[Core] Set scenario result as step finishes (#1430)

* Clean up pom * Update scenario result when step finishes The results of a step were set on the scenario after a test step finished. This meant that the result of the step was not available in its after hooks. By adding the result of the step to the scenario in TestStep we ensure that scenario always contains the most recent result. This removes the need to pass the scenario results around. Additionally: * Scenario has moved from cucumber.runtime.ScenarioImpl to cucumber.runner.Scenario and made package private. This prevents the implementation being used outside of the API. Users of ScenarioImpl.getError should use an event listener to create their reports. * TestStep.run no longer requires a language parameter. With Gherkin v5 the language is not used anywhere.
cucumber · Aug 25, 2018 · 1b0680b · 1b0680b
1 parent 532be3e
commit 1b0680b
Show file tree

Hide file tree

Showing 36 changed files with 393 additions and 418 deletions.
diff --git a/core/src/main/java/cucumber/api/Result.java b/core/src/main/java/cucumber/api/Result.java
@@ -3,8 +3,9 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.Comparator;
+import java.util.Objects;
 
-public class Result {
+public final class Result {
 
     public final static Comparator<Result> SEVERITY = new Comparator<Result>() {
 
@@ -14,8 +15,6 @@ public int compare(Result a, Result b) {
         }
     };
 
-    private static final long serialVersionUID = 1L;
-
     private final Type status;
     private final Long duration;
     private final Throwable error;
@@ -104,4 +103,19 @@ public String toString() {
             ", error=" + error +
             '}';
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        Result result = (Result) o;
+        return status == result.status &&
+            Objects.equals(duration, result.duration) &&
+            Objects.equals(error, result.error);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(status, duration, error);
+    }
 }
diff --git a/core/src/main/java/cucumber/runner/PickleStepTestStep.java b/core/src/main/java/cucumber/runner/PickleStepTestStep.java
@@ -1,19 +1,13 @@
 package cucumber.runner;
 
-import cucumber.api.Result;
-import cucumber.api.Scenario;
 import cucumber.api.TestCase;
 import cucumber.runtime.DefinitionArgument;
 import cucumber.runtime.PickleStepDefinitionMatch;
 import gherkin.pickles.PickleStep;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
-import static cucumber.api.Result.SEVERITY;
-import static java.util.Collections.max;
-
 class PickleStepTestStep extends TestStep implements cucumber.api.PickleStepTestStep {
     private final String uri;
     private final PickleStep step;
@@ -40,23 +34,20 @@ class PickleStepTestStep extends TestStep implements cucumber.api.PickleStepTest
     }
 
     @Override
-    Result run(TestCase testCase, EventBus bus, String language, Scenario scenario, boolean skipSteps) {
+    boolean run(TestCase testCase, EventBus bus, Scenario scenario, boolean skipSteps) {
         boolean skipNextStep = skipSteps;
-        List<Result> results = new ArrayList<Result>();
 
         for (HookTestStep before : beforeStepHookSteps) {
-            Result result = before.run(testCase, bus, language, scenario, skipSteps);
+            skipNextStep |= before.run(testCase, bus,  scenario, skipSteps);
-            skipNextStep |= !result.is(Result.Type.PASSED);
-            results.add(result);
         }
 
-        results.add(super.run(testCase, bus, language, scenario, skipNextStep));
+        skipNextStep |= super.run(testCase, bus,  scenario, skipNextStep);
 
         for (HookTestStep after : afterStepHookSteps) {
-            results.add(after.run(testCase, bus, language, scenario, skipSteps));
+            skipNextStep |=  after.run(testCase, bus,  scenario, skipSteps);
         }
 
-        return max(results, SEVERITY);
+        return skipNextStep;
     }
 
     List<HookTestStep> getBeforeStepHookSteps() {

diff --git a/core/src/main/java/cucumber/runner/Runner.java b/core/src/main/java/cucumber/runner/Runner.java
@@ -10,8 +10,8 @@
 import cucumber.runtime.Glue;
 import cucumber.runtime.HookDefinition;
 import cucumber.runtime.HookDefinitionMatch;
-import cucumber.runtime.RuntimeOptions;
 import cucumber.runtime.PickleStepDefinitionMatch;
+import cucumber.runtime.RuntimeOptions;
 import cucumber.runtime.UndefinedPickleStepDefinitionMatch;
 import gherkin.events.PickleEvent;
 import gherkin.pickles.PickleStep;

diff --git a/...n/java/cucumber/runtime/ScenarioImpl.java → ...c/main/java/cucumber/runner/Scenario.java b/...n/java/cucumber/runtime/ScenarioImpl.java → ...c/main/java/cucumber/runner/Scenario.java
@@ -1,50 +1,27 @@
-package cucumber.runtime;
+package cucumber.runner;
 
 import cucumber.api.Result;
-import cucumber.api.Scenario;
-import cucumber.api.TestCase;
 import cucumber.api.event.EmbedEvent;
 import cucumber.api.event.WriteEvent;
-import cucumber.runner.EventBus;
-import gherkin.events.PickleEvent;
-import gherkin.pickles.Pickle;
-import gherkin.pickles.PickleLocation;
 import gherkin.pickles.PickleTag;
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
 import static java.util.Collections.max;
 
-public class ScenarioImpl implements Scenario {
+class Scenario implements cucumber.api.Scenario {
 
     private final List<Result> stepResults = new ArrayList<Result>();
-    private final List<PickleTag> tags;
-    private final String uri;
-    private final String scenarioName;
-    private final String scenarioId;
-    private final List<Integer> scenarioLines;
     private final EventBus bus;
     private final TestCase testCase;
 
-    public ScenarioImpl(EventBus bus, TestCase testCase, PickleEvent pickleEvent) {
+    Scenario(EventBus bus, TestCase testCase) {
         this.bus = bus;
         this.testCase = testCase;
-        Pickle pickle = pickleEvent.pickle;
-        this.tags = pickle.getTags();
-        this.uri = pickleEvent.uri;
-        this.scenarioName = pickle.getName();
-        List<PickleLocation> locations = pickle.getLocations();
-        this.scenarioId = pickleEvent.uri + ":" + Integer.toString(locations.get(0).getLine());
-        ArrayList<Integer> lines = new ArrayList<Integer>();
-        for (PickleLocation location : locations) {
-            lines.add(location.getLine());
-        }
-        this.scenarioLines = Collections.unmodifiableList(lines);
     }
 
     public void add(Result result) {
@@ -53,12 +30,12 @@ public void add(Result result) {
 
     @Override
     public Collection<String> getSourceTagNames() {
-        Set<String> result = new HashSet<String>();
+        Set<String> result = new HashSet<>();
-        for (PickleTag tag : tags) {
+        for (PickleTag tag : testCase.getTags()) {
             result.add(tag.getName());
         }
         // Has to be a List in order for JRuby to convert to Ruby Array.
-        return new ArrayList<String>(result);
+        return new ArrayList<>(result);
     }
 
     @Override
@@ -91,22 +68,22 @@ public void write(String text) {
 
     @Override
     public String getName() {
-        return scenarioName;
+        return testCase.getName();
     }
 
     @Override
     public String getId() {
-        return scenarioId;
+        return testCase.getUri() + ":" + testCase.getLine();
     }
 
     @Override
     public String getUri() {
-        return uri;
+        return testCase.getUri();
     }
 
     @Override
     public List<Integer> getLines() {
-        return scenarioLines;
+        return testCase.getLines();
     }
 
     public Throwable getError() {

diff --git a/core/src/main/java/cucumber/runner/TestCase.java b/core/src/main/java/cucumber/runner/TestCase.java
@@ -4,7 +4,6 @@
 import cucumber.api.TestStep;
 import cucumber.api.event.TestCaseFinished;
 import cucumber.api.event.TestCaseStarted;
-import cucumber.runtime.ScenarioImpl;
 import gherkin.events.PickleEvent;
 import gherkin.pickles.PickleLocation;
 import gherkin.pickles.PickleTag;
@@ -35,33 +34,27 @@ void run(EventBus bus) {
         boolean skipNextStep = this.dryRun;
         Long startTime = bus.getTime();
         bus.send(new TestCaseStarted(startTime, this));
-        ScenarioImpl scenarioResult = new ScenarioImpl(bus, this, pickleEvent);
+        Scenario scenario = new Scenario(bus, this);
 
         for (HookTestStep before : beforeHooks) {
-            Result stepResult = before.run(this, bus, pickleEvent.pickle.getLanguage(), scenarioResult, dryRun);
+            skipNextStep |= before.run(this, bus, scenario, dryRun);
-            skipNextStep |= !stepResult.is(Result.Type.PASSED);
-            scenarioResult.add(stepResult);
         }
 
         for (PickleStepTestStep step : testSteps) {
-            Result stepResult = step.run(this, bus, pickleEvent.pickle.getLanguage(), scenarioResult, skipNextStep);
+            skipNextStep |= step.run(this, bus, scenario, skipNextStep);
-            skipNextStep |= !stepResult.is(Result.Type.PASSED);
-            scenarioResult.add(stepResult);
         }
 
         for (HookTestStep after : afterHooks) {
-            Result stepResult = after.run(this, bus, pickleEvent.pickle.getLanguage(), scenarioResult, dryRun);
+            after.run(this, bus, scenario, dryRun);
-            scenarioResult.add(stepResult);
         }
 
         Long stopTime = bus.getTime();
-        bus.send(new TestCaseFinished(stopTime, this, new Result(scenarioResult.getStatus(), stopTime - startTime, scenarioResult.getError())));
+        bus.send(new TestCaseFinished(stopTime, this, new Result(scenario.getStatus(), stopTime - startTime, scenario.getError())));
     }
 
     @Override
     public List<TestStep> getTestSteps() {
-        List<TestStep> testSteps = new ArrayList<TestStep>();
+        List<TestStep> testSteps = new ArrayList<TestStep>(beforeHooks);
-        testSteps.addAll(beforeHooks);
         for (PickleStepTestStep step : this.testSteps) {
             testSteps.addAll(step.getBeforeStepHookSteps());
             testSteps.add(step);
@@ -91,6 +84,14 @@ public int getLine() {
         return pickleEvent.pickle.getLocations().get(0).getLine();
     }
 
+    public List<Integer> getLines() {
+        List<Integer> lines = new ArrayList<>();
+        for (PickleLocation location : pickleEvent.pickle.getLocations()) {
+            lines.add(location.getLine());
+        }
+        return lines;
+    }
+
     private String fileColonLine(PickleLocation location) {
         return pickleEvent.uri + ":" + location.getLine();
     }

diff --git a/core/src/main/java/cucumber/runner/TestStep.java b/core/src/main/java/cucumber/runner/TestStep.java
@@ -2,7 +2,6 @@
 
 import cucumber.api.Pending;
 import cucumber.api.Result;
-import cucumber.api.Scenario;
 import cucumber.api.TestCase;
 import cucumber.api.event.TestStepFinished;
 import cucumber.api.event.TestStepStarted;
@@ -34,29 +33,39 @@ public String getCodeLocation() {
         return stepDefinitionMatch.getCodeLocation();
     }
 
-    Result run(TestCase testCase, EventBus bus, String language, Scenario scenario, boolean skipSteps) {
+    /**
+     * Runs a test step.
+     *
+     * @param testCase
+     * @param bus
+     * @param scenario
+     * @param skipSteps
+     * @return true iff subsequent skippable steps should be skipped
+     */
+    boolean run(TestCase testCase, EventBus bus, Scenario scenario, boolean skipSteps) {
         Long startTime = bus.getTime();
         bus.send(new TestStepStarted(startTime, testCase, this));
         Result.Type status;
         Throwable error = null;
         try {
-            status = executeStep(language, scenario, skipSteps);
+            status = executeStep(scenario, skipSteps);
         } catch (Throwable t) {
             error = t;
             status = mapThrowableToStatus(t);
         }
         Long stopTime = bus.getTime();
         Result result = mapStatusToResult(status, error, stopTime - startTime);
+        scenario.add(result);
         bus.send(new TestStepFinished(stopTime, testCase, this, result));
-        return result;
+        return !result.is(Result.Type.PASSED);
     }
 
-    private Result.Type executeStep(String language, Scenario scenario, boolean skipSteps) throws Throwable {
+    private Result.Type executeStep(cucumber.api.Scenario scenario, boolean skipSteps) throws Throwable {
         if (!skipSteps) {
-            stepDefinitionMatch.runStep(language, scenario);
+            stepDefinitionMatch.runStep(scenario);
             return Result.Type.PASSED;
         } else {
-            stepDefinitionMatch.dryRunStep(language, scenario);
+            stepDefinitionMatch.dryRunStep(scenario);
             return Result.Type.SKIPPED;
         }
     }

diff --git a/core/src/main/java/cucumber/runtime/AmbiguousPickleStepDefinitionsMatch.java b/core/src/main/java/cucumber/runtime/AmbiguousPickleStepDefinitionsMatch.java
@@ -15,13 +15,13 @@ public AmbiguousPickleStepDefinitionsMatch(String uri, PickleStep step, Ambiguou
     }
 
     @Override
-    public void runStep(String language, Scenario scenario) throws Throwable {
+    public void runStep(Scenario scenario) {
         throw exception;
     }
 
     @Override
-    public void dryRunStep(String language, Scenario scenario) throws Throwable {
+    public void dryRunStep(Scenario scenario) {
-        runStep(language, scenario);
+        runStep(scenario);
     }
 
     @Override

diff --git a/core/src/main/java/cucumber/runtime/FailedPickleStepInstantiationMatch.java b/core/src/main/java/cucumber/runtime/FailedPickleStepInstantiationMatch.java
@@ -1,8 +1,8 @@
 package cucumber.runtime;
 
-import io.cucumber.stepexpression.Argument;
 import cucumber.api.Scenario;
 import gherkin.pickles.PickleStep;
+import io.cucumber.stepexpression.Argument;
 
 import java.util.Collections;
 
@@ -15,13 +15,13 @@ public FailedPickleStepInstantiationMatch(String uri, PickleStep step, Throwable
     }
 
     @Override
-    public void runStep(String language, Scenario scenario) throws Throwable {
+    public void runStep(Scenario scenario) throws Throwable {
         throw throwable;
     }
 
     @Override
-    public void dryRunStep(String language, Scenario scenario) throws Throwable {
+    public void dryRunStep(Scenario scenario) throws Throwable {
-        runStep(language, scenario);
+        runStep(scenario);
     }
 
 }
diff --git a/core/src/main/java/cucumber/runtime/HookDefinitionMatch.java b/core/src/main/java/cucumber/runtime/HookDefinitionMatch.java
@@ -10,12 +10,12 @@ public HookDefinitionMatch(HookDefinition hookDefinition) {
     }
 
     @Override
-    public void runStep(String language, Scenario scenario) throws Throwable {
+    public void runStep(Scenario scenario) throws Throwable {
         hookDefinition.execute(scenario);
     }
 
     @Override
-    public void dryRunStep(String language, Scenario scenario) throws Throwable {
+    public void dryRunStep(Scenario scenario) {
         // Do nothing
     }
 

diff --git a/core/src/main/java/cucumber/runtime/NoStepDefinition.java b/core/src/main/java/cucumber/runtime/NoStepDefinition.java
@@ -1,7 +1,7 @@
 package cucumber.runtime;
 
-import io.cucumber.stepexpression.Argument;
 import gherkin.pickles.PickleStep;
+import io.cucumber.stepexpression.Argument;
 
 import java.util.List;
 
@@ -23,7 +23,7 @@ public Integer getParameterCount() {
     }
 
     @Override
-    public void execute(String language, Object[] args) {
+    public void execute(Object[] args) {
     }
 
     @Override