devlooped / moq

The most popular and friendly mocking framework for .NET
Other
5.88k stars 801 forks source link

VerifyNoOtherCalls fails reporting issue with different mock when using factory pattern #1181

Open benrobot opened 3 years ago

benrobot commented 3 years ago

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();
        }
    }

Back this issue Back this issue

stakx commented 3 years ago

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 commented 2 years ago

@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[bot] commented 3 weeks ago

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.