Provide a way to opt out of VerifyAll()

[ashmind]

My goal is similar to #747, so I am using VerifyAll() to detect unused setups. However there are a few very common setups done in common helpers that I don't want to detect.

It would be great to have some way to opt-out of VerifyAll in specific cases, e.g. by using Verifiable(false).

[stakx]

@ashmind, suppose there were a new setup.Verifiable(bool) method overload where:

• .Verifiable(true) is equivalent to today's .Verifiable(), and
• .Verifiable(false) is a no-op, i.e. equivalent to omitting .Verifiable() altogether.

Would that be sufficient for you to be able to parameterize your helpers? You'd then use mock.Verify() instead of mock.VerifyAll() and therefore test only setups marked as setup.Verifiable([true]), but exclude those setup[.Verifiable(false)].

[ashmind]

Would this require me to opt-in with Verifiable(true) on most setups?
I'll elaborate on my situation a bit.

My Project Scenario 1

(most common, any developer in the project)

public void SomeTest() {
     GetMock<X1>().Setup(...);
     GetMock<X2>().Setup(...);

     // Act
     // Assert
}

No Verifiable anywhere (and developer might not even know it exists). Since "ambient" mocks are retrieved from test scope, base test class can call VerifyAll() on those mocks in after-test method, to ensure there are no unnecessary setups unused by the test.

Most of the time developers might not even be aware base class does that.

My Project Scenario 2

(rare, only a few most widely used shared helpers)

public void SomeSetupHelper() {
     GetMock<X1>().Setup(...).Verifiable(false);
     GetMock<X2>().Setup(...).Verifiable(false);
}

Can explicitly opt out of verification -- not perfect, but makes helper simpler and it's too widely used to guarantee both setups will be used (would be cool to guarantee at least one was used, but well).

There are only a few, and the authors of those will be more experienced developers, aware of Verifiable(false) if added.

[stakx]

Would this require me to opt-in with Verifiable(true) on most setups?
I'll elaborate on my situation a bit. [...]

Yes, and based on your two scenarios, I can see how this wouldn't help a lot. My immediate thoughts:

• Are (some of) your tests possibly too complicated / trying to do too much, if some developers aren't aware of all that's going on? Might that possibly be the problem that needs to be solved by refactoring your test suite, instead of extending Moq's API? (You're in the better position to judge whether that's the case or not.)

• If we did what you suggest, i.e. .Verifiable(false) leading to these setups being excluded from .VerifyAll(), we'll end up with an even messier API situation than we already have with Verifiable(): You can choose which setups to .Verify() by marking them with .Verifiable(), and you can choose which setups not to .VerifyAll() by marking them with .Verifiable(false). People already don't understand the difference between .Verify() and .VerifyAll() too well, and we'll make the situation even more opaque. To me, that's most visible in the fact that .VerifyAll() will no longer do what its name claims. To me, that's a pretty serious API design flaw.

Any ideas?

[ashmind]

For the refactoring, it's a fair point, but the tests themselves are actually quite nice, even if infrastructure is a bit complex in places. Not unsimilar to Moq/xUnit, tbh, some complexity inside but most people just get the nice API.

Completely agree that changing VerifyAll might be a mess.

I'll try to think about some alternatives.

One immediate option: what if the framework provided enough information for me to do the verification myself? E.g. add something like .Tag("arbitrary") to Setup and also expose lists of Setups on the mock with the following (at minimum): interface ISetup { void Verify(); IReadOnlyList<string>Tags { get; } } -- then I can iterate the setups, pick ones I want to verify, and verify them.

Pros:

• Allows users to implement complex verification scenarios without effort from the framework
• Allows me to check that at least one method from each helper was called (if none were called, helper was not useful)

Cons:

• Leads to custom implementations instead of suggesting a standard way
• Adds complexity to the framework API that might only get used by a few people

I'll think about some other ideas.

[stakx]

To follow up on your idea above:

I'd very much like to see the Mock class to expose its .Setups collection, like we already did .Invocations (and IIRC that has already happened in Moq 5). Working out which parts of a setup to expose is a bit of work that has yet to happen, as the setup internals (MethodCall & its related type hierarchy) are bit of a mess TBH.

Let me check with Moq 5 whether it has anything like the .Tag(...) you suggest, too, or something that could be used toward the same end. (I suspect Moq 5 might allow you to simply store the setup in a variable for later reference.)

[stakx]

@ashmind, I've done some quick prototyping.

The following additions to Moq's API would allow you to discover all of a mock's setups and verify them.

namespace Moq
{
    partial class Mock
    {
        /// <summary>
        ///   Gets a list of live (i.e. non-overridden) setups on this mock.
        ///   Setups are returned in the opposite order in which they were added to the mock.
        /// </summary>
        public ISetupList Setups => ...;    
    }

    /// <summary>
    ///   A list of setups on a mock.
    /// </summary>
    public interface ISetupList : IEnumerable<ISetup>
    {
    }

    /// <summary>
    ///   Describes a setup on a mock.
    /// </summary>
    public interface ISetup
    {
        /// <summary>
        ///   Gets the expression that was passed to <c>.Setup()</c>.
        ///   Recursive setup expressions such as <c>mock => mock.A().B().C() are split up
        ///   and distributed over several mocks; see <see cref="ReturnsMockObject"/>.
        /// </summary>
        LambdaExpression Expression { get; }

        /// <summary>
        ///   Gets a value whether this setup was marked as verifiable using the <c>.Verifiable()</c> verb.
        /// </summary>
        bool IsVerifiable { get; }

        /// <summary>
        ///   Checks whether this setup is configured to return a mock object.
        ///   If so, the <see langword="out"/> parameter <paramref name="mock"/> will be set to the corresponding <see cref="Mock"/>.
        /// </summary>
        /// <param name="mock">
        ///   If this setup is configured to return a mock object, this parameter will be set to the corresponding <see cref="Mock"/>.
        /// </param>
        /// <returns>
        ///   <see langword="true"/> if this setup is configured to return a mock object;
        ///   otherwise, <see langword="false"/>.
        /// </returns>
        bool ReturnsMockObject(out Mock mock);

        /// <summary>
        ///   Verifies this setup.
        /// </summary>
        /// <exception cref="MockException">This setup was not matched by any invocations.</exception>
        void Verify();
    }
}

The ReturnsMockObject method is meant to help you discover "inner" mocks, i.e. if you have a composite setup expression mock => mock.GetA().GetB() and you're currently looking at the setup representing the mock => mock.GetA() part, ReturnsMockObject would help you get at the returned A mock containing the setup for (anonymous mock) => (anonymous mock).GetB().

Btw. such "inner mock" setups, when verified, verify all setups on the inner mock, which might not be super-obvious.

What's still missing here is something for your tags. If I'm looking at the correct code, Moq 5 setups have a State bag attached to them. We could add something similar to Moq 4 setups.

[ashmind]

That looks great! Some notes:

Btw. such "inner mock" setups, when verified, verify all setups on the inner mock, which might not be super-obvious.

You mean that calling Verify() on mock => mock.GetA().GetB() verifies GetB() as well? If so, I think it's expected and quite welcome, extracting inner mocks just to verify would be a hassle.

Expression -- if Verify verifies the whole thing, why not have the full expression here? For something like logging I feel it would be useful to have the whole thing. Not required for most cases I can think of though -- if I wanted to find all setups for a given method something like MemberInfo Member would be more straightforward.

ReturnsMockObject might be slightly confusing, I think your term InnerMock makes sense (kind of like InnerException). To match, if Expression was the whole thing, you could use OuterExpression for mock => mock.GetA() maybe?

[stakx]

You mean that calling Verify() on mock => mock.GetA().GetB() [...]

To be more precise here: There really is no mock => mock.GetA().GetB() once the corresponding Setup call is done. That expression gets split up into two setups on two mocks:

1. mock => mock.GetA() (which gets set up to return an anonymous mock object) on the original outer "parent" mock
2. (anonymous) => (anonymous).GetB() on that anonymous inner "child" mock

Verify()-ing the first setup will include verification of the second one.

Expression -- if Verify verifies the whole thing, why not have the full expression here?

That's a reasonable request. Ideally, we'd distinguish between two expressions: the exact one that got passed to Setup, and the (split-up) one that more accurately describes a setup.

Note that if Expression always returned what you passed to Setup, i.e. mock => mock.GetA().GetB() in our example, it would be easy to assume that Verify would only verify these calls on the inner "child" mocks — but that's not the case. If you add other setups to the same child mock, e.g.

mock.Setup(m => m.GetA().GetB())...; 
mock.Setup(m => m.GetA().Frobble())...; 

then Verify-ing the first setup (which would have an Expression equal to m.GetA().GetB()) would also verify the second setup. This is a somewhat unfortunate consequence of Moq's ownership rules: with regard to such "child mocks", the parent mock doesn't own a single setup on the child mock; it owns the whole child mock itself (including all of its setups). (There's more about this in e.g. #892 (comment).)

ReturnsMockObject might be slightly confusing, I think your term InnerMock makes sense

Yes, "inner mock" is what the child mocks are called internally, I've tried to keep it out of public parlance because I thought it would be too technical but I'll take your statement as proof to the contrary. :-)

[stakx]

FYI @ashmind, I've pushed a release to NuGet, however that doesn't yet include the ability to tag setups. I haven't forgotten it, though.

In the meantime, if you're in a hurry, you can verify single setups now, so it should be possible to loop over all setups and .Verify() only those that you want. For example, one could create two (admittedly thread-unsafe) helper extension methods, setup.NotVerifiable() which puts the last setup on a mock in some list, and mock.MySpecialVerify() which loops over a mock's setups and calls .Verify() on all that aren't memorized in that list. It'll be a tad more complicated once fluent setups come into play, but still feasible. Let me know if you need an example.

A future version of Moq will likely make that easier by adding arbitrary user state to setups, and through a new mock.Verify(bool recursive, Func<ISetup, bool> predicate) method overload to select what you want to include in verification.

[drdamour]

i came here looking for a Verifiable(false) to opt out of such a setup causing an exception wth VerifyNoOtherCalls(), is something like that supported?

[stakx]

@ashmind, it's been a long time since anything happened here, and you've probably moved on. In case you're still interested, I'm probably going to merge #1319, which will give you a way to opt out of mock.VerifyAll() by doing e.g. setup.Verifiable(Times.AtMost(int.MaxValue)). (Of course you can create an alias variable such as var anyNumberOfTimes = Times.AtMost(int.MaxValue), which may improve readability.)

@drdamour, I'm not sure yet how this is going to affect mock.VerifyNoOtherCalls() – it probably won't – but I'll give it some more thought.