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

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

@@ -24,6 +24,7 @@ import org.springframework.lang.Nullable;
 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 @@ class BeanOverrideContextCustomizerFactory implements ContextCustomizerFactory {
 	}
 
 	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)));
 	}
 
 }

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

@@ -19,18 +19,20 @@ package org.springframework.test.context.bean.override;
 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 @@ class BeanOverrideContextCustomizerFactoryTests {
 				.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,15 +91,15 @@ class BeanOverrideContextCustomizerFactoryTests {
 	}
 
 	@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 {
@@ -96,17 +107,25 @@ class BeanOverrideContextCustomizerFactoryTests {
 		@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;
+	}
+
 }

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 @@ public class TestBeanForByNameLookupIntegrationTests {
 		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 TestBeanForByNameLookupIntegrationTests {
 		@TestBean(name = "nestedField")
 		String nestedField;
 
-		@TestBean(name = "nestedField")
-		String renamed2;
-
 		@TestBean(name = "methodRenamed2", methodName = "nestedField")
 		String methodRenamed2;
 
@@ -93,12 +81,6 @@ public class TestBeanForByNameLookupIntegrationTests {
 			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 @@ public class TestBeanForByNameLookupIntegrationTests {
 			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 @@ public class TestBeanForByNameLookupIntegrationTests {
 				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 @@ public class TestBeanForByNameLookupIntegrationTests {
 				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 @@ public class TestBeanForByNameLookupIntegrationTests {
 	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");
 			}
 		}
 	}

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 @@ public class MockitoBeanForByNameLookupIntegrationTests {
 		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,10 +78,6 @@ public class MockitoBeanForByNameLookupIntegrationTests {
 		@Qualifier("field")
 		ExampleService localField;
 
-		@Autowired
-		@Qualifier("field")
-		ExampleService localRenamed;
-
 		@Autowired
 		@Qualifier("nonExistingBean")
 		ExampleService localNonExisting;
@@ -94,9 +85,6 @@ public class MockitoBeanForByNameLookupIntegrationTests {
 		@MockitoBean("nestedField")
 		ExampleService nestedField;
 
-		@MockitoBean("nestedField")
-		ExampleService nestedRenamed;
-
 		@MockitoBean("nestedNonExistingBean")
 		ExampleService nestedNonExisting;
 
@@ -106,11 +94,9 @@ public class MockitoBeanForByNameLookupIntegrationTests {
 			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 @@ public class MockitoBeanForByNameLookupIntegrationTests {
 			assertThat(ctx.getBean("nestedField"))
 					.isInstanceOf(ExampleService.class)
 					.satisfies(MockitoAssertions::assertIsMock)
-					.isSameAs(nestedField)
-					.isSameAs(nestedRenamed);
+					.isSameAs(nestedField);
 		}
 
 		@Test

spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanDuplicateTypeAndNameIntegrationTests.java

@@ -1,83 +0,0 @@
-/*
- * Copyright 2002-2024 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.test.context.bean.override.mockito;
-
-import java.util.List;
-
-import org.junit.jupiter.api.Test;
-
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Configuration;
-import org.springframework.test.context.bean.override.example.ExampleService;
-import org.springframework.test.context.bean.override.example.RealExampleService;
-import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
-
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.springframework.test.mockito.MockitoAssertions.assertIsNotSpy;
-import static org.springframework.test.mockito.MockitoAssertions.assertIsSpy;
-
-/**
- * Integration tests for duplicate {@link MockitoSpyBean @MockitoSpyBean}
- * declarations for the same target bean, selected by-name.
- *
- * @author Sam Brannen
- * @since 6.2.1
- * @see MockitoBeanDuplicateTypeIntegrationTests
- * @see MockitoSpyBeanDuplicateTypeIntegrationTests
- */
-@SpringJUnitConfig
-public class MockitoSpyBeanDuplicateTypeAndNameIntegrationTests {
-
-	@MockitoSpyBean("exampleService1")
-	ExampleService service1;
-
-	@MockitoSpyBean("exampleService1")
-	ExampleService service2;
-
-	@Autowired
-	ExampleService exampleService2;
-
-	@Autowired
-	List<ExampleService> services;
-
-
-	@Test
-	void duplicateMocksShouldHaveBeenCreated() {
-		assertThat(service1).isSameAs(service2);
-		assertThat(services).containsExactly(service1, exampleService2);
-
-		assertIsSpy(service1, "service1");
-		assertIsNotSpy(exampleService2, "exampleService2");
-	}
-
-
-	@Configuration(proxyBeanMethods = false)
-	static class Config {
-
-		@Bean
-		ExampleService exampleService1() {
-			return new RealExampleService("@Bean 1");
-		}
-
-		@Bean
-		ExampleService exampleService2() {
-			return new RealExampleService("@Bean 2");
-		}
-	}
-
-}

spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanForByNameLookupIntegrationTests.java

@@ -46,9 +46,6 @@ public class MockitoSpyBeanForByNameLookupIntegrationTests {
 	@MockitoSpyBean("field1")
 	ExampleService field;
 
-	@MockitoSpyBean("field1")
-	ExampleService renamed;
-
 
 	@Test
 	void fieldHasOverride(ApplicationContext ctx) {
@@ -60,15 +57,6 @@ public class MockitoSpyBeanForByNameLookupIntegrationTests {
 		assertThat(field.greeting()).isEqualTo("bean1");
 	}
 
-	@Test
-	void renamedFieldHasOverride(ApplicationContext ctx) {
-		assertThat(ctx.getBean("field1"))
-				.isInstanceOf(ExampleService.class)
-				.satisfies(MockitoAssertions::assertIsSpy)
-				.isSameAs(renamed);
-
-		assertThat(renamed.greeting()).isEqualTo("bean1");
-	}
 
 	@Nested
 	@DisplayName("With @MockitoSpyBean in enclosing class and in @Nested class")
@@ -78,16 +66,9 @@ public class MockitoSpyBeanForByNameLookupIntegrationTests {
 		@Qualifier("field1")
 		ExampleService localField;
 
-		@Autowired
-		@Qualifier("field1")
-		ExampleService localRenamed;
-
 		@MockitoSpyBean("field2")
 		ExampleService nestedField;
 
-		@MockitoSpyBean("field2")
-		ExampleService nestedRenamed;
-
 		@Test
 		void fieldHasOverride(ApplicationContext ctx) {
 			assertThat(ctx.getBean("field1"))
@@ -98,16 +79,6 @@ public class MockitoSpyBeanForByNameLookupIntegrationTests {
 			assertThat(localField.greeting()).isEqualTo("bean1");
 		}
 
-		@Test
-		void renamedFieldHasOverride(ApplicationContext ctx) {
-			assertThat(ctx.getBean("field1"))
-					.isInstanceOf(ExampleService.class)
-					.satisfies(MockitoAssertions::assertIsSpy)
-					.isSameAs(localRenamed);
-
-			assertThat(localRenamed.greeting()).isEqualTo("bean1");
-		}
-
 		@Test
 		void nestedFieldHasOverride(ApplicationContext ctx) {
 			assertThat(ctx.getBean("field2"))
@@ -117,16 +88,6 @@ public class MockitoSpyBeanForByNameLookupIntegrationTests {
 
 			assertThat(nestedField.greeting()).isEqualTo("bean2");
 		}
-
-		@Test
-		void nestedRenamedFieldHasOverride(ApplicationContext ctx) {
-			assertThat(ctx.getBean("field2"))
-					.isInstanceOf(ExampleService.class)
-					.satisfies(MockitoAssertions::assertIsSpy)
-					.isSameAs(nestedRenamed);
-
-			assertThat(nestedRenamed.greeting()).isEqualTo("bean2");
-		}
 	}
 
 	@Configuration(proxyBeanMethods = false)