Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support @MockitoBean in @Configuration classes #33934

Open
tobias-lippert opened this issue Nov 21, 2024 · 11 comments
Open

Support @MockitoBean in @Configuration classes #33934

tobias-lippert opened this issue Nov 21, 2024 · 11 comments
Assignees
Labels
in: test Issues in the test module status: waiting-for-triage An issue we've not yet triaged or decided on

Comments

@tobias-lippert
Copy link

tobias-lippert commented Nov 21, 2024

Versions
Spring Boot 3.4.0
Spring Framework 6.2.0

Hi Spring team,

While upgrading our apps to Spring Boot 3.4.0, I've encountered that some of our tests don't start because the application context cannot be created due to a missing bean dependency that should exist as mocked bean.

We define some MockitoBeans in a config class annotated with @TestConfiguration and import this class using @Import.
When I define a MockBean instead, the tests succeed.

You can find an example that reproduces the behavior here: https://github.com/tobias-lippert/spring-mockitobean-test-configuration-import-bug
The test passes on the branch mockbean-working and fails on the branch mockitobean-not-working.

I couldn't find anything in the docs pointing out that this should no longer work. If this is not a bug, but intended behavior please advise how to fix this.
The easiest, but not the prettiest workaround I've found is getting rid of the separate TestConfigurations and placing all MockitoBeans in the actual test class.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 21, 2024
@sbrannen sbrannen added the in: test Issues in the test module label Nov 22, 2024
@sbrannen sbrannen self-assigned this Nov 22, 2024
@olsavmic
Copy link

We have encountered the same problem.

The reason why we define @MockitoBean (and @MockitoSpyBean) is to prevent new Application Context creation - mocks are defined in one place and used by all tests importing the same configuration.

We could use a base class for tests but that goes against the extensible design of JUnit 🤔

@EugenMayer
Copy link

Everything in the issue queue points to MockitoBean/MockitoSpyBean has not been field test well (at all?). There are so many limitations right now, so that deprecating MockBean/MockSpy sounds like a well overpaced decision that should be rolled back with 3.4.1 until MockitoBean/MockitoSpyBean is actually able to replace the main cases in the first place.

I have nothing agains a better implementation, but this is not a "toy" to play around, neither is the migration "a little thing". For us, this is huge i feeling pushed into a playground in this area feels like being let down. This field needs to be rock solid - and when it is, deprecation can come in.

Expamples

  1. MockitoBean cannot be used on a class level to mock away a list of bean which one will not operate on - they are not important to the setup, they are just needed to be available. This is currently not possible
  2. MockitoBean as a field annotation does not behahave the same way as MockBean in several cases, i had to roll back in several cases until i decided to rollback the entire migration in the first place

there are other issues in the queue pointing at similar, severe issues.

I beg you, please roll back the deprecation until the actual target to migrate to is "ready for battle".

@sbrannen sbrannen changed the title MockitoBean in TestConfiguration is not picked up whereas MockBean is Support @MockitoBean on @Configuration classes Nov 24, 2024
@adase11
Copy link

adase11 commented Nov 25, 2024

I can confirm I'm running into the same issues when moving from spring boot 3.3.5 to 3.4.0 (and thus Spring Framework 6.2 ). Much like @olsavmic we have one shared configuration class for all tests - specifically @WebMvcTest tests. We do this because there are some beans involved in web security configuration that we need mocked for these tests and they are the same across all of those tests.

The test's that are failing due to missing beans are set up almost exactly like @tobias-lippert 's example. With a @WebMvcTest that uses @Import to pull in the configuration class in which we have those mock beans declared.

I also confirmed that adding the mock bean declarations to the test class (as well as alternatively a base abstract test class) directly fixes the problem (like @tobias-lippert mentioned).

@cfredri4
Copy link
Contributor

Same here. We also define @MockBean in a separate @TestConfiguration and this does not work with @MockitoBean.

@adase11
Copy link

adase11 commented Nov 25, 2024

I mentioned this on the Spring Boot repo spring-projects/spring-boot#43282 . @TestConfiguration and @WebMvcTest are both from Spring Boot.

@wilkinsona
Copy link
Member

wilkinsona commented Nov 25, 2024

I'm a Spring Boot maintainer and only speaking for the Boot team below.

Everything in the issue queue points to MockitoBean/MockitoSpyBean has not been field test well (at all?). There are so many limitations right now, so that deprecating MockBean/MockSpy sounds like a well overpaced decision that should be rolled back with 3.4.1 until MockitoBean/MockitoSpyBean is actually able to replace the main cases in the first place.

There's a tricky balance to strike here. Replacing @MockBean and @SpyBean with something built on Spring Framework's support for bean replacement is definitely the right thing to do. To do it well, we need feedback from the community. Unfortunately, our experience shows that if something isn't deprecated, it's very hard to get the feedback that we need and we just end up in a situation very similar to the one that we're in now, albeit 6-12 months later.

I beg you, please roll back the deprecation until the actual target to migrate to is "ready for battle".

IMO, rolling back the deprecation would be counter-productive. Instead, I'd like to try to reassure folks by noting that functionality deprecated in Spring Boot 3.4 won't be going away any time soon. We have Spring Boot 3.5 planned for May 2025 and it won't be removed in that release. Assuming the same pattern as previous releases, OSS support for Spring Boot 3.5 would then be provided until May 2026.

Please keep the constructive feedback coming and then address the deprecation either through a suppression or by using an alternative, even if it is one where you feel there's room for improvement.

@EugenMayer
Copy link

The important first

Please keep the constructive feedback coming and then address the deprecation either through a suppression or by using an alternative, even if it is one where you feel there's room for improvement.

I understand the critic above is harsher in it's tone, but i really try to keep it constructive in the first place (and i gave suggestions / opinions on how to improve it)

There's a tricky balance to strike here. Replacing @MockBean and @SpyBean with something built on Spring Framework's support for bean replacement is definitely the right thing to do. To do it well, we need feedback from the community. Unfortunately, our experience shows that if something isn't deprecated, it's very hard to get the feedback that we need and we just end up in a situation very similar to the one that we're in now, albeit 6-12 months later.

I'am following the project rather open-minded and i never heard of the MockitoBean/MockitoSpyBean effort, nor it's release nor the question for feedback. Can you point me at sources like release notes, blogs or anything else where this has been asked for 6-12 months ago?

@wilkinsona
Copy link
Member

I was talking in general terms about our experience with getting a sizeable portion of the user base to try out a new feature, not specifically the introduction of @MockitoBean and @MockitoSpyBean. Regardless, I think this is a case in point as https://spring.io/blog/2024/04/16/spring-framework-6-2-0-m1-overriding-beans-in-tests, published in April 2024, introduced the new Framework feature and invited feedback and suggestions for improvement. This was reinforced in Spring Boot 3.4.0-M1, released in July 2024, where @MockBean and @SpyBean were deprecated in favor of @MockitoBean and @MockitoSpyBean.

@EugenMayer
Copy link

EugenMayer commented Nov 25, 2024

I was talking in general terms about our experience with getting a sizeable portion of the user base to try out a new feature, not specifically the introduction of @MockitoBean and @MockitoSpyBean. Regardless, I think this is a case in point as https://spring.io/blog/2024/04/16/spring-framework-6-2-0-m1-overriding-beans-in-tests, published in April 2024, introduced the new Framework feature and invited feedback and suggestions for improvement. This was reinforced in Spring Boot 3.4.0-M1, released in July 2024, where @MockBean and @SpyBean were deprecated in favor of @MockitoBean and @MockitoSpyBean.

That's fair - missed all of those. That one is one me and surely proves you point of not getting substantial feedback if it is not thrown into our faces.

Yes, but i have a "but". Use the deprecation as a tool, for a annotation that is used 100+ in the code base (triggering a warning for each), for this specific case, was just not a good choice. For us, it is rather 1000+ and this means, every CI checking testing logs needs massive scrolling, not to talk about proper deprecation warnings which are entirely hidden now.
For me, you used a bazooka here (sorry if this sounds harsh again).

Constructively, i would love to offer something like

  • "use this in gradle to suppress the deprecation" - but there is none (all or nothing)
  • add "suppress to every MockBean/SpytBean usage .. well this means cluttering more then 100+ places with an @SupressWarning - one can surely use string/replace, but this is really... a lot of clutter.

So honestly, i have no constructive option on how to fix what has been enforced here while MockitoBean is not in shape (yet).

If you ask me, you could have make the entire MockitoBean migration visible in the test, when you detect it's usage and showing a warning in the search logs (once), and this warning should be able to be deactivated via properties (but is on by default). This is a sensible way of doing this, and still get visibility / exposure for feedback.

Do not get me wrong, things move on, break and needs migration. That is what it is and we all have to move. This is fair.
But currently one cannot even migrate and fix it, we are just forced to take it.

I understand, it's probably too late, but at least take my feedback as bring attention to changing such sensible topics. This is not "spring session redis" or "spring security SAML", "jpa" only affecting parts of spring boots ecosystems.

This changed affected all ecosystems at once, all of them. And this makes this a sensible one.

Thank you for your feedback and comment, really appreciated to at least have a dialog - maybe we do not agree, but still, thank you for being responsive.

@HenrikPublic
Copy link

When your policy is to compile with -Werror it's even more frustrating ;-)

@vpavic
Copy link
Contributor

vpavic commented Nov 26, 2024

When your policy is to compile with -Werror it's even more frustrating ;-)

Same frustrations here, and it's a bit ironic since both Spring Framework and Spring Boot use those as well 🙂:

Using suppressions tactically here and there is OK, but for non-trivial projects this can easily mean hundreds of suppressions which is really not cool.

I understand and agree that it's harder to get quality feedback without doing deprecations, but at the same time there appear to be quite a few common cases that developers rely on and don't have a clear migration path so I feel like this could easily turn out to be a prohibitively expensive upgrade for some projects (which also translates into less feedback).

So I'm personally in the camp that would like to see the deprecation rolled back, at least until clear migration path for common use cases is provided.

@sbrannen sbrannen changed the title Support @MockitoBean on @Configuration classes Support @MockitoBean in @Configuration classes Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module status: waiting-for-triage An issue we've not yet triaged or decided on
Projects
None yet
Development

No branches or pull requests

10 participants