Make VerifyNoOtherCalls take into account previous calls to parameterless Verify() and VerifyAll()
#659
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stakx
commented
Aug 19, 2018
| Invoked = 0, | ||
| InvokedAndMatchedBySetup, | ||
| InvokedAndMatchedByVerifiableSetup, | ||
| Verified, |
There was a problem hiding this comment.
Note that the logic implemented in the new Invocation methods ensures that we only ever progress from Invoked towards Verified, but it should never be possible to regress back to an earlier flag.
| @@ -252,6 +252,11 @@ public void Verify() | |||
|
|
|||
| private bool TryVerify(out MockException error) | |||
| { | |||
| foreach (Invocation invocation in this.MutableInvocations) | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| @@ -282,6 +287,11 @@ public void VerifyAll() | |||
|
|
|||
| private bool TryVerifyAll(out MockException error) | |||
| { | |||
| foreach (Invocation invocation in this.MutableInvocations) | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| } | ||
|
|
||
| [Fact] | ||
| public void VerifyNoOtherCalls_works_together_with_parameterless_Verify() |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
stakx
force-pushed
the
verifynoothercalls
branch
from
70c90b4
to
c385d53
Compare
|
@uecasm, may I ask you to briefly review this? I'd be glad if you could take a look at the tests I added in the first commit. Do we need to test anything else? |
stakx
force-pushed
the
verifynoothercalls
branch
from
c385d53
to
cb5babc
Compare
stakx
changed the title
VerifyNoOtherCalls play along nicely with parameterless Verify() and VerifyAll()VerifyNoOtherCalls take into account previous calls to parameterless Verify() and VerifyAll()
stakx
force-pushed
the
verifynoothercalls
branch
from
cb5babc
to
c089cbf
Compare
Add two tests that demonstrate how `VerifyNoOtherCalls` ought to interact with both parameterless and parameterized `Verify` methods. Currently, `VerifyNoOtherCalls` only works together with parameterized `Verify`, but not with the parameterless variant (which internally is actually very different as it targets setups, not invocations). These tests are taken from: devlooped#607
In order to get `VerifyNoOtherCalls` to interact properly with param- eterless `Verify[All]()`, we need to check, at verification time, for all invocations whether they were matched by a (verifiable) setup. If we have M setups and N invocations, that would be up to M*N checks. We can do something more efficient. Moq already checks upon each invocation whether there is a setup that matches it. So why not cache the result of that check with the invoc- ation? That allows us to do a linear (N) scan during verification and flag all setups as verified that were previously matched. In order to support both `Verify()` and `VerifyAll()`, we need to distinguish between verifiable setups and regular setups--that's why the `verified` flag in `Invocation` becomes an enum with that many options.
stakx
force-pushed
the
verifynoothercalls
branch
from
c089cbf
to
f065570
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This closes #607. See commit messages for details.
/cc @uecasm