I need to run Spring Test concurrently. For this reason, a separate method rule has been created. The logic of this rule (code included in reproducer) is to execute the test method multiple times in a dedicated executor.
Unfortunately injection flow does not work as excepted and throws the following exception:
org.springframework.beans.factory.BeanCreationException: Could not inject field: <xxx>;
nested exception is java.lang.IllegalStateException: The field <xxx> cannot have an existing value
at org.springframework.boot.test.mock.mockito.MockitoPostProcessor.inject(MockitoPostProcessor.java:364)
Reproducer and instruction are here: https://github.com/alexey-anufriev/spring-test-concurrent-execution-problem
Comment From: wilkinsona
For reference, this was originally discussed on Gitter.
Comment From: philwebb
Unfortunately I don't think the approach you're trying to take will work for mocked beans. The ConcurrentTestStatement is calling the evaluate method multiple times which is trying to call TestContextManager.prepareTestInstance multiple times with the same instance. My understanding is that this method is usually called only once, and with unique instances that are created by JUnit before the test method is run.
Even if we didn't have the "field cannot have an existing value" check, I think you'd hit trouble with the Mockito stubbing. The when(this.someBean.getData()).thenReturn("some other data") statement could be called multiple times in different threads and each would be trying to work on the same someBean instance. Things would get even worse if you had multiple @ConcurrentTest methods trying to setup different stubbing rules.
I'm not really sure what to suggest you try. It feels like you probably needs a fresh ApplicationContext for each invocation of a @ConcurrentTest method. Perhaps @sbrannen might have some ideas.
Comment From: philwebb
I'm going to close this one here because I don't think there's anything we can do in Spring Boot to help.
Comment From: alexey-anufriev
@philwebb, this approach has actually being inspired by @Repeat annotation. But I think @Repeat will also not work. Just tried to replace @ConcurrentTest with @Repeat and got the same error. So maybe we can adjust this issue to fix @Repeat and consequently can find a way to make it work with @Concurrent.
Comment From: philwebb
I updated your sample application and can confirm that @Repeat does indeed fail in the same way. That does feel like a bug to me.
Comment From: philwebb
@alexey-anufriev I'd prefer to keep backtick markup out of the issue titles since it doesn't render so well on the release notes
Comment From: alexey-anufriev
I also pushed one commit to reproduce the problem: https://github.com/alexey-anufriev/spring-test-concurrent-execution-problem/commit/7be5f8e355d6232a04c532e2a27729af917b552b
Comment From: wilkinsona
This still feels like a bug in Spring Framework's test framework rather than Spring Boot to me. The javadoc for prepareTestInstance states that it "should be called immediately after instantiation of the test". This doesn't hold true when using @Repeat which is what feels like a Framework bug.
For the record, the same problem does not occur with JUnit 5's @RepeatedTest where a new instance of the test class is created for each invocation of the repeated test.
Comment From: philwebb
@sbrannen Any thoughts on this one?
Comment From: sbrannen
This still feels like a bug in Spring Framework's test framework rather than Spring Boot to me. The javadoc for
prepareTestInstancestates that it "should be called immediately after instantiation of the test". This doesn't hold true when using@Repeatwhich is what feels like a Framework bug.
The Javadoc for @Repeat states the following:
Note that the scope of execution to be repeated includes execution of the test method itself as well as any set up or tear down of the test fixture.
That does not claim to create a new instance of the test class. It also does not claim that the test instance will be prepared anew for each repetition.
That is in line with the implementation in SpringJUnit4ClassRunner.
However, in order to support prepareTestInstance with Spring's JUnit 4 rules, the SpringMethodRule does in fact use RunPrepareTestInstanceCallbacks to prepare the test instance for each repetition, as can be seen in the stack trace.
[2021-08-20 16:39:21.772] - 79836 SEVERE [pool-1-thread-3] --- org.springframework.test.context.TestContextManager: Caught exception while allowing TestExecutionListener [org.springframework.boot.test.mock.mockito.MockitoTestExecutionListener@2a8448fa] to prepare test instance [test.ConcurrentSampleTest@38e98121]
org.springframework.beans.factory.BeanCreationException: Could not inject field: private business_logic.SomeOtherBean test.ConcurrentSampleTest.someOtherBean; nested exception is java.lang.IllegalStateException: The field private business_logic.SomeOtherBean test.ConcurrentSampleTest.someOtherBean cannot have an existing value
at org.springframework.boot.test.mock.mockito.MockitoPostProcessor.inject(MockitoPostProcessor.java:364)
at org.springframework.boot.test.mock.mockito.MockitoPostProcessor.inject(MockitoPostProcessor.java:352)
at org.springframework.boot.test.mock.mockito.MockitoTestExecutionListener.lambda$injectFields$0(MockitoTestExecutionListener.java:94)
at org.springframework.boot.test.mock.mockito.MockitoTestExecutionListener.postProcessFields(MockitoTestExecutionListener.java:115)
at org.springframework.boot.test.mock.mockito.MockitoTestExecutionListener.injectFields(MockitoTestExecutionListener.java:94)
at org.springframework.boot.test.mock.mockito.MockitoTestExecutionListener.prepareTestInstance(MockitoTestExecutionListener.java:61)
at org.springframework.test.context.TestContextManager.prepareTestInstance(TestContextManager.java:244)
at org.springframework.test.context.junit4.statements.RunPrepareTestInstanceCallbacks.evaluate(RunPrepareTestInstanceCallbacks.java:63)
at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:84)
at org.springframework.test.context.junit4.statements.SpringFailOnTimeout.evaluate(SpringFailOnTimeout.java:87)
at org.springframework.test.context.junit4.statements.ProfileValueChecker.evaluate(ProfileValueChecker.java:103)
at ice.bricks.exceptions.ExceptionUtils.runSafe(ExceptionUtils.java:34)
at test_utils.ConcurrentTestStatement.lambda$1(ConcurrentTestStatement.java:32)
at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: java.lang.IllegalStateException: The field private business_logic.SomeOtherBean test.ConcurrentSampleTest.someOtherBean cannot have an existing value
at org.springframework.util.Assert.state(Assert.java:97)
at org.springframework.boot.test.mock.mockito.MockitoPostProcessor.inject(MockitoPostProcessor.java:358)
... 17 more
For the record, the same problem does not occur with JUnit 5's
@RepeatedTestwhere a new instance of the test class is created for each invocation of the repeated test.
That's correct, and that points out a very key difference.
The SpringJUnit4ClassRunner extends JUnit 4's BlockJUnit4ClassRunner which is simply not designed for concurrent invocations of test methods; whereas, JUnit Jupiter's @RepeatedTest is designed for such use cases.
For example, the following JUnit Jupiter based test class executes the repetitions concurrently, and they all pass.
To enable concurrent execution, you need to set the JVM System property: -Djunit.jupiter.execution.parallel.enabled=true.
@SpringBootTest(classes = { SomeBean.class, SomeOtherBean.class })
class ConcurrentSampleJupiterTests {
@MockBean
private SomeBean someBean;
@MockBean
private SomeOtherBean someOtherBean;
@BeforeEach
void configureMock() {
when(this.someBean.getData()).thenReturn("some mocked data");
System.err.printf(">> Thread: %s --> hashes for someBean/someOtherBean's:\t%d/%d%n",
Thread.currentThread().getName(), System.identityHashCode(this.someBean),
System.identityHashCode(this.someOtherBean));
}
@RepeatedTest(10)
@Execution(ExecutionMode.CONCURRENT)
void concurrentTest() {
assertEquals(this.someBean.getData(), "some mocked data");
}
}
Comment From: wilkinsona
Thanks, Sam. My concern was more with the javadoc of prepareTestInstance combined with the behaviour of @Repeat. The former states that prepareTestInstance "should be called immediately after instantiation of the test" whereas the latter causes it to be called some time after the test was instantiated. Those two feel contradictory to me and I think it would be good to try and clarify the javadoc.
On the Boot side, Phil and I talked about this a bit and we think we can avoid the problem by only failing if the post-processor would change the value of an already-initialized @MockBean field to use a different mock.
Comment From: sbrannen
Unfortunately I don't think the approach you're trying to take will work for mocked beans. The
ConcurrentTestStatementis calling theevaluatemethod multiple times which is trying to callTestContextManager.prepareTestInstancemultiple times with the same instance. My understanding is that this method is usually called only once, and with unique instances that are created by JUnit before the test method is run.
That's correct @philwebb. The "unique instances" part is crucial.
When you use @Repeat with JUnit 4, you end up with a for-loop that invokes the same methods/fields on the same instance, and if you do that concurrently like in the ConcurrentTestStatement you end up with race conditions for access to the @MockBean fields.
Even if we didn't have the "field cannot have an existing value" check, I think you'd hit trouble with the Mockito stubbing. The
when(this.someBean.getData()).thenReturn("some other data")statement could be called multiple times in different threads and each would be trying to work on the samesomeBeaninstance. Things would get even worse if you had multiple@ConcurrentTestmethods trying to setup different stubbing rules.
To be honest, I don't recall if Mockito has any thread-local support for concurrent use of a shared mock.
I'm not really sure what to suggest you try. It feels like you probably needs a fresh
ApplicationContextfor each invocation of a@ConcurrentTestmethod. Perhaps @sbrannen might have some ideas.
If the mock setup is exactly the same for all concurrent usage, using JUnit Jupiter as I've shown above might work reliably.
If you need different mock configuration/expectations for each concurrent test invocation, you'll have to see if Mockito has thread-local support for such use cases.
The only other option I can think of is adding support for @MockBean ThreadLocal myBean where Spring Boot would need to set the mocked bean in a ThreadLocal. The concurrently executing tests could then get() the current mocked bean from the ThreadLocal.
It feels like you probably needs a fresh
ApplicationContextfor each invocation of a@ConcurrentTestmethod.
Yeah, that might be another way to do it (except that we do not have support in the Spring TestContext for loading an ApplicationContext per test method), or maybe the mocked bean in the ApplicationContext could use the SimpleThreadScope instead of singleton.
Comment From: sbrannen
Thanks, Sam. My concern was more with the javadoc of
prepareTestInstancecombined with the behaviour of@Repeat. The former states thatprepareTestInstance"should be called immediately after instantiation of the test" whereas the latter causes it to be called some time after the test was instantiated.
OK. I get your point now.
If you're using the SpringJUnit4ClassRunner, prepareTestInstance does in fact get "called immediately after instantiation of the test."
But when you're using the SpringMethodRule, you're right: prepareTestInstance is "called some time after the test was instantiated."
That last part is unavoidable, since JUnit 4 does not have any support for instance-level callbacks with rules.
Those two feel contradictory to me and I think it would be good to try and clarify the javadoc.
For the SpringMethodRule, that is contradictory. We can definitely add a note to the Javadoc to explain the different behavior when using SpringMethodRule.
On a related note, I don't recall why I chose to have withTestInstancePreparation() invoked within repeated test invocations in SpringMethodRule. It seems that I could just as easily have invoked withTestInstancePreparation() before timeouts and repetitions. So I don't know if that's worth revisiting.
On the Boot side, Phil and I talked about this a bit and we think we can avoid the problem by only failing if the post-processor would change the value of an already-initialized
@MockBeanfield to use a different mock.
That sounds like a good enhancement.
Comment From: sbrannen
Those two feel contradictory to me and I think it would be good to try and clarify the javadoc.
I opened https://github.com/spring-projects/spring-framework/issues/27305 to track that.
Comment From: sbrannen
@alexey-anufriev, as a workaround... if you are able to use the SpringRunner instead of the SpringMethodRule, and if you limit your test class to a single @Test method, the following approach appears to work.
public class ConcurrentMethodRule implements MethodRule {
@Override
public Statement apply(Statement base, FrameworkMethod frameworkMethod, Object testInstance) {
return new ConcurrentTestStatement(base, frameworkMethod.getMethod());
}
}
@RunWith(SpringRunner.class)
@SpringBootTest(classes = { SomeBean.class, SomeOtherBean.class })
public class ConcurrentSampleRunnerTests {
@Rule
public final ConcurrentMethodRule concurrentMethodRule = new ConcurrentMethodRule();
@Autowired
private SomeBean someBean;
@MockBean
private SomeOtherBean someOtherBean;
@ConcurrentTest
@Test
public void concurrentTest() {
when(this.someOtherBean.getData()).thenReturn("some mocked data");
System.out.printf(">> Thread: %s --> hashes for someBean/someOtherBean's:\t%d/%d%n", currentThread().getName(),
identityHashCode(this.someBean), identityHashCode(this.someOtherBean));
assertEquals("some mocked data", this.someBean.getData());
}
}
Perhaps that is useful to you.
Comment From: alexey-anufriev
@sbrannen, thank you for this suggestion, yes, this has some limitations but still works well enough for me.
Comment From: alexey-anufriev
Will there be plans to support repetitions/concurrent execution at some point? Or it is easier to migrate to Junit 5?
Comment From: wilkinsona
With that changes in https://github.com/spring-projects/spring-boot/commit/f8ef90813f839f34787f362f2712411f5d9d9359, repetition via @Repeat will now work. On the Spring Boot side, we have no plans to make any further changes to support concurrent execution. My opinion is that you should migrate to JUnit 5 for that.
Comment From: alexey-anufriev
I have just checked the fix and it looks promising even for my case, but anyway, I agree, JUnit 5 is the best way to go. Thank you for your support.