Is it OK / good practice to put assertions inside Mockito.argThat?

Let's say you have a JUnit test with Mockito and you have an argThat matcher:

Mockito.argThat(obj -> {
  return "ok".equals(obj.getStuff()) ||
         "ok2".equals(obj.getStuff2()) ||
         "ok3".equals(obj.getStuff3()) ||
         "ok4".equals(obj.getStuff4());
});

But now when you run the test and it fails, it generically fails and it's hard to figure out what condition fails and you need to use debugger.

If I do this:

Mockito.argThat(processedNuMessage -> {
  Assert.assertEquals("ok", obj.getStuff());
  Assert.assertEquals("ok2", obj.getStuff2());
  Assert.assertEquals("ok3", obj.getStuff3());
  Assert.assertEquals("ok4", obj.getStuff4());
  return true;
});

Now I get much easier assertions to use. But if feels kinda wonky.

Is there a better way to do this?

Solution

No, it is explicitly marked as a bad practice in the ArgumentMatcher.matches documentation, emphasis in the original:

The method should never assert if the argument doesn't match. It should only return false.

This is important for a couple of reasons: First, if there are multiple calls to the method, one call may fail while another passes. By throwing in the argument matcher, Mockito will fail before it gets a chance to match other stubs. Also, Mockito trusts that it can call this function without side effects in methods like when:

when(yourMock.yourMethod(anyInt(), argThat(new ThrowingMatcher()))).thenReturn(...);
// this next line will fail at stubbing time because of the previous line
when(yourMock.yourMethod(eq(6), argThat(new ThrowingMatcher()))).thenReturn(...);

In the second stub above, Mockito will call yourMock.yourMethod before calling when, so Mockito will try to match a call to yourMethod(0, null). This will trigger your matcher, which will throw an exception due to the mismatch, preventing you from stubbing further. This mismatch is only an example: if you throw from within an ArgumentMatcher and violate the interface contract, Mockito isn't guaranteed to work.

If you want Mockito to fail faster or more clearly, you do have a few options:

For stubbing time, you can use thenAnswer:

when(yourMock.yourMethod(/* user= */ any())).thenAnswer(invocation -> {
  User user = invocation.getArgument(0);
  /* your asserts here */
  return yourReturnValue;
});

For verification time, you can use an ArgumentCaptor.

ArgumentCaptor<User> userCaptor = ArgumentCaptor.forClass(User.class);
verify(yourMock).yourMethod(userCaptor.capture());
User user = userCaptor.getValue();
/* your asserts here */

You can enable Strictness, which causes Mockito to fail on an unstubbed method call. This is more idiomatic than the older Mockito 1.x technique of using a throwing default answer.

A few notes:

You might benefit from giving your User a reasonable toString representation for debugging. There are helper libraries for this. ArgumentMatcher also documents that it uses its toString method when printing verifications and failed matches.
Depending on how carefully you match your methods, you might still run into the stubbing problem described at the top. Switching to doVerb methods like doReturn or doAnswer help, as the call to doVerb precedes the call to your mock so Mockito can deactivate stubbed behavior.
Though Hamcrest matchers offers an appealing-looking describeMismatch, this typically has not been valuable in Mockito because Mockito's invocation matching algorithm makes it more difficult to reason about when a call is missing versus when a call is mismatched. It might still be a good solution for you, but you'd have to invoke the Matcher manually using one of the above techniques rather than just calling MockitoHamcrest.argThat.