-
-
Notifications
You must be signed in to change notification settings - Fork 794
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
Verify that no unverified methods were called (alternative to Strict) #527
Comments
Can you please elaborate a little? What expectations (say: setups) do you find acceptable in the "arrange" part, and which do you want to avoid? You also mention |
Use -- setups that arrange things -- provide meaningful behavior or state related to the test in question. E.g. "set up the repository to return a specific User object from Get(id)", or "set up file Save to throw a specific exception". Avoid -- setups that are expectations -- do nothing, but prepare for the future
Sorry for the confusion, I forgot that |
@ashmind - I like this idea quite a lot. I have two questions: 1. Would this feature take Moq in the right direction?Is it wise to introduce an alternative to Those are questions about the direction where we want to take Moq. I personally think it would be better to have a slim, opinionated API, but perhaps in reality, Moq is more attractive if it's unopinionated and caters for different tastes, even if that makes the API surface more complex and somewhat "messier". Ideally, the decision would be made by the user community. 2. How would this feature be implemented?Let's take a quick look at the two syntaxes that you've proposed above:
For this syntax to work, verification would have to become an operation that changes state. Specifically, each For that reason, it's not perfect from a design standpoint, but might work just fine in practice. Implementation should be perfectly feasible, and probably even fairly easy. (If it's decided that this feature should be implemented, I'm happy to offer some guidance, if desired.)
With this approach, verification could remain an operation that doesn't change state. On the other hand, I believe it's not a good fit for Moq's API because one of the explicit design goals of Moq is to have a statically, strongly-typed API. Like the I'd be interested to hear what others think of this proposed feature. |
i think this does mean a strict mock should be obsolete. In addition to this, one thing i have never really liked with a strict mock is the way that something fails during the test execution as it's not stubbed/setup rather than as part of verification as it was called when it shouldn't have been. I personally would prefer the "VerifyNoOtherCalls" approach as a mocked interface whereby you expect a few things to be called would make the verifyany with never a bit cumbersome. |
@ashmind, @BlythMeister - I got curious, so I implemented a quick and dirty version of this feature: see develop...stakx:verify-no-other-calls. Example: public interface IFoo
{
void X(int n);
void Y();
}
[Fact]
public void Can_Verify_any_calls_then_ensure_no_other_calls_were_made_with_VerifyNoOtherCalls()
{
var mock = new Mock<IFoo>();
mock.Object.X(1);
mock.Object.X(2);
mock.Object.X(3);
mock.Object.Y();
mock.Verify(m => m.X(AnyEvenInt));
mock.Verify(m => m.Y());
mock.VerifyNoOtherCalls();
}
static int AnyEvenInt => Match.Create<int>(x => x % 2 == 0);
This would obviously need a little cleaning up and some optimizations. But is this what you had in mind? |
Yes with 1 little tweak. |
@BlythMeister - that would work just as well: ... // (mock instantiation and invocations as above)
mock.Verify(m => m.X(AnyIntLessThan(3)), Times.Exactly(2));
mock.Verify(m => m.Y());
mock.VerifyNoOtherCalls();
}
static int AnyIntLessThan(int n) => Match.Create<int>(x => x < n);
|
I assumed it would, but wanted to be sure. This feature is the one which will make me spend the time converting my test suite from Rhino to Moq 😁 I prepared to accept having to "Setup" and then verify times on returns, but not having to setup voids and still get the benefits of a strict mock makes me a very happy man. |
This makes me feel warm inside |
Let's do this. A |
Probably closed this a little too early. The current implementation probably doesn't yet work correctly...
Need to make sure the above are properly covered. |
@ashmind, @BlythMeister, @willsmith9182 - I just published a pre-release version |
@stakx i know i'm probably pushing it...but how about VerifyNoOtherCalls on a mock repository as well :) I've been creating all my mocks off a mock repository and then calling repo.VerifyAll() |
@BlythMeister - Good question. I see the similarity to If your repository mocks were invoked, however, you'll have to |
as an example, this is what i do
In the loose test, i am basically saying of all the mocks created, i only want the mocks i have already verified to be called. So, if my code called the "DoStuff" method twice, i want it to fail. Essentially, the behaviour to mirror a strict mock factory, but without the need to "Setup" on voids. |
Why not simply
There are no other mocks in your example. Why would you even instantiate mock objects that you don't need? Unfortunately, your example doesn't really represent a realistic use case for why |
agree, this is a very simplistic example Calling Being able to state that i want to call it for every mock that the repository has created would be really useful in my opinion. As an alternative, exposing a way that you can run any delegate against every mock the factory created would mean the user has full freedom. |
Fair enough, I guess this comes down to personal preferences in how you write your tests. I was assuming that in each test, you would only instantiate the mocks that you're actually going to need—therefore there would be no superfluous I would prefer if we left |
Ok, i understand. With regard to only creating mocks you need, it is more to satisfy the dependency being injected. |
Your reasoning is similar to the OP's of #390, who requested But in the end, that is probably also just a personal preference. |
Having just finished converting from rhino to Moq using strict factory I'm used to having a setup on void and a verify with times. Arguably, it's verbose. Doing it that way also makes you think more about the system under test and what it is doing, so creating lots of mocks but only using 1 probably means your class is too big or has too many responsibilities. My only reasoning to add to the factory was because verify and verifyall were already there, so seemed like a logical extension on that. 🙂 |
The reason why it behaves that way is that when an invocation is unexpected, you typically want to know right there and then, with a full stacktrace of the actual invocation so you can see (and fix) the test (or the code) as to why the invocation was performed. This typically requires inspecting the call stack and local variables since it's clearly an invocation you didn't expect. I believe that no matter how nice and verbose the failed Verify makes it look, it would be very hard to diagnose why a call you didn't expect ended up happening anyway, without debugging and actually stopping at the offending call. Which is way easier to do if you just break on all exceptions and let the debugger take you there by just executing the test with a debugger attached ;) |
My goal in few specific tests is to verify that no unverified methods were called, for example:
I know I can do this with
Strict
, but I don't like howStrict
affects the test structure and moves some expectations into the "Arrange" part of the test (Setup().Verifiable()
). It seems likeVerify(call)
is designed to provide a nicer alternative toStrict
, so it would be nice to have that as well.Alternatively, some kind of method matcher in
Verify
would work as well -- e.g.mailSender.VerifyAny(m => m.Name != nameof(SendSpecificThing), Times.Never);
The text was updated successfully, but these errors were encountered: