From 7134586310a378557113e08090b18bcfc399dd0a Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 14 Sep 2016 22:03:07 -0700 Subject: [PATCH] Ensure test @PostConstructs are only called once Rename AutoConfigureReportTestExecutionListener to SpringBootDependencyInjectionTestExecutionListener and ensure that it replaces any existing DependencyInjectionTestExecutionListener. Prior to this commit the registration of two DependencyInjection listeners would cause @PostConstruct methods on tests to be called twice. In order to allow the standard DependencyInjectionTestExecutionListener to be removed a new DefaultTestExecutionListenersPostProcessor interface has been introduced. Fixes gh-6874 --- ...ndencyInjectionTestExecutionListener.java} | 40 ++++++++---- .../main/resources/META-INF/spring.factories | 5 +- ...ListenerPostConstructIntegrationTests.java | 64 +++++++++++++++++++ ...yInjectionTestExecutionListenerTests.java} | 10 +-- ...ltTestExecutionListenersPostProcessor.java | 42 ++++++++++++ .../SpringBootTestContextBootstrapper.java | 15 +++++ ...pringBootTestContextBootstrapperTests.java | 9 +++ ...ltTestExecutionListenersPostProcessor.java | 54 ++++++++++++++++ .../test/resources/META-INF/spring.factories | 2 + 9 files changed, 223 insertions(+), 18 deletions(-) rename spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/{AutoConfigureReportTestExecutionListener.java => SpringBootDependencyInjectionTestExecutionListener.java} (60%) create mode 100644 spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/SpringBootDependencyInjectionTestExecutionListenerPostConstructIntegrationTests.java rename spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/{AutoConfigureReportTestExecutionListenerTests.java => SpringBootDependencyInjectionTestExecutionListenerTests.java} (87%) create mode 100644 spring-boot-test/src/main/java/org/springframework/boot/test/context/DefaultTestExecutionListenersPostProcessor.java create mode 100644 spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/TestDefaultTestExecutionListenersPostProcessor.java create mode 100644 spring-boot-test/src/test/resources/META-INF/spring.factories diff --git a/spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/AutoConfigureReportTestExecutionListener.java b/spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/SpringBootDependencyInjectionTestExecutionListener.java similarity index 60% rename from spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/AutoConfigureReportTestExecutionListener.java rename to spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/SpringBootDependencyInjectionTestExecutionListener.java index 1cc0622494..5b3fcbd0fc 100644 --- a/spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/AutoConfigureReportTestExecutionListener.java +++ b/spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/SpringBootDependencyInjectionTestExecutionListener.java @@ -16,34 +16,32 @@ package org.springframework.boot.test.autoconfigure; +import java.util.LinkedHashSet; +import java.util.Set; + import org.springframework.boot.autoconfigure.condition.ConditionEvaluationReport; import org.springframework.boot.autoconfigure.logging.ConditionEvaluationReportMessage; +import org.springframework.boot.test.context.DefaultTestExecutionListenersPostProcessor; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.test.context.TestContext; import org.springframework.test.context.TestExecutionListener; -import org.springframework.test.context.support.AbstractTestExecutionListener; import org.springframework.test.context.support.DependencyInjectionTestExecutionListener; /** - * {@link TestExecutionListener} to print the {@link ConditionEvaluationReport} when the - * context cannot be prepared. + * Alternative {@link DependencyInjectionTestExecutionListener} prints the + * {@link ConditionEvaluationReport} when the context cannot be prepared. * * @author Phillip Webb + * @since 1.4.1 */ -class AutoConfigureReportTestExecutionListener extends AbstractTestExecutionListener { - - private DependencyInjectionTestExecutionListener delegate = new DependencyInjectionTestExecutionListener(); - - @Override - public int getOrder() { - return this.delegate.getOrder() - 1; - } +public class SpringBootDependencyInjectionTestExecutionListener + extends DependencyInjectionTestExecutionListener { @Override public void prepareTestInstance(TestContext testContext) throws Exception { try { - this.delegate.prepareTestInstance(testContext); + super.prepareTestInstance(testContext); } catch (Exception ex) { ApplicationContext context = testContext.getApplicationContext(); @@ -56,4 +54,22 @@ class AutoConfigureReportTestExecutionListener extends AbstractTestExecutionList } } + static class PostProcessor implements DefaultTestExecutionListenersPostProcessor { + + @Override + public Set> postProcessDefaultTestExecutionListeners( + Set> listeners) { + Set> updated = new LinkedHashSet>( + listeners.size()); + for (Class listener : listeners) { + updated.add( + listener.equals(DependencyInjectionTestExecutionListener.class) + ? SpringBootDependencyInjectionTestExecutionListener.class + : listener); + } + return updated; + } + + } + } diff --git a/spring-boot-test-autoconfigure/src/main/resources/META-INF/spring.factories b/spring-boot-test-autoconfigure/src/main/resources/META-INF/spring.factories index eb62fa3f31..08517da27d 100644 --- a/spring-boot-test-autoconfigure/src/main/resources/META-INF/spring.factories +++ b/spring-boot-test-autoconfigure/src/main/resources/META-INF/spring.factories @@ -66,6 +66,10 @@ org.springframework.boot.autoconfigure.thymeleaf.ThymeleafAutoConfiguration,\ org.springframework.boot.autoconfigure.web.HttpMessageConvertersAutoConfiguration,\ org.springframework.boot.autoconfigure.web.WebMvcAutoConfiguration +# DefaultTestExecutionListenersPostProcessors +org.springframework.boot.test.context.DefaultTestExecutionListenersPostProcessor=\ +org.springframework.boot.test.autoconfigure.SpringBootDependencyInjectionTestExecutionListener$PostProcessor + # Spring Test ContextCustomizerFactories org.springframework.test.context.ContextCustomizerFactory=\ org.springframework.boot.test.autoconfigure.OverrideAutoConfigurationContextCustomizerFactory,\ @@ -74,6 +78,5 @@ org.springframework.boot.test.autoconfigure.properties.PropertyMappingContextCus # Test Execution Listeners org.springframework.test.context.TestExecutionListener=\ -org.springframework.boot.test.autoconfigure.AutoConfigureReportTestExecutionListener,\ org.springframework.boot.test.autoconfigure.restdocs.RestDocsTestExecutionListener,\ org.springframework.boot.test.autoconfigure.web.client.MockRestServiceServerResetTestExecutionListener diff --git a/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/SpringBootDependencyInjectionTestExecutionListenerPostConstructIntegrationTests.java b/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/SpringBootDependencyInjectionTestExecutionListenerPostConstructIntegrationTests.java new file mode 100644 index 0000000000..d93cd884c7 --- /dev/null +++ b/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/SpringBootDependencyInjectionTestExecutionListenerPostConstructIntegrationTests.java @@ -0,0 +1,64 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.test.autoconfigure; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.ArrayList; +import java.util.List; + +import javax.annotation.PostConstruct; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.junit4.SpringRunner; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Integration tests for {@link SpringBootDependencyInjectionTestExecutionListener}. + * + * @author Phillip Webb + */ +@RunWith(SpringRunner.class) +@SpringBootTest +public class SpringBootDependencyInjectionTestExecutionListenerPostConstructIntegrationTests { + + private List calls = new ArrayList(); + + @PostConstruct + public void postConstruct() { + StringWriter writer = new StringWriter(); + new RuntimeException().printStackTrace(new PrintWriter(writer)); + this.calls.add(writer.toString()); + } + + @Test + public void postConstructShouldBeInvokedOnlyOnce() throws Exception { + // gh-6874 + assertThat(this.calls).hasSize(1); + } + + @Configuration + static class Config { + + } + +} diff --git a/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/AutoConfigureReportTestExecutionListenerTests.java b/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/SpringBootDependencyInjectionTestExecutionListenerTests.java similarity index 87% rename from spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/AutoConfigureReportTestExecutionListenerTests.java rename to spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/SpringBootDependencyInjectionTestExecutionListenerTests.java index e4196b919d..9fe7a813fa 100644 --- a/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/AutoConfigureReportTestExecutionListenerTests.java +++ b/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/SpringBootDependencyInjectionTestExecutionListenerTests.java @@ -35,23 +35,23 @@ import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; /** - * Tests for {@link AutoConfigureReportTestExecutionListener}. + * Tests for {@link SpringBootDependencyInjectionTestExecutionListener}. * * @author Phillip Webb */ -public class AutoConfigureReportTestExecutionListenerTests { +public class SpringBootDependencyInjectionTestExecutionListenerTests { @Rule public OutputCapture out = new OutputCapture(); - private AutoConfigureReportTestExecutionListener reportListener = new AutoConfigureReportTestExecutionListener(); + private SpringBootDependencyInjectionTestExecutionListener reportListener = new SpringBootDependencyInjectionTestExecutionListener(); @Test - public void orderShouldBeBeforeDependencyInjectionTestExecutionListener() + public void orderShouldBeSameAsDependencyInjectionTestExecutionListener() throws Exception { Ordered injectionListener = new DependencyInjectionTestExecutionListener(); assertThat(this.reportListener.getOrder()) - .isLessThan(injectionListener.getOrder()); + .isEqualTo(injectionListener.getOrder()); } @Test diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/context/DefaultTestExecutionListenersPostProcessor.java b/spring-boot-test/src/main/java/org/springframework/boot/test/context/DefaultTestExecutionListenersPostProcessor.java new file mode 100644 index 0000000000..ee177ddfc2 --- /dev/null +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/context/DefaultTestExecutionListenersPostProcessor.java @@ -0,0 +1,42 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.test.context; + +import java.util.Set; + +import org.springframework.test.context.TestExecutionListener; + +/** + * Callback interface trigger from {@link SpringBootTestContextBootstrapper} that can be + * used to post-process the list of default {@link TestExecutionListener} classes to be + * used by a test. Can be used to add or remove existing listener classes. + * + * @author Phillip Webb + * @since 1.4.1 + * @see SpringBootTest + */ +public interface DefaultTestExecutionListenersPostProcessor { + + /** + * Post process the list of default {@link TestExecutionListener} classes to be used. + * @param listeners the source listeners + * @return the actual listeners that should be used + */ + Set> postProcessDefaultTestExecutionListeners( + Set> listeners); + +} diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootTestContextBootstrapper.java b/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootTestContextBootstrapper.java index e6f0dc486b..f6c678c9e3 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootTestContextBootstrapper.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootTestContextBootstrapper.java @@ -19,6 +19,7 @@ package org.springframework.boot.test.context; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -27,11 +28,13 @@ import org.springframework.boot.SpringBootConfiguration; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.env.Environment; +import org.springframework.core.io.support.SpringFactoriesLoader; import org.springframework.test.context.ContextConfigurationAttributes; import org.springframework.test.context.ContextLoader; import org.springframework.test.context.MergedContextConfiguration; import org.springframework.test.context.TestContext; import org.springframework.test.context.TestContextBootstrapper; +import org.springframework.test.context.TestExecutionListener; import org.springframework.test.context.support.DefaultTestContextBootstrapper; import org.springframework.test.context.web.WebAppConfiguration; import org.springframework.test.context.web.WebMergedContextConfiguration; @@ -82,6 +85,18 @@ public class SpringBootTestContextBootstrapper extends DefaultTestContextBootstr return context; } + @Override + protected Set> getDefaultTestExecutionListenerClasses() { + Set> listeners = super.getDefaultTestExecutionListenerClasses(); + List postProcessors = SpringFactoriesLoader + .loadFactories(DefaultTestExecutionListenersPostProcessor.class, + getClass().getClassLoader()); + for (DefaultTestExecutionListenersPostProcessor postProcessor : postProcessors) { + listeners = postProcessor.postProcessDefaultTestExecutionListeners(listeners); + } + return listeners; + } + @Override protected ContextLoader resolveContextLoader(Class testClass, List configAttributesList) { diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/SpringBootTestContextBootstrapperTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/SpringBootTestContextBootstrapperTests.java index 176f416d41..65a0e766ea 100644 --- a/spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/SpringBootTestContextBootstrapperTests.java +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/SpringBootTestContextBootstrapperTests.java @@ -46,6 +46,8 @@ public class SpringBootTestContextBootstrapperTests { @Autowired private SpringBootTestContextBootstrapperExampleConfig config; + public boolean defaultTestExecutionListenersPostProcessorCalled; + @Test public void findConfigAutomatically() throws Exception { assertThat(this.config).isNotNull(); @@ -61,6 +63,12 @@ public class SpringBootTestContextBootstrapperTests { assertThat(this.context.getBean(ExampleBean.class)).isNotNull(); } + @Test + public void defaultTestExecutionListenersPostProcessorShouldBeCalled() + throws Exception { + assertThat(this.defaultTestExecutionListenersPostProcessorCalled).isTrue(); + } + @TestConfiguration static class TestConfig { @@ -79,4 +87,5 @@ public class SpringBootTestContextBootstrapperTests { static class ExampleTestComponent { } + } diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/TestDefaultTestExecutionListenersPostProcessor.java b/spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/TestDefaultTestExecutionListenersPostProcessor.java new file mode 100644 index 0000000000..2e24c29ccb --- /dev/null +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/TestDefaultTestExecutionListenersPostProcessor.java @@ -0,0 +1,54 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.test.context.bootstrap; + +import java.util.Set; + +import org.springframework.boot.test.context.DefaultTestExecutionListenersPostProcessor; +import org.springframework.test.context.TestContext; +import org.springframework.test.context.TestExecutionListener; +import org.springframework.test.context.support.AbstractTestExecutionListener; + +/** + * Test {@link DefaultTestExecutionListenersPostProcessor}. + * + * @author Phillip Webb + */ +public class TestDefaultTestExecutionListenersPostProcessor + implements DefaultTestExecutionListenersPostProcessor { + + @Override + public Set> postProcessDefaultTestExecutionListeners( + Set> listeners) { + listeners.add(ExampleTestExecutionListener.class); + return listeners; + } + + static class ExampleTestExecutionListener extends AbstractTestExecutionListener { + + @Override + public void prepareTestInstance(TestContext testContext) throws Exception { + Object testInstance = testContext.getTestInstance(); + if (testInstance instanceof SpringBootTestContextBootstrapperTests) { + SpringBootTestContextBootstrapperTests test = (SpringBootTestContextBootstrapperTests) testInstance; + test.defaultTestExecutionListenersPostProcessorCalled = true; + } + } + + } + +} diff --git a/spring-boot-test/src/test/resources/META-INF/spring.factories b/spring-boot-test/src/test/resources/META-INF/spring.factories new file mode 100644 index 0000000000..5eecd4c7e9 --- /dev/null +++ b/spring-boot-test/src/test/resources/META-INF/spring.factories @@ -0,0 +1,2 @@ +org.springframework.boot.test.context.DefaultTestExecutionListenersPostProcessor=\ +org.springframework.boot.test.context.bootstrap.TestDefaultTestExecutionListenersPostProcessor