From ada255d5849ab4a286946b0a82a6ed22ce5da352 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sat, 24 Oct 2020 17:47:56 +0200 Subject: [PATCH] Ignore duplicate config metadata for cache key in TestContext framework 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 --- .../context/support/ContextLoaderUtils.java | 25 ++- .../support/TestPropertySourceAttributes.java | 121 ++++++++++- .../support/TestPropertySourceUtils.java | 69 ++++++- .../MergedContextConfigurationTests.java | 2 +- .../BootstrapTestUtilsMergedConfigTests.java | 188 ++++++++++++++++++ .../support/TestPropertySourceUtilsTests.java | 53 ++++- 6 files changed, 430 insertions(+), 28 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java b/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java index d7bf5243dc..983dc31401 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java @@ -242,14 +242,35 @@ abstract class ContextLoaderUtils { annotationType.getName(), testClass.getName())); List attributesList = new ArrayList<>(); + ContextConfiguration previousAnnotation = null; + Class previousDeclaringClass = null; while (descriptor != null) { - convertContextConfigToConfigAttributesAndAddToList(descriptor.synthesizeAnnotation(), - descriptor.getRootDeclaringClass(), attributesList); + ContextConfiguration currentAnnotation = descriptor.synthesizeAnnotation(); + // Don't ignore duplicate @ContextConfiguration declaration without resources, + // because the ContextLoader will likely detect default resources specific to the + // annotated class. + if (currentAnnotation.equals(previousAnnotation) && hasResources(currentAnnotation)) { + if (logger.isDebugEnabled()) { + logger.debug(String.format("Ignoring duplicate %s declaration on [%s], " + + "since it is also declared on [%s].", currentAnnotation, + previousDeclaringClass.getName(), descriptor.getRootDeclaringClass().getName())); + } + } + else { + convertContextConfigToConfigAttributesAndAddToList(currentAnnotation, + descriptor.getRootDeclaringClass(), attributesList); + } + previousAnnotation = currentAnnotation; + previousDeclaringClass = descriptor.getRootDeclaringClass(); descriptor = descriptor.next(); } return attributesList; } + private static boolean hasResources(ContextConfiguration contextConfiguration) { + return (contextConfiguration.locations().length > 0 || contextConfiguration.classes().length > 0); + } + /** * Convenience method for creating a {@link ContextConfigurationAttributes} * instance from the supplied {@link ContextConfiguration} annotation and diff --git a/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceAttributes.java b/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceAttributes.java index 52f14ae139..84362fcc0f 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceAttributes.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceAttributes.java @@ -17,7 +17,7 @@ package org.springframework.test.context.support; import java.util.ArrayList; -import java.util.Collections; +import java.util.Arrays; import java.util.List; import org.apache.commons.logging.Log; @@ -25,7 +25,9 @@ import org.apache.commons.logging.LogFactory; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.io.ClassPathResource; +import org.springframework.core.log.LogMessage; import org.springframework.core.style.ToStringCreator; +import org.springframework.lang.Nullable; import org.springframework.test.context.TestPropertySource; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -53,6 +55,8 @@ class TestPropertySourceAttributes { private final Class declaringClass; + private final MergedAnnotation rootAnnotation; + private final List locations = new ArrayList<>(); private final boolean inheritLocations; @@ -64,25 +68,83 @@ class TestPropertySourceAttributes { TestPropertySourceAttributes(MergedAnnotation annotation) { this.declaringClass = declaringClass(annotation); + this.rootAnnotation = annotation.getRoot(); this.inheritLocations = annotation.getBoolean("inheritLocations"); this.inheritProperties = annotation.getBoolean("inheritProperties"); - mergePropertiesAndLocations(annotation); + addPropertiesAndLocationsFrom(annotation); } - private void mergePropertiesAndLocations(MergedAnnotation annotation) { - String[] locations = annotation.getStringArray("locations"); - String[] properties = annotation.getStringArray("properties"); + /** + * Merge this {@code TestPropertySourceAttributes} instance with the + * supplied {@code TestPropertySourceAttributes}, asserting that the two sets + * of test property source attributes have identical values for the + * {@link TestPropertySource#inheritLocations} and + * {@link TestPropertySource#inheritProperties} flags and that the two + * underlying annotations were declared on the same class. + * @since 5.2 + */ + void mergeWith(TestPropertySourceAttributes attributes) { + Assert.state(attributes.declaringClass == this.declaringClass, + () -> "Detected @TestPropertySource declarations within an aggregate index " + + "with different sources: " + this.declaringClass.getName() + " and " + + attributes.declaringClass.getName()); + logger.trace(LogMessage.format("Retrieved %s for declaring class [%s].", + attributes, this.declaringClass.getName())); + assertSameBooleanAttribute(this.inheritLocations, attributes.inheritLocations, + "inheritLocations", attributes); + assertSameBooleanAttribute(this.inheritProperties, attributes.inheritProperties, + "inheritProperties", attributes); + mergePropertiesAndLocationsFrom(attributes); + } + + private void assertSameBooleanAttribute(boolean expected, boolean actual, + String attributeName, TestPropertySourceAttributes that) { + + Assert.isTrue(expected == actual, () -> String.format( + "@%s on %s and @%s on %s must declare the same value for '%s' as other " + + "directly present or meta-present @TestPropertySource annotations", + this.rootAnnotation.getType().getSimpleName(), this.declaringClass.getSimpleName(), + that.rootAnnotation.getType().getSimpleName(), that.declaringClass.getSimpleName(), + attributeName)); + } + + private void addPropertiesAndLocationsFrom(MergedAnnotation mergedAnnotation) { + String[] locations = mergedAnnotation.getStringArray("locations"); + String[] properties = mergedAnnotation.getStringArray("properties"); + addPropertiesAndLocations(locations, properties, declaringClass(mergedAnnotation), false); + } + + private void mergePropertiesAndLocationsFrom(TestPropertySourceAttributes attributes) { + addPropertiesAndLocations(attributes.getLocations(), attributes.getProperties(), + attributes.getDeclaringClass(), true); + } + + private void addPropertiesAndLocations(String[] locations, String[] properties, + Class declaringClass, boolean prepend) { + if (ObjectUtils.isEmpty(locations) && ObjectUtils.isEmpty(properties)) { - Collections.addAll(this.locations, detectDefaultPropertiesFile(annotation)); + addAll(prepend, this.locations, detectDefaultPropertiesFile(declaringClass)); } else { - Collections.addAll(this.locations, locations); - Collections.addAll(this.properties, properties); + addAll(prepend, this.locations, locations); + addAll(prepend, this.properties, properties); } } - private String detectDefaultPropertiesFile(MergedAnnotation annotation) { - Class testClass = declaringClass(annotation); + /** + * Add all of the supplied elements to the provided list, honoring the + * {@code prepend} flag. + *

If the {@code prepend} flag is {@code false}, the elements will appended + * to the list. + * @param prepend whether the elements should be prepended to the list + * @param list the list to which to add the elements + * @param elements the elements to add to the list + */ + private void addAll(boolean prepend, List list, String... elements) { + list.addAll((prepend ? 0 : list.size()), Arrays.asList(elements)); + } + + private String detectDefaultPropertiesFile(Class testClass) { String resourcePath = ClassUtils.convertClassNameToResourcePath(testClass.getName()) + ".properties"; ClassPathResource classPathResource = new ClassPathResource(resourcePath); if (!classPathResource.exists()) { @@ -153,6 +215,45 @@ class TestPropertySourceAttributes { return this.inheritProperties; } + boolean isEmpty() { + return (this.locations.isEmpty() && this.properties.isEmpty()); + } + + @Override + public boolean equals(@Nullable Object other) { + if (this == other) { + return true; + } + if (other == null || other.getClass() != getClass()) { + return false; + } + + TestPropertySourceAttributes that = (TestPropertySourceAttributes) other; + if (!this.locations.equals(that.locations)) { + return false; + } + if (!this.properties.equals(that.properties)) { + return false; + } + if (this.inheritLocations != that.inheritLocations) { + return false; + } + if (this.inheritProperties != that.inheritProperties) { + return false; + } + + return true; + } + + @Override + public int hashCode() { + int result = this.locations.hashCode(); + result = 31 * result + this.properties.hashCode(); + result = 31 * result + (this.inheritLocations ? 1231 : 1237); + result = 31 * result + (this.inheritProperties ? 1231 : 1237); + return result; + } + /** * Provide a String representation of the {@code @TestPropertySource} * attributes and declaring class. diff --git a/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java b/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java index 8c75f86336..5d5a47f66f 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.Map; import java.util.Properties; import java.util.stream.Collectors; -import java.util.stream.Stream; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -44,6 +43,7 @@ import org.springframework.core.env.PropertySources; import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; import org.springframework.core.io.support.ResourcePropertySource; +import org.springframework.lang.Nullable; import org.springframework.test.context.TestContextAnnotationUtils; import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.util.TestContextResourceUtils; @@ -76,18 +76,71 @@ public abstract class TestPropertySourceUtils { static MergedTestPropertySources buildMergedTestPropertySources(Class testClass) { - List attributesList = - findRepeatableAnnotations(testClass, TestPropertySource.class) - .map(TestPropertySourceAttributes::new) - .collect(Collectors.toList()); + List attributesList = new ArrayList<>(); + + TestPropertySourceAttributes previousAttributes = null; + // Iterate over all aggregate levels, where each level is represented by + // a list of merged annotations found at that level (e.g., on a test + // class in the class hierarchy). + for (List> aggregatedAnnotations : + findRepeatableAnnotations(testClass, TestPropertySource.class)) { + + // Convert all of the merged annotations for the current aggregate + // level to a list of TestPropertySourceAttributes. + List aggregatedAttributesList = + aggregatedAnnotations.stream().map(TestPropertySourceAttributes::new).collect(Collectors.toList()); + // Merge all TestPropertySourceAttributes instances for the current + // aggregate level into a single TestPropertySourceAttributes instance. + TestPropertySourceAttributes mergedAttributes = mergeTestPropertySourceAttributes(aggregatedAttributesList); + if (mergedAttributes != null) { + if (!duplicationDetected(mergedAttributes, previousAttributes)) { + attributesList.add(mergedAttributes); + } + previousAttributes = mergedAttributes; + } + } if (attributesList.isEmpty()) { return MergedTestPropertySources.empty(); } - return new MergedTestPropertySources(mergeLocations(attributesList), mergeProperties(attributesList)); } + @Nullable + private static TestPropertySourceAttributes mergeTestPropertySourceAttributes( + List aggregatedAttributesList) { + + TestPropertySourceAttributes mergedAttributes = null; + TestPropertySourceAttributes previousAttributes = null; + for (TestPropertySourceAttributes currentAttributes : aggregatedAttributesList) { + if (mergedAttributes == null) { + mergedAttributes = currentAttributes; + } + else if (!duplicationDetected(currentAttributes, previousAttributes)) { + mergedAttributes.mergeWith(currentAttributes); + } + previousAttributes = currentAttributes; + } + + return mergedAttributes; + } + + private static boolean duplicationDetected(TestPropertySourceAttributes currentAttributes, + TestPropertySourceAttributes previousAttributes) { + + boolean duplicationDetected = + (currentAttributes.equals(previousAttributes) && !currentAttributes.isEmpty()); + + if (duplicationDetected && logger.isDebugEnabled()) { + logger.debug(String.format("Ignoring duplicate %s declaration on %s, " + + "since it is also declared on %s.", currentAttributes, + previousAttributes.getDeclaringClass().getName(), + currentAttributes.getDeclaringClass().getName())); + } + + return duplicationDetected; + } + private static String[] mergeLocations(List attributesList) { List locations = new ArrayList<>(); for (TestPropertySourceAttributes attrs : attributesList) { @@ -273,12 +326,12 @@ public abstract class TestPropertySourceUtils { return map; } - private static Stream> findRepeatableAnnotations( + private static List>> findRepeatableAnnotations( Class clazz, Class annotationType) { List>> listOfLists = new ArrayList<>(); findRepeatableAnnotations(clazz, annotationType, listOfLists, new int[] {0}); - return listOfLists.stream().flatMap(List::stream); + return listOfLists; } private static void findRepeatableAnnotations( diff --git a/spring-test/src/test/java/org/springframework/test/context/MergedContextConfigurationTests.java b/spring-test/src/test/java/org/springframework/test/context/MergedContextConfigurationTests.java index 26bac7594c..25c8530c49 100644 --- a/spring-test/src/test/java/org/springframework/test/context/MergedContextConfigurationTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/MergedContextConfigurationTests.java @@ -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(), diff --git a/spring-test/src/test/java/org/springframework/test/context/support/BootstrapTestUtilsMergedConfigTests.java b/spring-test/src/test/java/org/springframework/test/context/support/BootstrapTestUtilsMergedConfigTests.java index 8462555acb..4336ec5c83 100644 --- a/spring-test/src/test/java/org/springframework/test/context/support/BootstrapTestUtilsMergedConfigTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/support/BootstrapTestUtilsMergedConfigTests.java @@ -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 enclosingClass = EmptyConfigTestCase.class; + Class 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 { + } + } + } diff --git a/spring-test/src/test/java/org/springframework/test/context/support/TestPropertySourceUtilsTests.java b/spring-test/src/test/java/org/springframework/test/context/support/TestPropertySourceUtilsTests.java index 385180cae2..7797a2455d 100644 --- a/spring-test/src/test/java/org/springframework/test/context/support/TestPropertySourceUtilsTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/support/TestPropertySourceUtilsTests.java @@ -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 { + } + }