diff --git a/CHANGELOG.md b/CHANGELOG.md index c8d24857d1..7636a6aae9 100644 --- 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 [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 index 9e8c636133..bb2370f598 100644 --- 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 index a1cd13dec8..f91fd7be1b 100644 --- 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 index d3e04a88f4..81e0f1c8db 100644 --- 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 { + + } }