Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Ignore duplicate config metadata for cache key in TestContext framework

Browse Source 
Prior to this commit, if a developer accidentally copied and pasted the
same @ContextConfiguration or @TestPropertySource declaration from a
test class to one of its subclasses or nested test classes, the Spring
TestContext Framework (TCF) would merge the inherited configuration
with the local configuration, resulting in different sets of
configuration metadata which in turn resulted in a different
ApplicationContext instance being loaded for the test classes. This
behavior led to unnecessary creation of identical application contexts
in the context cache for the TCF stored under different keys.

This commit ignores duplicate configuration metadata when generating
the ApplicationContext cache key (i.e., MergedContextConfiguration) in
the TCF. This is performed for the following annotations.

- @ContextConfiguration
- @ActiveProfiles (support already existed prior to this commit)
- @TestPropertySource

Specifically, if @ContextConfiguration or @TestPropertySource is
declared on a test class and its subclass or nested test class with the
exact same attributes, only one instance of the annotation will be used
to generate the cache key for the resulting ApplicationContext. The
exception to this rule is an "empty" annotation declaration. An empty
@ContextConfiguration or @TestPropertySource declaration signals that
Spring (or a third-party SmartContextLoader) should detect default
configuration specific to the annotated class. Thus, multiple empty
@ContextConfiguration or @TestPropertySource declarations within a test
class hierarchy are not considered to be duplicate configuration and
are therefore not ignored.

Since @TestPropertySource is a @Repeatable annotation, the same
duplicate configuration detection logic is applied for multiple
@TestPropertySource declarations on a single test class or test
interface.

In addition, this commit reinstates validation of the rules for
repeated @TestPropertySource annotations that was removed when support
for @NestedTestConfiguration was introduced.

Closes gh-25800
This commit is contained in:
Sam Brannen
2020-10-24 17:47:56 +02:00
parent 7aef0c78a0
commit ada255d584
6 changed files with 430 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
spring-test/src/test/java/org/springframework/test/context/MergedContextConfigurationTests.java
View File

@@ -345,7 +345,7 @@ class MergedContextConfigurationTests {
@Test
void equalsWithSameDuplicateProfiles() {
String[] activeProfiles1 = new String[] { "catbert", "dogbert" };
String[] activeProfiles2 = new String[] { "catbert", "dogbert", "catbert", "dogbert", "catbert" };
String[] activeProfiles2 = new String[] { "dogbert", "catbert", "dogbert", "catbert" };
MergedContextConfiguration mergedConfig1 = new MergedContextConfiguration(getClass(),
EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles1, loader);
MergedContextConfiguration mergedConfig2 = new MergedContextConfiguration(getClass(),

188
spring-test/src/test/java/org/springframework/test/context/support/BootstrapTestUtilsMergedConfigTests.java
View File

@@ -24,11 +24,14 @@ import java.lang.annotation.Target;
import org.assertj.core.api.AssertionsForClassTypes;
import org.junit.jupiter.api.Test;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.BootstrapTestUtils;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.ContextLoader;
import org.springframework.test.context.MergedContextConfiguration;
import org.springframework.test.context.TestPropertySource;
import org.springframework.test.context.support.BootstrapTestUtilsMergedConfigTests.EmptyConfigTestCase.Nested;
import org.springframework.test.context.web.WebDelegatingSmartContextLoader;
import org.springframework.test.context.web.WebMergedContextConfiguration;
@@ -352,6 +355,109 @@ class BootstrapTestUtilsMergedConfigTests extends AbstractContextConfigurationUt
array(BarConfig.class), AnnotationConfigContextLoader.class);
}
/**
* @since 5.3
*/
@Test
void buildMergedConfigWithDuplicateConfigurationOnSuperclassAndSubclass() {
compareApplesToApples(AppleConfigTestCase.class, DuplicateConfigAppleConfigTestCase.class);
compareApplesToApples(DuplicateConfigAppleConfigTestCase.class, SubDuplicateConfigAppleConfigTestCase.class);
compareApplesToOranges(ApplesAndOrangesConfigTestCase.class, DuplicateConfigApplesAndOrangesConfigTestCase.class);
compareApplesToOranges(DuplicateConfigApplesAndOrangesConfigTestCase.class, SubDuplicateConfigApplesAndOrangesConfigTestCase.class);
}
/**
* @since 5.3
*/
@Test
void buildMergedConfigWithDuplicateConfigurationOnEnclosingClassAndNestedClass() {
compareApplesToApples(AppleConfigTestCase.class, AppleConfigTestCase.Nested.class);
compareApplesToApples(AppleConfigTestCase.Nested.class, AppleConfigTestCase.Nested.DoubleNested.class);
}
private void compareApplesToApples(Class<?> parent, Class<?> child) {
MergedContextConfiguration parentMergedConfig = buildMergedContextConfiguration(parent);
assertMergedConfig(parentMergedConfig, parent, EMPTY_STRING_ARRAY, array(AppleConfig.class),
DelegatingSmartContextLoader.class);
MergedContextConfiguration childMergedConfig = buildMergedContextConfiguration(child);
assertMergedConfig(childMergedConfig, child, EMPTY_STRING_ARRAY, array(AppleConfig.class),
DelegatingSmartContextLoader.class);
assertThat(parentMergedConfig.getActiveProfiles()).as("active profiles")
.containsExactly("apples")
.isEqualTo(childMergedConfig.getActiveProfiles());
assertThat(parentMergedConfig).isEqualTo(childMergedConfig);
}
private void compareApplesToOranges(Class<?> parent, Class<?> child) {
MergedContextConfiguration parentMergedConfig = buildMergedContextConfiguration(parent);
assertMergedConfig(parentMergedConfig, parent, EMPTY_STRING_ARRAY, array(AppleConfig.class),
DelegatingSmartContextLoader.class);
MergedContextConfiguration childMergedConfig = buildMergedContextConfiguration(child);
assertMergedConfig(childMergedConfig, child, EMPTY_STRING_ARRAY, array(AppleConfig.class),
DelegatingSmartContextLoader.class);
assertThat(parentMergedConfig.getActiveProfiles()).as("active profiles")
.containsExactly("apples", "oranges")
.isEqualTo(childMergedConfig.getActiveProfiles());
assertThat(parentMergedConfig).isEqualTo(childMergedConfig);
}
/**
* @since 5.3
*/
@Test
void buildMergedConfigWithEmptyConfigurationOnSuperclassAndSubclass() {
// not equal because different defaults are detected for each class
assertEmptyConfigsAreNotEqual(EmptyConfigTestCase.class, SubEmptyConfigTestCase.class, SubSubEmptyConfigTestCase.class);
}
private void assertEmptyConfigsAreNotEqual(Class<?> parent, Class<?> child, Class<?> grandchild) {
MergedContextConfiguration parentMergedConfig = buildMergedContextConfiguration(parent);
assertMergedConfig(parentMergedConfig, parent, EMPTY_STRING_ARRAY,
array(EmptyConfigTestCase.Config.class), DelegatingSmartContextLoader.class);
MergedContextConfiguration childMergedConfig = buildMergedContextConfiguration(child);
assertMergedConfig(childMergedConfig, child, EMPTY_STRING_ARRAY,
array(EmptyConfigTestCase.Config.class, SubEmptyConfigTestCase.Config.class), DelegatingSmartContextLoader.class);
assertThat(parentMergedConfig.getActiveProfiles()).as("active profiles")
.isEqualTo(childMergedConfig.getActiveProfiles());
assertThat(parentMergedConfig).isNotEqualTo(childMergedConfig);
MergedContextConfiguration grandchildMergedConfig = buildMergedContextConfiguration(grandchild);
assertMergedConfig(grandchildMergedConfig, grandchild, EMPTY_STRING_ARRAY,
array(EmptyConfigTestCase.Config.class, SubEmptyConfigTestCase.Config.class, SubSubEmptyConfigTestCase.Config.class),
DelegatingSmartContextLoader.class);
assertThat(childMergedConfig.getActiveProfiles()).as("active profiles")
.isEqualTo(grandchildMergedConfig.getActiveProfiles());
assertThat(childMergedConfig).isNotEqualTo(grandchildMergedConfig);
}
/**
* @since 5.3
*/
@Test
void buildMergedConfigWithEmptyConfigurationOnEnclosingClassAndExplicitConfigOnNestedClass() {
Class<EmptyConfigTestCase> enclosingClass = EmptyConfigTestCase.class;
Class<Nested> nestedClass = EmptyConfigTestCase.Nested.class;
MergedContextConfiguration enclosingMergedConfig = buildMergedContextConfiguration(enclosingClass);
assertMergedConfig(enclosingMergedConfig, enclosingClass, EMPTY_STRING_ARRAY,
array(EmptyConfigTestCase.Config.class), DelegatingSmartContextLoader.class);
MergedContextConfiguration nestedMergedConfig = buildMergedContextConfiguration(nestedClass);
assertMergedConfig(nestedMergedConfig, nestedClass, EMPTY_STRING_ARRAY,
array(EmptyConfigTestCase.Config.class, AppleConfig.class), DelegatingSmartContextLoader.class);
assertThat(enclosingMergedConfig.getActiveProfiles()).as("active profiles")
.isEqualTo(nestedMergedConfig.getActiveProfiles());
assertThat(enclosingMergedConfig).isNotEqualTo(nestedMergedConfig);
}
@ContextConfiguration
@Retention(RetentionPolicy.RUNTIME)
@@ -396,4 +502,86 @@ class BootstrapTestUtilsMergedConfigTests extends AbstractContextConfigurationUt
static class RelativeFooXmlLocation {
}
static class AppleConfig {
}
@ContextConfiguration(classes = AppleConfig.class)
@ActiveProfiles("apples")
static class AppleConfigTestCase {
@ContextConfiguration(classes = AppleConfig.class)
@ActiveProfiles({"apples", "apples"})
class Nested {
@ContextConfiguration(classes = AppleConfig.class)
@ActiveProfiles({"apples", "apples", "apples"})
class DoubleNested {
}
}
}
@ContextConfiguration(classes = AppleConfig.class)
@ActiveProfiles({"apples", "apples"})
static class DuplicateConfigAppleConfigTestCase extends AppleConfigTestCase {
}
@ContextConfiguration(classes = AppleConfig.class)
@ActiveProfiles({"apples", "apples", "apples"})
static class SubDuplicateConfigAppleConfigTestCase extends DuplicateConfigAppleConfigTestCase {
}
@ContextConfiguration(classes = AppleConfig.class)
@ActiveProfiles({"apples", "oranges"})
static class ApplesAndOrangesConfigTestCase {
@ContextConfiguration(classes = AppleConfig.class)
@ActiveProfiles(profiles = {"oranges", "apples"}, inheritProfiles = false)
class Nested {
@ContextConfiguration(classes = AppleConfig.class)
@ActiveProfiles(profiles = {"apples", "oranges", "apples"}, inheritProfiles = false)
class DoubleNested {
}
}
}
@ContextConfiguration(classes = AppleConfig.class)
@ActiveProfiles(profiles = {"oranges", "apples"}, inheritProfiles = false)
static class DuplicateConfigApplesAndOrangesConfigTestCase extends ApplesAndOrangesConfigTestCase {
}
@ContextConfiguration(classes = AppleConfig.class)
@ActiveProfiles(profiles = {"apples", "oranges", "apples"}, inheritProfiles = false)
static class SubDuplicateConfigApplesAndOrangesConfigTestCase extends DuplicateConfigApplesAndOrangesConfigTestCase {
}
@ContextConfiguration
static class EmptyConfigTestCase {
@ContextConfiguration(classes = AppleConfig.class)
class Nested {
// inner classes cannot have static nested @Configuration classes
}
@Configuration
static class Config {
}
}
@ContextConfiguration
static class SubEmptyConfigTestCase extends EmptyConfigTestCase {
@Configuration
static class Config {
}
}
@ContextConfiguration
static class SubSubEmptyConfigTestCase extends SubEmptyConfigTestCase {
@Configuration
static class Config {
}
}
}

53
spring-test/src/test/java/org/springframework/test/context/support/TestPropertySourceUtilsTests.java
View File

@@ -20,7 +20,7 @@ import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.Map;
import org.junit.jupiter.api.Disabled;
import org.assertj.core.api.SoftAssertions;
import org.junit.jupiter.api.Test;
import org.springframework.context.ConfigurableApplicationContext;
@@ -81,7 +81,6 @@ class TestPropertySourceUtilsTests {
}
@Test
@Disabled("Validation for repeated @TestPropertySource annotations has been removed")
void repeatedTestPropertySourcesWithConflictingInheritLocationsFlags() {
assertThatIllegalArgumentException()
.isThrownBy(() -> buildMergedTestPropertySources(RepeatedPropertySourcesWithConflictingInheritLocationsFlags.class))
@@ -91,7 +90,6 @@ class TestPropertySourceUtilsTests {
}
@Test
@Disabled("Validation for repeated @TestPropertySource annotations has been removed")
void repeatedTestPropertySourcesWithConflictingInheritPropertiesFlags() {
assertThatIllegalArgumentException()
.isThrownBy(() -> buildMergedTestPropertySources(RepeatedPropertySourcesWithConflictingInheritPropertiesFlags.class))
@@ -124,6 +122,33 @@ class TestPropertySourceUtilsTests {
asArray("classpath:/foo1.xml", "classpath:/foo2.xml"), asArray("k1a=v1a", "k1b: v1b"));
}
/**
* @since 5.3
*/
@Test
void locationsAndPropertiesDuplicatedLocally() {
assertMergedTestPropertySources(LocallyDuplicatedLocationsAndProperties.class,
asArray("classpath:/foo1.xml", "classpath:/foo2.xml"), asArray("k1a=v1a", "k1b: v1b"));
}
/**
* @since 5.3
*/
@Test
void locationsAndPropertiesDuplicatedOnSuperclass() {
assertMergedTestPropertySources(DuplicatedLocationsAndPropertiesPropertySources.class,
asArray("classpath:/foo1.xml", "classpath:/foo2.xml"), asArray("k1a=v1a", "k1b: v1b"));
}
/**
* @since 5.3
*/
@Test
void locationsAndPropertiesDuplicatedOnEnclosingClass() {
assertMergedTestPropertySources(LocationsAndPropertiesPropertySources.Nested.class,
asArray("classpath:/foo1.xml", "classpath:/foo2.xml"), asArray("k1a=v1a", "k1b: v1b"));
}
@Test
void extendedLocationsAndProperties() {
assertMergedTestPropertySources(ExtendedPropertySources.class,
@@ -149,7 +174,6 @@ class TestPropertySourceUtilsTests {
asArray("classpath:/baz.properties"), KEY_VALUE_PAIR);
}
@Test
void addPropertiesFilesToEnvironmentWithNullContext() {
assertThatIllegalArgumentException()
@@ -269,9 +293,11 @@ class TestPropertySourceUtilsTests {
String[] expectedProperties) {
MergedTestPropertySources mergedPropertySources = buildMergedTestPropertySources(testClass);
assertThat(mergedPropertySources).isNotNull();
assertThat(mergedPropertySources.getLocations()).isEqualTo(expectedLocations);
assertThat(mergedPropertySources.getProperties()).isEqualTo(expectedProperties);
SoftAssertions.assertSoftly(softly -> {
softly.assertThat(mergedPropertySources).isNotNull();
softly.assertThat(mergedPropertySources.getLocations()).isEqualTo(expectedLocations);
softly.assertThat(mergedPropertySources.getProperties()).isEqualTo(expectedProperties);
});
}
@@ -318,6 +344,10 @@ class TestPropertySourceUtilsTests {
@TestPropertySource(locations = { "/foo1.xml", "/foo2.xml" }, properties = { "k1a=v1a", "k1b: v1b" })
static class LocationsAndPropertiesPropertySources {
@TestPropertySource(locations = { "/foo1.xml", "/foo2.xml" }, properties = { "k1a=v1a", "k1b: v1b" })
class Nested {
}
}
static class InheritedPropertySources extends LocationsAndPropertiesPropertySources {
@@ -339,4 +369,13 @@ class TestPropertySourceUtilsTests {
static class OverriddenLocationsAndPropertiesPropertySources extends LocationsAndPropertiesPropertySources {
}
@TestPropertySource(locations = { "/foo1.xml", "/foo2.xml" }, properties = { "k1a=v1a", "k1b: v1b" })
@TestPropertySource(locations = { "/foo1.xml", "/foo2.xml" }, properties = { "k1a=v1a", "k1b: v1b" })
static class LocallyDuplicatedLocationsAndProperties {
}
@TestPropertySource(locations = { "/foo1.xml", "/foo2.xml" }, properties = { "k1a=v1a", "k1b: v1b" })
static class DuplicatedLocationsAndPropertiesPropertySources extends LocationsAndPropertiesPropertySources {
}
}