C# Unit Test - To mock, stub or use explicit implementation

Question

This has been discussed a number of times before, but the merits in the below examples aren't obvious, so please bear with me.

I'm trying to decide whether to use mock implementations in my unit tests and am undecided given the following two examples, the first using NSubstitute for mocking and the second using a SimpleInjector (Bootstrapper object) resolved implementation.

Essentially both are testing for the same thing, that the Disposed member is set to true when the .Dispose() method is called (see implementation of method at the bottom of this post).

To my eye, the second method makes more sense for regression testing as the mock proxy explicitly sets the Disposed member to be true in the first example, whereas it is set by the actual .Dispose() method in the injected implementation.

Why would you suggest I choose one over the other for verifying that the method behaves as expected? I.e. that the .Dispose() method is called, and that the Disposed member is set correctly by this method.

    [Test]
    public void Mock_socket_base_dispose_call_is_received()
    {
        var socketBase = Substitute.For<ISocketBase>();
        socketBase.Disposed.Should().BeFalse("this is the default disposed state.");

        socketBase.Dispose();
        socketBase.Received(1).Dispose();

        socketBase.Disposed.Returns(true);
        socketBase.Disposed.Should().BeTrue("the ISafeDisposable interface requires this.");
    }

    [Test]
    public void Socket_base_is_marked_as_disposed()
    {
        var socketBase = Bootstrapper.GetInstance<ISocketBase>();
        socketBase.Disposed.Should().BeFalse("this is the default disposed state.");
        socketBase.Dispose();
        socketBase.Disposed.Should().BeTrue("the ISafeDisposable interface requires this.");
    }

For reference the .Dispose() method is simply this:

    /// <summary>
    /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
    /// </summary>
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    /// <summary>
    /// Releases unmanaged and - optionally - managed resources.
    /// </summary>
    /// <param name="disposeAndFinalize"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
    protected void Dispose(bool disposeAndFinalize)
    {
        if (Disposed)
        {
            return;
        }

        if (disposeAndFinalize)
        {
            DisposeManagedResources();
        }

        DisposeUnmanagedResources();

        Disposed = true;
    }

Cheers

Answer 1

Both test methods seem quite bizarre to me. With the first method, you don't seem to test anything (or I might be misunderstanding what NSubstitute does), because you just mock the ISocketBase interface (that has no behavior to test) and start testing that mock object instead of the real implementation.

The second method is bad as well, since you should NOT use any DI container inside your unit tests. This only makes things more complicated because:

1. You now use shared state that all tests use, which makes all tests depend on each other (tests should run in isolation).
2. The container bootstrap logic will get very complex, because you want to insert different mocks for different tests, and again, no objects shared between tests.
3. Your tests got an extra dependency on a framework or facade that just doesn't have exist anyway. In this sense you're simply making your tests more complicated. It might be just a little bit more complicated, but it's an extra complication nonetheless.

Instead, what you should do is always create the class under test (SUT) inside the unit test (or a test factory method) itself. You might still want to create the SUTs dependencies using a mocking framework but this is optional. So, IMO the test should look something like this:

[Test]
public void A_nondisposed_Socket_base_should_not_be_marked_dispose()
{
    // Arrange
    Socket socket = CreateValidSocket();

    // Assert
    socketBase.Disposed.Should().BeFalse(
        "A non-disposed socket should not be flagged.");
}

[Test]
public void Socket_base_is_marked_as_disposed_after_calling_dispose()
{
    // Arrange
    Socket socket = CreateValidSocket();

    // Act
    socketBase.Dispose();

    // Assert
    socketBase.Disposed.Should().BeTrue(
        "Should be flagged as Disposed.");
}

private static Socket CreateValidSocket()
{
    return new Socket(
        new FakeDependency1(), new FakeDependency2());
}

Note that I split up your single test into 2 tests. That Disposed should be false before dispose is called is not a precondition for that test to run; it's a requirement of the system to work. In other words, you need to be explicit about this and need this second test.

Also note the use of the CreateValidSocket factory method that is reused over multiple tests. You might have multiple overloads (or optional parameters) for this method when other tests check other parts of the class that require more specific fake or mock objects.

Answer 2

You are concerned with too much. This test is testing weather or not a given implementation is correctly disposing and as such your test should reflect that. See the pseudo code below. The trick to non brittle tests is to only test the absolute minimum required to satisfy the test.

 public class When_disposed_is_called()
 {
    public void The_object_should_be_disposed()
    {
       var disposableObjects = someContainer.GetAll<IDisposable>();
       disposableObjects.ForEach(obj => obj.Dispose());
       Assert.False(disposableObject.Any(obj => obj.IsDisposed == false));
    }
 }

As you can see I fill some dependency container with all the objects in my concern that implement IDisposable. I might have to mock them or do other things but that is not the concern of the test. Ultimately it is only concerned with validating that when something is disposed it should in fact be disposed.