[Spring] Invoke all TestContextManager methods

To make writing tests with Spring easier Spring provides a `TestContextManager`. This classes provides call backs for various `TestExecutionListeners`. These are then used by various extensions such as the `MockitoTestExecutionListener` which injects `@MockBeans` into test instances. When all methods are not invoked this leads to problems such as (#2654,#2655,#2656) While this was initially (#1470) not a problem, it appears that various listener implementations have started to assume that all methods would be invoked. Closes: #2655 Fixes: #2654, #2572
cucumber · Dec 11, 2022 · 6ee38e0 · 6ee38e0
1 parent 9de55cf
commit 6ee38e0
Show file tree

Hide file tree

Showing 4 changed files with 313 additions and 67 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,25 +12,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Added
-- Enabled reproducible builds ([2641](https://github.com/cucumber/cucumber-jvm/issues/2641) Hervé Boutemy )
-- [Core] Mark Allure 5 and 6 plugins as incompatible ([2652](https://github.com/cucumber/cucumber-jvm/issues/2652) M.P. Korstanje)
+- Enabled reproducible builds ([#2641](https://github.com/cucumber/cucumber-jvm/issues/2641) Hervé Boutemy )
+- [Core] Mark Allure 5 and 6 plugins as incompatible ([#2652](https://github.com/cucumber/cucumber-jvm/issues/2652) M.P. Korstanje)
+- [Spring] Invoke all `TestContextManager` methods ([#2661](https://github.com/cucumber/cucumber-jvm/pull/2661) M.P. Korstanje)
 
 ## Fixed
--  [Core] Emit exceptions on failure to handle test run finished events ([2651](https://github.com/cucumber/cucumber-jvm/issues/2651) M.P. Korstanje)
+- [Core] Emit exceptions on failure to handle test run finished events ([#2651](https://github.com/cucumber/cucumber-jvm/issues/2651) M.P. Korstanje)
+- [Spring] @MockBean annotation not working with JUnit5 ([#2654](https://github.com/cucumber/cucumber-jvm/pull/2654) Alexander Kirilov, M.P. Korstanje)
 
 ## [7.9.0] - 2022-11-01
 ### Changed
 - [Core] Update dependency io.cucumber:gherkin to v25.0.2. Japanese Rule translation changed from Rule to ルール.
 
 ### Added
-- [Spring] Support @CucumberContextConfiguration as a meta-annotation ([2491](https://github.com/cucumber/cucumber-jvm/issues/2491) Michael Schlatt)
+- [Spring] Support @CucumberContextConfiguration as a meta-annotation ([#2491](https://github.com/cucumber/cucumber-jvm/issues/2491) Michael Schlatt)
 
 ### Changed
 - [Core] Update dependency io.cucumber:gherkin to v24.1
-- [Core] Delegate encoding and BOM handling to gherkin ([2624](https://github.com/cucumber/cucumber-jvm/issues/2624) M.P. Korstanje)
+- [Core] Delegate encoding and BOM handling to gherkin ([#2624](https://github.com/cucumber/cucumber-jvm/issues/2624) M.P. Korstanje)
 
 ### Fixed
-- [Core] Don't swallow parse errors on the CLI ([2632](https://github.com/cucumber/cucumber-jvm/issues/2632) M.P. Korstanje)
+- [Core] Don't swallow parse errors on the CLI ([#2632](https://github.com/cucumber/cucumber-jvm/issues/2632) M.P. Korstanje)
 
 ### Security
 - [Core] Update dependency com.fasterxml.jackson to v2.13.4.20221012

diff --git a/cucumber-spring/src/main/java/io/cucumber/spring/TestContextAdaptor.java b/cucumber-spring/src/main/java/io/cucumber/spring/TestContextAdaptor.java
@@ -10,9 +10,12 @@
 import org.springframework.test.context.TestContextManager;
 
 import java.lang.reflect.Method;
+import java.util.ArrayDeque;
 import java.util.Collection;
+import java.util.Deque;
 
 import static io.cucumber.spring.CucumberTestContext.SCOPE_CUCUMBER_GLUE;
+import static org.springframework.beans.factory.config.AutowireCapableBeanFactory.AUTOWIRE_NO;
 
 class TestContextAdaptor {
 
@@ -21,6 +24,7 @@ class TestContextAdaptor {
     private final TestContextManager delegate;
     private final ConfigurableApplicationContext applicationContext;
     private final Collection<Class<?>> glueClasses;
+    private final Deque<Runnable> stopInvocations = new ArrayDeque<>();
     private Object delegateTestInstance;
 
     TestContextAdaptor(
@@ -44,23 +48,70 @@ public final void start() {
             registerGlueCodeScope(applicationContext);
             registerStepClassBeanDefinitions(applicationContext.getBeanFactory());
         }
+        stopInvocations.push(this::notifyTestContextManagerAboutAfterTestClass);
         notifyContextManagerAboutBeforeTestClass();
-        CucumberTestContext.getInstance().start();
+        stopInvocations.push(this::stopCucumberTestContext);
+        startCucumberTestContext();
+        stopInvocations.push(this::disposeTestInstance);
+        createAndPrepareTestInstance();
+        stopInvocations.push(this::notifyTestContextManagerAboutAfterTestMethod);
         notifyTestContextManagerAboutBeforeTestMethod();
+        stopInvocations.push(this::notifyTestContextManagerAboutAfterTestExecution);
+        notifyTestContextManagerAboutBeforeExecution();
     }
 
-    private void notifyTestContextManagerAboutBeforeTestMethod() {
+    private void notifyContextManagerAboutBeforeTestClass() {
+        try {
+            delegate.beforeTestClass();
+        } catch (Exception e) {
+            throw new CucumberBackendException(e.getMessage(), e);
+        }
+    }
+
+    private void startCucumberTestContext() {
+        CucumberTestContext.getInstance().start();
+    }
+
+    private void createAndPrepareTestInstance() {
+        // Unlike JUnit, Cucumber does not have a single test class.
+        // Springs TestContext however assumes we do, and we are expected to
+        // create an instance of it using the default constructor.
+        //
+        // Users of Cucumber would however like to inject their step
+        // definition classes into other step definition classes. This requires
+        // that the test instance exists in the application context as a bean.
+        //
+        // Normally when a bean is pulled from the application context with
+        // getBean it is also autowired. This will however conflict with
+        // Springs DependencyInjectionTestExecutionListener. So we create
+        // a raw bean here.
+        //
+        // This probably free from side effects, but at some point in the
+        // future we may have to accept that the only way forward is to
+        // construct instances annotated with @CucumberContextConfiguration
+        // using their default constructor and now allow them to be injected
+        // into other step definition classes.
         try {
             Class<?> delegateTestClass = delegate.getTestContext().getTestClass();
-            delegateTestInstance = applicationContext.getBean(delegateTestClass);
-            Method dummyMethod = TestContextAdaptor.class.getMethod("cucumberDoesNotHaveASingleTestMethod");
+            Object delegateTestInstance = applicationContext.getBeanFactory().autowire(delegateTestClass, AUTOWIRE_NO,
+                false);
+            delegate.prepareTestInstance(delegateTestInstance);
+            this.delegateTestInstance = delegateTestInstance;
+        } catch (Exception e) {
+            throw new CucumberBackendException(e.getMessage(), e);
+        }
+    }
+
+    private void notifyTestContextManagerAboutBeforeTestMethod() {
+        try {
+            Method dummyMethod = getDummyMethod();
             delegate.beforeTestMethod(delegateTestInstance, dummyMethod);
         } catch (Exception e) {
             throw new CucumberBackendException(e.getMessage(), e);
         }
     }
 
-    final void registerGlueCodeScope(ConfigurableApplicationContext context) {
+    private void registerGlueCodeScope(ConfigurableApplicationContext context) {
         while (context != null) {
             ConfigurableListableBeanFactory beanFactory = context.getBeanFactory();
             // Scenario scope may have already been registered by another
@@ -73,15 +124,15 @@ final void registerGlueCodeScope(ConfigurableApplicationContext context) {
         }
     }
 
-    private void notifyContextManagerAboutBeforeTestClass() {
+    private void notifyTestContextManagerAboutBeforeExecution() {
         try {
-            delegate.beforeTestClass();
+            delegate.beforeTestExecution(delegateTestInstance, getDummyMethod());
         } catch (Exception e) {
             throw new CucumberBackendException(e.getMessage(), e);
         }
     }
 
-    final void registerStepClassBeanDefinitions(ConfigurableListableBeanFactory beanFactory) {
+    private void registerStepClassBeanDefinitions(ConfigurableListableBeanFactory beanFactory) {
         BeanDefinitionRegistry registry = (BeanDefinitionRegistry) beanFactory;
         for (Class<?> glue : glueClasses) {
             registerStepClassBeanDefinition(registry, glue);
@@ -102,18 +153,23 @@ private void registerStepClassBeanDefinition(BeanDefinitionRegistry registry, Cl
     }
 
     public final void stop() {
-        // Don't invoke after test method when before test class was not invoked
-        // this is implicit in the existence of an active the test context
-        // session. This is not ideal, but Cucumber only supports 1 set of
-        // before/after semantics while JUnit and Spring have 2 sets.
-        if (CucumberTestContext.getInstance().isActive()) {
-            if (delegateTestInstance != null) {
-                notifyTestContextManagerAboutAfterTestMethod();
-                delegateTestInstance = null;
+        // Cucumber only supports 1 set of before/after semantics while JUnit
+        // and Spring have 2 sets. So here we use a stack to ensure we don't
+        // invoke only the matching after methods for each before methods.
+        CucumberBackendException lastException = null;
+        for (Runnable stopInvocation : stopInvocations) {
+            try {
+                stopInvocation.run();
+            } catch (CucumberBackendException e) {
+                if (lastException != null) {
+                    e.addSuppressed(lastException);
+                }
+                lastException = e;
             }
-            CucumberTestContext.getInstance().stop();
         }
-        notifyTestContextManagerAboutAfterTestClass();
+        if (lastException != null) {
+            throw lastException;
+        }
     }
 
     private void notifyTestContextManagerAboutAfterTestClass() {
@@ -124,11 +180,35 @@ private void notifyTestContextManagerAboutAfterTestClass() {
         }
     }
 
+    private void stopCucumberTestContext() {
+        CucumberTestContext.getInstance().stop();
+    }
+
+    private void disposeTestInstance() {
+        delegateTestInstance = null;
+    }
+
     private void notifyTestContextManagerAboutAfterTestMethod() {
         try {
             Object delegateTestInstance = delegate.getTestContext().getTestInstance();
-            Method dummyMethod = TestContextAdaptor.class.getMethod("cucumberDoesNotHaveASingleTestMethod");
-            delegate.afterTestMethod(delegateTestInstance, dummyMethod, null);
+            // Cucumber tests can throw exceptions, but we can't currently
+            // get at them. So we provide null intentionally.
+            // Cucumber also doesn't a single test method, so we provide a
+            // dummy instead.
+            delegate.afterTestMethod(delegateTestInstance, getDummyMethod(), null);
+        } catch (Exception e) {
+            throw new CucumberBackendException(e.getMessage(), e);
+        }
+    }
+
+    private void notifyTestContextManagerAboutAfterTestExecution() {
+        try {
+            Object delegateTestInstance = delegate.getTestContext().getTestInstance();
+            // Cucumber tests can throw exceptions, but we can't currently
+            // get at them. So we provide null intentionally.
+            // Cucumber also doesn't a single test method, so we provide a
+            // dummy instead.
+            delegate.afterTestExecution(delegateTestInstance, getDummyMethod(), null);
         } catch (Exception e) {
             throw new CucumberBackendException(e.getMessage(), e);
         }
@@ -138,6 +218,14 @@ final <T> T getInstance(Class<T> type) {
         return applicationContext.getBean(type);
     }
 
+    private Method getDummyMethod() {
+        try {
+            return TestContextAdaptor.class.getMethod("cucumberDoesNotHaveASingleTestMethod");
+        } catch (NoSuchMethodException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
     public void cucumberDoesNotHaveASingleTestMethod() {
 
     }

diff --git a/cucumber-spring/src/test/java/io/cucumber/spring/SpringFactoryTest.java b/cucumber-spring/src/test/java/io/cucumber/spring/SpringFactoryTest.java
@@ -347,15 +347,10 @@ void shouldBeStoppableWhenFacedWithMissingContextConfiguration() {
         assertDoesNotThrow(factory::stop);
     }
 
-    @ParameterizedTest
-    @ValueSource(classes = {
-            FailedBeforeTestClassContextConfiguration.class,
-            FailedBeforeTestMethodContextConfiguration.class,
-            FailedTestInstanceContextConfiguration.class
-    })
-    void shouldBeStoppableWhenFacedWithFailedApplicationContext(Class<?> contextConfiguration) {
+    @Test
+    void shouldBeStoppableWhenFacedWithFailedApplicationContext() {
         final ObjectFactory factory = new SpringFactory();
-        factory.addClass(contextConfiguration);
+        factory.addClass(FailedTestInstanceCreation.class);
 
         assertThrows(CucumberBackendException.class, factory::start);
         assertDoesNotThrow(factory::stop);
@@ -414,40 +409,9 @@ public static class WithoutContextConfiguration {
 
     @CucumberContextConfiguration
     @ContextConfiguration("classpath:cucumber.xml")
-    @TestExecutionListeners(FailedBeforeTestClassContextConfiguration.FailingListener.class)
-    public static class FailedBeforeTestClassContextConfiguration {
-
-        public static class FailingListener implements TestExecutionListener {
-
-            @Override
-            public void beforeTestClass(TestContext testContext) throws Exception {
-                throw new StubException();
-            }
-
-        }
-
-    }
-
-    @CucumberContextConfiguration
-    @ContextConfiguration("classpath:cucumber.xml")
-    @TestExecutionListeners(FailedBeforeTestMethodContextConfiguration.FailingListener.class)
-    public static class FailedBeforeTestMethodContextConfiguration {
-
-        public static class FailingListener implements TestExecutionListener {
-
-            @Override
-            public void beforeTestMethod(TestContext testContext) throws Exception {
-                throw new StubException();
-            }
-
-        }
-
-    }
-    @CucumberContextConfiguration
-    @ContextConfiguration("classpath:cucumber.xml")
-    public static class FailedTestInstanceContextConfiguration {
+    public static class FailedTestInstanceCreation {
 
-        public FailedTestInstanceContextConfiguration() {
+        public FailedTestInstanceCreation() {
             throw new RuntimeException();
         }
     }