Isolate or Redundant Test?

Question

I have lots of code like the following. An "Entity" type that has some numerical properties. To be able to reuse the arithmetic I write the arithmetic functions against an interface. I use extension methods, which are easy to use. I use similar extension methods to convert the numerical values into formatted text to display. And then finally I have a type that stores the display text for the UI to use. (A "row".)

public class Entity : IEntity {
   public int NumberOfAs { get; set; }
   public int NumberOfBs { get; set; }
}

public interface IEntity {
   int NumberOfAs { get; }
   int NumberOfBs { get; }
}

public static class EntityFormula {
   public static int GetTotalNumber(this IEntity me) =>
      me.NumberOfAs + me.NumberOfBs;
}

public static class EntityFormatter {
   public static string GetTotalNumberText(this IEntity me) =>
      me.GetTotalNumber().ToString("#,##0");
}

public sealed class EntityRow {
   public string TotalNumberText { get; }

   public EntityRow(
      Entity e
   ) {
      TotalNumberText = e.GetTotalNumberText();
   }
}

This allows my UI to reuse the arithmetic for display purposes.

The trouble I'm having is in testing. It's easy to test GetTotalNumber(), but I notice that when I test GetTotalNumberText() and EntityRow that I am re-testing the arithmetic again.

Recently I started to learn to use Moq to generate mocks. What I would like to do is mock IEntity and/or Entity so that the call to GetTotalNumber() is not re-tested in GetTotalNumberText(), and the same for GetTotalNumberText() in the EntityRow class.

However, since these are extension methods, they are not "stubbed" when IEntity is mocked. They are essentially static methods and you can't mock static methods.

There are some options here, but I'm not sure the best approach.

One option, I think, is to convert the static extension method classes into concrete classes and then extract interfaces from them:

public sealed class EntityFormula : IEntityFormula {
   readonly IEntity mEntity;

   public EntityFormula(IEntity entity) => mEntity = entity;

   public int GetTotalNumber() => mEntity.NumberOfAs + mEntity.NumberOfBs;
}

public interface IEntityFormula {
   int GetTotalNumber();
}

This allows me to inject/mock the formula and formatter classes so that I can stub the method calls and not retest them. But it's a little less convenient to use and it creates a bunch of interfaces that wouldn't need to exist otherwise, so I'm not sure I want to commit to doing that to my entire project.

Another option is to just retest the logic and forget about isolating this code. That's what I was going to do but when I learned about isolation I really started liking the idea that I was testing the code in this method only and not re-testing the code in that other method that I've already tested.

Is one of these the correct choice or are there maybe other options I'm not thinking of (such as maybe organizing this code differently in the first place)?

Answer 1

I am not sure there is one solution that fits all cases here. In the simple case (such as your example code) I would be tempted to simply re-test the total logic when I call the formatter - the complexity of refactoring / more complex usage, just isn't worth it.

However I understand that some cases may be non trivial, in which case I think you are correct that the crux of the problem is the usage of static methods, initially I see several options for refactoring:

1. Adapter

Create a class that wraps your entity and exposes a different interface - this is the option you described. I am acknowledging that it would work, but I don't like it for the same reasons you don't - you're only really creating it to support testing (it doesn't add to the clarity of the production code).

2. Utility Class

Simply move the static methods into their own class - where the methods are not static. This class would not have a reference to the entity. Hence the methods would still take the entity as a parameter.

This is slightly more memory efficient, since you can create a singleton / inject the utility class at runtime - but it still feels like a hack to support testing.

3. Encapsulate/Delegate the logic

If you inject the logic as part of the construction of the Entity class you can expose the methods on the entities themselves (which simply delegate to the logic class).

I like this, since it improves the interfaces of the entities (they directly provide the additional functionality).

4. Add logic to the interface

I am not sure if C# supports this, but in other languages you can add default implementations to interfaces, such that a class that implements the interface gets the default implementation from the interface.

Since mocks can be created off of the interface, you can test this code without needing to instance an entity - so if supported this would be my preferred solution.

Answer 2

You want to test the behavior of EntityRow. Perhaps the real code isn't so simple, but keep the fine-grained tests for GetTotalNumber. Rather than repeat every permutation of those tests for EntityRow, reduce the number of test cases to the minimum required to verify the outward behavior of EntityRow.

There might be a little duplication, but stay focused on the testing the logic in EntityRow rather than being worried about duplicate test cases. And once you have comprehensively tested EntityRow, stop. Don't hit the edge cases for GetTotalNumber. Those are already covered by tests focused on the extension method.

Answer 3

In your example, EntityRow and GetTotalNumberText don't allow to exchange or parametrize their internal logic. Hence it should be an implementation detail how they are implemented "under the hood" and that they might be using the same function GetTotalNumber internally. And as you surely know, tests should not incorporate too much knowledge about implementation details.

Hence, I think it is best to ignore the knowledge that the current implementation of those components might lead to re-testing some arithmetic. I would recommend to write your tests as if those methods are black boxes and ignore the fact you know that there exist some extra tests for GetTotalNumber alone.

The situation becomes different when GetTotalNumber is a very complex calculation, and your unit test setup for testing GetTotalNumber and GetTotalNumberText requires complex and somewhat duplicate logic, triggered by the underlying identical business logic. Of course, you can refactor that logic in your tests to some common method as well. However, in such a situation implementing GetTotalNumber as a static (extension) method is probably the wrong approach, and it should be replaced by something "mockable", by one of the approaches scetched in @DavidT's answer.