How not to write unit tests

In doing some major refactoring of code recently I was extremely glad we had unit test coverage in place.  Unfortunately there were some fairly problematic issues in the implementation of the unit tests that ended up costing a lot more time than it should.

The problem was two-fold.  Firstly there was a mismatch of custom hand-crafted stub classes and automatically mocked interfaces.  Secondly there was no consistency over the level of dependencies that were stubbed.  In some cases "true" (i.e. non-stubbed) implementations of dependencies were used three or four dependencies deep, and others only the direct dependency was mocked.

The combination of this meant it was really difficult to update the unit tests as part of the refactoring.  Each time a test failed I had to check whether it was due to a manually crafted stub that needed to be revised, a top level dependency that needed to be updated, or a dependency deep in the dependency tree that needed to be updated.

If the unit tests were configured appropriately, the only time I would need to update the unit tests would have been when a direct dependency changed.

What should have been done?

One of the more important features of Unit Tests is isolation and repeatable consistency of testing.  This ensures your unit test is appropriately testing the code you are targeting, and not the multitude of dependencies that the code may have.

If a direct dependent class has a nested dependency that is modified but the direct dependency retains the same expected behaviour, then you don't need to change your unit test.  Without this separation, major refactoring of your code becomes far more labourious, as you need to both refactor your code as well as all of the unit tests that have a dependency on the altered code at any level.

For this reason mocking or stubbing the direct dependencies of a method has become standard practice when working with unit tests.  Back in the old days this used to mean creating custom stub classes for all your dependencies.  This is one reason why Unit Testing was so often implemented as full end-to-end tests.
These days there is no excuse, Tools like Moq and RhinoMocks provide the ability to automatically create mocked versions of your dependencies, return appropriate objects based on the the calling parameters, and confirm that the services were called with the expected parameters.  Using these tools allows you to focus on testing the "unit of code" without worrying about what dependencies of dependencies might be doing.

Given an arbitrary method that calculates the cart total cost with a discount based on the total cost and number of items purchased.

        public decimal CalculateCart(Item[] items, IDiscountCalculator discountCalculator)
        {
            decimal total = 0;

            foreach (var item in items)
            {
                total += item.Cost;
            }

            var discountedAmount = discountCalculator.calculate(total, items.Length);

            return total - discountedAmount;
        }

IDiscountCalculator has its own unit tests to verify its behaviour, so we don't want to be testing this as part of the cart calculation.  We also don't want our CalculateCart to fail if our discountCalculator algorithm changes - as long as we trust the discountCalculator is going to work we don't need it to do the actual calculation and can return an arbitrary value.

public void TestCalculateCart()
{
    var cart = new Cart();
    Mock<IDiscountCalculator> discountCalculator = new Mock<IDiscountCalculator>();
           
    //arrange
    var expectedDiscount = (decimal)2;
    var expectedOriginalTotal = 10 + 10;
    var expectedDiscountedTotal = 10 + 10 - (decimal)2;

    var items = new Item[] {new Item() {Cost = 10}, new Item() {Cost = 10}};
    discountCalculator
        .Setup(x =>
            x.calculate(It.IsAny<decimal>(), It.IsAny<int>())
        )
        .Returns(expectedDiscount);

    //act
    var total = cart.CalculateCart(items, discountCalculator.Object);

    //assert
    //confirm the expected total is returned
    Assert.AreEqual(expectedDiscountedTotal, total );
           
    //confirm the discount calculator was called with the appropriate input
    discountCalculator.Verify(mock =>
        mock.calculate(It.Is<decimal>(x => x.Equals(expectedOriginalTotal)), It.Is<int>(x => x.Equals(2)))
        , Times.Once
        );

}

Now no matter what the implementation of DiscountCalculator, as long as we trust our DiscountCalculator is behaving (i.e through valid unit tests) then we don't need to change our unit test for the Cart calculation.

If our cart calculator was extended to throw an InvalidDiscountException in the event the discount is great than the total, we can arrange the discountCalculator to return 12, and assert that the exception was thrown without changing our 'happy path test' and with the same level of confidence.

Proper isolation and repeatability means refactoring is far simpler and your tests become less brittle.