spring-framework icon indicating copy to clipboard operation
spring-framework copied to clipboard

Spring test context reloaded when MockBean and ContextHierarchy (on single class) are mixed

Open jonenst opened this issue 5 years ago • 6 comments

Hi, attached is a testcase showing a testcontest that is wrongly reloaded when using @MockBean in conjunction with using @ContextHierarchy in single class mode.

The testcase is in the attached zip, in the "notworking" folder. the "working" folder is present only for comparison to illustrate the bug.

run $ mvn package at the root to run the testcases. mockbean-hiearchy-singleclass-bug.zip

[INFO] aggregator 0.0.1-SNAPSHOT .......................... SUCCESS [  0.559 s]
[INFO] working ............................................ SUCCESS [  2.830 s]
[INFO] notworking 0.0.1-SNAPSHOT .......................... FAILURE [  1.718 s]

the testcase shows that a context that should be cached is instead reloaded when multiple classes using the same context as the parent of a contexthiearchy, and using mockbean on a bean in a child context (so the parent should still be reusable):

DOESN'T WORK

@ContextHierarchy({
    @ContextConfiguration(classes = ErrorIfContextReloaded.class),
    @ContextConfiguration(classes = DefaultFooService.class),
})
@ExtendWith(SpringExtension.class)
class Demo1ApplicationTests {

    @MockBean
    DefaultFooService fooService;

}
@ContextHierarchy({
    @ContextConfiguration(classes = ErrorIfContextReloaded.class),
    @ContextConfiguration(classes = DefaultFooService2.class),
})
@ExtendWith(SpringExtension.class)
class Demo2ApplicationTests {

    @MockBean
    DefaultFooService2 fooService;

}

The bug does not happen when using @ContextHiearchy with subclassing: WORKS

@ContextHierarchy({
    @ContextConfiguration(classes = ErrorIfContextReloaded.class),
})
@ExtendWith(SpringExtension.class)
class Demo1ApplicationTests {
}

@ContextHierarchy({ @ContextConfiguration(classes = DefaultFooService.class), })
class Demo1ApplicationChildTests extends Demo1ApplicationTests {
    @MockBean
    DefaultFooService fooService;
}
@ContextHierarchy({
    @ContextConfiguration(classes = ErrorIfContextReloaded.class),
})
@ExtendWith(SpringExtension.class)
class Demo2ApplicationTests {
}

@ContextHierarchy({ @ContextConfiguration(classes = DefaultFooService2.class), })
class Demo2ApplicationChildTests extends Demo2ApplicationTests {
    @MockBean
    DefaultFooService2 fooService;
}

This prevents reliable context caching, useful for example when expensive beans (like test databases and databases connections) are created only once in a parent context for every test classes.

jonenst avatar Apr 23 '20 12:04 jonenst

Thanks for the sample. This is behaving as I would expect given how the test framework, @ContextHierarchy, context customization, and context caching are currently implemented.

The test framework creates a MergedContextConfiguration for each application context that it may create. The MergedContextConfiguration is used as a cache key to decide whether or not an existing application context can be reused or if a new one needs to be created. In the notworking example four context configurations are involved:

  1. ErrorIfContextReloaded with mock bean definitions from Demo1ApplicationTests
  2. DefaultFooService with mock bean definitions from Demo1ApplicationTests
  3. ErrorIfContextReloaded with mock bean definitions from Demo2ApplicationTests
  4. DefaultFooService2 with mock bean definitions from Demo2ApplicationTests

As the mock bean definitions of Demo1ApplicationTests and Demo2ApplicationTests are different, the merged context configurations for 1 and 3 are also different so the context created for 1 cannot be reused by 3. This leads to a second attempt to create ErrorIfContextReloaded which fails.

I'm not sure there's much that we can do to improve the situation here. Perhaps we could allow @MockBean to be targeted at a particular context in the hierarchy but I'm not sure that the ContextCustomizerFactory API provides us with sufficient information to identify for what level of the hierarchy the factory is being called. Do you have any suggestions, @sbrannen?

wilkinsona avatar Apr 23 '20 13:04 wilkinsona

Hi, thanks for the quick reply.

if it's not possible to automatically determine which contexts in the hierarchy have which mockbeans, then perhaps adding a parameter to @MockBean to manually specify the context name is a good alternative ?

Also, maybe the technique of using a dummy inheritance structure to group the mockbeans definitions with the context they are associated with can be documented ? Although it won't work when your class needs to subclass another class.. In which case the (automatic or manual) association of mockbeans to their context in the hieararchy seems like the only way to solve the problem ?

Cheers, Jon

jonenst avatar Apr 23 '20 15:04 jonenst

@sbrannen When you have a moment, could you please take a look at my comment above?

wilkinsona avatar Jul 08 '20 13:07 wilkinsona

I'm not sure there's much that we can do to improve the situation here. Perhaps we could allow @MockBean to be targeted at a particular context in the hierarchy but I'm not sure that the ContextCustomizerFactory API provides us with sufficient information to identify for what level of the hierarchy the factory is being called. Do you have any suggestions, @sbrannen?

Well, ContextCustomizerFactory.createContextCustomizer(Class<?>, List<ContextConfigurationAttributes>) is invoked with a list of ContextConfigurationAttributes, and you can access the "the name of the context hierarchy level that was declared via @ContextConfiguration."

So, if @MockBean allows the user to specify a name attribute that matches the name attribute from @ContextConfiguration, then your ContextCustomizerFactory could decide if it supports the current context level based on that.

Does that meet your needs?

sbrannen avatar Jul 09 '20 14:07 sbrannen

Thanks, Sam.

Sorry that it has taken so long to get back to this.

It looks like name on @ContextConfiguration can be used in a way that would meet our needs. With a corresponding context attribute added to @MockBean and @SpyBean it becomes possible to indicate that the creation of a mock or spy bean should target a specific context. I've prototyped this in this branch. Passing the name of the "target" context around is a little bit clunky as we have a few places where, previously, we'd just create a new DefinitionsParser and we now need to create one that's aware of the name of the target context so it can ignore @MockBeans and @SpyBeans that target a different context.

I'll flag this one so that we can decide if this is something that we want to do, or if we're happy with the current situation where test-class inheritance can be used to provide the desired separation.

wilkinsona avatar Oct 18 '21 11:10 wilkinsona

I've moved this issue to framework given the support for mock bean has been moved to framework.

@sbrannen perhaps we could do something a bit more direct now the support is in spring-test.

snicoll avatar Jul 31 '24 07:07 snicoll

It looks like name on @ContextConfiguration can be used in a way that would meet our needs. With a corresponding context attribute added to @MockBean and @SpyBean it becomes possible to indicate that the creation of a mock or spy bean should target a specific context. I've prototyped this in this branch. Passing the name of the "target" context around is a little bit clunky as we have a few places where, previously, we'd just create a new DefinitionsParser and we now need to create one that's aware of the name of the target context so it can ignore @MockBeans and @SpyBeans that target a different context.

@wilkinsona, I wanted to take a look at the prototype you created for Spring Boot, but that branch unfortunately no longer exists.

In any case, I have prototyped a solution for Spring Framework's bean override support, and I would appreciate it if you could take a look at #34723 and let me know your thoughts on the proposal.

Thanks in advance!

sbrannen avatar Apr 07 '25 16:04 sbrannen

I have put considerable thought into this and created #34723 to address this and similar issues. So, feel free to subscribe to that PR for further updates.

In light of that, I am closing this issue as:

  • Superseded by #34723

sbrannen avatar Apr 07 '25 16:04 sbrannen