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.

Sign up for GitHub

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

Use deterministic unique ids in Descriptions #1121

Closed
wants to merge 1 commit into from
Closed

Use deterministic unique ids in Descriptions #1121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 7 additions & 5 deletions junit/src/main/java/cucumber/runtime/junit/ExamplesRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,23 @@

public class ExamplesRunner extends Suite {
private final CucumberExamples cucumberExamples;
private final IdProvider idProvider;
private Description description;
private JUnitReporter jUnitReporter;

protected ExamplesRunner(Runtime runtime, CucumberExamples cucumberExamples, JUnitReporter jUnitReporter) throws InitializationError {
super(ExamplesRunner.class, buildRunners(runtime, cucumberExamples, jUnitReporter));
protected ExamplesRunner(Runtime runtime, CucumberExamples cucumberExamples, JUnitReporter jUnitReporter, IdProvider idProvider) throws InitializationError {
super(ExamplesRunner.class, buildRunners(runtime, cucumberExamples, jUnitReporter, idProvider));
this.cucumberExamples = cucumberExamples;
this.jUnitReporter = jUnitReporter;
this.idProvider = idProvider;
}

private static List<Runner> buildRunners(Runtime runtime, CucumberExamples cucumberExamples, JUnitReporter jUnitReporter) {
private static List<Runner> buildRunners(Runtime runtime, CucumberExamples cucumberExamples, JUnitReporter jUnitReporter, IdProvider idProvider) {
List<Runner> runners = new ArrayList<Runner>();
List<CucumberScenario> exampleScenarios = cucumberExamples.createExampleScenarios();
for (CucumberScenario scenario : exampleScenarios) {
try {
ExecutionUnitRunner exampleScenarioRunner = new ExecutionUnitRunner(runtime, scenario, jUnitReporter);
ExecutionUnitRunner exampleScenarioRunner = new ExecutionUnitRunner(runtime, scenario, jUnitReporter, idProvider);
runners.add(exampleScenarioRunner);
} catch (InitializationError initializationError) {
initializationError.printStackTrace();
Expand All @@ -45,7 +47,7 @@ protected String getName() {
@Override
public Description getDescription() {
if (description == null) {
description = Description.createSuiteDescription(getName(), cucumberExamples.getExamples());
description = Description.createSuiteDescription(getName(), idProvider.next());
for (Runner child : getChildren()) {
description.addChild(describeChild(child));
}
Expand Down
23 changes: 8 additions & 15 deletions junit/src/main/java/cucumber/runtime/junit/ExecutionUnitRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@ public class ExecutionUnitRunner extends ParentRunner<Step> {
private final Runtime runtime;
private final CucumberScenario cucumberScenario;
private final JUnitReporter jUnitReporter;
private final IdProvider idProvider;
private Description description;
private final Map<Step, Description> stepDescriptions = new HashMap<Step, Description>();
private final List<Step> runnerSteps = new ArrayList<Step>();

public ExecutionUnitRunner(Runtime runtime, CucumberScenario cucumberScenario, JUnitReporter jUnitReporter) throws InitializationError {
public ExecutionUnitRunner(Runtime runtime, CucumberScenario cucumberScenario, JUnitReporter jUnitReporter, IdProvider idProvider) throws InitializationError {
super(ExecutionUnitRunner.class);
this.runtime = runtime;
this.cucumberScenario = cucumberScenario;
this.jUnitReporter = jUnitReporter;
this.idProvider = idProvider;
}

public List<Step> getRunnerSteps() {
Expand All @@ -53,21 +55,12 @@ public String getName() {
@Override
public Description getDescription() {
if (description == null) {
description = Description.createSuiteDescription(getName(), cucumberScenario.getGherkinModel());
description = Description.createSuiteDescription(getName(), idProvider.next());

if (cucumberScenario.getCucumberBackground() != null) {
for (Step backgroundStep : cucumberScenario.getCucumberBackground().getSteps()) {
// We need to make a copy of that step, so we have a unique one per scenario
Step copy = new Step(
backgroundStep.getComments(),
backgroundStep.getKeyword(),
backgroundStep.getName(),
backgroundStep.getLine(),
backgroundStep.getRows(),
backgroundStep.getDocString()
);
description.addChild(describeChild(copy));
runnerSteps.add(copy);
for (Step step : cucumberScenario.getCucumberBackground().getSteps()) {
description.addChild(describeChild(step));
runnerSteps.add(step);
}
}

Expand All @@ -89,7 +82,7 @@ protected Description describeChild(Step step) {
} else {
testName = step.getKeyword() + step.getName();
}
description = Description.createTestDescription(getName(), testName, step);
description = Description.createTestDescription(getName(), testName, idProvider.next());
stepDescriptions.put(step, description);
}
return description;
Expand Down
8 changes: 5 additions & 3 deletions junit/src/main/java/cucumber/runtime/junit/FeatureRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ public class FeatureRunner extends ParentRunner<ParentRunner> {

private final CucumberFeature cucumberFeature;
private final Runtime runtime;
private final IdProvider idProvider;
private final JUnitReporter jUnitReporter;
private Description description;

public FeatureRunner(CucumberFeature cucumberFeature, Runtime runtime, JUnitReporter jUnitReporter) throws InitializationError {
super(null);
this.cucumberFeature = cucumberFeature;
this.runtime = runtime;
this.idProvider = new IdProvider(cucumberFeature.getPath());
this.jUnitReporter = jUnitReporter;
buildFeatureElementRunners();
}
Expand All @@ -40,7 +42,7 @@ public String getName() {
@Override
public Description getDescription() {
if (description == null) {
description = Description.createSuiteDescription(getName(), cucumberFeature.getGherkinFeature());
description = Description.createSuiteDescription(getName(), idProvider.next());
for (ParentRunner child : getChildren()) {
description.addChild(describeChild(child));
}
Expand Down Expand Up @@ -76,9 +78,9 @@ private void buildFeatureElementRunners() {
try {
ParentRunner featureElementRunner;
if (cucumberTagStatement instanceof CucumberScenario) {
featureElementRunner = new ExecutionUnitRunner(runtime, (CucumberScenario) cucumberTagStatement, jUnitReporter);
featureElementRunner = new ExecutionUnitRunner(runtime, (CucumberScenario) cucumberTagStatement, jUnitReporter, idProvider);
} else {
featureElementRunner = new ScenarioOutlineRunner(runtime, (CucumberScenarioOutline) cucumberTagStatement, jUnitReporter);
featureElementRunner = new ScenarioOutlineRunner(runtime, (CucumberScenarioOutline) cucumberTagStatement, jUnitReporter, idProvider);
}
children.add(featureElementRunner);
} catch (InitializationError e) {
Expand Down
56 changes: 56 additions & 0 deletions junit/src/main/java/cucumber/runtime/junit/IdProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package cucumber.runtime.junit;

import java.io.Serializable;

/**
* JUnit Descriptions require an unique id. However this id should be identical between test
* re-runs. This allows descriptions to be used to determine which failed tests should be rerun.
*
* IdProvider generates predictable unique ids. The feature name is used as a base to disambiguate
* between different features.
*/
class IdProvider {

private long id = 0;
private final String base;

IdProvider(String base) {
this.base = base;
}

Serializable next() {
return new Id(base, id++);
Copy link
Contributor

Choose a reason for hiding this comment

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

Using a sequence counter as a base for the id-object makes them unpredictable. As I understand it using

cucumber/examples/java/calculator/basic_arithmetic.feature:7:12

and

cucumber/examples/java/calculator/basic_arithmetic.feature:12

would result in different ids for the test case from the scenario on line 12 (feature file from the java-calculator example in the repo).

The id should instead be base on the uri(/path) of the feature file and the line number of the feature/scenario/etc.

Copy link
Contributor Author

@mpkorstanje mpkorstanje May 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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


~~~For example:~~~

```feature
Feature: Basic Arithmetic
  Background: A Calculator
    Given a calculator I just turned on

  Scenario: Addition
    When I add 4 and 5
    Then the result is 9

  Scenario: Another Addition
    When I add 4 and 7
    Then the result is 11
```

~~~Results in a tree of executors that looks like this:~~~

```
FeatureRunner => Feature: Basic Arithmetic
  |- ScenarioRunner => Scenario: Addition
  |   |- ExecutionUnitRunner => 
  |       |- Background step: Given a calculator I just turned on
  |       |- When I add 4 and 5
  |       |- Then the result is 9
  |- ScenarioRunner => Scenario: Another Addition
    |- ExecutionUnitRunner => 
        |- Background step: Given a calculator I just turned on
        |- When I add 4 and 7
        |- Then the result is 11
```
~~~This tree can then be numbered in depth first order:~~~
```
0. FeatureRunner => Feature: Basic Arithmetic
1.   |- ScenarioRunner => Scenario: Addition
2.  |   |- ExecutionUnitRunner => 
2.  |       |- Background step: Given a calculator I just turned on
4.  |       |- When I add 4 and 5
5.  |       |- Then the result is 9
6.  |- ScenarioRunner => Scenario: Another Addition
7.    |- ExecutionUnitRunner => 
8.        |- Background step: Given a calculator I just turned on
9.        |- When I add 4 and 7
10.       |- Then the result is 11
```

~~~Which means that for a given feature file creating the numbering of descriptions is entirely predictable. So I guess I don't quite understand your comment.~~~

~~~> The id should instead be base on the uri(/path) of the feature file and the line number of the feature/scenario/etc.~~~

~~~Using the feature uri and the line number alone is not feasible as background steps are repeated.  A failing background step in the execution of Scenario `Another Addition` would result in both `Another Addition` and `Addition` being rerun. Their descriptions both contain a child Description that uses line 3 .~~~

~~~It would be possible to use the uri and line number provided we also used some other other identifier to disambiguate between background steps in different scenarios. So as far as I can see numbering the tree by depth first order is sufficient and simple.~~~

Copy link
Contributor Author

@mpkorstanje mpkorstanje May 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

After reviewing the changes in Gherkin v4 I've changed my opinion. It is indeed much better to use a line based identification.

  • FeatureRunner: feature-uri
  • ExecutionRunner: feature-uri:scenario-line
  • Step: feature-uri:scenario-line:step-line

}

private static final class Id implements Serializable {

private final String base;
private final long id;

private Id(String base, long id) {
this.base = base;
this.id = id;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

Id id1 = (Id) o;
return id == id1.id && base.equals(id1.base);

}

@Override
public int hashCode() {
int result = base.hashCode();
result = 31 * result + (int) (id ^ (id >>> 32));
return result;
}
}
}




12 changes: 7 additions & 5 deletions junit/src/main/java/cucumber/runtime/junit/ScenarioOutlineRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,20 @@
public class ScenarioOutlineRunner extends Suite {
private final CucumberScenarioOutline cucumberScenarioOutline;
private final JUnitReporter jUnitReporter;
private final IdProvider idProvider;
private Description description;

public ScenarioOutlineRunner(Runtime runtime, CucumberScenarioOutline cucumberScenarioOutline, JUnitReporter jUnitReporter) throws InitializationError {
super(null, buildRunners(runtime, cucumberScenarioOutline, jUnitReporter));
public ScenarioOutlineRunner(Runtime runtime, CucumberScenarioOutline cucumberScenarioOutline, JUnitReporter jUnitReporter, IdProvider idProvider) throws InitializationError {
super(null, buildRunners(runtime, cucumberScenarioOutline, jUnitReporter, idProvider));
this.cucumberScenarioOutline = cucumberScenarioOutline;
this.jUnitReporter = jUnitReporter;
this.idProvider = idProvider;
}

private static List<Runner> buildRunners(Runtime runtime, CucumberScenarioOutline cucumberScenarioOutline, JUnitReporter jUnitReporter) throws InitializationError {
private static List<Runner> buildRunners(Runtime runtime, CucumberScenarioOutline cucumberScenarioOutline, JUnitReporter jUnitReporter, IdProvider idProvider) throws InitializationError {
List<Runner> runners = new ArrayList<Runner>();
for (CucumberExamples cucumberExamples : cucumberScenarioOutline.getCucumberExamplesList()) {
runners.add(new ExamplesRunner(runtime, cucumberExamples, jUnitReporter));
runners.add(new ExamplesRunner(runtime, cucumberExamples, jUnitReporter, idProvider));
}
return runners;
}
Expand All @@ -39,7 +41,7 @@ public String getName() {
@Override
public Description getDescription() {
if (description == null) {
description = Description.createSuiteDescription(getName(), cucumberScenarioOutline.getGherkinModel());
description = Description.createSuiteDescription(getName(), idProvider.next());
for (Runner child : getChildren()) {
description.addChild(describeChild(child));
}
Expand Down
50 changes: 14 additions & 36 deletions junit/src/test/java/cucumber/runtime/junit/ExecutionUnitRunnerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ public void shouldAssignUnequalDescriptionsToDifferentOccurrencesOfSameStepInASc
ExecutionUnitRunner runner = new ExecutionUnitRunner(
null,
(CucumberScenario) features.get(0).getFeatureElements().get(0),
createStandardJUnitReporter()
createStandardJUnitReporter(),
createStandardIdGenerator()
);

// fish out the two occurrences of the same step and check whether we really got them
Expand Down Expand Up @@ -55,7 +56,8 @@ public void shouldIncludeScenarioNameAsClassNameInStepDescriptions() throws Exce
ExecutionUnitRunner runner = new ExecutionUnitRunner(
null,
(CucumberScenario) features.get(0).getFeatureElements().get(0),
createStandardJUnitReporter()
createStandardJUnitReporter(),
createStandardIdGenerator()
);

// fish out the data from runner
Expand All @@ -67,31 +69,6 @@ public void shouldIncludeScenarioNameAsClassNameInStepDescriptions() throws Exce
assertEquals("description includes step keyword and name as method name", step.getKeyword() + step.getName(), stepDescription.getMethodName());
}

@Test
public void shouldPopulateRunnerStepsWithStepsUsedInStepDescriptions() throws Exception {
CucumberFeature cucumberFeature = TestFeatureBuilder.feature("featurePath", "" +
"Feature: feature name\n" +
" Background:\n" +
" Given background step\n" +
" Scenario:\n" +
" Then scenario name\n");

ExecutionUnitRunner runner = new ExecutionUnitRunner(
null,
(CucumberScenario) cucumberFeature.getFeatureElements().get(0),
createStandardJUnitReporter()
);

// fish out the data from runner
Description runnerDescription = runner.getDescription();
Description backgroundStepDescription = runnerDescription.getChildren().get(0);
Description scenarioStepDescription = runnerDescription.getChildren().get(1);
Step runnerBackgroundStep = runner.getRunnerSteps().get(0);
Step runnerScenarioStep = runner.getRunnerSteps().get(1);

assertDescriptionHasStepAsUniqueId(backgroundStepDescription, runnerBackgroundStep);
assertDescriptionHasStepAsUniqueId(scenarioStepDescription, runnerScenarioStep);
}

@Test
public void shouldUseScenarioNameForRunnerName() throws Exception {
Expand All @@ -103,7 +80,8 @@ public void shouldUseScenarioNameForRunnerName() throws Exception {
ExecutionUnitRunner runner = new ExecutionUnitRunner(
null,
(CucumberScenario) cucumberFeature.getFeatureElements().get(0),
createStandardJUnitReporter()
createStandardJUnitReporter(),
createStandardIdGenerator()
);

assertEquals("Scenario: scenario name", runner.getName());
Expand All @@ -119,7 +97,8 @@ public void shouldUseStepKeyworkAndNameForChildName() throws Exception {
ExecutionUnitRunner runner = new ExecutionUnitRunner(
null,
(CucumberScenario) cucumberFeature.getFeatureElements().get(0),
createStandardJUnitReporter()
createStandardJUnitReporter(),
createStandardIdGenerator()
);

assertEquals("Then it works", runner.getDescription().getChildren().get(0).getMethodName());
Expand All @@ -135,24 +114,23 @@ public void shouldConvertTextFromFeatureFileForNamesWithFilenameCompatibleNameOp
ExecutionUnitRunner runner = new ExecutionUnitRunner(
null,
(CucumberScenario) cucumberFeature.getFeatureElements().get(0),
createJUnitReporterWithOption("--filename-compatible-names")
createJUnitReporterWithOption("--filename-compatible-names"),
createStandardIdGenerator()
);

assertEquals("Scenario__scenario_name", runner.getName());
assertEquals("Then_it_works", runner.getDescription().getChildren().get(0).getMethodName());
}

private void assertDescriptionHasStepAsUniqueId(Description stepDescription, Step step) {
// Note, JUnit uses the the serializable parameter (in this case the step)
// as the unique id when comparing Descriptions
assertEquals(stepDescription, Description.createTestDescription("", "", step));
}

private JUnitReporter createStandardJUnitReporter() {
return new JUnitReporter(null, null, false, new JUnitOptions(Collections.<String>emptyList()));
}

private JUnitReporter createJUnitReporterWithOption(String option) {
return new JUnitReporter(null, null, false, new JUnitOptions(Arrays.asList(option)));
}

private IdProvider createStandardIdGenerator() {
return new IdProvider("dummy base");
}
}
56 changes: 56 additions & 0 deletions junit/src/test/java/cucumber/runtime/junit/FeatureRunnerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@
import cucumber.runtime.io.ClasspathResourceLoader;
import cucumber.runtime.model.CucumberFeature;
import org.junit.Test;
import org.junit.runner.Description;
import org.junit.runner.notification.RunNotifier;
import org.junit.runners.model.InitializationError;

import java.io.IOException;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import static java.util.Arrays.asList;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;

public class FeatureRunnerTest {
Expand Down Expand Up @@ -143,4 +148,55 @@ private String runFeatureWithFormatterSpy(CucumberFeature cucumberFeature) throw
return formatterSpy.toString();
}

@Test
public void shouldPopulateDescriptionsWithStableUniqueIds() throws Exception {
FeatureRunner runner = createFeatureRunner();
FeatureRunner rerunner = createFeatureRunner();

Set<Description> descriptions = new HashSet<Description>();
assertDescriptionIsUnique(runner.getDescription(), descriptions);
assertDescriptionIsPredictable(rerunner.getDescription(), descriptions);
}

private FeatureRunner createFeatureRunner() throws IOException, InitializationError {
CucumberFeature cucumberFeature = TestFeatureBuilder.feature("featurePath", "" +
"Feature: feature name\n" +
" Background:\n" +
" Given background step\n" +
" Scenario: A\n" +
" Then scenario name\n" +
" Scenario: B\n" +
" Then scenario name\n" +
" Scenario Outline: C\n" +
" Then scenario <name>\n" +
" Examples:\n" +
" | name |\n" +
" | C |\n" +
" | D |\n" +
" | E |\n"

);

return new FeatureRunner(cucumberFeature, null, createStandardJUnitReporter());
}

private static void assertDescriptionIsUnique(Description description, Set<Description> descriptions) {
// Note, JUnit uses the the serializable parameter (in this case the step)
// as the unique id when comparing Descriptions
assertTrue(descriptions.add(description));
for (Description each : description.getChildren()) {
assertDescriptionIsUnique(each, descriptions);
}
}

private static void assertDescriptionIsPredictable(Description description, Set<Description> descriptions) {
assertTrue(descriptions.contains(description));
for (Description each : description.getChildren()) {
assertDescriptionIsPredictable(each, descriptions);
}
}

private static JUnitReporter createStandardJUnitReporter() {
return new JUnitReporter(null, null, false, new JUnitOptions(Collections.<String>emptyList()));
}
}