MockRepository.Verify() also verifies mocks created outside of the repo

[matkoliptak]

Since 4.14, calling MockRepository.VerifyAll() also verifies mocks created outside of the repo but returned from a mocked method. In 4.13.1, the following Test passes, in 4.14 if fails because the IProduct.GetString() was not called. I never wanted to verify that IProduct.GetString() was called, this is why I created the IProduct mock by new Mock rather than repo.Create. I believe this might be a bug.

Expected behavior: the Test below passed.
Actual behavior: the TEst below fails with the following output:

Moq.MockException : The mock repository failed verification due to the following:

   Mock<TestClosure.IFactory:1>:
   This mock failed verification due to the following:
   
      TestClosure.IFactory m => m.Create():
      
         Mock<TestClosure.IProduct:1>:
         This mock failed verification due to the following:
         
            TestClosure.IProduct m => m.GetString():
            This setup was not matched.
   v Moq.MockFactory.VerifyMocks(Action`1 verifyAction)
   v Moq.MockFactory.VerifyAll()
   v MoqBug.TestClosure.Test() 

    public class TestClosure
    {
        public interface IFactory
        {
            IProduct Create();
        }

        public interface IProduct
        {
            string GetString();
        }

        public class FactoryCaller
        {
            private readonly IFactory _factory;

            public FactoryCaller(IFactory factory)
            {
                _factory = factory;
            }

            public void CallFactory()
            {
                _factory.Create();
            }
        }

        [Test]
        public void Test()
        {
            var repo = new MockRepository(MockBehavior.Loose);
            var factoryMock = repo.Create<IFactory>();
            
            var productMock = new Mock<IProduct>();                 // created out of the mock repo
            productMock.Setup(m => m.GetString()).Returns("hey");   // not really expected to be called, setting up "just in case"
            
            factoryMock.Setup(m => m.Create()).Returns(productMock.Object);

            var sut = new FactoryCaller(factoryMock.Object);
            sut.CallFactory();

            repo.VerifyAll();
        }
    }

[stakx]

Recursive verification doesn't get much love, and you're not the first to complain about it. 😃 There has been a recent issue (I believe it was #892) were we discussed changing Moq semantics so this kind of problem would no longer occur, but I found out that this would break long-standing, specified behavior, so I opted to close that issue without acting on it. (I would have liked to change things so mocks don't own other mocks (and therefore include them during verification); mocks would only own setups, which would likely be much more logical from a user perspective... I haven't yet dared making that breaking change.)

The reason why there's a functional change in version 4.14.0 is because I have rectified some Moq internals and made them more consistent (once again). What you're observing is very likely a consequence of that.

In your particular case, I'll note two things:

• Setting up a mock "just in case" is perhaps not a terribly good idea. Unit tests are supposed to be very short and to the point, so if you don't actually need that product mock, remove it from the test—problem solved, and your test will become shorter and easier to read.

• If you want to keep that unused product around for whatever reason, change this line:

  -factoryMock.Setup(m => m.Create()).Returns(      productMock.Object);
  +factoryMock.Setup(m => m.Create()).Returns(() => productMock.Object);

  That will make the product mock undiscoverable to Moq as it doesn't execute user-provided delegates just to discover relationships between mocks.

[stakx]

P.S. regarding my above comment:

[...] but I found out that this would break long-standing, specified behavior, so I opted to close that issue without acting on it. [...]

Thanks to some very recent internal changes, it might now have become possible to actually fix this without introducing a breaking change. I'll give this another close look soon.

That notwithstanding, the two bullet points above are still valid advice and can help you fix this in the shorter term.

[matkoliptak]

Thank you so much for reviewing it - I definitely agree "just in case" setups are in fact avoidable but it comes with a price (especially when reusing the test code for multiple test cases) of few "if" statements that are excellent in making code less readable. Your second advice with the lambda is exactly what I was looking for, thank you!

[stakx]

Note to self: Making this change ("mocks own setups, not other mocks") is likely a prerequisite for doing behavior-driven setups (#1002) properly; the reason being that we currently have different kinds of setups (those for stubbed properties, those for inner mocks, regular setups, sequence setups), some of which act slightly differently when it comes to verification. If we do introduce behavior-driven setups, you'll be able to turn any kind of setup into a totally different (custom) one, meaning it will no longer make sense for setups to behave differently during verification, depending on how they were originally instantiated. So the task here is to find a verification algorithm that works in exactly the same way for all kinds of setups. And we just might find that changing to a "mocks own setups, not other mocks" principle would be the most straightforward way to achieve this. I'll likely look into this very soon.

[killergege]

Hello,

I encountered the same issue but not really with a "just in case" setup, with nested properties :

The following code works in 4.13 but not in 4.14:

[Test]
public void Test()
{
    var range = new Mock<Microsoft.Office.Interop.Excel.Range>();
    range.Setup(r =>r.Rows.Count).Returns(2);
   //...

   range.Verify();
}

This works in 4.13 but throws "the mock failed verification due to the following: Range r =>r.Rows: this setup was not matched" in 4.14.
Which means that now it verifies methods we didn't set explicitly as Verify(), I'm suprised there's not more people having the issue.

Is there a workaround ? Because making it work for 4.14 would require additional "useless" code to setup manually Rows, and I have quite a lot of these.

[stakx]

@killergege, hard to say what's going on with a full minimal repro. Perhaps post such a one (preferably in a separate issue).

[github-actions]

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]

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.