Skip to content

Commit

Permalink
Reject identical Bean Overrides
Browse files Browse the repository at this point in the history
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 spring-projectsgh-34054
  • Loading branch information
sbrannen committed Dec 9, 2024
1 parent 2137750 commit cd60a00
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 189 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ public class TestBeanForByNameLookupIntegrationTests {
@TestBean(name = "field")
String field;

@TestBean(name = "field")
String renamed;

@TestBean(name = "methodRenamed1", methodName = "field")
String methodRenamed1;

Expand All @@ -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");
Expand All @@ -80,9 +71,6 @@ public class TestBeanFieldInEnclosingClassTests {
@TestBean(name = "nestedField")
String nestedField;

@TestBean(name = "nestedField")
String renamed2;

@TestBean(name = "methodRenamed2", methodName = "nestedField")
String methodRenamed2;

Expand All @@ -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");
Expand All @@ -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");
Expand All @@ -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");
Expand All @@ -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");
Expand All @@ -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");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ public class MockitoBeanForByNameLookupIntegrationTests {
@MockitoBean("field")
ExampleService field;

@MockitoBean("field")
ExampleService renamed;

@MockitoBean("nonExistingBean")
ExampleService nonExisting;

Expand All @@ -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
Expand All @@ -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;

Expand All @@ -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
Expand All @@ -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
Expand Down

This file was deleted.

Loading

0 comments on commit cd60a00

Please sign in to comment.