Add VerifyNoOtherCalls to MockFactory

[BlythMeister]

Enables single call to verify no other calls on all mocks generated by the factory

Based on the discussion with @stakx in #675 it appeared that i was not using the framework correctly.
The reason i chose to do it this was way the inability to perform a VerifyNoOtherCalls on all mocks generated from a single factory.

This new method means you can create many mocks from a single factory, verify each expectation in turn and then at the end call Factory.VerifyNoOtherCalls to ensure that only the items you have verified occurred for every mock generated in a given test.

Current rather nasty workaround

    public class MyMockRepository : MockRepository
    {
        public MyMockRepository(MockBehavior defaultBehavior) : base(defaultBehavior)
        {
        }

        public virtual void VerifyNoOtherCalls()
        {
            VerifyMocks(m =>
            {
                dynamic objectAsType = Convert.ChangeType(m, m.GetType());
                objectAsType.VerifyNoOtherCalls();
            });
        }
    }

[stakx]

Thanks for your PR. Could you please take another look at the following few things? Also, could you please add an entry to CHANGELOG.md (under the Unpublished → Added heading) that briefly summarises this enhancement?

[stakx]

I suspect that this call won't do proper error aggregation. Let's say I'm adding another test for a repository with more than one mock:

[Fact]
public void VerifyNoOtherCalls_should_correctly_aggregate_unmatched_calls_from_more_than_one_mock()
{
    var repository = new MockRepository(MockBehavior.Default);
    repository.Create<IFoo>().Object.Do();
    repository.Create<IBar>().Object.Redo();

    var ex = Record.Exception(() => repository.VerifyNoOtherCalls());

    Assert.Contains("The following invocations were not verified:", ex.Message);
    Assert.Contains("IFoo.Do()", ex.Message);
    Assert.Contains("IBar.Redo()", ex.Message);
}

This test will fail because the error message is this:

The following invocations were not verified:
IFoo.Do()

No mention of the mock2.Redo() call.

Could you please add a test such as the above and look into error aggregation for VerifyNoOtherCalls? Hint: It might be enough to change the following line:

https://github.com/moq/moq4/blob/a8e2642fae53cc61fcd501b12b7bba6d1faa4edd/src/Moq/Obsolete/MockFactory.cs#L385

to ... when (error.Reason == MockExceptionReason.UnmatchedSetups || MockExceptionReason.UnverifiedInvocations).

As an inspiration for what properly aggregated errors might look like, here's how errors are aggregated for e.g. VerifyAll. Take this code:

var repo = new MockRepository(MockBehavior.Default);
repo.Create<IFoo>().Setup(m => m.Do());
repo.Create<IBar>().Setup(m => m.Redo());

repo.VerifyAll();

This will produce an error message such as the following:

The following setups on mock 'Mock<IFoo:00000001>' were not matched:
IFoo m => m.Do()

The following setups on mock 'Mock<IBar:00000001>' were not matched:
IBar m => m.Redo()

(This is basically a concatenation of the each "failed" mock's error message, with a blank line in-between.)

[BlythMeister]

This threw up an interesting issue of multiple types of exception needing to be grouped, so i added a new exception reason for this.

[stakx]

Thanks for the changes made so far. I have a few more. 🦄 The main issue here is with the way error aggregation is done. I think it'd be better not to make aggregation in VerifyMocks more general using a new MockExceptionReason.MultipleVerificationErrors. Instead, just perform specific aggregation right in MockFactory.VerifyNoOtherCalls.

That being said, greatly I appreciate the effort you made! Keep up the good work! 👍

[stakx]

There's two problems here:

1. Using string.Format on Resources.UnverifiedInvocations here will lead to duplicated text in the error message, since the same resource string will already be included at the beginning of each error.Message.

   It would be better to follow the pattern that can be seen in the MockException.UnmatchedSetups(IEnumerable<MockException> errors) method:

   return new MockException(
       MockExceptionReason.UnverifiedInvocations,
       string.Join(
           Environment.NewLine,
           errors.Select(error => error.Message)));

2. It would be important to include the failed mock's name in each original error.Message so that when multiple error messages are concatenated, the user can still tell which reported failures were caused by which mocks. This could be done as follows:

   • First, change the value of Resources.UnverifiedInvocations so it includes a placeholder for the mock's name: The following invocations on mock {0} were not verified:\n{1}.

   • In the MockException.UnverifiedInvocations(IEnumerable<Invocation> invocations) method, include the mock's name when string.Format-ting (which means that method needs an additional parameter for the mock:

     string.Format(
         CultureInfo.CurrentCulture,
         Resources.UnverifiedInvocations,
         mock.ToString(),
         string.Join(Environment.NewLine, invocations)));

[stakx]

Please remove this, along with the change in Resources.Designer.cs.

Instead, as mentioned in another review comment, change the value of UnverifiedInvocations (right above these lines) to include a placeholder for the mock's name. (The following invocations on mock {0} were not...; see other review comment for details.)

[BlythMeister]

@stakx I see where your coming from and I too was dubious over the way the usage was going, so I'll revert a bit and implement the aggregate exception nicer.

[BlythMeister]

@stakx all updated (i rebased with a squash to clean the history, hope that's OK)

[stakx]

@BlythMeister - Rebasing, squashing, fixing up, etc. your PR to keep it clean and tidy is very much appreciated! 👍 Will review again in a bit.

[stakx]

Thanks again for making (most of) the requested changes. There's just one bit missing now.

Let's say I do the following:

public interface IFoo
{
    void Do();
}

public interface IBar
{
    void Redo();
}

var repo = new MockRepository(MockBehavior.Default);
repo.Create<IFoo>().Object.Do();
repo.Create<IBar>().Object.Redo();
repo.VerifyNoOtherCalls();

I will get the following verification error message:

The following invocations were not verified:
IFoo.Do()
The following invocations were not verified:
IBar.Redo()

It should read as follows instead:

The following invocations on mock 'Mock<IFoo:00000001>' were not verified:
IFoo.Do()

The following invocations on mock 'Mock<IBar:00000001>' were not verified:
IBar.Redo()

(I didn't leave a review comment about it but of course this will require a small change in the last test you're adding.)

Please see the two unresolved comments in my previous review for some hints how to achieve this.

Thanks for the great work so far!

[BlythMeister]

Thank you for keeping bearing with me, I really appreciate it, and have already learnt a lot about moq!
I will take a look at the error messages in the morning.

[BlythMeister]

@stakx updates made, i did also update the test for the verify to make it match the new test which ensures the full error message is correct.

[stakx]

@BlythMeister - Thanks!

Did you see the two test failures? (They appear to be failing due to different line endings (CRLF vs LF). Instead of using "\r\n" (CRLF) in your test's string literals, try using Environment.NewLine (which will get resolved to whatever line separator your current platform uses).

(Btw. when you look at the AppVeyor log, you might see some � glyphs; never mind those, that's likely just a display / encoding problem on AppVeyor.)

[stakx]

Almost there! Just the two tests failing on AppVeyor. Should be easily fixed, however.

[BlythMeister]

@stakx hopefully there now!

[stakx]

AppVeyor is still complaining, but I'm going to merge this anyway. The thing is, this isn't the first time that this problem has shown up, and I'd like to finally create a proper assertion helper for comparing multi-line strings instead of off-loading this work on you (it's really a problem unrelated to this single PR). I'll fix this remaining issue in a separate step, hope that's OK with you.

Thanks a lot for the great work! 👍 It's been a joy working on this PR with you. You've responded very quickly to reviews and even taken the time to tidy up the PR intermittently. Couldn't ask for more!

[stakx]

@BlythMeister - I've just pushed Moq 4.10.0 (which includes your addition) to NuGet. It should be available shortly.