Unit testing smells and best practices

One of the most difficult task I know as a software engineer is to write good unit test. Of course multi-threading and distributed systems are also challenging but in this post I want to share some of the good and bad things I've learning doing unit testing over the last decade.

In a series of posts I'll cover mistakes I made in the past and good techniques I learned along the way. Come back often as this list will grow over time.

The smells

• The non-public class
• The class constructor is not easy to call
• The method is not public

Unit testing smells: The method is not public

A recurring pattern emerge often when unit testing, you find an interesting method you want to test but that method is private. Of course you could call that method through other public methods on the class but it might not be easy to cover all cases. This usually is a design smell for a Single Responsibility Principle violation. More on this later.

For now, to properly test the private method we have the following options:

• Use reflection
• Change the method visibility to public or internal
• Extract the method to another class

Again, if you previously read my other post on non public class and inaccessible constructor you know you should avoid using reflection in tests.

Let's take a look at the other options

Change the method visibility to public or internal

The first thing we should do is to challenge why that method is private in the first place. Why can't we just change its visibility to public or internal (and use the InternalsVisibleTo attribute)?

This breaks encapsulation of the class and will let developers call the method directly in production code. That might not be what you intended to do but can help you in the short term. Abusing this technique will burn you in the long run so be careful with it.

Fortunately, there is a simpler way: extract the method to another class.

Extract the method to another class

If you are lucky, the method you want to extract is static and don't call any other method on the original class. You should be able to extract that method to a static utility class.

But what to do when the method calls other methods and use fields or properties on the class? For that we must analyze the class to extract responsibilities using the Single Responsibility Principle.

Single Responsibility Principle

Can you tell what is the purpose of your class in one sentence without using words like: and, but, also, or, etc.? If not then your class is doing more than one thing. Each segments of the sentence could be in different classes that only have one responsibility each. If the method you want to extract use the same fields than other methods you may want to push them by parameter instead before extraction to the other class.

Find more about the Single Responsibility Principle in an old post of mine.

Unit testing smells: The class constructor is not easy to call

When we want to test an instance method on a class the first challenge is to create an instance of that class. Hopefully the class constructor is easy to call but that's not always the case.

Let's review a few cases where the constructor might prevent us to write tests easily:

• The constructor is private or internal
• It requires too many arguments or it is too hard to create/provide those arguments
• The constructor calls external dependencies

The case of the internal or private constructor

For this we can follow the same advices than with testing non-public class. But whatever you do please don't use reflection to call the constructor.

Also stated in testing non-public class, you should ask yourself if it would be better to test this class via another public class that use it. A private constructor usually means that there is a factory somewhere you should use to instantiate the class. You should figure out a way to use that factory or redesign it in case you have difficulties to work with it in unit tests.

When dealing with internal access modifier I usually use the InternalVisibleTo attribute for my test project.

[assembly: InternalsVisibleTo("Contoso.MyApp.UnitTests.dll")]

The case of the difficult constructor arguments

If the problem is that the constructor requires too many arguments then you should ask yourself if this class is too big, maybe it has too many responsibilities and don't follow the Single Responsibility Principle? In that case the class should be splitted into two or more classes. This way it will be easier to test the smaller class because its constructor should require less arguments.

Another case is when the arguments are difficult to create or provide. Maybe that in order to create one object to pass as parameter we need to create yet more objects. Micheal Feathers calls it the Onion Parameter in his book Working Effectively with Legacy Code. In that case I usually invest into a bit of testing infrastructure like factory methods or Test Data Builder in order to help me write tests faster afterwards.

Finally some parameters could be classes that wrap services or external dependencies. In that case I abstract the dependency using an interface and the Inversion of Control Principle. First extract an interface from the class. Next change the constructor to use this interface instead of the class. Finally use a mocking framework like Moq to create a fake (or create your own by hand if you don't like mocking frameworks) and pass it to the constructor.

public class MyService
{
    // public MyService(SomeDataAccessLayer dal, SomeExternalService externalService)
    public MyService(ISomeDataAccessLayer dal, ISomeExternalService externalService)
    {
        // ...
    }

    // ...
}

The case of the external dependencies calls

When the constructor itself calls an external dependency I usually refactor the constructor to inject this dependency via a parameter instead, this is called dependency injection and usually comes with the Inversion of Control Principle. This is also a good occasion to take a look at a IoC Container like Unity.

public class MyService
{
    // public MyService()
    public MyService(ISomeExternalService externalService)
    {
        // ...
        // var externalService = new ExternalService();
        externalService.CallService();
    }

    // ...
}

Unit test smells: The non-public class

Writing unit tests for a method on a class that is not public is doable but not straight forward. It could be done using a bit of reflection like this

var type = Type.GetType("MyProject.MyClass");
var methodInfo = type.GetMethod("TheMethod");

var classInstance = Activator.CreateInstance(type, null);
methodInfo.Invoke(classInstance, null);

That is quite a bit of work. Of course you could create some test infrastructure to avoid duplication in every tests or you may find libraries online for that. But still, writing tests like this smells funny to me.

In short, you should never write tests directly against a non-public class.
Let's take a look at the 2 possible cases for this: the private class and the internal class.

The case of the private class

For a class to be private means it is nested inside another class

public class A
{
    // ...

    private class B
    {
        // ...
    }
}

In a situation like this class B can only be used by class A. Anything class B do is only for serving class A. If you want to test a method in class B you should find out how class A use class B and write your tests against class A public API.

If you find there is no way to reach the method you want to test in class B through class A it simply means that you just found dead code! If you found a way but find it too hard to setup a test then maybe class B is not simply a private utility class. In that case I would extract class B from inside class A and change its access modifier to internal. Unfortunately, now any class inside the same assembly could use class B. It is a trade-off I'm willing to pay because this case is really rare and C# still lack a proper access modifier scoped to the current namespace.

The case of the internal class

Internal class could also be tested through reflection. Personally I prefer to use the InternalsVisibleToAttribute and give access to internals types to my unit test project. To do it you need to add the attribute to the project under test AssemblyInfo.cs file like this

[assembly: InternalsVisibleTo("Contoso.MyApp.UnitTests.dll")]

If your project is signed you will need to also provide the assembly's public key like this

[assembly: InternalsVisibleTo("Contoso.MyApp.UnitTests.dll, PublicKey=1234...789")]

Again, this is a trade-off: you give special access to your internal types only for testing but it is way better than changing the class access modifier to public. You should never change a type access modifier to public for testing!

Conclusion

I always try to write tests against public classes and methods. Sometime an internal class will grow to be very complex over time and become its own new component. In a solution were we create a lot of small projects we would simply create a new one for such component and expose it publicly but that is not what I do. I usually try to create the minimal number of projects in my solution so testing internal classes using the InternalsVisibleToAttribute is a good trade-off for me.

Windows Azure Caching and transient faults

When using remote services over the wire we should always plan for transient failures. Windows Azure Caching like any services in an Azure world is prone to such problem. Out of the box making calls to the cache server will fail from time to time due to network issues. Typically you will get those kind of exceptions:

Microsoft.ApplicationServer.Caching.DataCacheException: ErrorCode<ERRCA0017>:SubStatus<ES0006>:There is a temporary failure. Please retry later.

Microsoft.ApplicationServer.Caching.DataCacheException: ErrorCode<ERRCA0018>:SubStatus<ES0001>:The request timed out.

Microsoft.ApplicationServer.Caching.DataCacheException: ErrorCode<ERRCA0016>:SubStatus<ES0001>:The connection was terminated, possibly due to server or network problems or serialized Object size is greater than MaxBufferSize on server.

For that reason it is a best practice to implement some kind of retry logic around your code calling the cache server. We could have used the Transient Application Block to manage that. But a few months ago I found somewhere that from time to time the DataCache object lose it's internal connection to the cache server. A simple way to fix this is to re-create a DataCache instance and retry the operation.

In the implementation below I'm keeping a reference to the DataCacheFactory and DataCache objects (another best practice). The CreateDataCache factory method will come handy later.

public class CachingService
{
    private DataCacheFactory cacheFactory;
    private DataCache cache;
   
    private DataCache Cache
    {
        get
        {
            if (this.cache == null)
            {
                this.CreateDataCache();
            }

            return this.cache;
        }
    }

    private void CreateDataCache()
    {
        this.cacheFactory = new DataCacheFactory();
        this.cache = this.cacheFactory.GetDefaultCache();
    }

    // ...
}

Then I have this SafeCallFunction I use whenever I want to work with the DataCache object. Notice that the only thing I do to retry the operation is to call the factory method to re-create the DataCache object.

private object SafeCallFunction(Func<object> function)
{
    try
    {
        return function.Invoke();
    }
    catch (DataCacheException)
    {
        // Retry by first re-creating the DataCache
        try
        {
            this.CreateDataCache();
            return function.Invoke();
        }
        catch (DataCacheException)
        {
            // Log error
        }
    }

    return null;
}

Finally in the rest of the class I can use the SafeCallFunction like this

public object CacheGet(string key)
{
    return this.SafeCallFunction(() => this.Cache.Get(key));
}

public void CachePut(string key, object cacheObject)
{
    this.SafeCallFunction(() => this.Cache.Put(key, cacheObject));
}

public void CacheRemove(string key)
{
    this.SafeCallFunction(() => this.Cache.Remove(key));
}

So far after a few weeks of using this implementation the single retry never failed on us. Before that we had around 5-10 failures daily for about 500k calls to the cache server. I would still recommend using a more robust retry policy with Windows Azure Caching but I think it's interesting to know that simply instantiating a new DataCache can fix most failures.

References

Caching in Windows Azure
Best Practices for using Windows Azure Cache
Optimization Guidance for Windows Azure Caching
The Transient Fault Handling Application Block

Using Windows Azure Caching efficiently across multiple Cloud Service roles

Windows Azure Caching is a great way to improve performance of your Azure application at no additional cost. The cache is running alongside your application in Cloud Service roles. The only thing you need to decide is how much memory of the role you want use for it (for co-located cache role). You can also dedicate the all memory of a role to caching if you want (with dedicated cache role). Roles that host caching are called cache clusters.

Starting with Azure Caching is so easy that it can be a while before you fully understand the best way to use it. On a recent project my first tough was to enable caching on all the Cloud Service roles as co-located service. This was causing us problems.

First of all, been a developer I debug my application using the local Azure compute emulator. The emulator runs one cache service for each instances of roles with cache clusters. The application has 2 web roles and 1 worker role so when I start a debugging session with multiple instances per role I need a lot of memory to run everything. More importantly, cache clusters do not share cached data between each other. This caused us to have stale data in the application.

That is when I figured out that I needed to read a bit more on Azure Caching if I was to use it efficiently.

Understanding cache clusters

When you enable caching on an Azure role each instance of that role will run a cache service using a portion of the memory (or all of it if it's a dedicated cache role). The cache services running on each instance of a single role are managed as a single cache cluster. Cache services can talk to each other and synchronize data but only inside the same cache cluster (same role). That is why enabling caching of many roles might not be the best thing to do.

Another thing to mention is that cache clusters can only be created on small role instances or bigger. The reason is that with extra small instance you only get 768MB of RAM which is pretty much all used up by anything you run on those instances.

Now the enable a cache cluster on your role go to the role property page on the Caching tab.

Here you will notice that I also enabled notifications which will allow us to efficiently use local caches later.

For more information on the different configuration options for cache clusters go here.

Configuring roles to use cache clients

Now that we took care of the server side of caching configuration let's talk about the client side. Each instances of each roles inside the same cloud deployment can connect to a cache cluster. If you run only one cluster then you are guarantied to access the same cached data from whatever role you are inside your application (as long you have a valid configuration).

One nice feature we can enable in each role configuration is the local cache client. With this we can cache data locally in a role instance memory the data we recently fetched from the cache cluster for even faster access. Remember the Notification option we enabled on the server side? Using the configuration below in the Web.config or App.config of your role will ensure data stored in the local cache client gets updated whenever the cache server version of that data changes. Basically, the local cache client will invalidate data based on notifications received from the cache cluster.

For more information on client side configuration go here.

Other concerns

This post is only about an overview of the consideration of running multiple clusters versus a single one. Using Azure Caching there are a lot more configuration options you need to take a look at here. Also really important is how to use Azure Caching in your application.

Conclusions

I've spend a lot of time figuring out how all of this was working. I hope this post will help you with your learning experience.

Other useful links

• Caching in Windows Azure
• Optimization Guidance for Windows Azure Caching
• Best Practices for using Windows Azure Cache