Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot mock protected Dispose(It-IsAny-Bool) on System.ComponentModel.Component #614

Closed
quetzalcoatl opened this issue Apr 10, 2018 · 4 comments

Comments

@quetzalcoatl
Copy link
Contributor

quetzalcoatl commented Apr 10, 2018

This is a kind of a follow-up to #129 and #143

I'd like to point out that the suggested workaround of .As may be somewhat reasonable, however in some cases it will work, and in some cases will not. It's simply because implementing Dispose() from IDisposable in attempt to hide existing non-virtual Dispose() is not the same as implementing virtual Dispose on from a base class like System.ComponentModel.Component.

If some 3rd party code calls Dispose() by referencing it by the base class, then it won't hit proxy-based IDisposable interface. It will call it directly from the base class, and will end up calling not-mocked protected Dispose(bool) and throw exception due to strict behavior.

In my case the 3rdparty code is from is ... the GarbageCollector. It's not really critical, GC silences those exceptions, but still it smells.

I was able to set up base Dispose(bool) for true and false calls. However when I try to set up for It.IsAny<bool> then it does not work reliably in all cases.

I prepared some tests below. Tests prefixed NoSetup_ just show the original problem of having base protected Dispose(bool). Next tests try to use some Setups for that. The test called SetupBaseDisposeAny_DoesNotThrow_OnDisposeTrue shows the current problem. It seems that It.IsAny filter is not respected there and the setup does not work, and it ends up with exception. However an identical setup is respected in test that follows, SetupBaseDisposeAny_MockExceptions_DoNotOccur_DuringCollection_OnDisposeFalse. I would expect both of them to pass (well, rather obvious, this setup seems simple) or both of them to fail (meaning that It.IsAny is simply not supported in Protected() ?) - but the outcome is mixed.. Finally, last tests use setups with concrete values and confirm that Protected() is able to mock the base Dispose(bool), so the core of the mechanism probably works fine and only It.IsAny glitched.

For me, it seems like there's some bug out there..

        [Fact]
        public void NoSetup_ThrowsException_OnDisposeTrue()
        {
            var mock = new Mock<System.ComponentModel.Component>(MockBehavior.Strict);
            //mock.As<IDisposable>().Setup(it => it.Dispose()); - won't help with exception as we call Dispose directly by base class

            // non-virtual Dispose calls into base Dispose(true)
            var ex = Record.Exception(() => mock.Object.Dispose()); // Message: Moq.MockException : Component.Dispose(True)

            var mex = Assert.IsType<MockException>(ex);
            Assert.Contains("Component.Dispose(True)", mex.Message);
        }

        [Fact]
        public void NoSetup_GarbageCollector_DoesNotCrashOnMockExceptions_OnDisposeFalse()
        {
            var mock = new Mock<System.ComponentModel.Component>(MockBehavior.Strict);
            var obj = mock.Object;

            obj = null;
            mock = null;

            // GarbageCollector calls into base Dispose(false)
            GC.Collect(0, GCCollectionMode.Forced, true, false);
            GC.WaitForPendingFinalizers(); // First-Chance-Exception: Moq.MockException : Component.Dispose(False)
            // .. but we don't see it -- exception was CAUGHT and SILENCED by the GC

            // but.. was there really an exception? let's prepare a special component
            // and see what really is happening -> NoSetup_GarbageCollector_DoesNotReport_MockExceptions_OnDisposeFalse
        }

        public class StateHolder { public Exception DisposerException; }
        public class FinalizerFailureRecordingComponent : System.ComponentModel.Component
        {
            public StateHolder StateHolder;
            ~FinalizerFailureRecordingComponent()
            {
                try { Dispose(false); }
                catch (Exception ex) { StateHolder.DisposerException = ex; }
            }
        }

        [Fact]
        public void NoSetup_MockExceptions_ReallyDoOccur_DuringCollection_OnDisposeFalse()
        {
            var state = new StateHolder();

            var mock = new Mock<FinalizerFailureRecordingComponent>(MockBehavior.Strict);
            //mock.As<IDisposable>().Setup(it => it.Dispose()); - won't help with exception because GC calls Finalizer and Finalizer calls Dispose directly
            var obj = mock.Object;
            obj.StateHolder = state;

            // allow GC to sweep the mock, but we keep holding the ExceptionHolder
            // so information about any exceptions should still be there
            obj = null;
            mock = null;

            // GarbageCollector calls into base Dispose(false)
            GC.Collect(0, GCCollectionMode.Forced, true, false);
            GC.WaitForPendingFinalizers(); // First-Chance-Exception: Moq.MockException : Component.Dispose(False)
            // .. but we don't see it -- exception was CAUGHT and SILENCED by the GC

            var mex = Assert.IsType<MockException>(state.DisposerException);
            Assert.Contains("Component.Dispose(False)", mex.Message);
            // yup, there was an exception there - AS EXPECTED
        }

        // now .. let's try set It.Is setups

        [Fact] // FAILS
        public void SetupBaseDisposeAny_DoesNotThrow_OnDisposeTrue()
        {
            var state = new StateHolder();

            var mock = new Mock<FinalizerFailureRecordingComponent>(MockBehavior.Strict);
            mock.Protected().Setup("Dispose", It.IsAny<bool>());
            //mock.Protected().Setup("Dispose", true);
            mock.Object.StateHolder = state;

            // non-virtual Dispose calls into base Dispose(true)
            var ex = Record.Exception(() => mock.Object.Dispose());

            Assert.Null(ex);
        }

        [Fact]
        public void SetupBaseDisposeAny_MockExceptions_DoNotOccur_DuringCollection_OnDisposeFalse()
        {
            var state = new StateHolder();

            var mock = new Mock<FinalizerFailureRecordingComponent>(MockBehavior.Strict);
            mock.Protected().Setup("Dispose", It.IsAny<bool>()); // why does it work here if it did not in SetupBaseDisposeAny_DoesNotThrow_OnDisposeTrue?
            var obj = mock.Object;
            obj.StateHolder = state;

            // allow GC to sweep the mock, but we keep holding the ExceptionHolder
            // so information about any exceptions should still be there
            obj = null;
            mock = null;

            // GarbageCollector calls into base Dispose(false)
            GC.Collect(0, GCCollectionMode.Forced, true, false);
            GC.WaitForPendingFinalizers();

            Assert.Null(state.DisposerException);
        }

        // now .. let's try explicit setups

        [Fact]
        public void SetupBaseDisposeTrue_DoesNotThrow_OnDisposeTrue()
        {
            var state = new StateHolder();

            var mock = new Mock<FinalizerFailureRecordingComponent>(MockBehavior.Strict);
            mock.Protected().Setup("Dispose", true);
            mock.Object.StateHolder = state;

            // non-virtual Dispose calls into base Dispose(true)
            var ex = Record.Exception(() => mock.Object.Dispose());

            Assert.Null(ex);
            // as expected - setup for Dispose(true) was respected, no exceptions noticed
        }

        [Fact]
        public void SetupBaseDisposeFalse_MockExceptions_DoNotOccur_DuringCollection_OnDisposeFalse()
        {
            var state = new StateHolder();

            var mock = new Mock<FinalizerFailureRecordingComponent>(MockBehavior.Strict);
            mock.Protected().Setup("Dispose", false);
            var obj = mock.Object;
            obj.StateHolder = state;

            // allow GC to sweep the mock, but we keep holding the ExceptionHolder
            // so information about any exceptions should still be there
            obj = null;
            mock = null;

            // GarbageCollector calls into base Dispose(false)
            GC.Collect(0, GCCollectionMode.Forced, true, false);
            GC.WaitForPendingFinalizers();

            Assert.Null(state.DisposerException);
            // as expected - setup for Dispose(false) was respected, no exceptions noticed
        }

During these tests I've used:

#region Assembly Moq, Version=4.7.142.0, Culture=neutral, PublicKeyToken=69f491c39445e920
// C:\Users\User\.nuget\packages\moq\4.7.142\lib\net45\Moq.dll
#endregion
@quetzalcoatl
Copy link
Contributor Author

I've just noticed I'm a bit behind and updated packages, and got the same outcome with the now-newest

#region Assembly Moq, Version=4.8.0.0, Culture=neutral, PublicKeyToken=69f491c39445e920
// C:\Users\User\.nuget\packages\moq\4.8.2\lib\net45\Moq.dll
#endregion

@stakx
Copy link
Contributor

stakx commented Apr 11, 2018

@quetzalcoatl - This is a lot to take in, so please forgive me if I don't focus on the important bits right away.

TL;DR: When using .Protected(), remember to use ItExpr instead of It.

SetupBaseDisposeAny_DoesNotThrow_OnDisposeTrue

Your main question appears to be why this test fails:

[Fact] // FAILS
public void SetupBaseDisposeAny_DoesNotThrow_OnDisposeTrue()
{
    var state = new StateHolder();

    var mock = new Mock<FinalizerFailureRecordingComponent>(MockBehavior.Strict);
    mock.Protected().Setup("Dispose", It.IsAny<bool>());
    //mock.Protected().Setup("Dispose", true);
    mock.Object.StateHolder = state;

    // non-virtual Dispose calls into base Dispose(true)
    var ex = Record.Exception(() => mock.Object.Dispose());

    Assert.Null(ex);
}

First, one thing to note that It.IsAny<bool>() returns default(bool) (i.e. false), not true. So you're not setting up the right call here.

Second, and more to the point, you're trying to use It matchers with .Protected(), which is not going to work. Like the documentation points out, you should use ItExpr instead:

it-vs-itexpr

Once you change It.IsAny<bool>() to ItExpr.IsAny<bool>(), this test will pass.

The reason why you must use ItExpr instead of It is simple:

  • It matchers are supposed to be be used in a lambda that gets converted to an expression tree at compile time, so that they the matchers can be inspected. They do not return meaningful return values: It.IsAny<bool>() has a return value to satisfy type checking at compile-time.

  • Now, when using .Protected(), you no longer have a lambda that can get converted to an expression tree; you specify a plain list of argument values. Therefore, Moq is not able to recognize the matcher, as all it sees is the return value it produced; that's why you must use ItExpr instead: ItExpr.IsAny<bool>() won't return default(bool), but an expression tree representing the call to It.IsAny<bool>(). That's something Moq can recognize and work with during setup.)

SetupBaseDisposeAny_MockExceptions_DoNotOccur_DuringCollection_OnDisposeFalse

mock.Protected().Setup("Dispose", It.IsAny<bool>()); // why does it work here if it did not in SetupBaseDisposeAny_DoesNotThrow_OnDisposeTrue?

This works because It.IsAny<bool>() happens to evaluate to false. See full explanation above.

NoSetup_MockExceptions_ReallyDoOccur_DuringCollection_OnDisposeFalse

(Your code suggests that you expect this test to pass, but at least on my machine, it doesn't.)

@quetzalcoatl
Copy link
Contributor Author

quetzalcoatl commented Apr 11, 2018

@stakx - Regards net-core vs net-framework - that is fine. Due to Oracle not having their libs built for net-core, I'm actually have a mixed runtime: Project uses net-core-2.0 libraries but it's running on net-47 runtime required by Oracle. Totally fine, they are compatible in that direction (run core-2.0 on net-4.7, not the other way :) )

Regarding NoSetup_MockExceptions_ReallyDoOccur_DuringCollection_OnDisposeFalse - that's interesting. Does it throw an exception from WaitForPendingFinalizers or is it a failure from assertions at the end of the test? My not-so-smart mechanism for observing what happens in finalizers (FinalizerFailureRecordingComponent) probably isn't perfect. It works (exception gets recorded, test passes) on my machine, just checked again, but it's totally possible that it will behave differently on other runtimes.. However, I would expect it to either work in all tests, or not-work in all tests. You say that only this one failed, so other tests using of FinalizerFailureRecordingComponent were ok? Strange.. Anyways, FinalizerFailureRecordingComponent is just a helper I tried to create to automate the test and record finalizer exceptions... True test is running the code with debugger attached and breakpoint set in finalizer's catch clause. If my helper failed like this, then you could try that way if you'd like to reliably check what really happened.

Regarding ItExpr - oh god, I knew I missed something basic!! Thank you very much, I totally forgot about that. It's easy to forget when you use Protected() once in a year. I've just rechecked using ItExpr and it of course works. I'm really sorry for bothering you with an RTFM-kind of an issue!

@stakx
Copy link
Contributor

stakx commented Apr 11, 2018

Regarding NoSetup_MockExceptions_ReallyDoOccur_DuringCollection_OnDisposeFalse - that's interesting. Does it throw an exception from WaitForPendingFinalizers or is it a failure from assertions at the end of the test?

The latter: Assert.IsType<MockException>(state.DisposerException) fails because state.DisposerException holds a null reference.

I assume you're well aware that relying on finalizers to set observable state is probably a pretty bad idea (since finalizers and the way they execute are rather special) and not something you should ever have to do under normal circumstances, so I opted not to look into this too deeply. It would be an interesting puzzle to solve, though.

Regarding ItExpr - oh god, I knew I missed something basic!!

It happens to the best of us 😜 so no worries!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants