Overmocking

One of the most powerful tool available to developers testing a legacy code base is the ability to mock out classes that their code depends on. This is of great importance since our unit tests need to limit the scope, and we do this by trying to limit our dependencies. Through mocking we can exchange one dependency on the infrastructure of our application for an in-memory mock.

Sometimes mocking is overused, and I am not just talking about cases where every objected gets mocked to the point where we’re testing nothing. I am talking about a different issue in mocking. I am talking about where developers put on their mocking blinders when unit testing. It’s an easy thing to do, and I’ve done it plenty of times myself.

Setting Up Our Example

We will start of by defining our example. We will have a method to do some sort of calculation (an ideal and easy testing scenario). It will be a legacy code base, which means that it is untested code and probably hard to test. We’ll start off by making it a private static method and put in a nice dependency that one might try to mock to avoid.

private static decimal CalculateFooOnXyz(Xyz xyzItem, 
    decimal calculationParameter1)
{
    var numbersInCalculation = Repository.GetNumbers()
        .Where(n => n.IsActive);

    foreach (Number number in numbersInCalculation)
    {
        // Some code that executes for each one.
    }
}

private static decimal CalculateFooOnXyz(Xyz xyzItem, 
    decimal calculationParameter1, IRepository repository)
{
    var numbersInCalculation = repository.GetNumbers()
        .Where(n => n.IsActive);

    foreach (Number number in numbersInCalculation)
    {
        // Some code that executes for each one.
    }
}

Why This is Wrong

My first question is, what is the name of the method? CalculateFooOnXyz. Notice that I didn’t say, “GetDataFromTheDatabase”. That’s because we shouldn’t be doing that. It isn’t part of the calculation. The numbers returned from the database are required for calculating, so that means that we should have that object as our dependency instead.

How We Change It

So instead of making the repository our parameter, we should make the collection of numbers our parameter. This way we’re depending on the in-memory collection of CLR objects. This is much less of a dependency, and in is not one that needs to be mocked. By doing this alternative we’re better following the Single Responsibility and Dependency Inversion principles.

Our best code for testing will look like this.

private static decimal CalculateFooOnXyz(Xyz xyzItem, 
    decimal calculationParameter1, List<CalcNumbers> numbersInCalculation)
{
  foreach (Number number in numbersInCalculation)
  {
    // Some code that executes for each one.
  }
}