verify(someMethod).called(0) fails with unintuitive error. about mockito

[srawlins]

I'd say that's a bug.

[srawlins]

Ack, unfortunately, verify(...).called(0);would require a breaking change ☹️ .

When you call verify or verifyNever, we don't require any further methods to be called on the resulting object (a _VerifyCall). This is common with verifyNever(...);, but not very common with verify(...);, which basically (as you have learned the hard way), expects that at least one invocation has been logged. Users may depend on this behavior today.

In further detail: let's say you call verify(something).called(0);. While executing the verify(...) bit, we wish we could see that in the future, called() will be called, but we can't see the future. Also, when verify sees that no invocations have been logged, it calls package:test's fail method, which immediately throws. There's no way to reach that called() call, without changing the current behavior.

Now, I'm all for changing the behavior 😃 , as I don't think many people rely on verify(...);, but I'll discuss with a few folks on the versioning, etc. In the mean time, I'll clean up the messaging, as you suggest.

[srawlins]

@jonahwilliams how about the following breaking change for Mockito 3:

• Calling verify(a); does not verify anything about the call count of a.
• Calling verify(a); followed, at some point, by a call to when or verifyX throws an error, stating that a previous verify call was made without a subsequent call to called(). (But if verify(a); is the last statement in a test, it just silently does nothing; I don't think the test framework provides us with any APIs to register hooks at the end of a test. [1])
• The argument to called can become optional, so that verify(a).called() just sort of, verifies that a was called more than 0 times. This provides the same functionality as the old verify(a); (which was never documented), but I think is easier to read for maintainability; someone unfamiliar would not be expected to guess what verify(a); does, but could make an educated guess at what verify(a).called(); does.

[1] The single downside to this change that I can see is that verify(a); becomes just a silent no-op, which might be surprising. We could be more heavy-handed and register a expectAsync0 when verify is called, and then call the expected callback in called(), but this seems heavy handed...

[jonahwilliams]

If verify(a) does nothing by itself, why not combine it with the expectation? You could arrange the naming so that it corresponds with the current state. For example,

verify(a, called: 1) or verify.called(a)/verify.never(a)

That said, I think the proposed changes are perfectly good. We don't throw an error if a user writes a test case without an expect, so I wouldn't expect verify to have it's own error prevention here.

[matanlurey]

@srawlins Is this going to happen for Mockito 3? If not, do you either want to:

• Milestone it for Mockito 4
• Close the issue as not planned

[srawlins]

I found a case where verify(someMethod) is called without .called(n):

var verification = verify(someMethod);
expect(verification.captured[0], equals('hey'));
expect(verification.captured[1], startsWith('hi ho'));

So I don't think we can make the changes I proposed, except I'd like to still make the argument to called() be optional:

the argument to called can become optional, so that verify(a).called() just sort of, verifies that a was called more than 0 times.