Invocations.Clear() does not work on recursive mocks

[aaronburro]

IDE: VS 2017 Test Runner: ReSharper 2019.1 Moq: 4.9.0 and above Castle.Core: 4.4.1 NUnit: 3.7.1 and above .NET Version: 4.5.2

In the example below, the Invocations on the Bar mock are not cleared. This appears to have always been the case, but we noticed it in 4.15.1 because that version began including event (de)registrations in the Invocations, which caused some of our memory leak tests to start failing. I would expect recursively-created mocks (i.e., Foo in mock.Setup(m => m.Foo.DoIt()) to be cleared by the main mock, since that's how they were set up. Instead, we have to use Mock.Get(), which can be a bit of a crap-shoot in its own right on what it returns versus what was actually used (I recall having to muck around with a few tests to find the "right" mock. Different issue, but just mentioning it to describe why this is a pain point). I would also point out that the prior code which worked on Mock.ResetCalls() did seem to clear invocations on recursive mocks, which is why we probably relied on it. NOTE: I could see an argument either way for a mock assigned by SetupGet().

public interface IFoo { IBar Bar { get; } }
public interface IBar { void DoIt(); }
[TestFixture]
public class Tests
{
    [Test]
    public void Test()
    {
        var mock = new Mock<IFoo>();
        mock.Setup(m => m.Bar.DoIt()).Callback(() => Console.WriteLine("Here we are!"));

        mock.Object.Bar.DoIt();
        mock.Invocations.Clear();
        Assert.AreEqual(0, mock.Invocations.Count, "Mock's Invocations did not get cleared");

        // the next line has never worked
        Assert.AreEqual(0, mock.Object.Bar.Invocations.Count, "Mock.Bar's Invocations did not get cleared");
    }
}

[aaronburro]

Reading a few other issues, this seems to be in the same vein as #1093

[stakx]

In the example below, the Invocations on the Bar mock are not cleared. This appears to have always been the case [.]

I'd have to check to be sure, but if memory serves, you're right, clearing invocations has never automatically done the same on submocks.

Reading a few other issues, this seems to be in the same vein as #1093

You're correct, that "recursive" principle is very much relevant here. People keep complaining about Moq automatically including submocks in verification when that isn't expected at all.

Making mock.Invocations.Clear() also such a "recursive" operation would likely cause even more complaints, and with good reason: I dare say that most users would find this unexpected, unintuitive, and/or illogical.

With recursive verification, there's actually a good reason why verification of some foo mock may include a bar mock; after all, setups can span mocks: fooMock.Setup(foo => foo.Bar.X).

However, there's no such thing as a single invocation spanning several mocks; each invocation happens on exactly one method of one mock object. So the reason for "recursiveness" doesn't apply here.

That's why I believe we shouldn't make invocation clearing include submocks; it would be a change for the worse.

[aaronburro]

Right, I would agree that full recursive clearing of invocations might be surprising and unintuitive, but:

With recursive verification, there's actually a good reason why verification of some foo mock may include a bar mock; after all, setups can span mocks: fooMock.Setup(foo => foo.Bar.X).

This is the case where it's unintuitive that the Invocations don't clear. While I get that the Invocation is clearly on the submock, I set that submock up using the main mock (Moq did it for me magically), so I would expect Invocations to behave the same way Verifications would.

Maybe I'm just one of those oddballs who expects the recursive behaviour. In this case it is a little annoying because clearing the submocks seems to have been the behaviour before Invocations got stored on a per-mock basis, instead of on the top level.

[stakx]

Note to self: this may be related to #1120.

[toras9000]

Recently, I encountered the following situation that puzzled me.

public interface IFeature { void Operation(); }
public interface IService { IFeature Feature { get; } }

[TestMethod]
public void Test()
{
    var mock = new Mock<IService>();
    mock.Setup(m => m.Feature.Operation());

    mock.Object.Feature.Operation();
    mock.Verify(m => m.Feature.Operation(), Times.Once);  // pass

    mock.Invocations.Clear();
    mock.Verify(m => m.Feature.Operation(), Times.Never); // pass

    mock.Object.Feature.Operation();
    mock.Verify(m => m.Feature.Operation(), Times.Once);  // fail: Expected invocation on the mock once, but was 2 times
}

I understand this to a degree because of this issue, but I find it a bit strange. This may be related to #1120 and I am guessing that this is an issue that will be fundamentally addressed in #1093.

However, when looking at just Clear(), I think it could be made a little more obvious from the current situation. I agree that Clear() is not recursive by default, but I think it could be a signature for Clear(bool recursive = false). I think most people can quickly figure out what is going on when they see this signature in IntelliSense.

[stakx]

Clear(bool recursive = false). I think most people can quickly figure out what is going on when they see this signature in IntelliSense.

I agree.

The issue with such a method overload, however, is that it shouldn't exist. The whole "recursiveness" thing in Moq is inconsistent and overall (IMHO) a mistake. It shouldn't be further cemented by codifying it in the public API. (Unfortunately I made that exact error with ISetup.Verify.)

[toras9000]

Okay, I understand about the thinking of consistency. And certainly should not be addressed ad hoc with library APIs.

[github-actions[bot]]

Due to lack of recent activity, this issue has been labeled as 'stale'. It will be closed if no further activity occurs within 30 more days. Any new comment will remove the label.

[github-actions[bot]]

This issue will now be closed since it has been labeled 'stale' without activity for 30 days.