Hello Spring Boot team!
Today a colleague of mine came across strange behaviour when writing some JUnit 5 tests. It seems that @MochBean annotated fields, or more specifically the actual mocks behind that, are not reset for @Test methods residing in @Nested inner classes.
Example:
@SpringBootTest
@ExtendWith(SpringExtension.class)
class FailingTests {
@MockBean
ServiceB serviceB;
@Autowired
ServiceA cut;
@Nested
class NestedTest {
@Test
void mockWasInvokedOnce() {
cut.foo();
verify(serviceB, times(1)).bar();
}
@Test
void mockWasInvokedTwice() {
cut.foo();
cut.foo();
verify(serviceB, times(2)).bar();
}
}
}
One of these tests will fail because the mock will have been invoked 3 times at that point.
Tested with Spring Boot 2.0.1.BUILD-SNAPSHOT and JUnit 5.1.0 on 2018-03-13 at around 8:00 PM. (also tested with 2.0.0.RELEASE earlier)
I created a simple example application (thx start.spring.io ;) ) to reproduce the issue: https://github.com/slu-it/bug-spring-mockbean-reset
I also tried to find the cause of the issue, but I might have underestimated the complexity of the spring-test and spring-boot-test libraries... As far as I can see, the application context returned from the TestContext instance for tests within the @Nested class does not include references to the ServiceB bean. (From debugging MockitoTestExecutionListener)
When I debug the same location for a test without the @Nested part, the returned context contains a clearly identifiable instance of ServiceB.
I think the correct behaviour would be to have a parent child relationships between application contexts in case of inner classes (like @Nested JUnit 5 test classes).
Comment From: slu-it
As a side note: I think this might also be the reason why Spring REST Docs did not work with @Nested JUnit 5 tests last I checked (somewhere around M5). But I'll have to see, if this is still true.
Comment From: wilkinsona
Thanks for the minimal sample, @slu-it.
The problem is that NestedTest is using a separate application context to the one that's created for FailingTests. This means that the before and after test method callbacks for NestedTest.mockWasInvokedOnce() and NestedTest.mockWasInvokedTwice() are called with the application context created for NestedTest. It doesn't contain any mocks so there's nothing to reset. There's no link that I can see between the two contexts (no parent-child relationship, for example) so there's also no obvious way for us to get from the context for NestedTest to the context for FailingTests.
I'm rather surprised that there's a second application context created for NestedTest. My hunch was that it would share the context that was created for FailingTests. There's no mention of @Nested in the relevant section of Spring Framework's reference documentation so I'm not sure what is the right way to use @Nested with Spring Framework's testing support.
@sbrannen can you help us to get to the bottom of this one please?
Comment From: wilkinsona
A bit of hunting around led me to NestedTestsWithSpringAndJUnitJupiterTestCase.java . It's clear from those tests that the behaviour I described above is intentional and that my hunch was wrong. It's also taught me that in this case NestedTest should be annotated with @SpringBootTest. This gives it a chance to reuse the context that was created and cached for FailingTests. In fact, it would appear that it's only prevented from doing so due to there being no mock for serviceB in NestedTest. If FailingTests and NestedTest have the same mocks, the context is reused and the tests pass.
The change described above makes the test code look like this:
package example;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.test.context.junit.jupiter.SpringExtension;
@SpringBootTest
@ExtendWith(SpringExtension.class)
class FailingTests {
@Autowired
ServiceA cut;
@MockBean
ServiceB serviceB;
@Nested
@SpringBootTest
class NestedTest {
@MockBean
ServiceB serviceB;
@Test
void mockWasInvokedOnce() {
cut.foo();
verify(serviceB, times(1)).bar();
}
@Test
void mockWasInvokedTwice() {
cut.foo();
cut.foo();
verify(serviceB, times(2)).bar();
}
}
}
I wonder if we can do something when calculating the context cache key for NestedTest that would allow us to reuse the context from FailingTests without having to declare the mock for serviceB in both places.
Comment From: slu-it
In the meantime I checked whether or not the Spring REST Docs issue still exists. Tested it with Spring Boot 2.0.1-BUILD-SNAPSHOT in this repository: https://github.com/slu-it/bug-spring-restdocs-nested-tests
Failing with @Nested tests as well. Might be related, or it might just be coincidence.. I'll create a separate issue for that in the Spring REST Docs repository and link it here.
Comment From: wilkinsona
Here's a possible fix. Can you take a look please, @philwebb?
With the fix in place, these failing tests are now actually working tests:
package example;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.test.context.junit.jupiter.SpringExtension;
@SpringBootTest
@ExtendWith(SpringExtension.class)
class FailingTests {
@Autowired
ServiceA cut;
@MockBean
ServiceB serviceB;
@Nested
@SpringBootTest
class NestedTest {
@Test
void mockWasInvokedOnce() {
cut.foo();
verify(serviceB, times(1)).bar();
}
@Test
void mockWasInvokedTwice() {
cut.foo();
cut.foo();
verify(serviceB, times(2)).bar();
}
}
}
Adding a unit test for this is a challenge. As soon as I add an optional dependency on the JUnit Jupiter API to spring-boot-test, Eclipse defaults to using the JUnit 5 runner for all the tests in that project and you have to manually tell it to use JUnit 4 instead. I think that's too high a price to pay. We could add some use of @Nested to the JUnit 5 sample instead.
Comment From: slu-it
@wilkinsona is the second @SpringBootTest on the @Nested still necessary?
Comment From: wilkinsona
Yes. Without it, you get a second application context created for NestedTest. That second context is created as its configuration doesn't have anything to do with Spring Boot. Adding @SpringBootTest means that both NestedTest and FailingTests use SpringBootTestContextBootstrapper which finds example.Application as the basis for the test context's configuration in both cases.
Comment From: slu-it
Ok, makes sense.
Comment From: sbrannen
I'm rather surprised that there's a second application context created for
NestedTest.
I'm actually very surprised by that. I thought that a @Nested would by default not have an ApplicationContext loaded for it.
In fact, I'm nearly 100% certain that an ApplicationContext will not be loaded for such a @Nested class when using the Spring TestContext Framework (TCF) without Spring Boot Test support.
So, perhaps it's just the magic of Spring Boot Test that is automagically finding the configuration for the test ApplicationContext.
But I'd have to debug that to see what's really going on.
My hunch was that it would share the context that was created for
FailingTests.
Many people actually assume that would be the case, since the TCF supports configuration inheritance within a test class hierarchy.
However, Spring does not yet have any support for "inheriting" configuration from an enclosing class.
See SPR-15366 for details.
Unfortunately, resolving SPR-15366 is not so easy (if even feasible) since the entire annotation processing infrastructure within the core Spring Framework (and Spring portfolio projects for that matter) has zero support for searching for annotations on an enclosing class. π
There's no mention of
@Nestedin the relevant section of Spring Framework's reference documentation so I'm not sure what is the right way to use @Nested with Spring Framework's testing support.
The "right way" for the time being is simply to redeclare all of the annotations declared on the enclosing class. A custom composed annotation can alleviate the pain here, but in general I admit that it is quite a nuisance.
Comment From: sbrannen
Yes. Without it, you get a second application context created for NestedTest. That second context is created as its configuration doesn't have anything to do with Spring Boot.
Hmmm... what is the second context created from?
For example, what @Configuration class(es) is/are used to create that context?
Comment From: sbrannen
Here's a possible fix. Can you take a look please, @philwebb?
With the fix in place, these failing tests are now actually working tests:
That of course sounds nice for now, but I'm wondering what the implications would be if SPR-15366 is resolved at a later date. Maybe the result of SPR-15366 would be an opt-in feature for turning on "inheritance from enclosing class" in order to remain backwards compatible.
Will have to ponder it... π
Comment From: sbrannen
@wilkinsona,
Without it, you get a second application context created for NestedTest.
If you have personally confirmed that behavior with Boot, I would appreciate if you could add a corresponding comment to SPR-15366.
TIA
Comment From: wilkinsona
Hmmm... what is the second context created from?
For example, what
@Configurationclass(es) is/are used to create that context?
I'm not sure as I didn't dig that far. I can do though.
If you have personally confirmed that behavior with Boot, I would appreciate if you could add a corresponding comment to SPR-15366.
I have. I'll do a bit more digging so I can provide some more details and comment on SPR-15366.
Comment From: sbrannen
Thanks, @wilkinsona! π
Comment From: sbrannen
I'm not sure as I didn't dig that far. I can do though.
No need to do that: your input in SPR-15366 is sufficient.
Comment From: philwebb
We're going to wait for the SPR-15366 fix before looking at this.
Comment From: szymonslosarczyk
Hello, I'm having similar issue with BDDMockito.willThrow
willThrow(SomeException.class).given(service).someMethod(anyString());
It throws this exception in other nested tests, that use this method.
Comment From: hantsy
Same problem here, add reset to reset all mocks in afterEach to make it works again. https://github.com/hantsy/spring-reactive-jwt-sample/blob/master/src/test/java/com/example/demo/PostControllerTest.java#L85-L91
Comment From: jehanzebqayyum
Adding springbootcontext in nested classes doesnt work for me. I had to reset mock after each test
Comment From: sbrannen
Please note that https://github.com/spring-projects/spring-framework/issues/19930 has been resolved in time for Spring Framework 5.3 RC2.
See the "Deliverables" section of that issue (and the following commits) for details on what's included in RC2.
- https://github.com/spring-projects/spring-framework/commit/6641dbc852dddd46276820501cda0cac5e519eaa
- https://github.com/spring-projects/spring-framework/commit/fbb3c5cce73f07679afebbb39980fb971efb9f4f
- https://github.com/spring-projects/spring-framework/commit/0af09e076b61d7f63dde9df8a033599648d22248
Comment From: wilkinsona
I've tried a slightly updated version of the original sample with Spring Boot 2.4.0-M4 which uses Framework 5.3.0-RC2. The problem still occurs, however I think that's to be expected as we're not making use of any of the changes made in Framework to find test configuration in an enclosing class. This results in a cache miss as the merged context configurations for FailingTests and FailingTests.NestedTest differ. I think we need to make some changes to use searchEnclosingClass() and perhaps also update the @MockBean support to find annotated fields on an enclosing class. The latter would be a revision of this commit, updated to consider the @NestedTestConfiguration setting.
Comment From: sbrannen
Spring Framework 5.3 is now GA, and the searchEnclosingClass() method resides in the new org.springframework.test.context.TestContextAnnotationUtils class.
@wilkinsona, let me know if you run into any issues getting everything working with the new nested configuration support.
Comment From: wimdeblauwe
I just tried this with Spring Boot 2.4.0-RC1, but the @MockBean is not reset between tests like if I don't use nesting. You can view my example at https://github.com/wimdeblauwe/blog-example-code/tree/feature/nestedtests/nestedtests/src/test/java/com/wimdeblauwe/examples/nestedtests/music/web
There are 2 tests there: MusicRestControllerTest and MusicRestControllerNestedTest which only differ in the fact that nesting is used or not. The normal one succeeds, the nested one fails because the mock bean is not reset.
If you want me to create a separate issue, I can also do that.
Comment From: wilkinsona
Thanks for trying out the RC, @wimdeblauwe. Please do open a new issue. It's not immediately apparent to me what the difference is between this test class that was part of the commit that closed this issue and your example. It may be the use of @WebMvcTest vs @SpringBootTest.
Comment From: wimdeblauwe
Issue https://github.com/spring-projects/spring-boot/issues/23984 created.