[Spring] Throw exception when step definitions are annotated with component

mpkorstanje · mpkorstanje · commit db8eeaaf6eb4 · 2017-10-11T15:29:54.000+02:00
When step definitions are annotated with @component or other related annotations they can be picked up by springs class path scanning. This conflicts with cucumbers class path scanning and may result in multiple bean definitions for the same class. This problem is hard to understand and hard to trace. By making the problem explicit and providing a clear instruction on how to resolve this we can hopefully avoid future confusion. The current implementation only checks the a subset of all annotations. This will hopefully be sufficient. Closes #1225
diff --git a/core/pom.xml b/core/pom.xml
@@ -41,7 +41,7 @@
         </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
+            <artifactId>mockito-core</artifactId>
             <scope>test</scope>
         </dependency>
         <dependency>
diff --git a/gosu/pom.xml b/gosu/pom.xml
@@ -53,10 +53,5 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
-            <scope>test</scope>
-        </dependency>
     </dependencies>
 </project>
diff --git a/groovy/pom.xml b/groovy/pom.xml
@@ -46,7 +46,7 @@
         </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
+            <artifactId>mockito-core</artifactId>
             <scope>test</scope>
         </dependency>
     </dependencies>
diff --git a/guice/pom.xml b/guice/pom.xml
@@ -42,10 +42,5 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
-            <scope>test</scope>
-        </dependency>
     </dependencies>
 </project>
diff --git a/java/pom.xml b/java/pom.xml
@@ -29,7 +29,7 @@
         </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
+            <artifactId>mockito-core</artifactId>
             <scope>test</scope>
         </dependency>
     </dependencies>
diff --git a/junit/pom.xml b/junit/pom.xml
@@ -33,7 +33,7 @@
 
         <dependency>
             <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
+            <artifactId>mockito-core</artifactId>
             <scope>test</scope>
         </dependency>
     </dependencies>
diff --git a/jython/pom.xml b/jython/pom.xml
@@ -35,11 +35,6 @@
             <artifactId>cucumber-junit</artifactId>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
-            <scope>test</scope>
-        </dependency>
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
diff --git a/needle/pom.xml b/needle/pom.xml
@@ -42,7 +42,7 @@
         </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
+            <artifactId>mockito-core</artifactId>
             <scope>test</scope>
         </dependency>
         <dependency>
diff --git a/osgi/pom.xml b/osgi/pom.xml
@@ -37,11 +37,6 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
-            <scope>test</scope>
-        </dependency>
         <dependency>
             <groupId>org.osgi</groupId>
             <artifactId>org.osgi.core</artifactId>
diff --git a/pom.xml b/pom.xml
@@ -44,6 +44,7 @@
         <!-- TestNG 6.11 makes the tests of the testng module to fail -->
         <testng.version>6.10</testng.version>
         <junit.version>4.12</junit.version>
+        <hamcrest.version>1.3</hamcrest.version>
         <jython.version>2.7.1</jython.version>
         <mockito.version>1.10.19</mockito.version>
         <selenium.version>3.5.2</selenium.version>
@@ -349,6 +350,11 @@
                 <artifactId>junit</artifactId>
                 <version>${junit.version}</version>
             </dependency>
+            <dependency>
+                <groupId>org.hamcrest</groupId>
+                <artifactId>hamcrest-core</artifactId>
+                <version>${hamcrest.version}</version>
+            </dependency>
             <dependency>
                 <groupId>org.testng</groupId>
                 <artifactId>testng</artifactId>
@@ -359,15 +365,16 @@
                 <artifactId>jython-standalone</artifactId>
                 <version>${jython.version}</version>
             </dependency>
-            <dependency>
-                <groupId>org.mockito</groupId>
-                <artifactId>mockito-all</artifactId>
-                <version>${mockito.version}</version>
-            </dependency>
             <dependency>
                 <groupId>org.mockito</groupId>
                 <artifactId>mockito-core</artifactId>
                 <version>${mockito.version}</version>
+                <exclusions>
+                    <exclusion>
+                        <groupId>org.hamcrest</groupId>
+                        <artifactId>hamcrest-core</artifactId>
+                    </exclusion>
+                </exclusions>
             </dependency>
             <dependency>
                 <groupId>org.jsoup</groupId>
diff --git a/rhino/pom.xml b/rhino/pom.xml
@@ -44,7 +44,7 @@
         </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
+            <artifactId>mockito-core</artifactId>
             <scope>test</scope>
         </dependency>
     </dependencies>
diff --git a/spring/pom.xml b/spring/pom.xml
@@ -69,7 +69,7 @@
         </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
+            <artifactId>mockito-core</artifactId>
             <scope>test</scope>
         </dependency>
         <dependency>
diff --git a/spring/src/main/java/cucumber/runtime/java/spring/SpringFactory.java b/spring/src/main/java/cucumber/runtime/java/spring/SpringFactory.java
@@ -13,34 +13,42 @@
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import org.springframework.context.support.GenericApplicationContext;
+import org.springframework.stereotype.Component;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.ContextHierarchy;
 import org.springframework.test.context.TestContextManager;
 
 import java.lang.annotation.Annotation;
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.Set;
+import java.util.Stack;
 
 /**
  * Spring based implementation of ObjectFactory.
  * <p/>
  * <p>
  * <ul>
  * <li>It uses TestContextManager to manage the spring context.
- * Configuration via: @ContextConfiguration or @ContextHierarcy
- * At least on step definition class needs to have a @ContextConfiguration or
- * @ContextHierarchy annotation. If more that one step definition class has such
+ * Configuration via: @{@link ContextConfiguration} or @{@link ContextHierarchy}
+ * At least one step definition class needs to have a @ContextConfiguration
+ * or @ContextHierarchy annotation. If more that one step definition class has such
  * an annotation, the annotations must be equal on the different step definition
- * classes. If no step definition class with @ContextConfiguration or
- * @ContextHierarcy is found, it will try to load cucumber.xml from the classpath.
+ * classes. If no step definition class with @ContextConfiguration or @ContextHierarchy
+ * is found, it will try to load cucumber.xml from the classpath.
  * </li>
  * <li>The step definitions class with @ContextConfiguration or @ContextHierarchy
- * annotation, may also have a @WebAppConfiguration or @DirtiesContext annotation.
+ * annotation, may also have a @{@link org.springframework.test.context.web.WebAppConfiguration}
+ * or @{@link org.springframework.test.annotation.DirtiesContext} annotation.
  * </li>
- * <li>The step definitions added to the TestContextManagers context and
+ * <li>The step definitions are added to the TestContextManagers context and
  * is reloaded for each scenario.</li>
+ * <li>Step definitions  should not be annotated with @{@link Component} or
+ * other annotations that mark it as eligible for detection by classpath scanning.</li>
+ * <li>When a step definition class is annotated by @Component or an annotation that has the @Component stereotype an
+ * exception will be thrown</li>
+ * </li>
  * </ul>
- * </p>
  * <p/>
  * <p>
  * Application beans are accessible from the step definitions using autowiring
@@ -61,6 +69,7 @@ public SpringFactory() {
     @Override
     public boolean addClass(final Class<?> stepClass) {
         if (!stepClasses.contains(stepClass)) {
+            checkNoComponentAnnotations(stepClass);
             if (dependsOnSpringContext(stepClass)) {
                 if (stepClassWithSpringContext == null) {
                     stepClassWithSpringContext = stepClass;
@@ -73,6 +82,42 @@ public boolean addClass(final Class<?> stepClass) {
         return true;
     }
 
+    private static void checkNoComponentAnnotations(Class<?> type) {
+        for (Annotation annotation : type.getAnnotations()) {
+            if (hasComponentStereoType(annotation)) {
+                throw new CucumberException(String.format("" +
+                        "Glue class %1$s was annotated with @%2$s; marking it as a candidate for auto-detection by " +
+                        "Spring. Glue classes are detected and registered by Cucumber. Auto-detection of glue classes by " +
+                        "spring may lead to duplicate bean definitions. Please remove the @%2$s annotation",
+                    type.getName(),
+                    annotation.annotationType().getSimpleName()));
+            }
+        }
+    }
+
+    private static boolean hasComponentStereoType(Annotation annotation) {
+        Set<Class<? extends Annotation>> seen = new HashSet<Class<? extends Annotation>>();
+        Stack<Class<? extends Annotation>> toCheck = new Stack<Class<? extends Annotation>>();
+        toCheck.add(annotation.annotationType());
+
+        while (!toCheck.isEmpty()) {
+            Class<? extends Annotation> annotationType = toCheck.pop();
+            if (Component.class.equals(annotationType)) {
+                return true;
+            }
+
+            seen.add(annotationType);
+            for (Annotation annotationTypesAnnotations : annotationType.getAnnotations()) {
+                if (!seen.contains(annotationTypesAnnotations.annotationType())) {
+                    toCheck.add(annotationTypesAnnotations.annotationType());
+                }
+            }
+
+        }
+        return false;
+    }
+
+
     private void checkAnnotationsEqual(Class<?> stepClassWithSpringContext, Class<?> stepClass) {
         Annotation[] annotations1 = stepClassWithSpringContext.getAnnotations();
         Annotation[] annotations2 = stepClass.getAnnotations();
diff --git a/spring/src/test/java/cucumber/runtime/java/spring/SpringFactoryTest.java b/spring/src/test/java/cucumber/runtime/java/spring/SpringFactoryTest.java
@@ -7,14 +7,18 @@
 import cucumber.runtime.java.spring.commonglue.AutowiresThirdStepDef;
 import cucumber.runtime.java.spring.commonglue.OneStepDef;
 import cucumber.runtime.java.spring.commonglue.ThirdStepDef;
+import cucumber.runtime.java.spring.componentannotation.WithComponentAnnotation;
+import cucumber.runtime.java.spring.componentannotation.WithControllerAnnotation;
 import cucumber.runtime.java.spring.metaconfig.general.BellyMetaStepdefs;
 import cucumber.runtime.java.spring.contextconfig.BellyStepdefs;
 import cucumber.runtime.java.spring.contextconfig.WithSpringAnnotations;
 import cucumber.runtime.java.spring.contexthierarchyconfig.WithContextHierarchyAnnotation;
 import cucumber.runtime.java.spring.contexthierarchyconfig.WithDifferentContextHierarchyAnnotation;
 import cucumber.runtime.java.spring.dirtiescontextconfig.DirtiesContextBellyStepDefs;
 import cucumber.runtime.java.spring.metaconfig.dirties.DirtiesContextBellyMetaStepDefs;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.springframework.transaction.PlatformTransactionManager;
 
 import static org.junit.Assert.assertEquals;
@@ -25,6 +29,9 @@
 
 public class SpringFactoryTest {
 
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
     @Test
     public void shouldGiveUsNewStepInstancesForEachScenario() {
         final ObjectFactory factory = new SpringFactory();
@@ -241,10 +248,31 @@ public void shouldAllowClassesWithSameSpringAnnotations() {
         factory.addClass(BellyStepdefs.class);
     }
 
-    @Test(expected=CucumberException.class)
+    @Test
     public void shouldFailIfClassesWithDifferentSpringAnnotationsAreFound() {
+        expectedException.expect(CucumberException.class);
+        expectedException.expectMessage("Annotations differs on glue classes found: cucumber.runtime.java.spring.contexthierarchyconfig.WithContextHierarchyAnnotation, cucumber.runtime.java.spring.contexthierarchyconfig.WithDifferentContextHierarchyAnnotation");
         final ObjectFactory factory = new SpringFactory();
         factory.addClass(WithContextHierarchyAnnotation.class);
         factory.addClass(WithDifferentContextHierarchyAnnotation.class);
     }
+
+    @Test
+    public void shouldFailIfClassWithSpringComponentAnnotationsIsFound() {
+        expectedException.expect(CucumberException.class);
+        expectedException.expectMessage("Glue class cucumber.runtime.java.spring.componentannotation.WithComponentAnnotation was annotated with @Component");
+        expectedException.expectMessage("Please remove the @Component annotation");
+        final ObjectFactory factory = new SpringFactory();
+        factory.addClass(WithComponentAnnotation.class);
+    }
+
+    @Test
+    public void shouldFailIfClassWithAnnotationAnnotatedWithSpringComponentAnnotationsIsFound() {
+        expectedException.expect(CucumberException.class);
+        expectedException.expectMessage("Glue class cucumber.runtime.java.spring.componentannotation.WithControllerAnnotation was annotated with @Controller");
+        expectedException.expectMessage("Please remove the @Controller annotation");
+        final ObjectFactory factory = new SpringFactory();
+        factory.addClass(WithControllerAnnotation.class);
+
+    }
 }
diff --git a/spring/src/test/java/cucumber/runtime/java/spring/componentannotation/WithComponentAnnotation.java b/spring/src/test/java/cucumber/runtime/java/spring/componentannotation/WithComponentAnnotation.java
@@ -0,0 +1,23 @@
+package cucumber.runtime.java.spring.componentannotation;
+
+import cucumber.runtime.java.spring.beans.DummyComponent;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.ContextHierarchy;
+
+@Component
+public class WithComponentAnnotation {
+
+    private boolean autowired;
+
+    @Autowired
+    public void setAutowiredCollaborator(DummyComponent collaborator) {
+        autowired = true;
+    }
+
+    public boolean isAutowired() {
+        return autowired;
+    }
+
+}
diff --git a/spring/src/test/java/cucumber/runtime/java/spring/componentannotation/WithControllerAnnotation.java b/spring/src/test/java/cucumber/runtime/java/spring/componentannotation/WithControllerAnnotation.java
@@ -0,0 +1,22 @@
+package cucumber.runtime.java.spring.componentannotation;
+
+import cucumber.runtime.java.spring.beans.DummyComponent;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Controller;
+import org.springframework.stereotype.Repository;
+
+@Controller
+public class WithControllerAnnotation {
+
+    private boolean autowired;
+
+    @Autowired
+    public void setAutowiredCollaborator(DummyComponent collaborator) {
+        autowired = true;
+    }
+
+    public boolean isAutowired() {
+        return autowired;
+    }
+
+}
diff --git a/testng/pom.xml b/testng/pom.xml
@@ -22,7 +22,12 @@
         </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.hamcrest</groupId>
+            <artifactId>hamcrest-core</artifactId>
             <scope>test</scope>
         </dependency>
     </dependencies>
