From f5f1ace0fdf3d1e85ea3435ca999365489e6ed84 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Sun, 10 Apr 2022 15:20:22 +0200 Subject: [PATCH] [Spring] Start and stop test context once per scenario (#2517) The `TestContextAdaptor` would start the scenario scope once at the start of each test, then another one to run the after test method hooks. Refactored code to make the order of operations more obvious. Also prevented scenario scopes from being registered more then once. Though because `CucumberScenarioScope` delegates everything to `CucumberTestContext` this effectively only reduces the log spam a bit at debug level. Fixes: #2516 --- CHANGELOG.md | 1 + .../spring/CucumberScenarioScope.java | 4 +- .../cucumber/spring/CucumberTestContext.java | 5 ++- .../io/cucumber/spring/SpringFactory.java | 4 +- .../cucumber/spring/TestContextAdaptor.java | 40 +++++++++---------- .../io/cucumber/spring/SpringFactoryTest.java | 25 ++++++++++++ 6 files changed, 53 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc8190e855..ad671b6d1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `org.glassfish.jaxb:jaxb-runtime` - [TestNG] Remove spurious Optional\[] from test name ([#2488](https://github.com/cucumber/cucumber-jvm/pull/2488) M.P. Korstanje) * [BOM] Add missing dependencies to bill of materials ([#2496](https://github.com/cucumber/cucumber-jvm/pull/2496) M.P. Korstanje) +* [Spring] Start and stop test context once per scenario ([#2517](https://github.com/cucumber/cucumber-jvm/pull/2517) M.P. Korstanje) ## [7.2.3] (2022-01-13) diff --git a/spring/src/main/java/io/cucumber/spring/CucumberScenarioScope.java b/spring/src/main/java/io/cucumber/spring/CucumberScenarioScope.java index 93116e0858..694d6687e6 100644 --- a/spring/src/main/java/io/cucumber/spring/CucumberScenarioScope.java +++ b/spring/src/main/java/io/cucumber/spring/CucumberScenarioScope.java @@ -37,7 +37,9 @@ public Object resolveContextualObject(String key) { @Override public String getConversationId() { CucumberTestContext context = CucumberTestContext.getInstance(); - return context.getId(); + return context.getId() + .map(id -> "cucumber_test_context_" + id) + .orElse(null); } } diff --git a/spring/src/main/java/io/cucumber/spring/CucumberTestContext.java b/spring/src/main/java/io/cucumber/spring/CucumberTestContext.java index 073cd8039e..9e8c636133 100644 --- a/spring/src/main/java/io/cucumber/spring/CucumberTestContext.java +++ b/spring/src/main/java/io/cucumber/spring/CucumberTestContext.java @@ -4,6 +4,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; @API(status = API.Status.STABLE) @@ -31,8 +32,8 @@ void start() { sessionId = sessionCounter.incrementAndGet(); } - String getId() { - return "cucumber_test_context_" + sessionId; + Optional getId() { + return Optional.ofNullable(sessionId); } void stop() { diff --git a/spring/src/main/java/io/cucumber/spring/SpringFactory.java b/spring/src/main/java/io/cucumber/spring/SpringFactory.java index 65c93d91ae..40ae720b24 100644 --- a/spring/src/main/java/io/cucumber/spring/SpringFactory.java +++ b/spring/src/main/java/io/cucumber/spring/SpringFactory.java @@ -21,8 +21,6 @@ import java.util.HashSet; import java.util.Set; -import static io.cucumber.spring.TestContextAdaptor.createTestContextManagerAdaptor; - /** * Spring based implementation of ObjectFactory. *

@@ -151,7 +149,7 @@ public void start() { // a singleton and reused between scenarios and shared between // threads. TestContextManager testContextManager = new TestContextManager(withCucumberContextConfiguration); - testContextAdaptor = createTestContextManagerAdaptor(testContextManager, stepClasses); + testContextAdaptor = new TestContextAdaptor(testContextManager, stepClasses); testContextAdaptor.start(); } diff --git a/spring/src/main/java/io/cucumber/spring/TestContextAdaptor.java b/spring/src/main/java/io/cucumber/spring/TestContextAdaptor.java index 1bdeb7ac1f..a1cd13dec8 100644 --- a/spring/src/main/java/io/cucumber/spring/TestContextAdaptor.java +++ b/spring/src/main/java/io/cucumber/spring/TestContextAdaptor.java @@ -2,6 +2,7 @@ import io.cucumber.core.backend.CucumberBackendException; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.config.Scope; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.context.ConfigurableApplicationContext; @@ -13,7 +14,7 @@ import static io.cucumber.spring.CucumberTestContext.SCOPE_CUCUMBER_GLUE; -abstract class TestContextAdaptor { +class TestContextAdaptor { private static final Object monitor = new Object(); @@ -21,26 +22,16 @@ abstract class TestContextAdaptor { private final ConfigurableApplicationContext applicationContext; private final Collection> glueClasses; - protected TestContextAdaptor( - TestContextManager delegate, - ConfigurableApplicationContext applicationContext, - Collection> glueClasses - ) { - this.delegate = delegate; - this.applicationContext = applicationContext; - this.glueClasses = glueClasses; - } - - static TestContextAdaptor createTestContextManagerAdaptor( + TestContextAdaptor( TestContextManager delegate, Collection> glueClasses ) { TestContext testContext = delegate.getTestContext(); ConfigurableApplicationContext applicationContext = (ConfigurableApplicationContext) testContext .getApplicationContext(); - return new TestContextAdaptor(delegate, applicationContext, glueClasses) { - - }; + this.delegate = delegate; + this.applicationContext = applicationContext; + this.glueClasses = glueClasses; } public final void start() { @@ -50,15 +41,15 @@ public final void start() { // #1153, #1148, #1106) we do this serially. synchronized (monitor) { registerGlueCodeScope(applicationContext); - notifyContextManagerAboutTestClassStarted(); registerStepClassBeanDefinitions(applicationContext.getBeanFactory()); } + notifyContextManagerAboutBeforeTestClass(); + CucumberTestContext.getInstance().start(); notifyTestContextManagerAboutBeforeTestMethod(); } private void notifyTestContextManagerAboutBeforeTestMethod() { try { - CucumberTestContext.getInstance().start(); Class testClass = delegate.getTestContext().getTestClass(); Object testContextInstance = applicationContext.getBean(testClass); Method dummyMethod = TestContextAdaptor.class.getMethod("cucumberDoesNotHaveASingleTestMethod"); @@ -70,12 +61,18 @@ private void notifyTestContextManagerAboutBeforeTestMethod() { final void registerGlueCodeScope(ConfigurableApplicationContext context) { while (context != null) { - context.getBeanFactory().registerScope(SCOPE_CUCUMBER_GLUE, new CucumberScenarioScope()); + ConfigurableListableBeanFactory beanFactory = context.getBeanFactory(); + // Scenario scope may have already been registered by another + // thread. + Scope registeredScope = beanFactory.getRegisteredScope(SCOPE_CUCUMBER_GLUE); + if (registeredScope == null) { + beanFactory.registerScope(SCOPE_CUCUMBER_GLUE, new CucumberScenarioScope()); + } context = (ConfigurableApplicationContext) context.getParent(); } } - private void notifyContextManagerAboutTestClassStarted() { + private void notifyContextManagerAboutBeforeTestClass() { try { delegate.beforeTestClass(); } catch (Exception e) { @@ -106,6 +103,10 @@ private void registerStepClassBeanDefinition(BeanDefinitionRegistry registry, Cl public final void stop() { notifyTestContextManagerAboutAfterTestMethod(); CucumberTestContext.getInstance().stop(); + notifyTestContextManagerAboutAfterTestClass(); + } + + private void notifyTestContextManagerAboutAfterTestClass() { try { delegate.afterTestClass(); } catch (Exception e) { @@ -115,7 +116,6 @@ public final void stop() { private void notifyTestContextManagerAboutAfterTestMethod() { try { - CucumberTestContext.getInstance().start(); Class testClass = delegate.getTestContext().getTestClass(); Object testContextInstance = applicationContext.getBean(testClass); Method dummyMethod = TestContextAdaptor.class.getMethod("cucumberDoesNotHaveASingleTestMethod"); diff --git a/spring/src/test/java/io/cucumber/spring/SpringFactoryTest.java b/spring/src/test/java/io/cucumber/spring/SpringFactoryTest.java index e310925d1e..d3e04a88f4 100644 --- a/spring/src/test/java/io/cucumber/spring/SpringFactoryTest.java +++ b/spring/src/test/java/io/cucumber/spring/SpringFactoryTest.java @@ -23,6 +23,8 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.test.context.ContextConfiguration; +import java.util.Optional; + import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; @@ -32,6 +34,8 @@ import static org.hamcrest.core.IsNull.notNullValue; 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; @@ -59,6 +63,27 @@ void shouldGiveUsNewStepInstancesForEachScenario() { () -> assertThat(o2, is(not(equalTo(o1))))); } + @Test + void shouldStartOneCucumberContextForEachScenario() { + final ObjectFactory factory = new SpringFactory(); + factory.addClass(BellyStepDefinitions.class); + + // Scenario 1 + assertTrue(CucumberTestContext.getInstance().getId().isEmpty()); + factory.start(); + Optional testContextId1 = CucumberTestContext.getInstance().getId(); + factory.stop(); + + // Scenario 2 + assertTrue(CucumberTestContext.getInstance().getId().isEmpty()); + factory.start(); + Optional testContextId2 = CucumberTestContext.getInstance().getId(); + factory.stop(); + assertTrue(CucumberTestContext.getInstance().getId().isEmpty()); + + assertEquals(testContextId1.get() + 1, testContextId2.get()); + } + @Test void shouldNeverCreateNewApplicationBeanInstances() { // Feature 1