Skip to content

Use deterministic unique ids in Descriptions #1134

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

Merged
merged 2 commits into from
May 27, 2017
Merged

Use deterministic unique ids in Descriptions #1134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19774fc
[JUnit] Use deterministic unique ids in Descriptions
mpkorstanje May 13, 2017
6bad552
[JUnit] Fix argument name.
brasmusson May 27, 2017
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
77 changes: 62 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 @@ -49,7 +49,7 @@ public String getName() {
public Description getDescription() {
if (description == null) {
String nameForDescription = getName().isEmpty() ? "EMPTY_NAME" : getName();
description = Description.createSuiteDescription(nameForDescription, new PickleWrapper(pickleEvent));
description = Description.createSuiteDescription(nameForDescription, new PickleId(pickleEvent));

for (PickleStep step : getChildren()) {
description.addChild(describeChild(step));
Expand All @@ -68,7 +68,7 @@ protected Description describeChild(PickleStep step) {
} else {
testName = step.getText();
}
description = Description.createTestDescription(getName(), testName, new PickleStepWrapper(step));
description = Description.createTestDescription(getName(), testName, new PickleStepId(pickleEvent, step));
stepDescriptions.put(step, description);
}
return description;
Expand All @@ -93,22 +93,69 @@ protected void runChild(PickleStep step, RunNotifier notifier) {
private String makeNameFilenameCompatible(String name) {
return name.replaceAll("[^A-Za-z0-9_]", "_");
}
}

class PickleWrapper implements Serializable {
private static final long serialVersionUID = 1L;
private PickleEvent pickleEvent;
private static final class PickleId implements Serializable {
private static final long serialVersionUID = 1L;
private final String uri;
private int pickleLine;

PickleWrapper(PickleEvent pickleEvent) {
this.pickleEvent = pickleEvent;
PickleId(PickleEvent pickleEvent) {
this.uri = pickleEvent.uri;
this.pickleLine = pickleEvent.pickle.getLocations().get(0).getLine();
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
PickleId that = (PickleId) o;
return pickleLine == that.pickleLine && uri.equals(that.uri);
}

@Override
public int hashCode() {
int result = uri.hashCode();
result = 31 * result + pickleLine;
return result;
}

@Override
public String toString() {
return uri + ":" + pickleLine;
}
}
}

class PickleStepWrapper implements Serializable {
private static final long serialVersionUID = 1L;
private PickleStep step;
private static final class PickleStepId implements Serializable {
private static final long serialVersionUID = 1L;
private final String uri;
private final int pickleLine;
private final int pickleStepLine;

PickleStepWrapper(PickleStep step) {
this.step = step;
PickleStepId(PickleEvent pickleEvent, PickleStep pickleStep) {
this.uri = pickleEvent.uri;
this.pickleLine = pickleEvent.pickle.getLocations().get(0).getLine();
this.pickleStepLine = pickleStep.getLocations().get(0).getLine();
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
PickleStepId that = (PickleStepId) o;
return pickleLine == that.pickleLine && pickleStepLine == that.pickleStepLine && uri.equals(that.uri);
}

@Override
public int hashCode() {
int result = pickleLine;
result = 31 * result + uri.hashCode();
result = 31 * result + pickleStepLine;
return result;
}

@Override
public String toString() {
return uri + ":" + pickleLine + ":" + pickleStepLine;
}
}
}
}
31 changes: 30 additions & 1 deletion 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 @@ -12,6 +12,7 @@
import org.junit.runners.ParentRunner;
import org.junit.runners.model.InitializationError;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -36,7 +37,7 @@ public String getName() {
@Override
public Description getDescription() {
if (description == null) {
description = Description.createSuiteDescription(getName(), cucumberFeature);
description = Description.createSuiteDescription(getName(), new FeatureId(cucumberFeature));
for (ParentRunner child : getChildren()) {
description.addChild(describeChild(child));
}
Expand Down Expand Up @@ -87,4 +88,32 @@ private void buildFeatureElementRunners(Runtime runtime, JUnitReporter jUnitRepo
}
}

private static final class FeatureId implements Serializable {
private static final long serialVersionUID = 1L;
private final String uri;

FeatureId(CucumberFeature feature) {
this.uri = feature.getPath();
}

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

}

@Override
public int hashCode() {
return uri.hashCode();
}

@Override
public String toString() {
return uri;
}
}

}
62 changes: 58 additions & 4 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 @@ -7,6 +7,7 @@
import cucumber.runtime.RuntimeOptions;
import cucumber.runtime.io.ClasspathResourceLoader;
import cucumber.runtime.model.CucumberFeature;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.Description;
import org.junit.runner.notification.RunNotifier;
Expand All @@ -15,8 +16,11 @@
import org.mockito.InOrder;

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

import static java.util.Arrays.asList;
import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.argThat;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -89,17 +93,67 @@ public void should_call_formatter_for_scenario_outline_with_two_examples_table_a
}

private RunNotifier runFeatureWithNotifier(CucumberFeature cucumberFeature) throws InitializationError {
FeatureRunner runner = createFeatureRunner(cucumberFeature);
RunNotifier notifier = mock(RunNotifier.class);
runner.run(notifier);
return notifier;
}

private FeatureRunner createFeatureRunner(CucumberFeature cucumberFeature) throws InitializationError {
final RuntimeOptions runtimeOptions = new RuntimeOptions("-p null");
final ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
final ClasspathResourceLoader resourceLoader = new ClasspathResourceLoader(classLoader);
final RuntimeGlue glue = mock(RuntimeGlue.class);
final Runtime runtime = new Runtime(resourceLoader, classLoader, asList(mock(Backend.class)), runtimeOptions, new TimeService.Stub(0l), glue);
FeatureRunner runner = new FeatureRunner(cucumberFeature, runtime, new JUnitReporter(runtime.getEventBus(), false, new JUnitOptions(Collections.<String>emptyList())));
RunNotifier notifier = mock(RunNotifier.class);
runner.run(notifier);
return notifier;
return new FeatureRunner(cucumberFeature, runtime, new JUnitReporter(runtime.getEventBus(), false, new JUnitOptions(Collections.<String>emptyList())));
}


@Test
public void shouldPopulateDescriptionsWithStableUniqueIds() throws Exception {
Copy link
Contributor

@brasmusson brasmusson May 27, 2017

Choose a reason for hiding this comment

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

Most test method uses the non-idomatic form of using underscore, since some/many of us contributors find them easier to read (and in tests readability is even more important than elsewhere), see discussion at #302 (comment)

CucumberFeature cucumberFeature = TestPickleBuilder.parseFeature("path/test.feature", "" +
"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"

);

FeatureRunner runner = createFeatureRunner(cucumberFeature);
FeatureRunner rerunner = createFeatureRunner(cucumberFeature);

Set<Description> descriptions = new HashSet<Description>();
assertDescriptionIsUnique(runner.getDescription(), descriptions);
Copy link
Contributor

@brasmusson brasmusson May 27, 2017

Choose a reason for hiding this comment

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

It is a bit unusual for an assert-method to modify its arguments, but checking the uniqueness with a set makes sence - and then the set has been create and can be used subsequently.

assertDescriptionIsPredictable(runner.getDescription(), descriptions);
assertDescriptionIsPredictable(rerunner.getDescription(), descriptions);

}

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);
}
}
}

class DescriptionMatcher extends ArgumentMatcher<Description> {
Expand Down