-
Notifications
You must be signed in to change notification settings - Fork 403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add reducing method MERGE_FEATURES_WITH_RETEST_MARKING_FLACKY #986
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package net.masterthought.cucumber.json.support; | ||
|
||
import net.masterthought.cucumber.json.Embedding; | ||
|
||
/** | ||
* Ensures that class delivers method for embeddings. | ||
* | ||
* @author Vitaliya Ryabchuk (aqaexplorer@github) | ||
*/ | ||
public interface Embedded { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it looks more like a class than interface |
||
|
||
Embedding[] getEmbeddings(); | ||
|
||
void setEmbeddings(Embedding[] embeddings); | ||
|
||
boolean hasEmbeddings(); | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ | |
|
||
/** | ||
* Defines all possible statuses provided by cucumber library. | ||
* | ||
* | ||
* @author Damian Szczepanik (damianszczepanik@github) | ||
*/ | ||
@JsonDeserialize(using = StatusDeserializer.class) | ||
|
@@ -43,4 +43,5 @@ public String getLabel() { | |
public boolean isPassed() { | ||
return this == PASSED; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nothing has changed here, revert the file |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
package net.masterthought.cucumber.reducers; | ||
|
||
import static java.util.Arrays.stream; | ||
import static java.util.stream.Stream.concat; | ||
import static net.masterthought.cucumber.json.support.Status.FAILED; | ||
import static net.masterthought.cucumber.json.support.Status.PASSED; | ||
import static org.apache.commons.lang3.ArrayUtils.add; | ||
import static org.apache.commons.lang3.ArrayUtils.addAll; | ||
|
||
import net.masterthought.cucumber.json.Element; | ||
import net.masterthought.cucumber.json.Feature; | ||
import net.masterthought.cucumber.json.Hook; | ||
import net.masterthought.cucumber.json.Tag; | ||
import net.masterthought.cucumber.json.support.Resultsable; | ||
|
||
import java.util.List; | ||
import java.util.NoSuchElementException; | ||
import java.util.stream.Stream; | ||
|
||
/** | ||
* Merge list of given features in the same way as ReportFeatureWithRetestMerger plus marking flacky tests | ||
* (that were failed and after rerun are passed) not failing the last one and moves exception and attachments from | ||
* previously failed scenario for better analyzing | ||
* <p> | ||
* Uses when need to generate a report with rerun results analyzing flacky tests. | ||
*/ | ||
final class ReportFeatureWithRetestMarkingFlackyMerger extends ReportFeatureWithRetestMerger { | ||
|
||
@Override | ||
public boolean test(List<ReducingMethod> reducingMethods) { | ||
return reducingMethods != null | ||
&& reducingMethods.contains(ReducingMethod.MERGE_FEATURES_WITH_RETEST_MARKING_FLACKY); | ||
} | ||
|
||
@Override | ||
protected void replace(Feature feature, Element[] elements, int i, Element current, int indexOfPreviousResult, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
boolean hasBackground) { | ||
if (ifFlacky(feature.getElements()[indexOfPreviousResult], current)) { | ||
addFlackyTag(current); | ||
addExceptionWithEmbeddings(feature.getElements()[indexOfPreviousResult], current); | ||
} | ||
super.replace(feature, elements, i, current, indexOfPreviousResult, hasBackground); | ||
|
||
} | ||
|
||
boolean ifFlacky(Element target, Element candidate) { | ||
return target.isStatus(FAILED) && candidate.isStatus(PASSED); | ||
} | ||
|
||
void addFlackyTag(Element candidate) { | ||
Tag[] tags = add(candidate.getTags(), new Tag("@flacky")); | ||
candidate.setTags(tags); | ||
} | ||
|
||
void addExceptionWithEmbeddings(Element target, Element candidate) { | ||
Resultsable targetFailed = getFailedResultsable(mergeResultsable(target)); | ||
setErrorMessage(targetFailed, mergeResultsable(candidate)); | ||
setEmbeddingsInAfterHookIfPresent(target, candidate, targetFailed); | ||
} | ||
|
||
void setErrorMessage(Resultsable targetFailed, Stream<Resultsable> candidateResultsableStream) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. copyErrorMessage - what do you think? |
||
candidateResultsableStream.filter(resultsable -> resultsable.getMatch().getLocation() | ||
.equals(targetFailed.getMatch().getLocation())) | ||
.findFirst().ifPresent(step | ||
-> step.getResult().setErrorMessage(targetFailed.getResult().getErrorMessage())); | ||
} | ||
|
||
Resultsable getFailedResultsable(Stream<Resultsable> resultsableStream) { | ||
return resultsableStream.filter(resultsable -> resultsable.getResult().getStatus() == FAILED) | ||
.findFirst() | ||
.orElseThrow(() -> new NoSuchElementException("No hook or step with failed status was found")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when this exception may be thrown? are you catching such exception anywhere? |
||
} | ||
|
||
Stream<Resultsable> mergeResultsable(Element element) { | ||
Stream<Hook> candidateHooks = concat(stream(element.getBefore()), stream(element.getAfter())); | ||
return concat(candidateHooks, getStepsWithStepHooks(element)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sometimes with static import ... |
||
} | ||
|
||
Stream<Resultsable> getStepsWithStepHooks(Element previous) { | ||
Stream<Resultsable> stream = stream(previous.getSteps()); | ||
|
||
for (int i = 0; i <= previous.getSteps().length - 1; i++) { | ||
Resultsable[] before = previous.getSteps()[i].getBefore(); | ||
Resultsable[] after = previous.getSteps()[i].getAfter(); | ||
Stream<Resultsable> hooks = concat(stream(before), stream(after)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it proper to concat before and after ? |
||
stream = Stream.concat(stream, hooks); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ... sometimes not |
||
} | ||
return stream; | ||
} | ||
|
||
void setEmbeddingsInAfterHookIfPresent(Element previous, Element current, Resultsable previousFailed) { | ||
stream(previous.getAfter()) | ||
.filter(Hook::hasEmbeddings) | ||
.findFirst().ifPresent(previousEmbeddedAfter -> | ||
stream(current.getSteps()) | ||
.filter(hook -> hook.getMatch().getLocation().equals(previousFailed.getMatch().getLocation())) | ||
.findFirst().ifPresent(hook | ||
-> hook.setEmbeddings(addAll(hook.getEmbeddings(), previousEmbeddedAfter.getEmbeddings())))); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
method which start from isXXX should only perform fast checking and here I see some logic