-
-
Notifications
You must be signed in to change notification settings - Fork 802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Can specify Times a verifiable expectation must be met #401
Conversation
…invoked to pass mock Verify Defaults to AtLeastOnce to match current functionality Included test for mutating reference argument to demonstrate
This will probably have to wait a while until we have an answer in #373. Thanks for being patient! :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very promising to me, thanks for sharing this!
One aspect that might benefit from some more discussion is whether .Verifiable
is the best place for specifying a Times
. It's not unreasonable; I'm just wondering if it's generally obvious / discoverable / intuitive enough. I'd be interested to hear your thoughts on whether you've considered (a) adding Times
as an additional parameter on the various .Setup
methods; or (b) adding named methods to IOccurrence
and then de-obsolesce that interface; or (c) any other options I'm not thinking of right now. :) (I'd say let's discuss that bit over in #373.)
I'd like to request one simple change:
- Could you please rebase this one commit on top of the
develop
branch of Moq's repo? The "merge develop" commits shouldn't end up in thedevelop
branch. :)
I'd generally be OK with accepting this feature, but would prefer to leave the final decision up to @kzu whether anything like this gets merged at all, as mentioned in #373.
…invoked to pass mock Verify Defaults to AtLeastOnce to match current functionality Included test for mutating reference argument to demonstrate
Defaults to
AtLeastOnce
to match current functionality.We had an issue testing a method modified properties within a reference type shared to a dependency. The current "Verify" checks can only compare against the current reference type state (see new
CannotPostVerifyCallsByExpressionForModifiedReferenceTypeArgument
fact).The approach that we came up with was to support the "Times" concept directly in the "Verifiable" setup.
Given that the change is non-breaking by default and pretty clean, I thought it worth pushing back.
Full example in
CanPreVerifyExactCallsByExpressionForModifiedReferenceTypeArgument
fact, but in short: