When is it not worth writing a Unit Test?

Last Updated: October 1, 2015 | Created: October 1, 2015

I’m currently working on a e-commerce site based on ASP.NET MVC 5 for a new venture (in stealth mode, so I can’t say what it is). I have been working on it for over six months and we set a target date to get a new Beta2 version of the site up by end of September (yesterday).

That puts a bit of pressure on development and I had a couple of interesting decisions around Unit Testing that I thought I would share.

I also include some quotes from one of the giants of Software Design, Martin Fowler, and at the end I give some criteria to help you decide what to test, and maybe what you might not test.

Setting the scene

I expect everybody who comes to this article will know about Unit Testing (here is link to a definition in case you are new to this). I also expect you have an opinion, ranging from “you need to have 100% test coverage” through to “Unit Testing is a waste of valuable development time”.

All my articles are based on my real-life experiences. In this one I found, just before the deadline, that a small piece of code took maybe 10 minutes to write and 1+ hours to write the Unit Tests. On the other had I realised two days earlier I had written another piece of code which took about the same time to write, but I didn’t write any Unit Tests for it.

On reflection I decided that both decisions were correct. In this article I go through my reasoning to see why I think this way. You may disagree with my reasons (please do!), but hopefully I will make you think.

Before I start let me give you my first quote from Martin Fowler. If you read my history you will see I came back to programming in 2009 after a 21 year break. I gobbled up all the reading I could find, and Martin Fowler was a great help to put me on a better track.

It is better to write and run incomplete tests than not to run complete tests.

Martin Fowler in the book ‘Refactoring’

1. Code I tested

Below is the code I wrote in about 10 minutes, which I then took over an hour to test. To save you reading it the name of the method says what it does, i.e. if the justification of a field is changed, say from Left to Centre then it changes the x position to match.

private void HandleChangeOfJustifyAndCompensatePosition(
    FieldJson fieldJson, CrudAdvancedStylesDto dto)
{
    if (fieldJson.styleInfo.justify != dto.Justify)
    {
        //The justification has changed, so we compensate the position

        if (fieldJson.styleInfo.rotation != 0 || dto.Rotation != 0)
            throw new NotImplementedException(
            "HandleChangeOfJustifyAndCompensatePosition does not currently support rotation.");
        if (fieldJson.styleInfo.shear != 0 || dto.Shear != 0)
            throw new NotImplementedException(
            "HandleChangeOfJustifyAndCompensatePosition does not currently support shear.");

        var leftX = 0.0;
        switch (fieldJson.styleInfo.justify)
        {
            case JustifyOptions.Left:
                leftX = fieldJson.xPosMm;
                break;
            case JustifyOptions.Center:
                leftX = fieldJson.xPosMm - (fieldJson.widthmm/2);
                break;
            case JustifyOptions.Right:
                leftX = fieldJson.xPosMm - fieldJson.widthmm;
                break;
            default:
                throw new ArgumentOutOfRangeException();
        }

        switch (dto.Justify)
        {
            case JustifyOptions.Left:
                fieldJson.xPosMm = leftX;
                break;
            case JustifyOptions.Center:
                fieldJson.xPosMm = leftX + (fieldJson.widthmm / 2);
                break;
            case JustifyOptions.Right:
                fieldJson.xPosMm = leftX + fieldJson.widthmm;
                break;
            default:
                throw new ArgumentOutOfRangeException();
        }
    }
}

Now, you might ask, why did it take me so long to write a test for such a small method. Well, it is a private method so I can only get to it via the business logic that uses it (Note: I really don’t want to change the natural design the the software to make this method easier to test. I have found that leads to all sorts of other problems).

So, why did the tests take so long? Here is a quick summary:

  • The new method added to an existing existing piece of business logic that already had quite a few tests, so it wasn’t a green field site.
  • The previous tests needed a significant amount of set-up, especially of the database, to  run. These needed changing.
  • Because the original tests had been written some time ago they didn’t use the new database mock I had developed, which would help with the new tests. I therefore decided to move all the tests in this group over to the mock database.
  • The Mock database didn’t have some of the features I needed so I had to add them.
  • … I could go on, but I am sure you get the picture.

This leads me onto a key point.

I find 80%+ of the work of writing Unit Tests in about getting the right set-up to allow you to exercise the feature you want to test.

So let me explore that.

Set up of the test environment

In any significant software development there is normally some sort of data hierarchy that can be quite complex. Often to test say one bit of business logic you need to set up other parts of the system that the business logic relies on. When this is stored on a database as well then it makes setting up the right environment really complex.

This might sound wrong for Unit Testing, which is about isolating sections of code and just testing that, but I find in the real world the lines are blurred. In the example code about I really don’t want to make a separate class, which I inject at runtime, just to isolate that method for testing. IMHO that leads to bad design.

Over the years I have tried lots of approaches and here is what I do:

1. Write ‘Set-up Helpers’ to create data for Unit Tests.

My test code is has a Helpers directory with a range of Helper classes (27 so far in this six-month+ project) which create data suitable for testing specific parts of the system. In the past I skimped on these, but I soon learnt that was counter-productive.

Most build the data by hand, but a few load it from json or csv files held in the Test project.

2. Use mocks where you can.

Mocks, or fakes, are pieces of code that replace a more complex subsystem or class. Mostly they replace a system library, but sometimes replace some of your own code so you can trigger thing that the normal code would rarely do, i.e. error conditions.

Mocks/fakes are great and I used them as much as I can. They can provide really good control of what goes on for a small about of work. I personally use Moq, which I find good.

However there are some limits to replicate all the feature of a big system like the Entity Framework ORM (EF), or some of the MVC Controller systems. I do have a mock for Entity Framework, which is pretty good, but it doesn’t do relational fix-up and current doesn’t support .Include().

2. Decide on how you will use a real database

I have found out the hard way that I need to use a real database for testing. There is down side to that of testing taking a lot more time to start (EF6 has a fairly long start-up time). However I have found, up to now (see comment at end of this section), the only way to truly check some code out.

There are a number of ways you can go, from clearing and reloading the database for every test up to just adding to the database. I have found clearing and reloading pretty difficult/time consuming, especially with complex foreign keys, so I use unique strings, via GUIDs, in strategic places to create a unique set of entries for a test.

Whatever way you go you need to think through a system and keep to it, otherwise thinks get messy.

Possible future options on EF?

  • Two separates readers of this blog have recommended Effort Library, which provides an in-memory EF database. That sounds like a great idea, but once I am in a project I try not to make big changes to the system so I haven’t tried it yet, although EF 7 may be better (see next).
  • Entity Framework 7 will have an in-memory database, which sound perfect for Unit Testing. I definitely plan to look at that.

Another Martin Fowler quote:

I’ve often read books on testing, and my reaction has been to shy away from the mountain of stuff I have to do to test. This is counterproductive…

Martin Fowler in the book ‘Refactoring’

The Unit Test framework has a BIG effect

I have found that the Unit Test framework is crucial. I started using NUnit many years ago and found it great, especially when run from inside Visual Studio via Resharper. However, when I started testing JavaScript I found other Unit Testing frameworks had some really useful features.

I will only summarise my thoughts on this because I wrote a long article called Reflections on Unit Testing in C# and JavaScript, which gives more detail. However let me give you the places where NUnit skews my Unit Testing approach.

NUnit does not have nested set-up calls

As I said earlier I find most of the work goes into setting things up to run a test. NUnit has a FixtureSetup/FixtureTearDown, which is run once at the start/end of a group of tests in a class, and a Setup/TearDown that is run before/after each test. These are useful, but not sufficient.

JavaScript Unit Test frameworks, like like Mocha and Jasmine , have a BeforeEach and AfterEach, which can be nested. Now why is that useful? I find that for some of the more complex methods to test I want to check multiple things when it is finished, e.g. was it successful, did it return the right data and did it change the database in the way I expected.

In Mocha and Jasmine I would have a group of tests with maybe an outer BeforeEach that sets up the environment and then an inner BeforeEach which calls the method, followed by separate tests for each of the parts I want to check. This means that a) the setup code is in one place, b) the call to the item under test is in one place and c) the tests are separately shown.

In NUnit I tend to find I either duplicate code or test lots of things in one test. (Note: If this doesn’t make sense then there is an example of nested set-ups in my article Reflections on Unit Testing in C# and JavaScript).

Possible future options:

One of my readers,

NUnit does not support async Setup/TearDowns

.NET 4.5’s async/await is a big change – see my full article on async/await for more. I find I use async/await in 90%+ of my  database calls and maybe 80% of my business logic. NUnit supports async tests, but not the FixtureSetup/FixtureTearDown or Setup/TearDown. That causes me to have to put more setup code in the actual tests.

2. The code I didn’t test

We have been off on a journey into why testing can be difficult. Now I’m back to the decision whether or not to Unit Test. Below is a fairly simple piece of business logic. It most likely took a bit longer than the first bit of code, as there was a DTO class to set up an the normal interface plumbing. What is does is find a customer order connected to something called a FOL.

public class GetFilledOutLabelLinksAsync : IGetFilledOutLabelLinksAsync
{
    private readonly IGenericActionsDbContext _db;
    private readonly IGenericLogger _logger;

    public GetFilledOutLabelLinksAsync(IGenericActionsDbContext db)
    {
        _db = db;
        _logger = GenericLibsBaseConfig.GetLogger(GetType().Name);
    }

    /// <summary>
    /// This returns information on the FOL and any associated order
    /// </summary>
    /// <param name="folId"></param>
    /// <returns></returns>
    public async Task<ISuccessOrErrors<FolAssociatedData>>
         RunActionAsync (int folId)
    {
        var status = new SuccessOrErrors<FolAssociatedData>();

        var fol = await _db.Set<FilledOutLabel>()
              .SingleOrDefaultAsync(x => x.FilledOutLabelId == folId);
        if (fol == null)
        {
            //very odd, but we fail softly
            _logger.WarnFormat(
                 "Could not find the FOL of id {0}", folId);
            return status.AddSingleError(
                  "Could not find the Label");
        }

        //Note: of FOL should only be associated with one LabelOrder,
        //but we have written this in a 'safe' way
        var associatedOrder = await _db.Set<LabelOrder>()
            .Include(x => x.Order)
            .OrderByDescending(x => x.Order.DateCreatedUtc)
            .FirstOrDefaultAsync(x => x.FilledOutLabelGuid == fol.GroupBy);
            
        return status.SetSuccessWithResult(
             new FolAssociatedData(fol, associatedOrder), "Got data");
    }
}

So why didn’t I write a Unit Test for this, and more importantly why am I happy with that decision? Well let me start my list of decison points with another quote from Martin Fowler.

 The style I follow (for Unit Testing) is to look at all the things the class should do and test each one for any condition that might cause a class to fail. This is not the same as “test every public method”, which some programmers advocate.

Testing should be risk driven; remember, you are trying to find bugs now or in the future.

Martin Fowler in the book ‘Refactoring’

My Criteria for Testing/Not Testing

I don’t have any hard and fast rules for Unit Testing, but over the years I have found, sometimes the hard way, what works for me. Here are some guiding principals, starting with the most important, that I use to decide if I need to test something.

Will it fail silently?

If the code could ‘fail’ silently, i.e. it goes wrong but doesn’t throw an Exception, then it needs watching! Any method that does maths, manipulates strings etc. can get the wrong answer and you need to check it, as it could cause problems and you don’t even know.

What is the cost of it going wrong?

It is a risk/reward thing. The more important/central the piece of code is to your application then the more you should consider Unit Testing it. (I can tell you that the payment side of my application is covered with Unit Tests!)

Will it change in the future?

If some code is likely to be changed/refactored in the future then adding Unit Tests makes sure it doesn’t get broken by accident. I love Unit Tests when it comes to Refactoring – I can change vast chunks of code and the Unit Tests will check I didn’t break it in the process.

Does it have lots of conditional paths?

Method with lots of conditional paths or loops are complex by nature. The logic could be wrong and its hard to test all the paths with functional testing. Unit Tests allow you to exercise each path and check that the logic is correct.

Does it have special error handling?

If the path through a method is only triggered on errors then Unit Testing may be the only way to check them out. If you added error handling then it must be important so maybe it should be Unit Tested.

I’m sure there are more, but I’ll stop there. You can always leave a comment and tell me what I have missed.

I should say there are some negative points, mainly around how hard it is to test. For me this isn’t the deciding factor, but it does play into the risk/reward part.

Final quote from Martin Fowler.

You get many benefits from testing even if you do a little testing. The key is to test the areas that you are most worried about going wrong. That way you get the most benefit for your testing effort.

Martin Fowler in the book ‘Refractoring’

Conclusion

So, if you look at the criteria and look back at the two pieces of code you will see that the code I tested a) could fail silently, b) will change in the future (see the NotImplementedExceptions), and c) had lots of conditional paths. It was hard work writing the tests but it was a no-brainer decision. (Note: I did find one bug around the tests for the not implemented ‘rotate’ and ‘shear’ checks).

The second piece of code wouldn’t fail silently, is unlikely to change and has one error path and one success path. You might also note the defensive style of coding as this is about a potential order a customer wants to make, so I don’t want it going wrong. Time will tell if that was a good decision or not, but I am happy for now.

Please feel feed to add your own comments.