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

Recommend the use of AopTestUtils.getTargetObject() when setting expectations on a spied bean that Spring has proxied #22281

Closed
wilkinsona opened this issue Jul 9, 2020 · 7 comments
Assignees
Labels
type: documentation A documentation update
Milestone

Comments

@wilkinsona
Copy link
Member

With thanks to @kriegaex, see this question on Stack Overflow and the sample to which it links.

@wilkinsona
Copy link
Member Author

To fix this, we need to be able to automatically unwrap the proxy as is being done here:

doNothing()
    .when(
        // fetch spy bean by unwrapping the AOP proxy, if any
        AopTestUtils.<KafkaService>getTargetObject(kafkaService)
    )
    .send(ArgumentMatchers.any());

Unfortunately, I don't think there's anything in Mockito's current public API that will notify us of the when call and allow us to get the target object and have it be the value that's returned from when(). I think we need an equivalent of VerificationStarterListener (which was added to Mockito as a result of #10352) for when the expectations are being set up.

@mockitoguy If you have a moment, I'd love your expertise here please. I'm wondering if there's something that I've missed in Mockito that would provide the necessary interception point.

@wilkinsona wilkinsona added the status: blocked An issue that's blocked on an external project change label Jul 29, 2020
@mockitoguy
Copy link

Hey!

One thing to try is to see if "InvocationListener" works for your use case. I don't remember off top of my head if it reports stubbings or only regular invocations (and whether it works with when and doReturn stubbings)

Adding new listener is conceivable. My understanding is that the "sample" link in the description reproduces this very issue? I'm a little busy next couple of days but I'm happy help.

@wilkinsona
Copy link
Member Author

Thanks, @mockitoguy.

Thinking about this some more, I'm not sure that we can fix this at the Mockito level. If the method being stubbed returned something other than void, you'd write something like given(mock.method()).willReturn(42) and at that point the call to mock.method() will happen before there's any opportunity to unwrap the proxy. Requiring people to flip that around and write doReturn(42).when(mock).method(), doesn't feel like a good solution.

I'm starting to believe that we may not always be able to do the right thing when proxying is involved. We may just have to make working with that proxy and getting its underlying target easier and more obvious.

@wilkinsona wilkinsona added for: team-attention An issue we'd like other members of the team to review and removed status: blocked An issue that's blocked on an external project change labels Jul 29, 2020
@philwebb philwebb modified the milestones: 2.2.x, 2.4.x Aug 5, 2020
@philwebb philwebb added type: bug A general bug and removed for: team-attention An issue we'd like other members of the team to review type: bug A general bug labels Aug 5, 2020
@philwebb philwebb removed this from the 2.4.x milestone Aug 5, 2020
@wilkinsona
Copy link
Member Author

wilkinsona commented Aug 5, 2020

We've opened spring-projects/spring-framework#33743 to provide a nicer API for dealing with spies that are also Spring proxies. In the meantime, manually removing the Spring proxies by calling getTargetObject() is the way to go. We can use this issue to update the documentation to recommend its usage when spying on a Spring proxy.

@wilkinsona wilkinsona changed the title Advice is invoked when setting expectations on a @SpyBean Recommend the use of AopTestUtils.getTargetObject() when setting expectations on a spied bean that Spring has proxied Aug 5, 2020
@wilkinsona wilkinsona added type: documentation A documentation update and removed type: bug A general bug labels Aug 5, 2020
@wilkinsona wilkinsona added this to the 2.2.x milestone Aug 5, 2020
@wilkinsona wilkinsona modified the milestones: 2.2.x, 2.2.10 Aug 6, 2020
@sbrannen
Copy link
Member

sbrannen commented Aug 6, 2020

Commit b53f54f incorrectly states:

Use AopTestUtils.getTargetProxy(yourProxiedSpy) to do so.

Whereas, that should read:

Use AopTestUtils.getTargetObject(yourProxiedSpy) to do so.

@wilkinsona
Copy link
Member Author

Well spotted, @sbrannen. Thank you. I've corrected it in 63f7c75.

@mockitoguy
Copy link

Sounds good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

4 participants