Documenting smells for untestable .NET code with NSubstitute and NUnit

Retrofitting unit tests is hard work and the knowledge about the code could easily be forgotten until it will be refactored. We want to communicate to the developers (and to ourselves after enough time has passed to forget what we've learned about the code) our testing intentions, how the code could be more testable.

One of the unit testing axioms is: always keep production and test code separate. Now if you program in .NET/C# and create a separate test project, then friend assemblies or this pattern can help you to overcome the visibility problem: if I can only access the public interface of my code, there will be much unaccessible code that I still need to test.

But if you work with legacy code there will likely access much untestable code. Tight coupling, hidden dependencies, implementation dependency and many many others are common problems you would normally conquer by refactoring your code (see e.g. Michael C. Feather´s Dependency Breaking Techniques (Part III of his book)).

Sometimes though, you won't have the permission to change the production code. Sometimes you just need to document the code for future reference and prepare test cases so the knowledge you've built up won't get wasted until the code has been refactored..

In this world of retrofitting tests to legacy code you will often find it useful to document the smells or pathologies that make your testing so hard resp. impossible. Instead of writing bug reports, source comments, etc. you might consider next time the following pattern.

For example, imagine you have the following code you're not allowed to make changes to:

public class HasDependencyToImplementation{
  
  public HasDependencyToImplementation(Class c)
  {
    this.c = c; 
    ...
  }

  public void DoSomethingWithC(){
    this.privateProperty = this.c.DoSomething();
    this.PublicProperty = this.WorkWithPrivateProperty();
  }
  
  private T WorkWithPrivateProperty(){
    ... do something with this.privateProperty ...
  }

}

public class Class{...}

Now, if you need to take control over the method Class.DoSomething, you'll normally be advised to extract the interface of Class. Then the constructor admits passing it a mocked Class instance. But what if you're not allowed to do that? Do you want to call the developer to please refactor that code and check in again? Is your developer even in the mood or has time to listen to your pleas? Let's guess they won't. Then with NSubstitute and NUnit you would normally still be able to write a (failing) test like this:

...
[Test]
public void DoSomethingOnC_ChangesState(){
   var c = Substitute.For<Class>();
   var controllableValue = getValue();
   c.DoSomething().Returns(controllableValue);
   
   var hasDependency = new HasDependencyToImplementation(c);
   hasDependency.DoSomethingWithC();
   var expected = getExpectedFor(controllableValue);

   Assert.That(
       hasDependency.PublicProperty,
       Is.EqualTo(
       expected);
}

This test would normally compile, but by running it, we will get an exception from NSubstitute as Class is not abstract and the method Class.DoSomething isn't virtual, so NSubstitute cannot overwrite it. Our problem is: on one hand we want this test to be written for documentation and future testing, after refactoring is done, on the other hand the refactoring developers won't necessarily be able to interpret the test result as a todo for them and you might forget what the problem was until next time you'll work with the code. A solution to this problem might be the following:

We implement a custom assert on NUnit.Framework.Assert:

public static void HasSmell(int PRIO, string PROBLEM, string PROPOSAL){
        Assert.Fail(
                 helper.GetMessage(PRIO, PROBLEM, PROPOSAL));
}

and implement constants

public static Smells{
   public const string IrritatingParameter = "Irritating parameter";
   ...
}

public static Refactorings{
   public const string ExtractInterface = "Extract interface";
   ...
}

Then we can add documentation for developers:


...
[Test]
public void DoSomethingOnC_ChangesState(){

   Assert.HasSmell(
          PRIO: HIGH,
          PROBLEM: Smells.IrritatingParameter
                  +"\nClass c: cannot be mocked for checking.",
          PROPOSAL: Refactorings.ExtractInterface);

   var c = Substitute.For<Class>();
   var controllableValue = getValue();
   ...
}

Which not only will fail when run by developers, but the NUnit runner will tell them

Test DoSomethiongOnC_ChangesState failed:
SMELL PRIO HIGH
PROBLEM: Irritating parameter
Class c: cannot be mocked for checking.
PROPOSAL: Extract interface