devlooped / moq

The most popular and friendly mocking framework for .NET
Other
5.88k stars 801 forks source link

Invocation count incorrect when argument is a class-type and multiple calls are made with the same object reference #861

Closed mmccord-mdbuyline closed 5 years ago

mmccord-mdbuyline commented 5 years ago

Assume the following:

runnerMock.Setup(r => r.Execute(It.Is<LiquibaseOptions>(l => l.Equals(tagOptions))))
                .Returns(Task.FromResult(true));

runnerMock.Setup(r => r.Execute(It.Is<LiquibaseOptions>(l => l.Equals(updateOptions))))
                .Returns(Task.FromResult(true));

LiquibaseStartupTask task = new LiquibaseStartupTask(runnerMock.Object,
                Mock.Of<IOptions<LiquibaseStartupTaskOptions>>(o => o.Value == options),
                Mock.Of<ILogger<LiquibaseStartupTask>>());

await task.Execute();

runnerMock.Verify(r => r.Execute(It.Is<LiquibaseOptions>(l => l.Equals(tagOptions))), Times.Once());

Where tagOptions and updateOptions are of the same class-type with value-type semantics (overriden Equals, ==, !=...implements IEquatable). As is suggested by the above, code, the call to task.Execute() is expected to call Execute twice on the runnerMock. The first call should call the runnerMock's Execute with an argument equivalent to tagOptions and the second call should call runnerMock's Execute with an argument equivalent to updateOptions. Note that tagOptions and updateOptions do not refer to the same object reference. However, within the task.Execute method the same "options" object is re-used for both calls to the runner Execute() method with some properties changed in the interim. The code in task.Execute() looks something like this:

LiquibaseOptions options = new LiquibaseOptions();

options.MyCoolProperty = "initial value";

runner.Execute(options);

options.MyCoolProperty = "new value";

runner.Execute(options);

While the setups work correctly the invocation count at the Verify step is incorrect at 0 when it should be 1. If I alter the last line of my test code to be the following then the verification passes even though there is only one such call:

runnerMock.Verify(r => r.Execute(It.Is<LiquibaseOptions>(l => l.Equals(updateOptions))), Times.Exactly(2));

I believe this observed behavior is due to my re-use of the options object between calls to the runnerMock's Execute and the subsequent re-evaluation of the argument stored in Moq.Invocation to determine a "match" for verification. The fact that the "options" being passed as an argument is a class-type means that the argument reference stored in both Moq.Invocation objects point to the same object reference. This means that both Moq.Invocation objects appear to match the updateOptions call, hence the invalid invocation counts.

stakx commented 5 years ago

I believe this observed behavior is due to my re-use of the options object between calls to the runnerMock's Execute and the subsequent re-evaluation of the argument stored in Moq.Invocation to determine a "match" for verification. The fact that the "options" being passed as an argument is a class-type means that the argument reference stored in both Moq.Invocation objects point to the same object reference. This means that both Moq.Invocation objects appear to match the updateOptions call, hence the invalid invocation counts.

Yes, this is precisely it. Moq doesn't take deep-cloned snapshots of all arguments at the time of the invocation; instead, it simply stores the arguments as they were passed to the invoked method, i.e. it copies their values of value-typed arguments, and it copies the reference to reference-typed arguments.

Here's a few alternatives you can try:

P.S.: It would've been easier if you had posted a MCVE (minimal, complete, verifiable code example) instead of parts in code, parts in prose. Piecing together a runnable code example that way takes much more time on our side than is really necessary.

mmccord-mdbuyline commented 5 years ago

@stakx -- Fair enough, but would it not be possible to count invocations at execution time, and then read the count during verify? Seems like if you do that then cloning the arguments is a non-issue? I think that would also work for the VerifyAll mechanisms as well, assuming I have an understanding of how those work. Is that something that might be open for discussion?

stakx commented 5 years ago

would it not be possible to count invocations at execution time, and then read the count during verify?

No, that wouldn't be easily possible, because the invocation count would be kept in the various setups; however, a call to mock.Verify(callExpression) need not map to any setup(s) at all—it goes directly against the list of recorded invocations.

mock.Setup(m => m.Frobble(It.Is(n => n < 5)));
mock.Setup(m => m.Frobble(It.Is(n => n >= 5));
mock.Object.Frobble(...);
...
mock.Verify(m => m.Frobble(It.Is(n => n % 2 == 0)), Times...);

Where are you going to get the count from for this and other, possibly even more involved examples? In order to do that, you'd have to be able to "intersect" matchers, and that's quite probably not possible in general.

stakx commented 5 years ago

P.S.: I guess what we could do is something like a new form of verification:

var setup = mock.Setup(...);
...
Mock.Verify(setup, Times....);

I'm not a big fan of saving setups in variables; I'm not sure whether that goes well together with the fluent API.

(That being said, it probably would solve this long-standing issie that people keep asking about.)

mmccord-mdbuyline commented 5 years ago

Humm, well I had something else in mind but it's entirely probable I just don't understand the inner-workings of Moq enough to make a clear case. If I get a chance I'll mess around with Moq's inner workings some more and get back to you with some more concrete ideas. Thanks for the quick responses!