[Java] Call to Mockito method `verify`, `when` or `given` can be simplified

Mockito provides argument matchers for flexibly stubbing or verifying method calls.

Mockito.verify(), Mockito.when, Stubber.when and BDDMockito.given each have overloads with and without argument matchers.

However, the default matching behavior (i.e. without argument matchers) uses equals(). If only the matcher org.mockito.ArgumentMatchers.eq is used, the call is equivalent to the call without matchers, i.e. the eq is not necessary and can be omitted.

The resulting code is shorter and easier to read.

Noncompliant code examples

given(foo.bar(eq(val1), eq(val2), eq(val3))).willReturn(null);   // Noncompliant

when(foo.baz(eq(val4), eq(val5))).thenReturn("hi");   // Noncompliant

doThrow(new RuntimeException()).when(foo).quux(eq(42));    // Noncompliant

verify(foo).bar(eq(val1), eq(val2), eq(val3));   // Noncompliant

can be simplified to

Compliant code examples

given(foo.bar(val1, val2, val3)).willReturn(null);   // Compliant

when(foo.baz(val4, val5)).thenReturn("hi");   // Compliant

doThrow(new RuntimeException()).when(foo).quux(42);   // Compliant

verify(foo).bar(val1, val2, val3);   // Compliant

External references and/or language specifications

Type

  • type : Code smell

Tags

  • java
  • mockito
  • clumsy

On the other hand, leaving the eqs where they are would make it easier to change them to other matchers.

Hey Björn,

Nothing to add to your description. This rule is a perfect match, nicely written, and goes into the direction of writing simpler, nicer tests… Which is exactly one of the directions we want to push these days for our Java analyzer.

Here is are the related tickets I created from it:

Really, great work! Thanks a lot! Also, @bduderstadt, in order to thank you a bit more concretely for your contributions to this community, notably the new rules you have been suggesting over the past months, we would like to send you a SonarSource T-shirt as a small gift. I’ll you a private message to get your address and t-shirt size, and we will send it to you as soon as possible!

Regarding your remark:

Well, if there is another matcher to be used, it will still be possible to use it only on that parameter afterward, for instance with:

verify(foo).bar(val1, val2, anyString());

Cheers,
Michael

2 Likes

Hey Michael,

thanks for the appreciation.

Regarding the remark:

Well, if there is another matcher to be used, it will still be possible to use it only on that parameter afterward, for instance with:

verify(foo).bar(val1, val2, anyString());

I’m afraid that wouldn’t work. If you use argument matchers, all the parameters need to have matchers, otherwise an InvalidUseOfMatchersException would be thrown. See “Warning on argument matchers” in Mockito docs. So in this case, the eq becomes mandatory again. This is really a Mockito pitfall. I think Mockito needs to work on this, or at least IDEs should provide better support. And perhaps it’s worth even another SonarQube rule. :slight_smile:

Cheers,
Björn

Indeed! And I should have actually run my test instead of just checking if it was compiling.

I’m going to write another rule tied with the one you just suggested to cover the case of using argument matchers on all arguments, or none, classified as a bug this time. You are right that anyway the test will fail if executed, but still, it will save some time for users in SonarLint at test writing time. (or if people are not executing their tests :face_with_raised_eyebrow: )

Cheers,
Michael

1 Like

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.