Expose Mock.Setups, part 3: Individual setup verification
#987
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
changed the title
Mock.Setups, part 3: Individual setup verificationMock.Setups, part 3: Individual setup verification (WIP)
stakx
commented
Apr 1, 2020
Comment on lines
+194
to
+197
| foreach (Invocation invocation in this.mock.MutableInvocations) | ||
| { | ||
| invocation.MarkAsVerifiedIfMatchedBy(setup => setup == this); | ||
| } |
There was a problem hiding this comment.
Should this only happen if verification actually succeeds? Same question goes for the corresponding loop in Mock.Verify.
There was a problem hiding this comment.
While the current logic isn't perfect, it's generally good enough: if verification fails, an exception gets thrown anyway; it's unlikely someone catches that exception (Moq documentation explicitly recommends not doing that!) and then goes on to inspect the mock's state.
There may possibly be some odd corner cases regarding the verification status of invocations on a setup's inner mock; let's deal with those when they become an actual problem.
Despite the same name, they differ from the `Verify[All]` methods of
`Mock` mainly in one aspect: The setup on which the method is invoked
will always be verified, regardless of whether or not it is flagged
verifiable.
That is, `setup.Verify()` will verify `setup` even if it has not been
marked `.Verifiable()`. It wouldn't make sense for `setup.Verify()` to
be a no-op; otherwise, why would one make the call in the first place?
These new method signatures have the added benefit of allowing us to
later introduce (both on `ISetup` and `Mock`) an additional overload:
void Verify(bool recursive, Func<ISetup, bool> predicate)
which will allow custom setup selection without conflicting with any
existing `Verify` methods.
The exact location where this `foreach` loop gets added is important: The simplest solution with the fewest lines of code would be to mark matching invocations inside `Setup.TryVerify`, and to remove the corr- esponding loop from `Mock.TryVerify`. However, since `Setup.TryVerify` is called by `Mock.TryVerify` for each setup, that would mean iterating over invocations once per setup. It is much more efficient to iterate once per mock in `Setup.TryVerify`. This is why we are adding the new `foreach` loop in `Setup.Verify`, so it only executes when we enter verification at this setup (and so "by- passed" the batch logic in `Mock`).
stakx
changed the title
Mock.Setups, part 3: Individual setup verification (WIP)Mock.Setups, part 3: Individual setup verification
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 is a follow-up to #985.)
This adds
void Verify(bool recursive = true)andvoid VerifyAll()methods toISetup.