Unit testing – Making existing code testable

Posted on by

Introduction

Unit testing (and integration testing) is something every developer knows they should do, but quite often it can fall by the wayside – and I’m very guilty of that. When you start your a fresh project, it’s simple to design your application in a manner that will make unit testing easy, but what about when you want to test existing code Obviously, the answer completely depends on the code itself. However, from experience a lot of code out there is rigid and tightly coupled to other parts of the application, and I’ve found it nigh-on impossible to write any automated test for it.

I was recently tasked with implementing a new feature in one of our applications, one which already involves a ton of business logic. I decided I wanted to use this opportunity to make my code testable, and at the same time refactor my existing codebase to allow for unit testing.

Awful ‘legacy’ code

This is a contrived example, and has a lot of code smells, but please humor me on this! This doesn’t differ much from typical code in our codebase. It does the job, but it’s almost impossible to unit test

public class Importer : IDisposable
{
    public const string FILE_DIRECTORY = @"C:\Test\Files";
    private DBEntities _dataContext = new DBEntities();

    public void Import()
    {
        var files = System.IO.Directory.GetFiles(FILE_DIRECTORY, "*.csv");
        foreach (var file in files)
        {
            var lines = System.IO.File.ReadAllLines(file);
            foreach (var line in lines)
            {
                var split = line.Split(',');
                var newOrder = new Order
                {
                    OrderNumber = line[0],
                    CustomerName = line[1],
                    OrderValue = decimal.Parse(line[2])
                };
                _dataContext.Orders.Add(newOrder);
            }

            _dataContext.SaveChanges();
        }
    }

    public void Dispose()
    {
        _dataContext?.Dispose();
    }

}

Why is it hard to test?

It’s all well and good for me to say that it’s hard to test, but for a lot of people it may not be immediately clear why. So here’s a bit of a breakdown

Monolothic

Due to the structure of the class, we are unable to import a file directly – so the only way we can test at the moment is to put a file in the directory defined in the constant, and run the code. And even then, there’s a danger it may pick up files up while we’re testing.

Database dependency

We are using Entity Framework to connect to a SQL Server database, which is naturally an external dependency. If we want to run a test, we’d need to have it connect to the database and somehow clear itself up at the end.

File system dependancy

We are looking directly at our workstations file system, which again is an external dependency. Unit testing is all about isolating these external dependencies to allow us to test a block of code with no reliance on any other parts of the system. If we were to unit test with our real file system, there would end up being lots of prerequisites before we can run our tests (ensuring test import files are in the right directory).

Single Responsibility Principle

As the name implies, unit testing is about testing units of code. To be able to do this, your codebase needs to be modular. So the first we need to do is refactor this code out into methods which obey the Single Responsibility Principle, where our methods should only one responsibility. See below code that I have refactored

public class Importer : IDisposable
{
    public const string FILE_DIRECTORY = @"C:\Test\Files";
    private DBEntities _dataContext = new DBEntities();

    public void ImportAllFromDirectory()
    {
        var files = System.IO.Directory.GetFiles(FILE_DIRECTORY, "*.csv");
        foreach (var file in files)
        {
            ImportFile(file);
        }
    }

    public void ImportFile(string filePath)
    {
        var lines = System.IO.File.ReadAllLines(file);
        foreach (var line in lines)
        {
            var split = line.Split(',');
            var newOrder = new Order
            {
                OrderNumber = line[0],
                CustomerName = line[1],
                OrderValue = decimal.Parse(line[2])
            };
            _dataContext.Orders.Add(newOrder);
        }
        _dataContext.SaveChanges();
        System.IO.File.Delete(filePath);
    }

    public void Dispose()
    {
        _dataContext?.Dispose();
    }

}

Better! However, we still have some major problems with this, specifically due to the external dependencies we have in the code. We cannot unit test this code unless we have a way of ‘faking’ or ‘mocking’ these external dependancies

Interfaces & dependency injection

I’m going to assume here that you have some knowledge of interfaces – if you don’t, I suggest reading this great post which explains the basics of interfaces.

Interfaces provide us with a great way of being able to abstract away some of our dependencies. We can create an interface that tells our code what anything implementing it can do, and create 2 implementations – our real one, and a mock one used for testing (in practice you may have several real implementations and several mock implementations, but I’ll avoid getting into that complexity.)

Database access

The first of our dependencies is the access to the database. Our code does two things with our Entity Framework model – we add an item to the Orders table, and then we call SaveChanges to push our changes to the live DB. So let’s create an interface that has these two operations. I’ll also implement our ‘real’ version of the interface, which uses our Entity Framework data context. You may wonder why I also have a GetAllOrders method if we’re not using it – this will become apparent later.

public interface IOrderAccess : IDisposable
{
    void AddOrder(Order order);
    IQueryable<Order> GetAllOrders();
    void SaveChanges();
}
public class OrderAccess : IOrderAccess
{
    private DBEntities _dataContext = new DBEntities();
    public void AddOrder(Order order)
    {
        _dataContext.Orders.Add(newOrder);
    }

    public IQueryable<Order> GetAllOrders()
    {
        return _dataContext.Orders;
    }
    public void SaveChanges()
    {
        _dataContext.SaveChanges();
    }

    public void Dispose()
    {
        _dataContext?.Dispose();
    }
}

Great! Now we need a mock implementation to help us with our unit tests, so that our unit tests don’t need to touch the database. You may think it’s going to be really complicated to mock, but it’s really not. We’re not replicating everything a data context does, we’re mocking the small subset that our interface defines – the crucial ones being AddOrder, which adds to a list that we use as our ‘table’, and GetAllOrders which gives us access to said list.

public class MockOrderAccess : IOrderAccess
{
    private bool _saveCalled = false;
    private List<Order> _orders = new List<Order>();
    public void AddOrder(Order order)
    {
        _orders.Add(order);
    }
    public IQueryable<Order> GetAllOrders()
    {
        return _orders.AsQueryable();
    }

    public void SaveChanges()
    {
        _saveCalled = true;
    }
    public void Dispose()
    {
        //Nothing to dispose
    }
}

Depending on what you want to test, you may need some cleverness with mocking when SaveChanges is called in relation to when an order is added, but I won’t get too wrapped up in that.

We can then modify importer class as below to use our new abstraction

public class Importer : IDisposable
{
    public const string FILE_DIRECTORY = @"C:\Test\Files";
    private DBEntities _dataContext = new DBEntities();
    private IOrderAccess _dbAccess = new OrderAccess();

    public void ImportAllFromDirectory()
    {
        var files = System.IO.Directory.GetFiles(FILE_DIRECTORY, "*.csv");
        foreach (var file in files)
        {
            ImportFile(file);
        }
    }

    public void ImportFile(string filePath)
    {
        var lines = System.IO.File.ReadAllLines(file);
        foreach (var line in lines)
        {
            var split = line.Split(',');
            var newOrder = new Order
            {
                OrderNumber = line[0],
                CustomerName = line[1],
                OrderValue = decimal.Parse(line[2])
            };
            _dbAccess.AddOrder(newOrder);
        }

        _dbAccess.SaveChanges();
        System.IO.File.Delete(filePath);
    }

    public void Dispose()
    {
        _dbAccess?.Dispose();
    }
}

Great! Well, no – we’re explicitly creating the ‘real’ implementation of our OrderAccess class. That puts us back into the same point – it’s always going to access the database, and our mock implementation will never be used. The answer to this is dependency injection

Enter dependency injection

Dependency injection will allow us to pass through the implementation we want to use. This means that our real code can pass through the real OrderAccess class, and our unit tests can pass through a mock version! Note that I have provided two constructors – one that takes an IOrderAccess object (our implementation we want to use), and a parameterless constructor which will always use the real version. This saves us having to always pass through our implementation in our real code. 

public class Importer : IDisposable
{
    public const string FILE_DIRECTORY = @"C:\Test\Files";
    private DBEntities _dataContext = new DBEntities();
    private IOrderAccess _dbAccess;

    public Importer() : this(new OrderAccess())
    {
    }

    public Importer(IOrderAccess dbAccess)
    {
        if (dbAccess == null)
            throw new ArgumentNullException(nameof(dbAccess));

        _dbAccess = dbAccess;
    }

    public void ImportAllFromDirectory()
    {
        var files = System.IO.Directory.GetFiles(FILE_DIRECTORY, "*.csv");
        foreach (var file in files)
        {
            ImportFile(file);
        }
    }

    public void ImportFile(string filePath)
    {
        var lines = System.IO.File.ReadAllLines(file);
        foreach (var line in lines)
        {
            var split = line.Split(',');
            var newOrder = new Order
            {
                OrderNumber = line[0],
                CustomerName = line[1],
                OrderValue = decimal.Parse(line[2])
            };
            _dbAccess.AddOrder(newOrder);
        }

        _dbAccess.SaveChanges();
        System.IO.File.Delete(filePath);
    }

    public void Dispose()
    {
        _dbAccess?.Dispose();
    }
}

And there we go – we have now isolated our database access to allow for unit testing! However, we’re not done yet. We still need to abstract away our file system access to maximize our unit testing benefits.

File system access

I’ve already gone through the considerations when abstracting some functionality out to an interface and creating mock implementations, so I’ll cut to the chase and share the file system code below. Note the mock file system implementation here isn’t very good – you probably want something generic, that allows you to add files without hard coding the file contents in the class. But read on until the end for a solution to this…

public interface IFileAccess
{
    string[] GetFiles(string directory, string pattern);
    void Delete(string file);
    string[] ReadAllLines(string file);
}
public class FileAccess : IFileAccess
{
    public string[] GetFiles(string directory, string pattern)
    {
        return System.IO.Directory.GetFiles(directory, pattern);
    }

    public void Delete(string file)
    {
        System.IO.File.Delete(file);
    }
    public string[] ReadAllLines(string file)
    {
        return System.IO.File.ReadAllLines(file);
    }
}
public class MockFileAccess : IFileAccess
{
    List<string> _mockFiles = new List<string> { "valid.csv", "invalid.csv" };

    public string[] GetFiles(string directory, string pattern)
    {
        return _mockFiles.ToArray();
    }

    public void Delete(string file)
    {
        _mockFiles.Remove(file);
    }
    public string[] ReadAllLines(string file)
    {
        if (file == "valid.csv")
        {
            return "AAA000,Chris St Clair,9.90";
        }
        else if (file == "invalid.csv")
        {
            return "AAA000,Chris St Clair";
        }
        throw new FileNotFoundException(file);
    }
}

And now our new, totally modular Importer class

public class Importer : IDisposable
{
	public const string FILE_DIRECTORY = @"C:\Test\Files";
	private DBEntities _dataContext = new DBEntities();
	private IOrderAccess _dbAccess;
	private IFileAccess _fileAccess;
	
	public Importer() : this(new OrderAccess(),new FileAccess())
	{
	}
	
	public Importer(IOrderAccess dbAccess, IFileAccess fileAccess)
	{
		if (dbAccess == null)
			throw new ArgumentNullException(nameof(dbAccess));
			
		if (fileAccess)
			throw new ArgumentNullException(nameof(fileAccess));
			
		_dbAccess = dbAccess;
		_fileAccess = fileAccess;
	}
	
	public void ImportAllFromDirectory()
	{
		var files = _fileAccess.GetFiles(FILE_DIRECTORY, "*.csv");
		foreach (var file in files)
		{
			ImportFile(file);
		}
	}
	
	public void ImportFile(string filePath)
	{
		var lines = _fileAccess.ReadAllLines(file);
		foreach (var line in lines)
		{
			var split = line.Split(',');
			var newOrder = new Order
			{
				OrderNumber = line[0],
				CustomerName = line[1],
				OrderValue = decimal.Parse(line[2])
			};
			_dbAccess.AddOrder(newOrder);
		}
		
		_dbAccess.SaveChanges();
		_fileAccess.Delete(filePath);
	}
	
	public void Dispose()
	{
		_dbAccess?.Dispose();
	}
}

And there we have it – our new Importer class which is fully unit testable! Which brings us on to…

Typical unit tests

So now our code is pretty unit testable – what should we actually test? The obvious answer is the success path – see below.

[TestMethod]
public void Valid_File_Should_Import()
{
	using (IOrderAccess orderAccess = new MockOrderAccess())
	{
		using (var importer = new Importer(orderAccess, new MockFileAccess())
		{
			importer.ImportFile("valid.csv");
		}
		
		Assert.AreEqual(1, orderAccess.GetAllOrders().Count());
	}
}

We can also mock what happens if you pass in a file that doesn’t exist as below – the real file system implementation will throw a FileNotFoundException, and we’ve made our mock file system do that too, so we can have a test like the below

[TestMethod]
[ExpectedException(typeof(FileNotFoundException)]
public void Non_Existent_File_Should_Throw()
{
	using (IOrderAccess orderAccess = new MockOrderAccess())
	{
		using (var importer = new Importer(orderAccess, new MockFileAccess())
		{
			importer.ImportFile("notarealfile.csv");
		}		
	}
}

You may have noticed in my mock file system, I have an ‘invalid.csv’ which returns a CSV with only 2 columns. This is a perfect case for a unit test! Currently, it would throw an IndexOutOfRangeException, but for the sanity of your users you’ll want to do something a bit ‘nicer’ such as throw a more meaningful exception. Your unit test can assert that the exception was thrown, and you can also test to confirm no Orders have been added to our ‘database’ (our mock one!)

Test-driven development

You can imagine that there is a heck of a lot of possible scenarios that you can cover in your tests. Whatever you think of, it’s worth writing a test for it – even if you think it may never happen, you’ll know what your software will do if it does ever happen.

I’m not going to delve into detail here, but there’s a lot to be said for adopting the test-driven development approach, which fundamentally means writing your tests first and as little code as you can. This forces you to think about the scenarios upfront, and will likely greatly reduce the number of bugs you’ll have. It is difficult (in my opinion at least) to adopt this methodology, simply because it can more time consuming. But do as I say, not as I do!

Better mocking and dependency injection

While our mock classes do the job, they are not exactly flexible – especially the file system access. Do we really have to switch statements for each file we want? The answer is no – we don’t have to reinvent the wheel, as the fantastic System.IO.Abstractions exists, which includes interfaces for pretty much the entire System.IO namespace, and it also includes mock implementations which basically store a mock file system in memory, allowing you to add files to it in unit tests. How cool is that?! I implore you to check it out, and to use it

In terms of our database access mocking, our mock is much cleaner than the one we have for the file system. What we’ve implemented is a basic and rough version of the repository pattern , of which there is some controversy over whether it provides a needless distraction over data access. I’m on the fence on this to be honest, but you’ll be glad to know that mocking your data access with Entity Framework can be done in other ways too – in fact, you don’t even need to abstract your data access onto a new class if you really don’t want to. This can be done using the fantastic Moq library – there’s a great tutorial here which will explain in detail how to do this. If you’re using .NET Core, you can actually have your Entity Framework connection point to an in memory collection of objects. Very cool, although I’ve not tried this myself.

Also, what we’ve done in these examples is what is known as poor mans dependency injection. This is because we go through the process of passing through the correct implementation manually, or we have a parameterless constructor which uses a ‘default’ implementation, which can up end up with a lot of code repetition where you ‘new up’ the default implementation.

There’s a huge amount of frameworks out there that promise to make dependency injection, and most certainly live up their promises. Again, this is beyond the scope of this article as there are plenty of comparisons of these frameworks out there. I would say that I’ve used Ninject and Autofac and they are great!

Conclusion

So that concludes this article, and I hope at least someone reading this has found this useful! Unit testing can be difficult to get your head around , especially if your code base is especially monolithic, but the benefits usually heavily outweigh the pain of refactoring your code to be more modular.

error