spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.77k stars 38.16k forks source link

Spring Framework 6.2.0-RC2 no longer creates a spy for `@MockitoSpyBean` #33803

Closed JKatzwinkel closed 1 month ago

JKatzwinkel commented 1 month ago

Upgrading from Spring Boot 3.4.0-M3 (Spring Framework 6.2.0-RC1) to Spring Boot 3.4.0-RC1 (Spring Framework 6.2.0-RC3) breaks my use of a @MockitoSpyBean of the form doReturn(results).when(spyBean).method(argumentMatchers...).

That form is the one recommend by Mockito.

The error thrown says Argument passed to when() is not a mock!: https://github.com/JKatzwinkel/tla-es/actions/runs/11541063711/job/32122614536?pr=382#step:9:188 .

This problem did not occur with Spring Boot 3.4.0-M3 (Spring Framework 6.2.0-RC1).

This is where the spy (operations) gets stubbed:

https://github.com/JKatzwinkel/tla-es/blob/dependabot/gradle/spring-boot-bf23beedc8/src/test/java/tla/backend/service/SentenceServiceTest.java#L99-L103

        doReturn(hits).when(operations).search(
            any(org.springframework.data.elasticsearch.core.query.Query.class),
            eq(SentenceEntity.class),
            any()
        );

Please let me know if you need further information or if this is not an issue with spring boot. Thanks!

sbrannen commented 1 month ago

Hi @JKatzwinkel,

Thanks for reporting the issue you've encountered.

Please let me know if you need further information or if this is not an issue with spring boot.

It's difficult to assess based on the complexity of the tla-es project you've referenced.

Out of curiosity, why did you switch from a @MockBean for ElasticsearchOperations in the original tla-es project to a @MockitoSpyBean in your fork?

In any case, please provide a minimal sample application that reproduces the problem (that we can download and run), preferably one that does not require the use of Lombok or Docker containers.

Cheers,

Sam

sbrannen commented 1 month ago

@JKatzwinkel, I experimented with your application, and after some hacking... it appears that your use of @MockitoSpyBean may work with 6.2 snapshots.

In other words, this may effectively be a:

So, before you take the time to create a minimal sample application, can you please try with 6.2.0-SNAPSHOT for Spring Framework dependencies such as org.springframework:spring-test?

If you need help setting up Gradle to use snapshots, this wiki page should prove useful.

Cheers,

Sam

JKatzwinkel commented 1 month ago

thank you very much for the investigating and good news! I will look into that and let you know!

JKatzwinkel commented 1 month ago

@sbrannen adding org.springframework:spring-test:6.2.0-SNAPSHOT to the test dependencies does indeed make the error disappear! I am very grateful for your quick response and gathering insights without me having to come up with a minimal sample application, and of course for the fix in https://github.com/spring-projects/spring-framework/commit/81d89f478aa2441ecfa90907ba21b7cc20152059 !

I am looking forward to the next release containing the patched bean factory post processor! Thanks again!


Out of curiosity, why did you switch from a @MockBean for ElasticsearchOperations in the original tla-es project to a @MockitoSpyBean in your fork?

Using a @MockitoBean instead of a spy leads to failed dependency resolution during application context loading, because some other autowired spring components down the line also use ElasticsearchOperations in their @PostConstruct hook. They use it for creating Elasticsearch indexes in my fork, whereas in the original tla-es repo, index creation is being done automatically by spring-data-elasticsearch, so their ElasticsearchOperations mock isn't expected to actually do anything.

sbrannen commented 1 month ago

Hi @JKatzwinkel,

@sbrannen adding org.springframework:spring-test:6.2.0-SNAPSHOT to the test dependencies does indeed make the error disappear!

Great! Thanks for trying it out and confirming it solves your problem.

I am very grateful for your quick response and gathering insights without me having to come up with a minimal sample application, and of course for the fix in 81d89f4 !

You're welcome.

I am looking forward to the next release containing the patched bean factory post processor! Thanks again!

That next "release" should be 6.2 GA.

Thanks for clarifying why you switched to @MockitoSpyBean in your fork.

Although there were actually two "bugs" in the Bean Override support regarding FactoryBeans, the fix I pushed for #33800 actually resolved the issue you reported as well. In light of that I am closing this as a: