From aa55c976e656dd807c1d45279912fac876601fb1 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sun, 11 Dec 2022 16:28:43 +0100 Subject: [PATCH] [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 --- CHANGELOG.md | 14 +- .../cucumber/spring/TestContextAdaptor.java | 128 ++++++++++-- .../io/cucumber/spring/SpringFactoryTest.java | 46 +---- .../spring/TestTestContextAdaptorTest.java | 191 ++++++++++++++++++ 4 files changed, 312 insertions(+), 67 deletions(-) create mode 100644 cucumber-spring/src/test/java/io/cucumber/spring/TestTestContextAdaptorTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 12f4c84226..2474c0aac9 100644 --- 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 index 3b7c921fe8..9afb5bc428 100644 --- 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> glueClasses; + private final Deque 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 getInstance(Class 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 index 81926d7086..eae1d389cc 100644 --- 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(); } } diff --git a/cucumber-spring/src/test/java/io/cucumber/spring/TestTestContextAdaptorTest.java b/cucumber-spring/src/test/java/io/cucumber/spring/TestTestContextAdaptorTest.java new file mode 100644 index 0000000000..665ccb2f81 --- /dev/null +++ b/cucumber-spring/src/test/java/io/cucumber/spring/TestTestContextAdaptorTest.java @@ -0,0 +1,191 @@ +package io.cucumber.spring; + +import io.cucumber.core.backend.CucumberBackendException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InOrder; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestContextManager; +import org.springframework.test.context.TestExecutionListener; + +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.inOrder; + +@ExtendWith(MockitoExtension.class) +public class TestTestContextAdaptorTest { + + @Mock + TestExecutionListener listener; + + @AfterEach + void verifyNoMoroInteractions() { + Mockito.verifyNoMoreInteractions(listener); + } + + @Test + void invokesAllLiveCycleHooks() throws Exception { + TestContextManager manager = new TestContextManager(SomeContextConfiguration.class); + TestContextAdaptor adaptor = new TestContextAdaptor(manager, singletonList(SomeContextConfiguration.class)); + manager.registerTestExecutionListeners(listener); + InOrder inOrder = inOrder(listener); + + adaptor.start(); + inOrder.verify(listener).beforeTestClass(any()); + inOrder.verify(listener).prepareTestInstance(any()); + inOrder.verify(listener).beforeTestMethod(any()); + inOrder.verify(listener).beforeTestExecution(any()); + + adaptor.stop(); + inOrder.verify(listener).afterTestExecution(any()); + inOrder.verify(listener).afterTestMethod(any()); + inOrder.verify(listener).afterTestClass(any()); + } + + @Test + void invokesAfterClassIfBeforeClassFailed() throws Exception { + TestContextManager manager = new TestContextManager(SomeContextConfiguration.class); + TestContextAdaptor adaptor = new TestContextAdaptor(manager, singletonList(SomeContextConfiguration.class)); + manager.registerTestExecutionListeners(listener); + InOrder inOrder = inOrder(listener); + + doThrow(new RuntimeException()).when(listener).beforeTestClass(any()); + + assertThrows(CucumberBackendException.class, adaptor::start); + inOrder.verify(listener).beforeTestClass(any()); + + adaptor.stop(); + inOrder.verify(listener).afterTestClass(any()); + } + + @Test + void invokesAfterClassIfPrepareTestInstanceFailed() throws Exception { + TestContextManager manager = new TestContextManager(SomeContextConfiguration.class); + TestContextAdaptor adaptor = new TestContextAdaptor(manager, singletonList(SomeContextConfiguration.class)); + manager.registerTestExecutionListeners(listener); + InOrder inOrder = inOrder(listener); + + doThrow(new RuntimeException()).when(listener).prepareTestInstance(any()); + + assertThrows(CucumberBackendException.class, adaptor::start); + inOrder.verify(listener).beforeTestClass(any()); + + adaptor.stop(); + inOrder.verify(listener).afterTestClass(any()); + } + + @Test + void invokesAfterMethodIfBeforeMethodThrows() throws Exception { + TestContextManager manager = new TestContextManager(SomeContextConfiguration.class); + TestContextAdaptor adaptor = new TestContextAdaptor(manager, singletonList(SomeContextConfiguration.class)); + manager.registerTestExecutionListeners(listener); + InOrder inOrder = inOrder(listener); + + doThrow(new RuntimeException()).when(listener).beforeTestMethod(any()); + + assertThrows(CucumberBackendException.class, adaptor::start); + inOrder.verify(listener).beforeTestClass(any()); + inOrder.verify(listener).prepareTestInstance(any()); + inOrder.verify(listener).beforeTestMethod(any()); + + adaptor.stop(); + inOrder.verify(listener).afterTestMethod(any()); + inOrder.verify(listener).afterTestClass(any()); + } + + @Test + void invokesAfterTestExecutionIfBeforeTestExecutionThrows() throws Exception { + TestContextManager manager = new TestContextManager(SomeContextConfiguration.class); + TestContextAdaptor adaptor = new TestContextAdaptor(manager, singletonList(SomeContextConfiguration.class)); + manager.registerTestExecutionListeners(listener); + InOrder inOrder = inOrder(listener); + + doThrow(new RuntimeException()).when(listener).beforeTestExecution(any()); + + assertThrows(CucumberBackendException.class, adaptor::start); + inOrder.verify(listener).beforeTestClass(any()); + inOrder.verify(listener).prepareTestInstance(any()); + inOrder.verify(listener).beforeTestMethod(any()); + + adaptor.stop(); + inOrder.verify(listener).afterTestExecution(any()); + inOrder.verify(listener).afterTestMethod(any()); + inOrder.verify(listener).afterTestClass(any()); + } + + @Test + void invokesAfterTestMethodIfAfterTestExecutionThrows() throws Exception { + TestContextManager manager = new TestContextManager(SomeContextConfiguration.class); + TestContextAdaptor adaptor = new TestContextAdaptor(manager, singletonList(SomeContextConfiguration.class)); + manager.registerTestExecutionListeners(listener); + InOrder inOrder = inOrder(listener); + + doThrow(new RuntimeException()).when(listener).afterTestExecution(any()); + + adaptor.start(); + inOrder.verify(listener).beforeTestClass(any()); + inOrder.verify(listener).prepareTestInstance(any()); + inOrder.verify(listener).beforeTestMethod(any()); + inOrder.verify(listener).beforeTestExecution(any()); + + assertThrows(CucumberBackendException.class, adaptor::stop); + inOrder.verify(listener).afterTestExecution(any()); + inOrder.verify(listener).afterTestMethod(any()); + inOrder.verify(listener).afterTestClass(any()); + } + + @Test + void invokesAfterTesClassIfAfterTestMethodThrows() throws Exception { + TestContextManager manager = new TestContextManager(SomeContextConfiguration.class); + TestContextAdaptor adaptor = new TestContextAdaptor(manager, singletonList(SomeContextConfiguration.class)); + manager.registerTestExecutionListeners(listener); + InOrder inOrder = inOrder(listener); + + doThrow(new RuntimeException()).when(listener).afterTestMethod(any()); + + adaptor.start(); + inOrder.verify(listener).beforeTestClass(any()); + inOrder.verify(listener).prepareTestInstance(any()); + inOrder.verify(listener).beforeTestMethod(any()); + inOrder.verify(listener).beforeTestExecution(any()); + + assertThrows(CucumberBackendException.class, adaptor::stop); + inOrder.verify(listener).afterTestExecution(any()); + inOrder.verify(listener).afterTestMethod(any()); + inOrder.verify(listener).afterTestClass(any()); + } + + @Test + void invokesAllMethodsPriorIfAfterTestClassThrows() throws Exception { + TestContextManager manager = new TestContextManager(SomeContextConfiguration.class); + TestContextAdaptor adaptor = new TestContextAdaptor(manager, singletonList(SomeContextConfiguration.class)); + manager.registerTestExecutionListeners(listener); + InOrder inOrder = inOrder(listener); + + doThrow(new RuntimeException()).when(listener).afterTestExecution(any()); + + adaptor.start(); + inOrder.verify(listener).beforeTestClass(any()); + inOrder.verify(listener).prepareTestInstance(any()); + inOrder.verify(listener).beforeTestMethod(any()); + inOrder.verify(listener).beforeTestExecution(any()); + + assertThrows(CucumberBackendException.class, adaptor::stop); + inOrder.verify(listener).afterTestExecution(any()); + inOrder.verify(listener).afterTestMethod(any()); + inOrder.verify(listener).afterTestClass(any()); + } + + @CucumberContextConfiguration + @ContextConfiguration("classpath:cucumber.xml") + public static class SomeContextConfiguration { + + } + +}