cucumber · mpkorstanje · Jun 19, 2022 · Jun 13, 2022
diff --git a/CHANGELOG.md b/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

diff --git a/spring/src/main/java/io/cucumber/spring/CucumberTestContext.java b/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;
+    }
+
 }
diff --git a/spring/src/main/java/io/cucumber/spring/TestContextAdaptor.java b/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);
         }

diff --git a/spring/src/test/java/io/cucumber/spring/SpringFactoryTest.java b/spring/src/test/java/io/cucumber/spring/SpringFactoryTest.java
@@ -18,10 +18,15 @@
 import io.cucumber.spring.metaconfig.general.BellyMetaStepDefinitions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.function.Executable;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.springframework.beans.factory.BeanCreationException;
 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;
 
@@ -35,7 +40,6 @@
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -57,10 +61,10 @@ void shouldGiveUsNewStepInstancesForEachScenario() {
         factory.stop();
 
         assertAll(
-            () -> assertThat(o1, is(notNullValue())),
-            () -> assertThat(o2, is(notNullValue())),
-            () -> assertThat(o1, is(not(equalTo(o2)))),
-            () -> assertThat(o2, is(not(equalTo(o1)))));
+                () -> assertThat(o1, is(notNullValue())),
+                () -> assertThat(o2, is(notNullValue())),
+                () -> assertThat(o1, is(not(equalTo(o2)))),
+                () -> assertThat(o2, is(not(equalTo(o1)))));
     }
 
     @Test
@@ -101,10 +105,10 @@ void shouldNeverCreateNewApplicationBeanInstances() {
         factory2.stop();
 
         assertAll(
-            () -> assertThat(o1, is(notNullValue())),
-            () -> assertThat(o2, is(notNullValue())),
-            () -> assertThat(o1, is(equalTo(o1))),
-            () -> assertThat(o2, is(equalTo(o2))));
+                () -> assertThat(o1, is(notNullValue())),
+                () -> assertThat(o2, is(notNullValue())),
+                () -> assertThat(o1, is(equalTo(o1))),
+                () -> assertThat(o2, is(equalTo(o2))));
     }
 
     @Test
@@ -124,10 +128,10 @@ void shouldNeverCreateNewApplicationBeanInstancesUsingMetaConfiguration() {
         factory2.stop();
 
         assertAll(
-            () -> assertThat(o1, is(notNullValue())),
-            () -> assertThat(o2, is(notNullValue())),
-            () -> assertThat(o1, is(equalTo(o1))),
-            () -> assertThat(o2, is(equalTo(o2))));
+                () -> assertThat(o1, is(notNullValue())),
+                () -> assertThat(o2, is(notNullValue())),
+                () -> assertThat(o1, is(equalTo(o1))),
+                () -> assertThat(o2, is(equalTo(o2))));
     }
 
     @Test
@@ -143,10 +147,10 @@ void shouldFindStepDefsCreatedImplicitlyForAutowiring() {
         factory1.stop();
 
         assertAll(
-            () -> assertThat(o1.getThirdStepDef(), is(notNullValue())),
-            () -> assertThat(o2, is(notNullValue())),
-            () -> assertThat(o1.getThirdStepDef(), is(equalTo(o2))),
-            () -> assertThat(o2, is(equalTo(o1.getThirdStepDef()))));
+                () -> assertThat(o1.getThirdStepDef(), is(notNullValue())),
+                () -> assertThat(o2, is(notNullValue())),
+                () -> assertThat(o1.getThirdStepDef(), is(equalTo(o2))),
+                () -> assertThat(o2, is(equalTo(o1.getThirdStepDef()))));
     }
 
     @Test
@@ -162,10 +166,10 @@ void shouldReuseStepDefsCreatedImplicitlyForAutowiring() {
         factory1.stop();
 
         assertAll(
-            () -> assertThat(o1.getThirdStepDef(), is(notNullValue())),
-            () -> assertThat(o3.getThirdStepDef(), is(notNullValue())),
-            () -> assertThat(o1.getThirdStepDef(), is(equalTo(o3.getThirdStepDef()))),
-            () -> assertThat(o3.getThirdStepDef(), is(equalTo(o1.getThirdStepDef()))));
+                () -> assertThat(o1.getThirdStepDef(), is(notNullValue())),
+                () -> assertThat(o3.getThirdStepDef(), is(notNullValue())),
+                () -> assertThat(o1.getThirdStepDef(), is(equalTo(o3.getThirdStepDef()))),
+                () -> assertThat(o3.getThirdStepDef(), is(equalTo(o1.getThirdStepDef()))));
     }
 
     @Test
@@ -209,10 +213,10 @@ void shouldRespectDirtiesContextAnnotationsInStepDefs() {
         factory.stop();
 
         assertAll(
-            () -> assertThat(o1, is(notNullValue())),
-            () -> assertThat(o2, is(notNullValue())),
-            () -> assertThat(o1, is(not(equalTo(o2)))),
-            () -> assertThat(o2, is(not(equalTo(o1)))));
+                () -> assertThat(o1, is(notNullValue())),
+                () -> assertThat(o2, is(notNullValue())),
+                () -> assertThat(o1, is(not(equalTo(o2)))),
+                () -> assertThat(o2, is(not(equalTo(o1)))));
     }
 
     @Test
@@ -232,10 +236,10 @@ void shouldRespectDirtiesContextAnnotationsInStepDefsUsingMetaConfiguration() {
         factory.stop();
 
         assertAll(
-            () -> assertThat(o1, is(notNullValue())),
-            () -> assertThat(o2, is(notNullValue())),
-            () -> assertThat(o1, is(not(equalTo(o2)))),
-            () -> assertThat(o2, is(not(equalTo(o1)))));
+                () -> assertThat(o1, is(notNullValue())),
+                () -> assertThat(o2, is(notNullValue())),
+                () -> assertThat(o1, is(not(equalTo(o2)))),
+                () -> assertThat(o2, is(not(equalTo(o1)))));
     }
 
     @Test
@@ -257,9 +261,9 @@ void shouldFailIfMultipleClassesWithSpringAnnotationsAreFound() {
         Executable testMethod = () -> factory.addClass(BellyStepDefinitions.class);
         CucumberBackendException actualThrown = assertThrows(CucumberBackendException.class, testMethod);
         assertThat(actualThrown.getMessage(), startsWith(
-            "Glue class class io.cucumber.spring.contextconfig.BellyStepDefinitions and class io.cucumber.spring.SpringFactoryTest$WithSpringAnnotations are both annotated with @CucumberContextConfiguration.\n"
-                    +
-                    "Please ensure only one class configures the spring context"));
+                "Glue class class io.cucumber.spring.contextconfig.BellyStepDefinitions and class io.cucumber.spring.SpringFactoryTest$WithSpringAnnotations are both annotated with @CucumberContextConfiguration.\n"
+                        +
+                        "Please ensure only one class configures the spring context"));
     }
 
     @Test
@@ -269,7 +273,7 @@ void shouldFailIfClassWithSpringComponentAnnotationsIsFound() {
         Executable testMethod = () -> factory.addClass(WithComponentAnnotation.class);
         CucumberBackendException actualThrown = assertThrows(CucumberBackendException.class, testMethod);
         assertThat(actualThrown.getMessage(), is(equalTo(
-            "Glue class io.cucumber.spring.componentannotation.WithComponentAnnotation was annotated with @Component; 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 @Component annotation")));
+                "Glue class io.cucumber.spring.componentannotation.WithComponentAnnotation was annotated with @Component; 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 @Component annotation")));
     }
 
     @Test
@@ -279,7 +283,7 @@ void shouldFailIfClassWithAnnotationAnnotatedWithSpringComponentAnnotationsIsFou
         Executable testMethod = () -> factory.addClass(WithControllerAnnotation.class);
         CucumberBackendException actualThrown = assertThrows(CucumberBackendException.class, testMethod);
         assertThat(actualThrown.getMessage(), is(equalTo(
-            "Glue class io.cucumber.spring.componentannotation.WithControllerAnnotation was annotated with @Controller; 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 @Controller annotation")));
+                "Glue class io.cucumber.spring.componentannotation.WithControllerAnnotation was annotated with @Controller; 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 @Controller annotation")));
     }
 
     @Test
@@ -300,10 +304,10 @@ void shouldGlueScopedSpringBeanBehaveLikeGlueLifecycle() {
         factory.stop();
 
         assertAll(
-            () -> assertThat(glueInstance1, is(not(glueInstance2))),
-            () -> assertThat(glueInstance2, is(not(glueInstance1))),
-            () -> assertThat(bellyInstance1, is(bellyInstance2)),
-            () -> assertThat(bellyInstance2, is(bellyInstance1)));
+                () -> assertThat(glueInstance1, is(not(glueInstance2))),
+                () -> assertThat(glueInstance2, is(not(glueInstance1))),
+                () -> assertThat(bellyInstance1, is(bellyInstance2)),
+                () -> assertThat(bellyInstance2, is(bellyInstance1)));
     }
 
     @Test
@@ -327,7 +331,7 @@ void shouldBeStoppableWhenFacedWithInvalidConfiguration() {
 
         IllegalStateException exception = assertThrows(IllegalStateException.class, factory::start);
         assertThat(exception.getMessage(),
-            containsString("DelegatingSmartContextLoader was unable to detect defaults"));
+                containsString("DelegatingSmartContextLoader was unable to detect defaults"));
         assertDoesNotThrow(factory::stop);
     }
 
@@ -341,6 +345,19 @@ void shouldBeStoppableWhenFacedWithMissingContextConfiguration() {
         assertDoesNotThrow(factory::stop);
     }
 
+    @ParameterizedTest
+    @ValueSource(classes = {
+            FailedBeforeTestClassContextConfiguration.class,
+            FailedBeforeTestMethodContextConfiguration.class,
+    })
+    void shouldBeStoppableWhenFacedWithFailedApplicationContext(Class<?> contextConfiguration) {
+        final ObjectFactory factory = new SpringFactory();
+        factory.addClass(contextConfiguration);
+
+        assertThrows(CucumberBackendException.class, factory::start);
+        assertDoesNotThrow(factory::stop);
+    }
+
     @CucumberContextConfiguration
     @ContextConfiguration("classpath:cucumber.xml")
     public static class WithSpringAnnotations {
@@ -366,7 +383,7 @@ public String getProperty() {
     }
 
     @CucumberContextConfiguration
-    @ContextConfiguration()
+    @ContextConfiguration
     public static class WithEmptySpringAnnotations {
 
     }
@@ -376,4 +393,39 @@ 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();
+            }
+
+        }
+
+    }
+
+    public static class StubException extends Exception {
+
+    }
 }