Consider @Primary annotation when using @MockBean, make @MockBean/@SpyBean behave consistently

[neiser]

The discussion in #11066 revealed that the proposed solution is not ideal. Based on @alexandreBaronZnk's work, I created another pull request and tried to honor @philwebb's remarks.

I changed that getBeanName respects the @Primary annotation (and does not simply fail if more than one candidate is found) and I copied the primary flag from the removed bean to the mocked bean. This makes the tests work again (of course I removed the changes in findCandidateBeans first).

I cannot push to @alexandreBaronZnk's branch, I hope it's okay that I created another pull request.

[pivotal-issuemaster]

@neiser Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@neiser Thank you for signing the Contributor License Agreement!

[neiser]

@philwebb Can you explain why the handling of bean registration is so different for @MockBean and @SpyBean in MockitoPostProcessor? I added some TODO comments in the last commit, as the analogous test where you have a primary bean but want to spy a qualified one does not work.

Note that I didn't change anything in the current behaviour for @SpyBean, so it already had some handling for the @Primary annotation in MockitoPostProcessor.determinePrimaryCandidate, which is only used when registering spies (and not mocks).

This is all quite mysterious to me. I hope you can shed some light on it.

Update: I added also an integration test illustrating the problem with @SpyBean.

[philwebb]

@neiser Spy beans are quite tricky. With standard mocks we can replace the bean definition early and just create a mock instance when the bean is needed. For spy beans we need to allow the standard instance to be created, then wrap it with a spy.

In registerSpy we actually just keep track of the definitions. We then have a SpyPostProcessor class that gets called after every bean is created. It delegates back to MockitoPostProcessor.createSpyIfNecessary to actually wrap the real bean instance in a spy if we know that's what the user wants.

[bopfermann]

Any update on this?

[neiser]

@snicoll @philwebb I'm happy to resolve the merge conflicts which have appeared by now. I'd just like to know if there's any chance an updated PR will be reviewed after some reasonable time.

[philwebb]

@neiser Sorry this didn't make it into 2.0, we just ran out of time. I'll add it to the 2.1 backlog. There's no need to rebase, we can do that when we look at merging it.

[neiser]

@philwebb Any update on this? Is there an estimate when this is going to be reviewed?

[philwebb]

@neiser It's on the backlog which means we'd like to include it in the next release but I'm afraid I can't give you an estimate for when it might be reviewed.

[neiser]

@philwebb We're approaching now one year that this PR is open. Any chance this gets reviewed soon? I'd also consider this more a bug than an enhancement by the way, as using @Primary with @MockBean is indeed broken (though I haven't checked the most recent Spring Boot release).

[philwebb]

Thanks for bearing with us on this one @neiser! I've now pushed a version of it to master so it should be in 2.1. Unfortunately the PR didn't apply that cleanly so I've chopped it up a bit. The tests from your original PR (with some slight formatting changes) are in a5b3a26. The main code changes are in c777614 but they've been made directly to the post processor rather than introducing a new class.

Thanks again!

[philwebb]

I realized your @Author attribution also got lost in the mix. I've just fixed that in 0471037.

[neiser]

@philwebb Thanks a lot for merging it!