From 4dc9bbe127fdd4ca681985410b34170a29f65e51 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Wed, 22 Apr 2020 19:09:04 -0700 Subject: [PATCH] @SpringBootTest classes with different args shouldn't share a context Fixes gh-20866 --- .../test/context/SpringBootContextLoader.java | 11 +-- ...BootTestArgsTrackingContextCustomizer.java | 69 +++++++++++++++++++ .../SpringBootTestContextBootstrapper.java | 9 ++- ...pringBootTestContextBootstrapperTests.java | 40 ++++++++++- 4 files changed, 120 insertions(+), 9 deletions(-) create mode 100644 spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootTestArgsTrackingContextCustomizer.java diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java index 2bcaaebdb9f..744cb721322 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java @@ -82,8 +82,6 @@ import org.springframework.web.context.support.GenericWebApplicationContext; */ public class SpringBootContextLoader extends AbstractContextLoader { - private static final String[] NO_ARGS = new String[0]; - @Override public ApplicationContext loadContext(MergedContextConfiguration config) throws Exception { Class[] configClasses = config.getClasses(); @@ -149,11 +147,16 @@ public class SpringBootContextLoader extends AbstractContextLoader { * empty array. * @param config the source context configuration * @return the application arguments to use + * @deprecated since 2.2.7 * @see SpringApplication#run(String...) */ protected String[] getArgs(MergedContextConfiguration config) { - return MergedAnnotations.from(config.getTestClass(), SearchStrategy.TYPE_HIERARCHY).get(SpringBootTest.class) - .getValue("args", String[].class).orElse(NO_ARGS); + ContextCustomizer customizer = config.getContextCustomizers().stream() + .filter((c) -> c instanceof SpringBootTestArgsTrackingContextCustomizer).findFirst().orElse(null); + if (customizer != null) { + return ((SpringBootTestArgsTrackingContextCustomizer) customizer).getArgs(); + } + return SpringBootTestArgsTrackingContextCustomizer.NO_ARGS; } private void setActiveProfiles(ConfigurableEnvironment environment, String[] profiles) { diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootTestArgsTrackingContextCustomizer.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootTestArgsTrackingContextCustomizer.java new file mode 100644 index 00000000000..b1486bb7e4f --- /dev/null +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootTestArgsTrackingContextCustomizer.java @@ -0,0 +1,69 @@ +/* + * Copyright 2012-2020 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 + * + * https://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.Arrays; + +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.core.annotation.MergedAnnotations; +import org.springframework.test.context.ContextCustomizer; +import org.springframework.test.context.MergedContextConfiguration; + +/** + * {@link ContextCustomizer} to track application arguments that are used in a + * {@link SpringBootTest}. The application arguments are taken into account when + * evaluating a {@link MergedContextConfiguration} to determine if a context can be shared + * between tests. + * + * @author Madhura Bhave + */ +class SpringBootTestArgsTrackingContextCustomizer implements ContextCustomizer { + + static final String[] NO_ARGS = new String[0]; + + private String[] args; + + SpringBootTestArgsTrackingContextCustomizer(Class testClass) { + this.args = MergedAnnotations.from(testClass, MergedAnnotations.SearchStrategy.TYPE_HIERARCHY) + .get(SpringBootTest.class).getValue("args", String[].class).orElse(NO_ARGS); + } + + @Override + public void customizeContext(ConfigurableApplicationContext context, MergedContextConfiguration mergedConfig) { + + } + + /** + * Return the application arguments that are tracked by this customizer. + * @return the args + */ + String[] getArgs() { + return this.args; + } + + @Override + public boolean equals(Object obj) { + return (obj != null) && (getClass() == obj.getClass()) + && Arrays.equals(this.args, ((SpringBootTestArgsTrackingContextCustomizer) obj).args); + } + + @Override + public int hashCode() { + return Arrays.hashCode(this.args); + } + +} diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootTestContextBootstrapper.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootTestContextBootstrapper.java index 9301f864960..ef5a566b367 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootTestContextBootstrapper.java +++ b/spring-boot-project/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.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -40,6 +41,7 @@ import org.springframework.core.env.Environment; import org.springframework.core.io.support.SpringFactoriesLoader; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.ContextConfigurationAttributes; +import org.springframework.test.context.ContextCustomizer; import org.springframework.test.context.ContextHierarchy; import org.springframework.test.context.ContextLoader; import org.springframework.test.context.MergedContextConfiguration; @@ -355,11 +357,12 @@ public class SpringBootTestContextBootstrapper extends DefaultTestContextBootstr */ protected final MergedContextConfiguration createModifiedConfig(MergedContextConfiguration mergedConfig, Class[] classes, String[] propertySourceProperties) { + Set contextCustomizers = new LinkedHashSet<>(mergedConfig.getContextCustomizers()); + contextCustomizers.add(new SpringBootTestArgsTrackingContextCustomizer(mergedConfig.getTestClass())); return new MergedContextConfiguration(mergedConfig.getTestClass(), mergedConfig.getLocations(), classes, mergedConfig.getContextInitializerClasses(), mergedConfig.getActiveProfiles(), - mergedConfig.getPropertySourceLocations(), propertySourceProperties, - mergedConfig.getContextCustomizers(), mergedConfig.getContextLoader(), - getCacheAwareContextLoaderDelegate(), mergedConfig.getParent()); + mergedConfig.getPropertySourceLocations(), propertySourceProperties, contextCustomizers, + mergedConfig.getContextLoader(), getCacheAwareContextLoaderDelegate(), mergedConfig.getParent()); } } diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/SpringBootTestContextBootstrapperTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/SpringBootTestContextBootstrapperTests.java index a64bdd0ca22..ad026da4e34 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/SpringBootTestContextBootstrapperTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/bootstrap/SpringBootTestContextBootstrapperTests.java @@ -23,8 +23,11 @@ import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.boot.test.context.SpringBootTestContextBootstrapper; import org.springframework.test.context.BootstrapContext; import org.springframework.test.context.CacheAwareContextLoaderDelegate; +import org.springframework.test.context.TestContext; import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.test.util.ReflectionTestUtils; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -50,15 +53,33 @@ class SpringBootTestContextBootstrapperTests { buildTestContext(SpringBootTestMockWebEnvironmentAndWebAppConfiguration.class); } + @Test + void mergedContextConfigurationWhenArgsDifferentShouldNotBeConsideredEqual() { + TestContext context = buildTestContext(SpringBootTestArgsConfiguration.class); + Object contextConfiguration = ReflectionTestUtils.getField(context, "mergedContextConfiguration"); + TestContext otherContext2 = buildTestContext(SpringBootTestOtherArgsConfiguration.class); + Object otherContextConfiguration = ReflectionTestUtils.getField(otherContext2, "mergedContextConfiguration"); + assertThat(contextConfiguration).isNotEqualTo(otherContextConfiguration); + } + + @Test + void mergedContextConfigurationWhenArgsSameShouldBeConsideredEqual() { + TestContext context = buildTestContext(SpringBootTestArgsConfiguration.class); + Object contextConfiguration = ReflectionTestUtils.getField(context, "mergedContextConfiguration"); + TestContext otherContext2 = buildTestContext(SpringBootTestSameArgsConfiguration.class); + Object otherContextConfiguration = ReflectionTestUtils.getField(otherContext2, "mergedContextConfiguration"); + assertThat(contextConfiguration).isEqualTo(otherContextConfiguration); + } + @SuppressWarnings("rawtypes") - private void buildTestContext(Class testClass) { + private TestContext buildTestContext(Class testClass) { SpringBootTestContextBootstrapper bootstrapper = new SpringBootTestContextBootstrapper(); BootstrapContext bootstrapContext = mock(BootstrapContext.class); bootstrapper.setBootstrapContext(bootstrapContext); given((Class) bootstrapContext.getTestClass()).willReturn(testClass); CacheAwareContextLoaderDelegate contextLoaderDelegate = mock(CacheAwareContextLoaderDelegate.class); given(bootstrapContext.getCacheAwareContextLoaderDelegate()).willReturn(contextLoaderDelegate); - bootstrapper.buildTestContext(); + return bootstrapper.buildTestContext(); } @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) @@ -73,4 +94,19 @@ class SpringBootTestContextBootstrapperTests { } + @SpringBootTest(args = "--app.test=same") + static class SpringBootTestArgsConfiguration { + + } + + @SpringBootTest(args = "--app.test=same") + static class SpringBootTestSameArgsConfiguration { + + } + + @SpringBootTest(args = "--app.test=different") + static class SpringBootTestOtherArgsConfiguration { + + } + }