Try to replace calls to Mockito internals

[philwebb]

Long term it would be nice to replace our calls to internal Mockito code to only use their public API (see this comment).

This should be easier once #10247 has been fixed.

[wilkinsona]

All of our use of Mockito's internals in now isolated to MockitoAopProxyTargetInterceptor where we use MockingProgress, reset the mode with which verification was started, and update the argument matcher storage. I can't see any way to replace that with public API at the moment.

/cc @TimvdLippe

[mockitoguy]

Hey, let's work together to avoid using internal APIs!

Can you elaborate a bit on the use case? I've read MockitoAopProxyTargetInterceptor javadoc. It is not completely clear to me why it is needed.

[wilkinsona]

@szczepiq Thanks. There's some background about MockitoAopProxyTargetInterceptor in this issue and also in this commit where it was first introduced. Could you please have a read of those and let us know if they help to clear up what the interceptor's doing.

[mockitoguy]

@wilkinsona, I found this section of your comment very useful (#5837 (comment)):

@Test
public void test() {
    service.doSomething(99);
    Mockito.verify(repository).save(99);
    Mockito.verifyNoMoreInteractions(repository);
}

Things go wrong when save is called. Mockito's MockAwareVerificationMode believes that the mock is MyRepository$$EnhancerBySpringCGLIB$$b2127210, whereas its InvocationImpl believes that it's MyRepository$$EnhancerByMockitoWithCGLIB$$cd452aee. It sees that the two "mocks" are different and re-adds the verification mode to the ThreadSafeMockingProgress. From Mockito's perspective, this verification is never completed so it fails with an UnfinishedVerificationException.

The reason Mockito needs to re-add verification mode is to correctly handle Mockito test spies. Example: calling verify(mockitoSpy).foo() will trigger real implementation of “foo” method. “foo” may delegate to other methods on the spy object, let’s say “bar”. We want to verify “foo” method, not “bar” method so we need to re-add verification of “foo”. Hope it explains why we do it this way in Mockito. Now let's figure out how help out Spring integration :)

Here’s how I understand the Spring scenario, based on the code snippet above:

Repository is a Spring proxy object (SP - spring proxy) wrapped with Mockito mock (MP - mockito proxy).

1. Mockito.verify(repository).save(99); is called
2. verify(repository) passes the instance of MP to Mockito’s internal mocking progress state
3. save(99) is invoked, SP handles it first
4. MP handles it next, SP mock does not match MP mock and Mockito re-adds verification mode

How can I better understand what happens in step 3) Why does the invocation object has different mock that the mock in verification mode? Does Spring do something interesting when delegating to Mockito proxy?

Is there a unit test that demonstrates this behavior and easy steps to pull that code to IDE so that I can run it and reproduce the behavior?

Thank you for useful links! Looking forward to fixing this internal use of Mockito API!!!

[wilkinsona]

@szczepiq Thanks for taking a look. I've created a minimal reproduction of the problem when calling verifyNoMoreInteraction() with the double proxy. It also shows it working when there's only a single proxy.

[mockitoguy]

Great! I will investigate it. -- Szczepan Faber Founder @ http://mockito.org | Twitter @ https://twitter.com/mockitoguy Author @ https://www.linkedin.com/in/szczepiq/recent-activity/posts/

[mockitoguy]

@wilkinsona, the sample project is fantastic! Thank you.

I debugged it and I now I fully understand what's going on. Let me think about it a bit more and I'll come back with some options of growing Mockito public API to accommodate this use case.

[mockitoguy]

Dual proxy layer is an interesting caveat. There is no super clean solution :) Here’re the options we have I think:

1. Add an extension point to where we compare mocks. This way we can hook up custom logic that compares mocks. Then, we can make Mocktio consider outer proxy object same as inner proxy object.

2. Create new extension point: VerificationStartedListener. The instance of listener can be passed on mock creation. When verification is started Spring can replace outer proxy mock with inner proxy mock.

mock(Foo.class, withSettings().verificationStartedListeners(new VerificationStartedListener() {
  public void onVerifyStarted(VerificationStartedEvent event) {
    event.getMock();
    event.setMock(arbitrayObject);
  }
}

3. Add API for manipulating verification state. This way, we can keep most of the current Spring integration, just replace private API with public API.

VerificationState state = Mockito.framework().getVerificationState();
if (state.isVerificationStarted()) {
  state.restartVerification(arbitraryObject)
}

4. Other options? Suggestions?

My thoughts about options:

1. Seems hacky. There may be many places where we compare (!=) mocks. It’s easy to forget to use the extension point. The extension point itself very awkward, really hard to understand why it is needed.

2. If it works for you, I like it best. Do you control creation of mocks in Spring so that we can ensure the listener is added?

3. Should work but I’m not very happy with it. The API is very contrived, tailored for very specific use case.

I'm excited we can grow Mockito API to streamline integrations!!! Thoughts about the APIs?

[wilkinsona]

Thank you, @szczepiq. Your second option sounds like it will work for us. When someone uses @MockBean or @SpyBean we have complete control over the creation of the mock/spy so providing the listener should not be a problem. If you have the time to prototype something in a fork/branch that I could build locally, I'd be more than happy to try it out.

[mockitoguy]

Fantastic!!! I'll put together a prototype soon.

[mockitoguy]

Since this is a code change in Mockito, is it ok to move the discussion to the Mockito ticket?

I put together the prototype and documented it here: mockito/mockito#1191 (comment)

Please take a look! I'm excited to make Mockito public APIs more robust!

[wilkinsona]

Here's a prototype of an update to Spring Boot to use the new Mockito API (and remove some other internal API usage in our tests): https://github.com/wilkinsona/spring-boot/tree/gh-10352.

[TimvdLippe]

Mockito 2.11.0 (https://mvnrepository.com/artifact/org.mockito/mockito-core/2.11.0) has been released with the new API!

[wilkinsona]

Thank you, @mockitoguy and @TimvdLippe. We'll upgrade to Mockito 2.11 in 2.0 M6.