VerifyNoOtherCalls fails reporting issue with different mock when using factory pattern

[benrobot]

The code below passes unit tests successfully up to moq version 4.10.1. After that the code fails as described in the code.

I read through the release notes for version 4.11 and I didn't see anything to point to this change. In addition, the behavior is NOT expected (in my opinion) because the code is asking for VerifyNoOtherCalls() from a specific mock therefore I would NOT expect an error message citing a different mock.

In addition to the code below I have also created a repository that reproduces the pass on version 4.10.1 and the fail on the latest version of moq.

See https://github.com/benrobot/moq4BugReport

    public interface IInterfaceOne
    {
        IInterfaceTwo MethodOne();
    }

    public interface IInterfaceTwo
    {
        bool MethodTwo();
    }

    public class MyTestClass
    {
        [Fact]
        public void Test1()
        {
            var mockTwo = new Mock<IInterfaceTwo>();
            mockTwo.Setup(x => x.MethodTwo()).Returns(true);
            var mockOne = new Mock<IInterfaceOne>();
            mockOne.Setup(x => x.MethodOne()).Returns(mockTwo.Object);

            mockOne.Object.MethodOne();
            mockTwo.Object.MethodTwo();

            mockOne.Verify(x => x.MethodOne());

            // The following fails with:
            //     Moq.MockException
            //     Mock < IInterfaceTwo:1 >:
            //     This mock failed verification due to the following unverified invocations:
            //     
            //     IInterfaceTwo.MethodTwo()
            mockOne.VerifyNoOtherCalls();
        }
    }

[stakx]

Yes, this situation is not ideal. Moq's current recursive verification algorithm is indeed problematic.

Since it is mockOne that gets verified, verification should only fail for unmatched setups that were actually configured on mockOne, and not on some other mock.

That is, the observed failure would be OK if you had instead done this:

-mockTwo.Setup(x => x.MethodTwo()).Returns(true);
 mockOne.Setup(x => x.MethodOne()).Returns(mockTwo.Object);
+mockOne.Setup(x => x.MethodOne().MethodTwo()).Returns(true);

(But it's not OK to fail mockOne verification due to an unmatched setup on mockTwo, just because there's some path from mockOne to mockTwo that Moq can follow.)

I've added your issue to the list of use cases in #1093.

If you need a temporary workaround, you can make the following change:

-mockOne.Setup(x => x.MethodOne()).Returns(      mockTwo.Object);
+mockOne.Setup(x => x.MethodOne()).Returns(() => mockTwo.Object);

[benrobot]

@stakx Thank you. Somehow I saw your original response and never noticed that you provided a workaround after editing your response. Someone else at my organization tried to upgrade above 4.10.1 and ran into this issue.

I can confirm that the workaround you provided DOES work.

Thanks again.

[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.

[github-actions]

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