Reject identical Bean Overrides

Prior to this commit, the Bean Override feature in the Spring
TestContext Framework (for annotations such as @⁠MockitoBean and
@⁠TestBean) silently allowed one bean override to override another
"identical" bean override; however, Spring Boot's @⁠MockBean and
@⁠SpyBean support preemptively rejects identical overrides and throws
an IllegalStateException to signal the configuration error to the user.

To align with the behavior of @⁠MockBean and @⁠SpyBean in Spring Boot,
and to help developers avoid scenarios that are potentially confusing
or difficult to debug, this commit rejects identical bean overrides in
the Spring TestContext Framework.

Note, however, that it is still possible for a bean override to
override a logically equivalent bean override. For example, a
@⁠TestBean can override a @⁠MockitoBean, and vice versa.

Closes gh-34054

Author: sbrannen

Diff for: spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideContextCustomizerFactory.java

@@ -24,6 +24,7 @@
 import org.springframework.test.context.ContextConfigurationAttributes;
 import org.springframework.test.context.ContextCustomizerFactory;
 import org.springframework.test.context.TestContextAnnotationUtils;
+import org.springframework.util.Assert;
 
 /**
  * {@link ContextCustomizerFactory} implementation that provides support for
@@ -51,10 +52,13 @@ public BeanOverrideContextCustomizer createContextCustomizer(Class<?> testClass,
 	}
 
 	private void findBeanOverrideHandler(Class<?> testClass, Set<BeanOverrideHandler> handlers) {
-		handlers.addAll(BeanOverrideHandler.forTestClass(testClass));
 		if (TestContextAnnotationUtils.searchEnclosingClass(testClass)) {
 			findBeanOverrideHandler(testClass.getEnclosingClass(), handlers);
 		}
+		BeanOverrideHandler.forTestClass(testClass).forEach(handler ->
+				Assert.state(handlers.add(handler), () ->
+						"Duplicate BeanOverrideHandler discovered in test class %s: %s"
+							.formatted(testClass.getName(), handler)));
 	}
 
 }

Diff for: spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideContextCustomizerFactoryTests.java

@@ -19,18 +19,20 @@
 import java.util.Collections;
 import java.util.function.Consumer;
 
-import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.lang.Nullable;
 import org.springframework.test.context.bean.override.DummyBean.DummyBeanOverrideProcessor.DummyBeanOverrideHandler;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 
 /**
  * Tests for {@link BeanOverrideContextCustomizerFactory}.
  *
  * @author Stephane Nicoll
+ * @author Sam Brannen
+ * @since 6.2
  */
 class BeanOverrideContextCustomizerFactoryTests {
 
@@ -65,6 +67,15 @@ void createContextCustomizerWhenNestedTestHasBeanOverrideAsWellAsTheParent() {
 				.hasSize(2);
 	}
 
+	@Test  // gh-34054
+	void failsWithDuplicateBeanOverrides() {
+		Class<?> testClass = DuplicateOverridesTestCase.class;
+		assertThatIllegalStateException()
+				.isThrownBy(() -> createContextCustomizer(testClass))
+				.withMessageStartingWith("Duplicate BeanOverrideHandler discovered in test class " + testClass.getName())
+				.withMessageContaining("DummyBeanOverrideHandler");
+	}
+
 
 	private Consumer<BeanOverrideHandler> dummyHandler(@Nullable String beanName, Class<?> beanType) {
 		return dummyHandler(beanName, beanType, BeanOverrideStrategy.REPLACE);
@@ -80,33 +91,41 @@ private Consumer<BeanOverrideHandler> dummyHandler(@Nullable String beanName, Cl
 	}
 
 	@Nullable
-	BeanOverrideContextCustomizer createContextCustomizer(Class<?> testClass) {
+	private BeanOverrideContextCustomizer createContextCustomizer(Class<?> testClass) {
 		return this.factory.createContextCustomizer(testClass, Collections.emptyList());
 	}
 
+
 	static class Test1 {
 
 		@DummyBean
 		private String descriptor;
-
 	}
 
 	static class Test2 {
 
 		@DummyBean
 		private String name;
 
-		@Nested
+		// @Nested
 		class Orange {
 		}
 
-		@Nested
+		// @Nested
 		class Green {
 
 			@DummyBean(beanName = "counterBean")
 			private Integer counter;
-
 		}
 	}
 
+	static class DuplicateOverridesTestCase {
+
+		@DummyBean(beanName = "text")
+		String text1;
+
+		@DummyBean(beanName = "text")
+		String text2;
+	}
+
 }

Diff for: spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanForByNameLookupIntegrationTests.java

@@ -40,9 +40,6 @@ public class TestBeanForByNameLookupIntegrationTests {
 	@TestBean(name = "field")
 	String field;
 
-	@TestBean(name = "field")
-	String renamed;
-
 	@TestBean(name = "methodRenamed1", methodName = "field")
 	String methodRenamed1;
 
@@ -60,12 +57,6 @@ void fieldHasOverride(ApplicationContext ctx) {
 		assertThat(field).as("injection point").isEqualTo("fieldOverride");
 	}
 
-	@Test
-	void renamedFieldHasOverride(ApplicationContext ctx) {
-		assertThat(ctx.getBean("field")).as("applicationContext").isEqualTo("fieldOverride");
-		assertThat(renamed).as("injection point").isEqualTo("fieldOverride");
-	}
-
 	@Test
 	void fieldWithMethodNameHasOverride(ApplicationContext ctx) {
 		assertThat(ctx.getBean("methodRenamed1")).as("applicationContext").isEqualTo("fieldOverride");
@@ -80,9 +71,6 @@ public class TestBeanFieldInEnclosingClassTests {
 		@TestBean(name = "nestedField")
 		String nestedField;
 
-		@TestBean(name = "nestedField")
-		String renamed2;
-
 		@TestBean(name = "methodRenamed2", methodName = "nestedField")
 		String methodRenamed2;
 
@@ -93,12 +81,6 @@ void fieldHasOverride(ApplicationContext ctx) {
 			assertThat(field).as("injection point").isEqualTo("fieldOverride");
 		}
 
-		@Test
-		void renamedFieldHasOverride(ApplicationContext ctx) {
-			assertThat(ctx.getBean("field")).as("applicationContext").isEqualTo("fieldOverride");
-			assertThat(renamed).as("injection point").isEqualTo("fieldOverride");
-		}
-
 		@Test
 		void fieldWithMethodNameHasOverride(ApplicationContext ctx) {
 			assertThat(ctx.getBean("methodRenamed1")).as("applicationContext").isEqualTo("fieldOverride");
@@ -111,12 +93,6 @@ void nestedFieldHasOverride(ApplicationContext ctx) {
 			assertThat(nestedField).isEqualTo("nestedFieldOverride");
 		}
 
-		@Test
-		void nestedRenamedFieldHasOverride(ApplicationContext ctx) {
-			assertThat(ctx.getBean("nestedField")).as("applicationContext").isEqualTo("nestedFieldOverride");
-			assertThat(renamed2).isEqualTo("nestedFieldOverride");
-		}
-
 		@Test
 		void nestedFieldWithMethodNameHasOverride(ApplicationContext ctx) {
 			assertThat(ctx.getBean("methodRenamed2")).as("applicationContext").isEqualTo("nestedFieldOverride");
@@ -133,12 +109,6 @@ void fieldHasOverride(ApplicationContext ctx) {
 				assertThat(field).as("injection point").isEqualTo("fieldOverride");
 			}
 
-			@Test
-			void renamedFieldHasOverride(ApplicationContext ctx) {
-				assertThat(ctx.getBean("field")).as("applicationContext").isEqualTo("fieldOverride");
-				assertThat(renamed).as("injection point").isEqualTo("fieldOverride");
-			}
-
 			@Test
 			void fieldWithMethodNameHasOverride(ApplicationContext ctx) {
 				assertThat(ctx.getBean("methodRenamed1")).as("applicationContext").isEqualTo("fieldOverride");
@@ -151,12 +121,6 @@ void nestedFieldHasOverride(ApplicationContext ctx) {
 				assertThat(nestedField).isEqualTo("nestedFieldOverride");
 			}
 
-			@Test
-			void nestedRenamedFieldHasOverride(ApplicationContext ctx) {
-				assertThat(ctx.getBean("nestedField")).as("applicationContext").isEqualTo("nestedFieldOverride");
-				assertThat(renamed2).isEqualTo("nestedFieldOverride");
-			}
-
 			@Test
 			void nestedFieldWithMethodNameHasOverride(ApplicationContext ctx) {
 				assertThat(ctx.getBean("methodRenamed2")).as("applicationContext").isEqualTo("nestedFieldOverride");
@@ -170,25 +134,25 @@ void nestedFieldWithMethodNameHasOverride(ApplicationContext ctx) {
 	public class TestBeanFactoryMethodInEnclosingClassTests {
 
 		@TestBean(methodName = "nestedField", name = "nestedField")
-		String nestedField2;
+		String nestedField;
 
 		@Test
 		void nestedFieldHasOverride(ApplicationContext ctx) {
 			assertThat(ctx.getBean("nestedField")).as("applicationContext").isEqualTo("nestedFieldOverride");
-			assertThat(nestedField2).isEqualTo("nestedFieldOverride");
+			assertThat(nestedField).isEqualTo("nestedFieldOverride");
 		}
 
 		@Nested
 		@DisplayName("With factory method in the enclosing class of the enclosing class")
 		public class TestBeanFactoryMethodInEnclosingClassLevel2Tests {
 
-			@TestBean(methodName = "nestedField", name = "nestedField")
-			String nestedField2;
+			@TestBean(methodName = "nestedField", name = "nestedNestedField")
+			String nestedNestedField;
 
 			@Test
 			void nestedFieldHasOverride(ApplicationContext ctx) {
-				assertThat(ctx.getBean("nestedField")).as("applicationContext").isEqualTo("nestedFieldOverride");
-				assertThat(nestedField2).isEqualTo("nestedFieldOverride");
+				assertThat(ctx.getBean("nestedNestedField")).as("applicationContext").isEqualTo("nestedFieldOverride");
+				assertThat(nestedNestedField).isEqualTo("nestedFieldOverride");
 			}
 		}
 	}

Diff for: spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanForByNameLookupIntegrationTests.java

@@ -45,9 +45,6 @@ public class MockitoBeanForByNameLookupIntegrationTests {
 	@MockitoBean("field")
 	ExampleService field;
 
-	@MockitoBean("field")
-	ExampleService renamed;
-
 	@MockitoBean("nonExistingBean")
 	ExampleService nonExisting;
 
@@ -57,11 +54,9 @@ void fieldAndRenamedFieldHaveSameOverride(ApplicationContext ctx) {
 		assertThat(ctx.getBean("field"))
 				.isInstanceOf(ExampleService.class)
 				.satisfies(MockitoAssertions::assertIsMock)
-				.isSameAs(field)
-				.isSameAs(renamed);
+				.isSameAs(field);
 
 		assertThat(field.greeting()).as("mocked greeting").isNull();
-		assertThat(renamed.greeting()).as("mocked greeting").isNull();
 	}
 
 	@Test
@@ -83,20 +78,13 @@ public class MockitoBeanNestedTests {
 		@Qualifier("field")
 		ExampleService localField;
 
-		@Autowired
-		@Qualifier("field")
-		ExampleService localRenamed;
-
 		@Autowired
 		@Qualifier("nonExistingBean")
 		ExampleService localNonExisting;
 
 		@MockitoBean("nestedField")
 		ExampleService nestedField;
 
-		@MockitoBean("nestedField")
-		ExampleService nestedRenamed;
-
 		@MockitoBean("nestedNonExistingBean")
 		ExampleService nestedNonExisting;
 
@@ -106,11 +94,9 @@ void fieldAndRenamedFieldHaveSameOverride(ApplicationContext ctx) {
 			assertThat(ctx.getBean("field"))
 					.isInstanceOf(ExampleService.class)
 					.satisfies(MockitoAssertions::assertIsMock)
-					.isSameAs(localField)
-					.isSameAs(localRenamed);
+					.isSameAs(localField);
 
 			assertThat(localField.greeting()).as("mocked greeting").isNull();
-			assertThat(localRenamed.greeting()).as("mocked greeting").isNull();
 		}
 
 		@Test
@@ -128,8 +114,7 @@ void nestedFieldAndRenamedFieldHaveSameOverride(ApplicationContext ctx) {
 			assertThat(ctx.getBean("nestedField"))
 					.isInstanceOf(ExampleService.class)
 					.satisfies(MockitoAssertions::assertIsMock)
-					.isSameAs(nestedField)
-					.isSameAs(nestedRenamed);
+					.isSameAs(nestedField);
 		}
 
 		@Test