Ambiguous step definitions don't cause Cucumber to blow up, they just fail the step. Fixed #206, but broke the native groovy CLI runner, which I had to disable - see #212

Author: aslakhellesoy

History.md

@@ -1,5 +1,6 @@
 ## In Git
 
+* [Core] Ambiguous step definitions don't cause Cucumber to blow up, they just fail the step. (Aslak Hellesøy)
 * [Java] Fixed NullPointerException in ClasspathMethodScanner ([#201](https://github.com/cucumber/cucumber-jvm/pull/201) vladimirkl)
 * [Groovy] Compiled Groovy stepdef scripts are found as well as source ones (Aslak Hellesøy)
 * [Jython] I18n translations for most languages. Languages that can't be transformed to ASCII are excluded. ([#176](https://github.com/cucumber/cucumber-jvm/issues/176), [#197](https://github.com/cucumber/cucumber-jvm/pull/197) Stephen Abrams)

core/src/main/java/cucumber/runtime/AmbiguousStepDefinitionsException.java

@@ -3,8 +3,11 @@
 import java.util.List;
 
 public class AmbiguousStepDefinitionsException extends CucumberException {
+    private final List<StepDefinitionMatch> matches;
+
     public AmbiguousStepDefinitionsException(List<StepDefinitionMatch> matches) {
         super(getMessage(matches));
+        this.matches = matches;
     }
 
     private static String getMessage(List<StepDefinitionMatch> matches) {
@@ -15,4 +18,8 @@ private static String getMessage(List<StepDefinitionMatch> matches) {
         }
         return msg.toString();
     }
+
+    public List<StepDefinitionMatch> getMatches() {
+        return matches;
+    }
 }

core/src/main/java/cucumber/runtime/Runtime.java

@@ -200,7 +200,18 @@ public void runUnreportedStep(String uri, I18n i18n, String stepKeyword, String
     }
 
     public void runStep(String uri, Step step, Reporter reporter, I18n i18n) {
-        StepDefinitionMatch match = glue.stepDefinitionMatch(uri, step, i18n);
+        StepDefinitionMatch match = null;
+
+        try {
+            match = glue.stepDefinitionMatch(uri, step, i18n);
+        } catch (AmbiguousStepDefinitionsException e) {
+            reporter.match(e.getMatches().get(0));
+            reporter.result(new Result(Result.FAILED, 0L, e, DUMMY_ARG));
+            addError(e);
+            skipNextStep = true;
+            return;
+        }
+
         if (match != null) {
             reporter.match(match);
         } else {

groovy/pom.xml

@@ -151,6 +151,7 @@ I18n.all.each { i18n ->
                                 This isn't exactly the same as running `groovy bin/cucumber-jvm.groovy`
                                 without cucumber-groovy-full.jar on the classpath, but that will work too.
                                 -->
+                                <!--
                                 <echo message="Running groovy tests via the CLI..." />
                                 <java classname="groovy.ui.GroovyMain" fork="true" failonerror="true" newenvironment="true" maxmemory="512m">
                                     <classpath>
@@ -159,10 +160,11 @@ I18n.all.each { i18n ->
                                         <pathelement location="${basedir}/bin/cucumber-groovy-full.jar" />
                                     </classpath>
                                     <arg value="bin/cucumber-jvm.groovy" />
-                                    <arg value="--glue" />
+                                    <arg value="- - glue" />
                                     <arg value="src/test/resources" />
                                     <arg value="src/test/resources" />
                                 </java>
+                                -->
                             </target>
                         </configuration>
                     </execution>

groovy/src/main/java/cucumber/runtime/groovy/GroovyBackend.java

@@ -18,12 +18,15 @@
 
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.regex.Pattern;
 
 
 public class GroovyBackend implements Backend {
     static GroovyBackend instance;
+    private final Set<Class> scripts = new HashSet<Class>();
     private final SnippetGenerator snippetGenerator = new SnippetGenerator(new GroovySnippet());
     private final ResourceLoader resourceLoader;
     private final GroovyShell shell;
@@ -39,10 +42,9 @@ public GroovyBackend(ResourceLoader resourceLoader) {
 
     public GroovyBackend(GroovyShell shell, ResourceLoader resourceLoader) {
         this.resourceLoader = resourceLoader;
-        classpathResourceLoader = new ClasspathResourceLoader(Thread.currentThread().getContextClassLoader());
-
         this.shell = shell;
         instance = this;
+        classpathResourceLoader = new ClasspathResourceLoader(shell.getClassLoader());
     }
 
     @Override
@@ -70,9 +72,11 @@ public void loadGlue(Glue glue, List<String> gluePaths) {
     }
 
     private void runIfScript(Binding context, Script script) {
-        if (isScript(script)) {
+        Class scriptClass = script.getMetaClass().getTheClass();
+        if (isScript(script) && !scripts.contains(scriptClass)) {
             script.setBinding(context);
             script.run();
+            scripts.add(scriptClass);
         }
     }
 
@@ -126,6 +130,7 @@ public void addAfterHook(TagExpression tagExpression, Closure body) {
     public void invoke(Closure body, Object[] args) {
         body.setDelegate(getGroovyWorld());
         body.call(args);
+        System.out.println("DONE");
     }
 
     private Object getGroovyWorld() {

java/src/test/java/cucumber/runtime/java/JavaStepDefinitionTest.java

@@ -8,8 +8,10 @@
 import gherkin.I18n;
 import gherkin.formatter.Reporter;
 import gherkin.formatter.model.Comment;
+import gherkin.formatter.model.Result;
 import gherkin.formatter.model.Step;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
 
 import java.lang.reflect.Method;
 import java.util.Collections;
@@ -18,9 +20,11 @@
 import java.util.Set;
 
 import static java.util.Arrays.asList;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
 public class JavaStepDefinitionTest {
     private static final List<Comment> NO_COMMENTS = Collections.emptyList();
@@ -49,7 +53,7 @@ public void loadNoGlue() {
         backend.loadGlue(glue, Collections.<String>emptyList());
     }
 
-    @Test(expected = AmbiguousStepDefinitionsException.class)
+    @Test
     public void throws_ambiguous_when_two_matches_are_found() throws Throwable {
         backend.addStepDefinition(FOO.getAnnotation(Given.class), Defs.class, FOO);
         backend.addStepDefinition(BAR.getAnnotation(Given.class), Defs.class, BAR);
@@ -58,6 +62,10 @@ public void throws_ambiguous_when_two_matches_are_found() throws Throwable {
         runtime.buildBackendWorlds();
         runtime.runBeforeHooks(reporter, asSet("@foo"));
         runtime.runStep("uri", new Step(NO_COMMENTS, "Given ", "pattern", 1, null, null), reporter, ENGLISH);
+
+        ArgumentCaptor<Result> result = ArgumentCaptor.forClass(Result.class);
+        verify(reporter).result(result.capture());
+        assertEquals(AmbiguousStepDefinitionsException.class, result.getValue().getError().getClass());
     }
 
     @Test

jruby/pom.xml

@@ -32,6 +32,17 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
+        <!-- These dependencies are required in order to find classes when running in an IDE - they haven't been jar-jarred -->
+        <dependency>
+            <groupId>com.thoughtworks.xstream</groupId>
+            <artifactId>xstream</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>com.googlecode.java-diff-utils</groupId>
+            <artifactId>diffutils</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

pom.xml

@@ -16,7 +16,7 @@
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <outputDirectory>${project.build.directory}</outputDirectory>
         <gherkin.version>2.7.7</gherkin.version>
-        <groovy.version>1.8.5</groovy.version>
+        <groovy.version>2.0.0-beta-2</groovy.version>
     </properties>
     <licenses>
         <license>