[Spring] Cleanly stop after failure to start application context

Fixes: #2569

Author: mpkorstanje

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ## [Unreleased] (In Git)
 
 ### Added
+* [Core] Warn when glue path is passed as file scheme instead of classpath ([#2547](https://github.com/cucumber/cucumber-jvm/pull/2547) M.P. Korstanje)
 
 ### Changed
 
@@ -16,12 +17,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Removed
 
 ### Fixed
+* [Spring] Cleanly stop after failure to start application context
 
-## [7.3.5] (2022-05-10)
-
-### Added
-* [Core] Warn when glue path is passed as file scheme instead of classpath ([#2547](https://github.com/cucumber/cucumber-jvm/pull/2547) M.P. Korstanje)
- 
 ## [7.3.4] (2022-05-02)
 
 ### Fixed
@@ -1793,16 +1790,17 @@ in `cucumber.api` stable from now on, with proper deprecation warnings in case s
 
 <!-- Releases -->
 [Unreleased]: https://github.com/cucumber/cucumber-jvm/compare/v7.3.4...main
-[7.3.4]:  https://github.com/cucumber/cucumber-jvm/compare/v7.3.3-RC1...v7.3.4
-[7.3.3]:  https://github.com/cucumber/cucumber-jvm/compare/v7.3.2-RC1...v7.3.3
-[7.3.2]:  https://github.com/cucumber/cucumber-jvm/compare/v7.3.1-RC1...v7.3.2
-[7.3.1]:  https://github.com/cucumber/cucumber-jvm/compare/v7.3.0-RC1...v7.3.1
-[7.3.0]:  https://github.com/cucumber/cucumber-jvm/compare/v7.2.3-RC1...v7.3.0
-[7.2.3]:  https://github.com/cucumber/cucumber-jvm/compare/v7.2.2-RC1...v7.2.3
-[7.2.2]:  https://github.com/cucumber/cucumber-jvm/compare/v7.2.1-RC1...v7.2.2
-[7.2.1]:  https://github.com/cucumber/cucumber-jvm/compare/v7.2.0-RC1...v7.2.1
-[7.2.0]:  https://github.com/cucumber/cucumber-jvm/compare/v7.1.0-RC1...v7.2.0
-[7.1.0]:  https://github.com/cucumber/cucumber-jvm/compare/v7.0.0-RC1...v7.1.0
+[7.3.4]:  https://github.com/cucumber/cucumber-jvm/compare/v7.3.3...v7.3.4
+[7.3.4]:  https://github.com/cucumber/cucumber-jvm/compare/v7.3.3...v7.3.4
+[7.3.3]:  https://github.com/cucumber/cucumber-jvm/compare/v7.3.2...v7.3.3
+[7.3.2]:  https://github.com/cucumber/cucumber-jvm/compare/v7.3.1...v7.3.2
+[7.3.1]:  https://github.com/cucumber/cucumber-jvm/compare/v7.3.0...v7.3.1
+[7.3.0]:  https://github.com/cucumber/cucumber-jvm/compare/v7.2.3...v7.3.0
+[7.2.3]:  https://github.com/cucumber/cucumber-jvm/compare/v7.2.2...v7.2.3
+[7.2.2]:  https://github.com/cucumber/cucumber-jvm/compare/v7.2.1...v7.2.2
+[7.2.1]:  https://github.com/cucumber/cucumber-jvm/compare/v7.2.0...v7.2.1
+[7.2.0]:  https://github.com/cucumber/cucumber-jvm/compare/v7.1.0...v7.2.0
+[7.1.0]:  https://github.com/cucumber/cucumber-jvm/compare/v7.0.0...v7.1.0
 [7.0.0]:  https://github.com/cucumber/cucumber-jvm/compare/v7.0.0-RC1...v7.0.0
 [7.0.0-RC1]:  https://github.com/cucumber/cucumber-jvm/compare/v6.11.0...v7.0.0-RC1
 [6.11.0]:  https://github.com/cucumber/cucumber-jvm/compare/v6.10.4...v6.11.0

spring/src/main/java/io/cucumber/spring/CucumberTestContext.java

@@ -66,10 +66,14 @@ void registerDestructionCallback(String name, Runnable callback) {
     }
 
     void requireActiveScenario() {
-        if (sessionId == null) {
+        if (!isActive()) {
             throw new IllegalStateException(
                 "Scenario scoped beans can only be created while Cucumber is executing a scenario");
         }
     }
 
+    boolean isActive() {
+        return sessionId != null;
+    }
+
 }

spring/src/main/java/io/cucumber/spring/TestContextAdaptor.java

@@ -50,10 +50,10 @@ public final void start() {
 
     private void notifyTestContextManagerAboutBeforeTestMethod() {
         try {
-            Class<?> testClass = delegate.getTestContext().getTestClass();
-            Object testContextInstance = applicationContext.getBean(testClass);
+            Class<?> delegateTestClass = delegate.getTestContext().getTestClass();
+            Object delegateTestInstance = applicationContext.getBean(delegateTestClass);
             Method dummyMethod = TestContextAdaptor.class.getMethod("cucumberDoesNotHaveASingleTestMethod");
-            delegate.beforeTestMethod(testContextInstance, dummyMethod);
+            delegate.beforeTestMethod(delegateTestInstance, dummyMethod);
         } catch (Exception e) {
             throw new CucumberBackendException(e.getMessage(), e);
         }
@@ -101,8 +101,14 @@ private void registerStepClassBeanDefinition(BeanDefinitionRegistry registry, Cl
     }
 
     public final void stop() {
-        notifyTestContextManagerAboutAfterTestMethod();
-        CucumberTestContext.getInstance().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()) {
+            notifyTestContextManagerAboutAfterTestMethod();
+            CucumberTestContext.getInstance().stop();
+        }
         notifyTestContextManagerAboutAfterTestClass();
     }
 
@@ -116,10 +122,9 @@ private void notifyTestContextManagerAboutAfterTestClass() {
 
     private void notifyTestContextManagerAboutAfterTestMethod() {
         try {
-            Class<?> testClass = delegate.getTestContext().getTestClass();
-            Object testContextInstance = applicationContext.getBean(testClass);
+            Object delegateTestInstance = delegate.getTestContext().getTestInstance();
             Method dummyMethod = TestContextAdaptor.class.getMethod("cucumberDoesNotHaveASingleTestMethod");
-            delegate.afterTestMethod(testContextInstance, dummyMethod, null);
+            delegate.afterTestMethod(delegateTestInstance, dummyMethod, null);
         } catch (Exception e) {
             throw new CucumberBackendException(e.getMessage(), e);
         }

spring/src/test/java/io/cucumber/spring/SpringFactoryTest.java

@@ -2,6 +2,7 @@
 
 import io.cucumber.core.backend.CucumberBackendException;
 import io.cucumber.core.backend.ObjectFactory;
+import io.cucumber.spring.SpringFactoryTest.FailedContextConfiguration.FailingListener;
 import io.cucumber.spring.beans.Belly;
 import io.cucumber.spring.beans.BellyBean;
 import io.cucumber.spring.beans.DummyComponent;
@@ -22,6 +23,9 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.TestContext;
+import org.springframework.test.context.TestExecutionListener;
+import org.springframework.test.context.TestExecutionListeners;
 
 import java.util.Optional;
 
@@ -341,6 +345,15 @@ void shouldBeStoppableWhenFacedWithMissingContextConfiguration() {
         assertDoesNotThrow(factory::stop);
     }
 
+    @Test
+    void shouldBeStoppableWhenFacedWithFailedApplicationContext() {
+        final ObjectFactory factory = new SpringFactory();
+        factory.addClass(FailedContextConfiguration.class);
+
+        assertThrows(RuntimeException.class, factory::start);
+        assertDoesNotThrow(factory::stop);
+    }
+
     @CucumberContextConfiguration
     @ContextConfiguration("classpath:cucumber.xml")
     public static class WithSpringAnnotations {
@@ -366,7 +379,7 @@ public String getProperty() {
     }
 
     @CucumberContextConfiguration
-    @ContextConfiguration()
+    @ContextConfiguration
     public static class WithEmptySpringAnnotations {
 
     }
@@ -376,4 +389,20 @@ public static class WithoutContextConfiguration {
 
     }
 
+    @CucumberContextConfiguration
+    @ContextConfiguration("classpath:cucumber.xml")
+    @TestExecutionListeners(FailingListener.class)
+    public static class FailedContextConfiguration {
+
+        public static class FailingListener implements TestExecutionListener {
+
+            @Override
+            public void beforeTestClass(TestContext testContext) {
+                throw new RuntimeException("Boom");
+            }
+
+        }
+
+    }
+
 }