mockito
mockito copied to clipboard
verify(someMethod).called(0) fails with unintuitive error.
I initially thought that verify(something).called(0) would be equivalent to verifyNever. Instead the first fails with a no matching calls error. I am not sure if this is a bug or not, but if called doesn't support values <= 0 then it should throw a better error, perhaps pointing a user to verifyNever.
Minimal example:
import 'package:test/test.dart';
import 'package:mockito/mockito.dart';
void main() {
group('verify calls', () {
test('verifyNever passes', () {
var cat = new MockCat();
verifyNever(cat.countLives());
});
test('verify .called(0) fails', () {
var cat = new MockCat();
verify(cat.countLives()).called(0);
});
});
}
class Cat {
int countLives() => 9;
}
class MockCat extends Mock implements Cat {}
I'd say that's a bug.
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.
@jonahwilliams how about the following breaking change for Mockito 3:
- Calling
verify(a);does not verify anything about the call count ofa. - Calling
verify(a);followed, at some point, by a call towhenorverifyXthrows an error, stating that a previousverifycall was made without a subsequent call tocalled(). (But ifverify(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
calledcan become optional, so thatverify(a).called()just sort of, verifies thatawas called more than 0 times. This provides the same functionality as the oldverify(a);(which was never documented), but I think is easier to read for maintainability; someone unfamiliar would not be expected to guess whatverify(a);does, but could make an educated guess at whatverify(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...
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.
@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
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.