Skip navigation

Unit Testing Old Code

Brian Oliver - The C2 Group

The C2 Group

I was working on a legacy project recently – a project that has no unit tests whatsoever. Can I just say that jumping onto a project that doesn’t have a comprehensive suite of unit tests is one of the hardest tasks? (I know, I know… you’re going to answer with “No, I’m sorry, we don’t have time for you to say that right now.”) Unit tests are so incredibly valuable when it comes to long-term maintainability of a code base. The tests help document your system and provide reassurance that you can refactor the code to clean it up and the system will still work.

Luckily, the client has come to embrace this philosophy as well and agreed that as I work on the code I can clean and test as I go. This code is slowly improving, and it’s improving because I’m writing tests to expose additional bugs before I fix them. The new design philosophy is to loosely couple the components using principles of Onion Architecture and SOLID design; this makes it much easier to test components individually.

So, there I was with a bug to fix. Nothing was visible to the user of the web site, but under certain conditions I was catching a lot of XML parsing exceptions. The content authors were assigning plain HTML content blocks as “News Items” and not using the Ektron SmartForm. This meant that as I was mapping from Ektron’s TaxonomyItemData object into my NewsItem domain object, the code I had written to parse the XML SmartForm fields was failing. I was catching those errors, but I would rather have a clean error log.

Here’s the code that was failing:

XElement root = null;
try
{
    root = XElement.Parse(content);
}
catch (Exception ex)
{
    Elmah.ErrorSignal.FromCurrentContext().Raise(ex);
}

 

This was part of a slightly larger method. I know, I know; this violates the Singularity Principle. In other words, a function should do only one thing, and that “try” block counts as one thing. So we’ve identified that there’s plenty of room for refactoring, even without the existence of this XML parsing bug. My immediate concern, however, is (a) proving that this bug exists and (b) proving that I have fixed the bug.

To fix this, I needed to keep track of the exceptions. The existing code would produce a failing test, but the reason just felt wrong to me. It would fail because ELMAH would try to use the current HttpContext, and since there is no HttpContext when running unit tests, ELMAH would fail. I didn’t want to rely on ELMAH failing. I wanted a test that would fail on my assert. Ok. Time for the big guns. Time for MOQ.

I needed to mock the portion that called into the ELMAH library. To do that, I created a loose coupling between my mapper and the error logging. Enter the Error Logging Service!

public class ErrorLoggingService : IErrorLoggingService
{
	public void LogError(Exception exception)
	{
		Elmah.ErrorSignal.FromCurrentContext().Raise(exception);
	}
}

 

And, of course, the corresponding interface:

public interface IErrorLoggingService
{
	void LogError(Exception exception);
}

 

Now, instead of directly calling ELMAH from my mapper class, I modify the mapper to accept a parameter of type IErrorLoggingService in its constructor. The code I’m trying to test is now loosely coupled and completely testable.

XElement root = null;
try
{
	root = XElement.Parse(content);
}
catch (Exception ex)
{
	_errorLoggingService.LogError(ex);
}

 

So how do I test such a beast? Well, remember the big guns (a.k.a. MOQ)? With MOQ, I can wire up a mock implementation of the Error Logging Service that simply increments a counter every time an exception is logged.

int errorCount = 0;
var errorMock = new Moq.Mock<IErrorLoggingService>();
errorMock.Setup(error => error.LogError(It.IsAny<Exception>())).Callback(() => errorCount++);

// Some code left out...

Assert.AreEqual(0, errorCount);

 

Voila! I now have a test that fails because errorCount is equal to 1 when I’m expecting zero. I’ve proved the existence of the bug. Now I can make the test pass… the green portion of Red-Green-Refactor. I’ll leave that as an exercise for the reader. (I’ve always wanted to say that… I can’t tell you how many times my college professors would end a lecture with that line!)