Moq4: Deep verification of call arguments

[kzu]

When mocking a service interface, I want to make assertions that a method on the interface was called with a given set of arguments. Currently Moq lets me call Verify on my mock to check, but will only perform equality comparisons on expected and actual arguments using Equals. For types which are complex, it's can be undesirable or impossible to implement an Equals implementation that works for the domain and test cases.

Instead, I'm having to Setup my Moq in a way which captures the arguments so I can make assertions on them after asserting that a call has been made:

Bar actualBar = null;
var service = new Mock<IService>();
service
    .Setup(x => x.Foo(It.IsAny<Bar>())
    .Callback<Bar>(b => actualBar = b);

// ...Run the test...

// Assert the service was called once.
service.Verify(x => x.Foo(It.IsAny<Bar>(), Times.Once);

// Assert the service was given the correct information.
actualBar.ShouldBeEquivalentTo(expectedBar);

Is there some way to get access to the recorded invocations other than using Verify? Could there be a way to extend Verify to perform more complex assertions and report on failures more clearly?

NOTE: moved from https://github.com/moq/moq/issues/9 by @Tragedian

[stakx]

@Tragedian:

Moq lets me call Verify on my mock to check, but will only perform equality comparisons on expected and actual arguments using Equals.

This is not correct. You can use any matcher(s) you want, including custom ones (such as It.Is<T>(arg => condition(arg))).

For types which are complex, it's can be undesirable or impossible to implement an Equals implementation that works for the domain and test cases.

Can you give a example? Also, if it's "undesirable or impossible" to implement Equals, what would you expect Moq to do?

Instead, I'm having to Setup my Moq in a way which captures the arguments so I can make assertions on them after asserting that a call has been made

No, setups are only required for strict mocks. For loose mocks (which are the default), you can skip Setup and just have Verify calls.

var service = new Mock<IService>();

// ...Run the test...

// Assert the service was called once.
service.Verify(x => x.Foo(It.Is<Bar>(actualBar => actualBar.ShouldBeEquivalentTo(expectedBar)), Times.Once);

(All of that being said... yes, a mock's internal Invocations collection could be exposed. But I'd like to wait with discussing this until I understand your issue better.)

[stakx]

(It just dawned on me that you're probably referring to the problem where verifying argument values with Verify comes too late because the argument's type is a reference type, and Moq does not actually capture the precise state of the reference type at the moment when an invocation is happening. If that's indeed what you're struggling with, please see https://github.com/moq/moq4/issues/531#issuecomment-346867155.)

[Tragedian]

Sorry if my scenario hasn't been made clear. In the example given, I have used Fluent Assertions to check the value of the captured arguments, in this case performing deep comparison of object graphs to determine the argument had the values expected. This throws an exception when the actual value doesn't match the expected values, explaining what parts of the object caused the comparison to fail:

var expected = new Bar
{
    Property1 = "Paul",
    Property2 = "Teather",
    Property3 = "Mr",
    Property4 = "[email protected]"
};

new Bar().ShouldBeEquivalentTo(expected);

Message: Expected member Property3 to be "Mr", but found <null>.
Expected member Property1 to be "Paul", but found <null>.
Expected member Property2 to be "Teather", but found <null>.
Expected member Property4 to be "[email protected]", but found <null>.

With configuration:

• Use declared types and members
• Compare enums by value
• Match member by name (or throw)
• Be strict about the order of items in byte arrays

Whilst Moq can be set up to use arbitrary conditions for matching arguments with It.Is<T> during verification, this generates errors which aren't particularly helpful in explaining why your expected call didn't happen:

var service = new Mock<IService>();

var expected = new Bar
{
    Property1 = "Paul",
    Property2 = "Teather",
    Property3 = "Mr",
    Property4 = "[email protected]"
};

service.Object.Foo(new Bar());

service.Verify(
    svc => svc.Foo(
        It.Is<Bar>(
            bar => bar.Property1 == expected.Property1
                && bar.Property2 == expected.Property2
                && bar.Property3 == expected.Property3
                && bar.Property4 == expected.Property4)));

Message: Moq.MockException :
Expected invocation on the mock at least once, but was never performed: svc => svc.Foo(It.Is(bar => ((bar.Property1 == "Paul" && bar.Property2 == "Teather") && bar.Property3 == "Mr") && bar.Property4 == "[email protected]"))
No setups configured.

Performed invocations:
IService.Foo(TestLibrary.Bar)

Given one of the simplest (and perhaps the most common) scenarios is to set up for a single call with some expected arguments, Moq doesn't really give a whole lot of support once you move beyond primitive types. My experience has been that most application require passing more complex DTO-like arguments. Naturally, this only really makes sense when you are expecting a single call, or you can otherwise narrow down to a specific expected sequence.

Whilst it would be nice if the Moq library could directly support this kind of argument verification, giving a method to more directly examine the performed calls would make this type of deep-examination scenario a lot simpler to delegate to other, assertion-specific libraries like Fluent Validation.

[Tragedian]

I feel like I want to write extension methods:

service.VerifySingle(svc => svc.Foo(expected));
service.VerifySequence(svc => svc.Foo(expected1), svc => svc.Foo(expected2));

But right now the information is internal, so I need to have some Setup calls to capture the arguments for myself.

[stakx]

@Tragedian, thanks for replying. If I understand you correctly, your issue is mostly about getting useful diagnostic messages. You're saying that Moq's verification error messages are less helpful than they could be, which becomes apparent when they're contrasted with Fluent Assertions' messages. I agree that there is definitely room for improvement here.

Exposing a mock's Invocations collection so that specialized assertions libraries can take over from there would be fairly easy to do.

Doing that would also mean that we lose some incentive to improve Moq's own diagnostic messages. Perhaps now would be a good opportunity to once more see what we can do about them. (Something similar has been previously discussed in #84.) The hard thing is either...

1. to find one diagnostic format that suits most people and the most frequent use cases. Moq's current reliance on Equals / ToString is essentially that, the lowest common denominator in .NET when it comes to comparing and formatting objects. Or,
2. to find some kind of generic extensibility model that allows people to swap error diagnostics according to their needs.

Option (2) is made more difficult by the fact that you don't always have a 1:1 relationship between an expected object and an actual object, like in your above example. You can have many invocations, so you need to somehow group them: Which invocations logically belong together? If grouped by the precise method called, you can then have multiple invocations and therefore multiple actual objects to be compared against... just one? An invoked method can also have multiple parameters. You'd need to consider all these things when producing a diagnostic message (and probably some more), so a message might easily get really long and far too detailed, which would again be unhelpful.

Perhaps I'm overthinking this. Do you have a specific suggestion on how to improve Moq's verification error messages? (Please take the discussion in #84 into consideration.)

[Tragedian]

My aspirations are twofold:

1. Clearer messages explaining what actually happened and why it didn't meet the test expectations.
2. Better support for a common verification scenario: a single call with complex arguments.

I think there's probably a lot of overlap in these things: you can make clearer error messages if you understand the scenario better, knowing more about the expectations, and adding support for more specific scenarios gives you that additional knowledge.

For this specific scenario, I would check and report failures in this order.

1. Was the method call at all? > Expected method Foo(Bar) to be called once, but no calls were performed.`
2. Was the method called more than once? > Expected method Foo(Bar) to be called once, but N calls were made.
3. Was the method called with the expected arguments, left-to-right, performing property-value based comparisons? > Expected methodFoo(Bar)` to be called once with a specified argument, but the argument does not match:
   > Expected member Property3 to be "Mr", but found .
   > Expected member Property1 to be "Paul", but found .
   > Expected member Property2 to be "Teather", but found .
   > Expected member Property4 to be "[email protected]", but found .

In short, what I want to see from my failing scenario is a message expressing where the expectations failed.

Unfortunately, there's no getting away from the points raised by the discussion of #84: there is no one-size-fits-all solution. I'm going to keep referring to Fluent Assertions (because they really do seem to have a firm grasp of what's really involved in scenario-based testing) where their model uses a configuration object to customise how the comparison of complex types is made. They already deal with the pain of walking through an object graph and dealing with the dangers of cyclic references, etc, and give you control to exclude/include properties, whether ordering matters in collections and other nuanced details of object comparisons.

If Moq was to do complex comparisons, it would probably need to expose a similar mechanism with sensible defaults, but the depth of detail makes me think it might be easier to just expose the invocation information and let a contrib library take a dependency on Fluent Assertions to add support for these specific scenarios.

[Tragedian]

So, assuming the right path is to open Moq to allow for "custom" verification by directly interacting with the invocation, what would that API look like? Verify(Action<Invocation>) ?

[stakx]

@Tragedian - the most straightforward thing I can think of is simply making the Mock.Invocations collection publicly accessible in a read-only manner.

[Tragedian]

Both strategies then raise the question: how much of the Invocation type should be made public?

[stakx]

The current type of Mock.Invocations (InvocationCollection) should not be made publicly visible in its current form. At the moment, it's a collection of very specific methods that synchronize access to an underlying List<Invocation>, but the type doesn't even implement IEnumerable<>.

how much of the Invocation type should be made public?

I haven't thought about it in detail, but the publicly visible Mock.Invocations would ideally appear to be a IReadOnlyList<IInvocation>, where the interface type IInvocation defines two properties MethodInfo Method { get; } and IReadOnlyList<object> Arguments { get; }. (Note that Moq doesn't currently record return values.)

Some technical difficulties in making Mock.Invocations public will be:

• Deciding whether to hide the actual types behind an interface, or whether to just make the actual types (Invocation, InvocationCollection) public but change some mebers' accessibility to internal. I think it would be better to expose internal types only through interfaces.

• Overloading the Mock.Invocations such that Moq's internals see the actual InvocationCollection type with all its specific methods, while the public property appears as a IEnumerable<> or IReadOnlyList<>. Overloading a property based on accessibility isn't actually possible (except through explicit interface implementation, but that's not an option), so we might have to juggle some things around.

• Thread-safety: Should user code receive a reference to the actual invocations collection, or a snapshot / copy of the actual invocations, whenever Mock.Invocations is queried? (The latter would have the advantage that the returned collection doesn't have to be synchronized.)

[Tragedian]

Creating an IInvocation interface may be overkill; the current class is already an abstract base with very little implementation. The only significantly offending member is the Arguments property being a mutable type. The Return methods could be marked internal and the Arguments property changed to IReadOnlyList<object>, and the type should be a public-safe representation.

Refactoring the internal Invocations collection property name is a fine idea; it shouldn't cause problems, unless the renaming tools miss something and exposing a new public IReadOnlyList<T> Invocations property is definitely preferable over working with the existing type.

Looking at the existing thread-safety code, there doesn't seem to be a way to get access to anything other than a snapshot of the current invocation collection. I don't think there's any issue continuing to use this strategy, though might be best to change the Invocation[] ToArray() call to IReadOnlyList<Invocation> GetSnapshot().

[Tragedian]

Additionally, should we be looking at marking an invocation as verified?

[stakx]

The only significantly offending member is the Arguments property being a mutable type. The Return methods could be marked internal and the Arguments property changed to IReadOnlyList<object>, and the type should be a public-safe representation.

Arguments needs to be mutable because of ref and out parameters. The contract defined by Invocation is that the Return methods should ensure that these get properly written back for the calling code. (Btw., a Throw finalization method is currently still missing.)

I think it would be better in this case to hide Invocation behind a public interface, so that we'll keep the freedom of refactoring the implementation type in the future without breaking user code. Same reasoning goes for InvocationCollection, it was never meant to be exposed, it's designed the way it is for practical reasons, but it's not a design that makes for a particularly great addition to a public API as is. When just publishing InvocationCollection in the public API I'd be especially concerned about having to be careful which interfaces it implements.

Perhaps it's best to think about redesign InvocationCollection first to a cleaner, more solid design that adheres to the usual .NET collection patterns better; perhaps then it would be ready to be exposed without an additional interface.

Additionally, should we be looking at marking an invocation as verified?

No, that should stay internal for now. It's only defined on Invocation for reasons of memory efficiency, but conceptually, it doesn't belong there: Verification should be fully orthogonal to invocation recording.

[Tragedian]

I took a stab at trying to implement this: #569

Notably, I did make the Invocation type public whilst maintaining its existing mutable array collection, which differs from the previous comment's suggestion. Looking for feedback.

[stakx]

@Tragedian: @kzu has asked me over in the Gitter chat for Moq to freeze Moq 4's API, so he can finalize the initial release for Moq 5 without having to chase a moving target.

This request comes at a somewhat awkward time regarding your PR (#569) because it would effect an API change and is still open (due to me taking longer than usual in reviewing). I mentioned this to @kzu, and he was suggesting that you migrate to Moq 5, which offers much better introspection into a mock's state and already includes the possibility to look at all invocations that have occurred on a mock.

I cannot judge whether migration to Moq 5 would actually be feasible for you, since I don't know the exact release date for Moq 5, nor whether it will be sufficiently feature-complete to cover your usage scenarios. Therefore I'd like to invite you to join Moq's Gitter chat so we can discuss your PR with @kzu.

[stakx]

@Tragedian, you've stated in your PR that you're going to focus on Moq 5 instead. So I hope you don't mind if I close this issue as well (but I'll tag it as "unresolved").

[Tragedian]

The resolution seems to be "wait for Moq 5". Closing is fair and I should have done so myself (but forgot about the Issue entirely).

[Tragedian]

New PR: https://github.com/moq/moq4/pull/627

[stakx]

@Tragedian - I've just published Moq v4.9.0 on NuGet.

[H-a-w-k]

One thing that works is

Func<Bar, bool> validate = actualBar => {
  actualBar.Should().BeEquivalentTo(expectedBar);
  return true;
}

var service = new Mock<IService>();
service.Setup(x => x.Foo(It.IsAny<Bar>());
service.Verify(x => x.Foo(It.IsAny<Bar>(z=>validate(z)), Times.Once);

[CraigComeOnThisNameCantBeTaken]

One thing that works is

Func<Bar, bool> validate = actualBar => {
  actualBar.Should().BeEquivalentTo(expectedBar);
  return true;
}

var service = new Mock<IService>();
service.Setup(x => x.Foo(It.IsAny<Bar>());
service.Verify(x => x.Foo(It.IsAny<Bar>(z=>validate(z)), Times.Once);

thanks to @H-a-w-k .

I wrote this to improve reusability a little:

    public static class ItExt
    {
        public static T IsDeep<T>(T expected)
        {
            Func<T, bool> validate = actual =>
            {
                actual.Should().BeEquivalentTo(expected);
                return true;
            };
            return Match.Create<T>(s => validate(s));
        }
    }

Usage:

       .Verify(x => x.myMethod(ItExt.IsDeep(expected)), Times.Once);