Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aqaexplorer
Copy link

@aqaexplorer aqaexplorer commented Jan 20, 2021

Works as MERGE_FEATURES_WITH_RETEST reducing method plus marking as FLACKY tests that after rerun were passed. Before merging it checks that scenario was failed and now it's passed and mark it as flacky (add @Flacky tag). Also it gets an exception from firstly failed scenario and sets it to passed scenario in the same step/hook so you can analyze the reason but not to fail test suite. Plus it will also move any attachments to passed scenario in the same step/hook as in firstly failed scenario from firstly failed scenario AFTER HOOK (The common cucumber solution to embed screenshot in after hook if scenario failed)

@codecov-io
Copy link

codecov-io commented Jan 20, 2021

Codecov Report

Merging #986 (cf5d024) into master (e2bac56) will decrease coverage by 0.54%.
The diff coverage is 87.01%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #986      +/-   ##
============================================
- Coverage     97.94%   97.39%   -0.55%     
- Complexity      557      587      +30     
============================================
  Files            54       55       +1     
  Lines          1166     1228      +62     
  Branches        106      109       +3     
============================================
+ Hits           1142     1196      +54     
- Misses            9       14       +5     
- Partials         15       18       +3     
Impacted Files Coverage Δ Complexity Δ
...et/masterthought/cucumber/json/support/Status.java 90.90% <0.00%> (-9.10%) 7.00 <1.00> (+1.00) ⬇️
.../cucumber/reducers/ReportFeatureMergerFactory.java 100.00% <ø> (ø) 3.00 <0.00> (ø)
...cumber/reducers/ReportFeatureWithRetestMerger.java 84.84% <66.66%> (+0.97%) 23.00 <2.00> (+1.00)
...rs/ReportFeatureWithRetestMarkingFlackyMerger.java 85.36% <85.36%> (ø) 18.00 <18.00> (?)
.../java/net/masterthought/cucumber/json/Element.java 98.27% <91.66%> (-1.73%) 29.00 <3.00> (+3.00) ⬇️
...ain/java/net/masterthought/cucumber/json/Hook.java 100.00% <100.00%> (ø) 11.00 <7.00> (+3.00)
...n/java/net/masterthought/cucumber/json/Result.java 100.00% <100.00%> (ø) 9.00 <1.00> (+1.00)
...ain/java/net/masterthought/cucumber/json/Step.java 94.73% <100.00%> (+0.45%) 20.00 <4.00> (+3.00)
...asterthought/cucumber/reducers/ReducingMethod.java 100.00% <100.00%> (ø) 1.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2bac56...cf5d024. Read the comment docs.

@aqaexplorer aqaexplorer force-pushed the add_new_reducing_method_MERGE_FEATURES_WITH_RETEST_MARKING_FLACKY branch 3 times, most recently from 4605327 to 953274f Compare January 21, 2021 15:12
@aqaexplorer aqaexplorer force-pushed the add_new_reducing_method_MERGE_FEATURES_WITH_RETEST_MARKING_FLACKY branch from 953274f to 3826b3f Compare January 21, 2021 16:42
@damianszczepanik
Copy link
Owner

Some checks are failing. Also no functional description provided

@aqaexplorer
Copy link
Author

@damianszczepanik Hi! I have updated the description. As for failing, it is coverage problem. But I have checked and covered failed but still have an issues. For example Step class is fully covered but it shows
image
Can you please help me to understand where to look?
As I see from the report https://codecov.io/gh/damianszczepanik/cucumber-reporting/compare/e2bac56c24ae93f1b9160503c3271e136b9868cb...3826b3fdab78e2eb50828cf88675f89709be04cb/diff
problems in Step and Element classes but everything is covered here. Will appreciate if you could show me where to start from :)

@@ -131,6 +135,18 @@ public String getFormattedDuration() {
return Util.formatDuration(duration);
}

public boolean isStatus(Status status) {
Copy link
Owner

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

*
* @author Vitaliya Ryabchuk (aqaexplorer@github)
*/
public interface Embedded {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks more like a class than interface

@@ -43,4 +43,5 @@ public String getLabel() {
public boolean isPassed() {
return this == PASSED;
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing has changed here, revert the file

setEmbeddingsInAfterHookIfPresent(target, candidate, targetFailed);
}

void setErrorMessage(Resultsable targetFailed, Stream<Resultsable> candidateResultsableStream) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyErrorMessage - what do you think?

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"));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when this exception may be thrown?

are you catching such exception anywhere?

MemberModifier.field(Step.class, "match").set(step, match);
MemberModifier.field(Match.class, "location").set(match, location);

MemberModifier.field(Step.class, "before").set(step,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to use try-catch when using Deencapsulation.setField


Stream<Resultsable> mergeResultsable(Element element) {
Stream<Hook> candidateHooks = concat(stream(element.getBefore()), stream(element.getAfter()));
return concat(candidateHooks, getStepsWithStepHooks(element));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sometimes with static import ...

Resultsable[] before = previous.getSteps()[i].getBefore();
Resultsable[] after = previous.getSteps()[i].getAfter();
Stream<Resultsable> hooks = concat(stream(before), stream(after));
stream = Stream.concat(stream, hooks);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... sometimes not

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));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it proper to concat before and after ?

}

@Override
protected void replace(Feature feature, Element[] elements, int i, Element current, int indexOfPreviousResult,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants