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

new ReducingMethod KEEP_ONLY_LATEST_SCENARIO_RUNS #914

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions src/main/java/net/masterthought/cucumber/ReportParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import com.google.common.collect.Lists;

import net.masterthought.cucumber.json.Element;
import net.masterthought.cucumber.json.Feature;
import net.masterthought.cucumber.reducers.ReducingMethod;
import org.apache.commons.configuration.ConfigurationException;
Expand All @@ -22,6 +25,8 @@
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;
import java.util.Optional;
import java.util.logging.Level;
import java.util.logging.Logger;

Expand Down Expand Up @@ -71,6 +76,9 @@ public List<Feature> parseJsonFiles(List<String> jsonFiles) {
continue;
}
Feature[] features = parseForFeature(jsonFile);
if (configuration.containsReducingMethod(ReducingMethod.KEEP_ONLY_LATEST_SCENARIO_RUNS)) {
keepOnlyLatestScenarioRuns(features);
}
LOG.log(Level.INFO, String.format("File '%s' contains %d features", jsonFile, features.length));
featureResults.addAll(Arrays.asList(features));
}
Expand All @@ -82,6 +90,42 @@ public List<Feature> parseJsonFiles(List<String> jsonFiles) {

return featureResults;
}

/**
* If the JSON file has the same scenarios run multiple times, keep only the
Copy link
Owner

Choose a reason for hiding this comment

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

AFAIR JSON does not have results for multiple times. For that case you would rather have more than one JSON file. Can you explain this case?

Copy link
Owner

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

@damianszczepanik - I know this is an old PR. This case happens when we use TestNGCucumberRunner and use TestNG's retry analyzer capability to retry tests to disregard flaky tests.

Assuming that we run 1 test with re-run enabled -

What this does is, in case of flaky behavior, where a test would fail first and then would pass when re-tried - it creates 2 entries in JSON for the same test - 1 for the failed test and 2nd for the same test, re-tried. So the final report shows that there were 2 tests.

If the re-tried scenario also fails, it is counted as another failed test. so total 2 failed tests
If the re-tried scenario passed, the report says, 1 failed and 1 passed.

This PR seems to solve that problem.

Copy link
Author

Choose a reason for hiding this comment

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

Exactly. Sorry I didn't respond earlier, got moved on to other priorities and never had a chance to come back and answer the questions/fixes.

* latest scenario's run.
*/
private void keepOnlyLatestScenarioRuns(Feature[] features) {
for (Feature feature : features) {
Element[] elements = feature.getElements();
List<Element> elementList = Lists.newArrayList(elements);
ListIterator<Element> li = elementList.listIterator(elementList.size());
Optional<Element> lastElement = Optional.empty();
int numRemoved = 0;
while (li.hasPrevious()) {
Element element = li.previous();
if (lastElement.isPresent() && element.getId().equals(lastElement.get().getId())) {
if (LOG.isLoggable(Level.FINE)) {
LOG.log(Level.FINE, String.format("Reducing method KEEP_ONLY_EARLIEST_SCENARIO_RUNS is removing an earlier test result of scenario %s", feature.getName()));
}
li.remove();
++numRemoved;
} else {
addRetryNumberToElmIfNeeded(lastElement, numRemoved);
lastElement = Optional.of(element);
numRemoved = 0;
}
}
addRetryNumberToElmIfNeeded(lastElement, numRemoved);
feature.setElements(elementList.toArray(new Element[0]));
}
}

private void addRetryNumberToElmIfNeeded(Optional<Element> lastElement, int numRemoved) {
Copy link
Owner

Choose a reason for hiding this comment

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

this is Java, we don't shorten names to ElmI

if (lastElement.isPresent() && numRemoved > 0) {
lastElement.get().appendToName(" [Retry count " + (numRemoved + 1) + "]");
}
}

/**
* Reads passed file and returns parsed features.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ private void buildGeneralParameters() {
context.put("run_with_jenkins", configuration.containsPresentationMode(PresentationMode.RUN_WITH_JENKINS));
context.put("expand_all_steps", configuration.containsPresentationMode(PresentationMode.EXPAND_ALL_STEPS));
context.put("hide_empty_hooks", configuration.containsReducingMethod(ReducingMethod.HIDE_EMPTY_HOOKS));
context.put("keep_only_earliest_scenario_runs", configuration.containsReducingMethod(ReducingMethod.KEEP_ONLY_LATEST_SCENARIO_RUNS));

context.put("trends_available", configuration.isTrendsAvailable());
context.put("build_project_name", configuration.getProjectName());
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/net/masterthought/cucumber/json/Element.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class Element implements Durationable {

// Start: attributes from JSON file report
private final String id = null;
private final String name = null;
private String name = null;
Copy link
Owner

Choose a reason for hiding this comment

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

name is immutable

Copy link
Author

Choose a reason for hiding this comment

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

i change this to mutable because i use the Name in the report because I append " (${numRetries)" so that it shows up in the report.

alternatively we could create a new element in the html/xslt or whatever.

private final String type = null;
private final String description = null;
private final String keyword = null;
Expand Down Expand Up @@ -157,4 +157,12 @@ private void calculateDuration() {
duration += step.getResult().getDuration();
}
}

/**
* Append a string to the name.
* @param str The string to append.
*/
public void appendToName(String str) {
name += str;
}
}
4 changes: 4 additions & 0 deletions src/main/java/net/masterthought/cucumber/json/Feature.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ public void addElements(Element[] newElements) {
System.arraycopy(newElements, 0, both, elements.length, newElements.length);
elements = both;
}

public void setElements(Element[] elements) {
this.elements = elements;
}

public Element[] getElements() {
return elements;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,11 @@ public enum ReducingMethod {
/**
* Does not display hooks (@Before and @After) which do not have attachment or error message.
*/
HIDE_EMPTY_HOOKS
HIDE_EMPTY_HOOKS,

/**
* Keep only the very latest scenario runs. This is useful in situations where you did retries of a
* flaky scenario and the subsequent attempt(s) passed. In this case you will want to disregard the first few failures.
*/
KEEP_ONLY_LATEST_SCENARIO_RUNS
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package net.masterthought.cucumber;

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.UUID;

import org.apache.commons.io.FileUtils;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

import net.masterthought.cucumber.json.support.Status;
import net.masterthought.cucumber.presentation.PresentationMode;
import net.masterthought.cucumber.reducers.ReducingMethod;

public class RemoveFailuresDueToRetriesTest {
private static final String WITH_RETRIES_JSON = "with-retries.json";

File reportDir;
File jsonFile;

@Before
public void before() throws IOException {
File target = new File("target");
reportDir = new File(target, UUID.randomUUID().toString());
reportDir.mkdirs();
jsonFile = new File(reportDir, WITH_RETRIES_JSON);

FileUtils.copyInputStreamToFile(this.getClass().getResourceAsStream("/json/" + WITH_RETRIES_JSON), jsonFile);
}

@After
public void after() throws IOException {
FileUtils.deleteDirectory(reportDir);
}

@Test
public void testRetryRemoval() throws IOException {

File reportOutputDirectory = reportDir;
List<String> jsonFiles = new ArrayList<>();
jsonFiles.add(jsonFile.getAbsolutePath());

String buildNumber = "1";
String projectName = "cucumberProject";

Configuration configuration = new Configuration(reportOutputDirectory, projectName);
// optional configuration - check javadoc for details
configuration.addPresentationModes(PresentationMode.RUN_WITH_JENKINS);
// do not make scenario failed when step has status SKIPPED
configuration.setNotFailingStatuses(Collections.singleton(Status.SKIPPED));
configuration.setBuildNumber(buildNumber);
// addidtional metadata presented on main page
configuration.addClassifications("Platform", "Windows");
configuration.addClassifications("Browser", "Google Chrome");
configuration.addClassifications("Branch", "release/1.0");
configuration.addReducingMethod(ReducingMethod.KEEP_ONLY_LATEST_SCENARIO_RUNS);


ReportBuilder reportBuilder = new ReportBuilder(jsonFiles, configuration);
Reportable result = reportBuilder.generateReports();

Assert.assertEquals("Should not report the retried steps as failures!", 0, result.getFailedSteps());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public void buildGeneralParameters_AddsCommonProperties() {

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(10);
assertThat(context.getKeys()).hasSize(11);

Object obj = context.get("counter");
assertThat(obj).isInstanceOf(Counter.class);
Expand Down Expand Up @@ -152,7 +152,7 @@ public void buildGeneralParameters_OnInvalidBuildNumber_SkipsBuildPreviousNumber

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(10);
assertThat(context.getKeys()).hasSize(11);
assertThat(context.get("build_time")).isNotNull();
}

Expand All @@ -168,7 +168,7 @@ public void buildGeneralParameters_OnBuildNumber_AddsBuildPreviousNumberProperty

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(10);
assertThat(context.getKeys()).hasSize(11);
assertThat(context.get("build_time")).isNotNull();
}

Expand All @@ -184,7 +184,7 @@ public void buildGeneralParameters_OnErrorPage_AddsExtraProperties() {

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(10);
assertThat(context.getKeys()).hasSize(11);
assertThat(context.get("build_previous_number")).isNull();
}

Expand All @@ -201,7 +201,7 @@ public void buildGeneralParameters_OnInvalidBuildNumber_DoesNotAddPreviousBuildN

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(11);
assertThat(context.getKeys()).hasSize(12);
assertThat(context.get("build_previous_number")).isEqualTo(33);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void prepareReport_AddsCustomProperties() {

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(13);
assertThat(context.getKeys()).hasSize(14);
assertThat(context.get("classifications")).isInstanceOf(List.class);
assertThat(context.get("output_message")).isEqualTo(ExceptionUtils.getStackTrace(exception));
assertThat(context.get("json_files")).isEqualTo(jsonReports);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void prepareReport_AddsCustomProperties() {

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(11);
assertThat(context.getKeys()).hasSize(12);

List<Element> elements = (List<Element>) context.get("failures");
assertThat(elements).hasSameElementsAs(failures);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void prepareReport_AddsCustomProperties() {

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(11);
assertThat(context.getKeys()).hasSize(12);
assertThat(context.get("feature")).isEqualTo(feature);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public void prepareReport_AddsCustomProperties() {

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(13);
assertThat(context.getKeys()).hasSize(14);

assertThat(context.get("all_features")).isEqualTo(features);
assertThat(context.get("report_summary")).isEqualTo(reportResult.getFeatureReport());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void prepareReport_AddsCustomProperties() {

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(15);
assertThat(context.getKeys()).hasSize(16);
assertThat(context.get("all_steps")).isEqualTo(steps);

int allOccurrences = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void prepareReport_AddsCustomProperties() {

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(11);
assertThat(context.getKeys()).hasSize(12);
assertThat(context.get("tag")).isEqualTo(tag);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void prepareReport_AddsCustomProperties() {

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(14);
assertThat(context.getKeys()).hasSize(15);

assertThat(context.get("all_tags")).isEqualTo(tags);
assertThat(context.get("report_summary")).isEqualTo(reportResult.getTagReport());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void prepareReport_AddsCustomProperties() {

// then
VelocityContext context = page.context;
assertThat(context.getKeys()).hasSize(21);
assertThat(context.getKeys()).hasSize(22);

assertThat(context.get("buildNumbers")).isEqualTo(new String[]{"01_first", "other build", "05last"});
assertThat(context.get("failedFeatures")).isEqualTo(new int[]{1, 2, 5});
Expand Down
Loading