Skip to content
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

Arguments in recursive expressions should not be treated like It.IsAny by default #780

Merged
merged 6 commits into from
Mar 9, 2019

Conversation

stakx
Copy link
Contributor

@stakx stakx commented Mar 9, 2019

This improves the accuracy with which Moq matches invocations against recursive / fluent verification expressions when using mock.Verify(expression). Error messages likewise contain more precise & more complete information about which invocations occurred, and on what (inner) mocks.

Here's an example of what call verification error message will look like when this gets merged:

public interface IX
{
    void Do();
    void Do(int arg);

    IX Inner { get; }
    IX GetInner(int arg);
}

var mock = new Mock<IX> { DefaultValue = DefaultValue.Mock };

mock.Object.GetInner(1).Do(1);
mock.Object.GetInner(1).Do(2);

var innerTwo = mock.Object.GetInner(2);
innerTwo.Do();
innerTwo.Do(2);

mock.Object.GetInner(3);

mock.Verify(m => m.GetInner(1).Do(It.IsAny<int>()), Times.Exactly(3));
Expected invocation on the mock once, but was 2 times: m => m.GetInner(It.IsAny<int>()).Do(2)
Performed invocations:

   Mock<IX:00000001> (m):

      IX.GetInner(1)  => Mock<IX:00000002>
      IX.GetInner(1)  => Mock<IX:00000002>
      IX.GetInner(2)  => Mock<IX:00000003>
      IX.GetInner(3)  => Mock<IX:00000004>

   Mock<IX:00000002>:

      IX.Do(1)
      IX.Do(2)

   Mock<IX:00000003>:

      IX.Do()
      IX.Do(2)

   Mock<IX:00000004>:
   No invocations performed.

In verification expressions of the kind `m => m.X(args).Y`, each arg-
ument in `args` gets treated by `Verify` as if it were `It.IsAny<T>`.
Instead of just taking *any* inner mock setup that happens to be for
the same method, try to pick one where argument expressions match too.
Now that we're more picky when choosing inner mock setups, we should
also be more accurate when adding them. Otherwise, the only matches
we'll ever get will be if all arguments use `It.IsAny<TArg>`, as that
is the kind of inner mock setup generated by `DefaultValue.Mock`.

This commit makes the tests pass that originally failed (_1 and _2),
but it now breaks those using `It.IsAny` for verification (_3).

This is an important breakage as it demonstrates a fundamental problem
in the current recursive verification approach: In a verification call
for `m => m.X.Y`, the inner mock `Y` to proceed to from `X` should not
be chosen from `X`'s setups, but from the return values of all invoc-
ations matching `X` (and there may be several different ones)!
Recorded invocations' return values are used to discover a full mocked
object graph. The split verification expression can then be distribu-
ted over those objects, and each one reports the number of matching
invocations, which are then summed up.

We also need to deal with error messages, too, which contain informa-
tion about all actual invocations. We show all invocations of all dis-
covered mocks in a flattened list, grouped by the mock on which they
occurred, sorted in the order in which those mocks were discovered
(i.e. by their approximate distance from the root object).
@stakx stakx added this to the 4.11.0 milestone Mar 9, 2019
@stakx stakx merged commit 5ba005c into devlooped:master Mar 9, 2019
@stakx stakx deleted the recursive-call-verification branch March 9, 2019 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant