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

mockito3.4.0 InlineByteBuddyMockMaker can't work with spring cglib proxy, but ByteBuddyMockMaker works #1980

Closed
jacky1193610322 opened this issue Jul 17, 2020 · 17 comments · Fixed by #2042

Comments

@jacky1193610322
Copy link

I upgrade mockito to 3.4.0, and the mockMaker is InlineByteBuddyMockMaker, I use SpyBean to spy a spring bean, and then verify the bean, it throws NotAMockException.
image

then i debug, find that the mocks.get(mock) can't find it because the mock is CGLIB proxy, not the original type.

when I use the verify((AppGroupClusterAction) AopTestUtils.getTargetObject(appGroupClusterAction)) to verify. it works.

but the before mockMaker ByteBuddyMockMaker works. so can the InlineByteBuddyMockMaker be compatible with the before?

@raphw
Copy link
Member

raphw commented Jul 17, 2020

In this case, the mock is not registered correctly. Mockito has no knowledge of Spring and could not do much about this. Have you adressed this issue with Spring? How do you get hold of this proxied instance?

@jacky1193610322
Copy link
Author

@SpringBootTest
@RunWith(SpringJUnit4ClassRunner.class)
@AutoConfigureMockMvc
public class AppGroupClusterActionTest {

  @Autowired
  private MockMvc mockMvc;

  @MockBean
  private AppGroupClusterService appGroupClusterService;

  @SpyBean
  private AppGroupClusterAction appGroupClusterAction;

  @Captor
  private ArgumentCaptor<Object[]> argumentCaptor;

  @Test
  public void testUpdateExecuteAuditSpel() throws Exception {
    ServiceResult mockRes = new ServiceResult();
    mockRes.setSuccess(true);
    doReturn(mockRes).when(appGroupClusterService).update(
        Mockito.any(), Mockito.any(), Mockito.any(),
        Mockito.any(), Mockito.any(), Mockito.any(),
        Mockito.any(), Mockito.any(), Mockito.any());

    mockMvc.perform(
        MockMvcRequestBuilders.post("/platforms/{platform}/appGroupNames/{appGroupName}/clusterNames/{clusterName}/update",
            "platform", "appGroupName", "clusterName").content("{}"))
        .andExpect(status().isOk())
        .andReturn();

//    not work
//    verify(appGroupClusterAction).addAfterAuditResult(argumentCaptor.capture());
    verify((AppGroupClusterAction) AopTestUtils.getTargetObject(appGroupClusterAction)).addAfterAuditResult(argumentCaptor.capture());
    assertArrayEquals(argumentCaptor.getValue(), new Object[]{"appGroupName", "platform", "clusterName", "{}"});

    verify((AppGroupClusterAction) AopTestUtils.getTargetObject(appGroupClusterAction)).addAfterAuditResult(argumentCaptor.capture());
    assertArrayEquals(argumentCaptor.getValue(), new Object[]{"appGroupName", "platform", "clusterName", "{}"});
  }
}

this is the code. I haven't addressed this issue with Spring, because it works before mockito3.4.0, I don't know whether the mockito need to be compatible with before?

@raphw
Copy link
Member

raphw commented Jul 20, 2020

I'd say this issue would need to be adressed in Spring. I assume that the MockBean does not actually get the object injected that Mockito issues.

@wilkinsona
Copy link

wilkinsona commented Aug 7, 2020

I've taken a look at this from the Spring Boot side of things. The TL;DR is that Spring's creating a CGLib proxy of the spy that Mockito has created. This proxied spy is passed into when and SubclassByteBuddyMockMaker correctly identifies that it's a mock while InlineByteBuddyMockMaker does not. You can work around the problem by passing the target of Spring's proxy (the Mockito spy) into when, but my expectation is that the two mock makers should have the same ability to identify a mock and both should cope with the spy having been proxied.

I haven't addressed this issue with Spring, because it works before mockito3.4.0

I should also note that, in my testing, this isn't the case. The problem exists with Mockito 3.4, 3.3, and 3.1 with the difference in behaviour depending on whether or not you're using mockito-inline.

@jacky1193610322
Copy link
Author

jacky1193610322 commented Aug 17, 2020

@raphw @mockitoguy can you help?

@TimvdLippe
Copy link
Contributor

Thanks for filing the issue. Could you please provide us a minimal reproduction case that we can clone/download to debug the issue? Thanks in advance!

@raphw
Copy link
Member

raphw commented Sep 14, 2020

Sorry, I missed this.

The reason this works with the SubclassByteBuddyMockMaker is that Spring proxies the (internal) MockAccess interface which we implement in the mock to extract the mock state which is stored in a field of the mock instance. We could just as much had read that field directly to avoid the interface; and this wouldn't even be such a bad idea as it less pollutes the reflection API. But at this time, when we call our internal interface method, it gets redirected to the actual proxy by Spring which is why this functions (accidentally).

The reason it does not work with the InlineByteBuddyMockMaker is that we cannot implement interfaces in a class after it has been loaded. The whole premise of the inline mock maker is to keep changes invisible to make use of the retransformation facilities even after class loading. Therefore, we need to identify mocks by the object identity which changes once Spring changes the object. We cannot change this, from our perspective nothing is broken.

From Mockito's side things are simple: if you do not provide "our" mock objects to us, we do not guarantee to work, not in the past or in the future. That it does currently work with the subclass mock maker facility is rather accidental and might change in the future. If Spring cannot do anything about this, you'd need to unproxy the objects explicitly.

@wilkinsona
Copy link

wilkinsona commented Sep 14, 2020

If Spring cannot do anything about this

As things stand, I don't believe we can as there's no opportunity for Spring to remove its proxy when the proxied spy is passed into given or when.

I've wondered in the past if Mockito could offer a low-level API that provides a mechanism for pre-processing the instance that's passed in, but concluded that it wouldn't be broadly usable. I'd be delighted if someone who's more familiar with Mockito's internals believes it could be done.

In the absence of some inspiration on the Mockito side, our current plan is to make it easier for users to work with both the Spring proxy and the "bare" Mockito spy.

@raphw
Copy link
Member

raphw commented Sep 14, 2020

I could certainly add a mechanism for this, it would not create too much overhead and we already have a plugin mechanism in place for it. We would just add a non-operational mechanism by default but allow frameworks to unwrap "their" mocks. You could declare this by Mockito's default plugin mechanism.

I will look into adding something like this to Mockito.

@raphw
Copy link
Member

raphw commented Sep 14, 2020

@wilkinsona

I just added an API. Could you try the mock-resolver-plugin branch where Spring would need to implement the MockResolver interface as a public class with a default constructor and add a file org.mockito.plugins.MockResolver to the mockito-extensions folder with the fully qualified class name. The resolver needs to check if a mock instance can be unwrapped and return this instance if applicable.

This way, I think this problem can be solved and Spring stops relying on the implementation detail.

@wilkinsona
Copy link

Thanks, @raphw. I think this moves things in the right direction and allows almost all of our Mockito-related tests to pass with the inline mock maker.

I am, however, still seeing a failure when calling Mockito.reset(T) with a spy that has been proxied with CGLib. The MockHandler is retrieved successfully (as our resolver removes the CGLib proxy) but it then fails in InlineByteBuddyMockMaker.resetMock(Object, MockHandler, MockCreationSettings) as the mock that is passed in is still CGLib-proxied. I wonder if reset should resolve the mock and use that both for getting the handler and for calling the mock maker? Something like this:

public static <T> void resetMock(T mock) {
    mock = resolve(mock);
    MockHandler oldHandler = getMockHandler(mock);
    MockCreationSettings settings = oldHandler.getMockSettings();
    MockHandler newHandler = createMockHandler(settings);

    mockMaker.resetMock(mock, newHandler, settings);
}

@raphw
Copy link
Member

raphw commented Sep 18, 2020

Good catch! I'll fix that up!

@raphw
Copy link
Member

raphw commented Sep 18, 2020

I updated the branch and doublechecked that I did not miss anything else. Let me know if this works for you!

@raphw
Copy link
Member

raphw commented Oct 10, 2020

@wilkinsona Did you have a chance to validate my latest revision of this?

@wilkinsona
Copy link

Not yet, sorry. I'll try and take another look at it this week.

@wilkinsona
Copy link

It took a little longer than I'd hoped, but I've just tried the updated branch and all of Spring Boot's Mockito-related tests now pass with the inline mock maker. Thank you.

@Kosmo1
Copy link

Kosmo1 commented Dec 20, 2022

@raphw @wilkinsona Can someone tell me how I can leverage or otherwise take advantage of whatever fix was made in #2042 and get my @SpyBeans to work correctly with Mockito of any version? I've tried changing my version from v3.4.0 to v3.6.0 (where the change was ultimately merged) but that alone has not improved the situation. Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants