Proposal to fix Issue577 -- improved junit runner

junit/src/main/java/cucumber/api/junit/Cucumber.java

@@ -10,7 +10,7 @@
 import cucumber.runtime.io.ResourceLoader;
 import cucumber.runtime.io.ResourceLoaderClassFinder;
 import cucumber.runtime.junit.Assertions;
-import cucumber.runtime.junit.FeatureRunner;
+import cucumber.runtime.junit.FeatureSuite;
 import cucumber.runtime.junit.JUnitReporter;
 import cucumber.runtime.model.CucumberFeature;
 import cucumber.runtime.snippets.SummaryPrinter;
@@ -38,9 +38,9 @@
  *
  * @see Options
  */
-public class Cucumber extends ParentRunner<FeatureRunner> {
+public class Cucumber extends ParentRunner<FeatureSuite> {
     private final JUnitReporter jUnitReporter;
-    private final List<FeatureRunner> children = new ArrayList<FeatureRunner>();
+    private final List<FeatureSuite> children = new ArrayList<FeatureSuite>();
     private final Runtime runtime;
 
     /**
@@ -51,7 +51,7 @@ public class Cucumber extends ParentRunner<FeatureRunner> {
      * @throws org.junit.runners.model.InitializationError
      *                             if there is another problem
      */
-    public Cucumber(Class clazz) throws InitializationError, IOException {
+    public Cucumber(Class<?> clazz) throws InitializationError, IOException {
         super(clazz);
         ClassLoader classLoader = clazz.getClassLoader();
         Assertions.assertNoCucumberAnnotatedMethods(clazz);
@@ -68,17 +68,17 @@ public Cucumber(Class clazz) throws InitializationError, IOException {
     }
 
     @Override
-    public List<FeatureRunner> getChildren() {
+    public List<FeatureSuite> getChildren() {
         return children;
     }
 
     @Override
-    protected Description describeChild(FeatureRunner child) {
+    protected Description describeChild(FeatureSuite child) {
         return child.getDescription();
     }
 
     @Override
-    protected void runChild(FeatureRunner child, RunNotifier notifier) {
+    protected void runChild(FeatureSuite child, RunNotifier notifier) {
         child.run(notifier);
     }
 
@@ -92,7 +92,7 @@ public void run(RunNotifier notifier) {
 
     private void addChildren(List<CucumberFeature> cucumberFeatures) throws InitializationError {
         for (CucumberFeature cucumberFeature : cucumberFeatures) {
-            children.add(new FeatureRunner(cucumberFeature, runtime, jUnitReporter));
+            children.add(new FeatureSuite(getTestClass().getJavaClass(), cucumberFeature, runtime, jUnitReporter));
         }
     }
 

junit/src/main/java/cucumber/runtime/junit/ExecutionUnitNotifier.java

@@ -0,0 +1,59 @@
+package cucumber.runtime.junit;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.internal.AssumptionViolatedException;
+import org.junit.internal.runners.model.EachTestNotifier;
+import org.junit.runner.Description;
+import org.junit.runner.notification.RunNotifier;
+
+/**
+ * This class makes sure that an execution unit with a missing step definition is properly
+ * marked as ignored. The main reason for being of this class is that in JUnit, a test
+ * that is to be ignored can't be started. While for a cuke, we only figure out that a step
+ * definition is missing when we get at that step.
+ */
+public class ExecutionUnitNotifier extends EachTestNotifier {
+
+    private final List<Throwable> failures = new ArrayList<Throwable>();
+    private boolean ignored;
+
+    public ExecutionUnitNotifier(RunNotifier notifier, Description description) {
+        super(notifier, description);
+    }
+
+    public void addFailure(Throwable targetException) {
+        this.failures.add(targetException);
+    }
+
+    public void addFailedAssumption(AssumptionViolatedException e) {
+        this.failures.add(e);
+    }
+
+    public void fireTestFinished() {
+        if (ignored) {
+            super.fireTestIgnored();
+        } else {
+            super.fireTestStarted();
+            for (Throwable t : failures) {
+                if (t instanceof AssumptionViolatedException) {
+                    super.addFailedAssumption((AssumptionViolatedException) t);
+                } else {
+                    super.addFailure(t);
+                }
+            }
+            super.fireTestFinished();
+        }
+        failures.clear();
+        ignored = false;
+    }
+
+    public void fireTestStarted() {
+    }
+
+    public void fireTestIgnored() {
+        ignored = true;
+    }
+
+}

junit/src/main/java/cucumber/runtime/junit/ExecutionUnitRunner.java

@@ -1,78 +1,37 @@
 package cucumber.runtime.junit;
 
-import cucumber.runtime.Runtime;
-import cucumber.runtime.model.CucumberScenario;
-import gherkin.formatter.model.Step;
 import org.junit.runner.Description;
+import org.junit.runner.Runner;
 import org.junit.runner.notification.RunNotifier;
-import org.junit.runners.ParentRunner;
 import org.junit.runners.model.InitializationError;
 
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import cucumber.runtime.Runtime;
+import cucumber.runtime.model.CucumberScenario;
 
 /**
  * Runs a scenario, or a "synthetic" scenario derived from an Examples row.
  */
-public class ExecutionUnitRunner extends ParentRunner<Step> {
+public class ExecutionUnitRunner extends Runner {
+
     private final Runtime runtime;
     private final CucumberScenario cucumberScenario;
     private final JUnitReporter jUnitReporter;
-    private Description description;
-    private final Map<Step, Description> stepDescriptions = new HashMap<Step, Description>();
+    private final Description description;
 
-    public ExecutionUnitRunner(Runtime runtime, CucumberScenario cucumberScenario, JUnitReporter jUnitReporter) throws InitializationError {
-        super(ExecutionUnitRunner.class);
+    public ExecutionUnitRunner(Class<?> testClass, Runtime runtime, String name, CucumberScenario cucumberScenario, JUnitReporter jUnitReporter) throws InitializationError {
         this.runtime = runtime;
         this.cucumberScenario = cucumberScenario;
         this.jUnitReporter = jUnitReporter;
+        this.description = Description.createTestDescription(testClass, replaceParenthesis(name));
     }
 
-    @Override
-    protected List<Step> getChildren() {
-        return cucumberScenario.getSteps();
-    }
-
-    @Override
-    public String getName() {
-        return cucumberScenario.getVisualName();
+    // eclipse implementation can't live with parenthesis in descriptions
+    private static String replaceParenthesis(String name) {
+        return name.replace('(', '<').replace(')','>');
     }
 
     @Override
     public Description getDescription() {
-        if (description == null) {
-            description = Description.createSuiteDescription(getName(), cucumberScenario.getGherkinModel());
-
-            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));
-                }
-            }
-
-            for (Step step : getChildren()) {
-                description.addChild(describeChild(step));
-            }
-        }
-        return description;
-    }
-
-    @Override
-    protected Description describeChild(Step step) {
-        Description description = stepDescriptions.get(step);
-        if (description == null) {
-            description = Description.createTestDescription(getName(), step.getKeyword() + step.getName(), step);
-            stepDescriptions.put(step, description);
-        }
         return description;
     }
 
@@ -84,11 +43,4 @@ public void run(final RunNotifier notifier) {
         jUnitReporter.finishExecutionUnit();
     }
 
-    @Override
-    protected void runChild(Step step, RunNotifier notifier) {
-        // The way we override run(RunNotifier) causes this method to never be called.
-        // Instead it happens via cucumberScenario.run(jUnitReporter, jUnitReporter, runtime);
-        throw new UnsupportedOperationException();
-        // cucumberScenario.runStep(step, jUnitReporter, runtime);
-    }
 }