Brendan Enrick

Daily Software Development

Write a Test Before Fixing a Bug

As I've said in previous posts, it is important to write tests for your code. A lot of the time I am talking about when refactoring code or when writing new code. Now I agree that bug fixing could be considered refactoring, but people seem to treat it differently. When fixing bugs they want to go in and quickly make the bug go away. That is a dangerous way to solve things, because it doesn't prevent the bug from returning.

If a bug exists in your system then there are test cases missing from the system. When you fix a bug, even if you do write a test case to handle the bug how do you know you've tested the right thing? If your test is not right it will not prevent the bug from returning. I know of only one way to be fairly confident that a bug is tested well. The test needs to be written before the fix. If it isn't written first you will not know. If you follow the TDD practice of Red, Green, Refactor, the Red lets you know that you've found the bug. Then when you make it green you can have confidence that you've fixed the bug.

RedGreenRefactor

If you had not tested first you don't even know that your test is accurate. How do you know you actually found what is causing the bug? You can't really be sure with any confidence, because all you saw were green tests. Your test couldn't fail. Either the test didn't actually test the bug or because you fixed it. You hope the latter, but it could be the former.

GreenGreenRefactor

This will not work. For other things I can agree with you that you can write code and then test it, but not for bugs. With bugs the test needs to be written first, so there is some confidence that the bug is being tested. Since code already exists the test needs to be in place first. This same practice should be adhered to when doing refactorings of working code as well. The point in that case is also to prevent software regressions caused by the refactoring. Tests help you hold existing functionality, and in the case of bug fixing they help to ensure that the bug has truly been fixed and tested to prevent a return.

Software bugs are like bad musicians. If you don't actively prevent them, they will make a comeback.

Example

As an example of this. We can assume that we have two classes. One of them is called Student and the other is called Course. To represent this example we are going to have some badly written code which is going to cause us to have a few nasty bugs (Badly written code makes for easier examples). From our knowledge  of the domain, we know that students take a few courses at a time, so there is a collection of courses associated with a student.

Say our student has a property called Courses and that property holds the collection.

public class Student
{
    public int ID { get; set; }
    public List<Course> Courses { get; set; }
}

Now we say that somewhere we are caching the Student objects based on their IDs. We want to make sure that if the student object is changed that we expire the cache, so we will cache it with a SqlCacheDependency on the database table for the Student object.

Assume we have a "View my courses" page for the student to get information about the courses. For ease, it will obviously grab the student object out of cache and use its collection of courses to display the information if the information is already there.

What bug have we created here?

Well we've now made it so that if any of the courses change this change will not be reflected on the courses page of any student currently in the cache. There are literally probably a dozen ways of fixing this bug. Plenty of ways we could have prevented it, but then we wouldn't have this nice example now would we?

All I am saying is that some unit tests and integration tests be written for these before we fix the problem. This lets us be sure that we've fixed the problem correctly.

The first test I would write is one that loads a student object (making sure that is gets cached) and then separately loads one of the courses the student is in and alters that course. I would then make sure that the change to the course succeeded and then I would get the student again, (it would come out of cache this time) and I would verify that it had the change I made to the course. I would expect that the first time I run it that the test will fail. If it doesn't fail, or fails for a reason other than what I expected, I need to fix it so it fails the way I expect.

Once it is failing correctly I can adjust the code to make it work correctly. I would call this an issue violating the single responsibility principle, because the Student object in my opinion shouldn't be holding a local copy of the Courses. If it doesn't have that then it will not be able to cache it. I think that property should just be a getter calling through to some Course-related class which can cache it if it wants to. Even though these are obviously domain objects, we should try to keep them separated a little bit better, because keeping them too close can cause some annoying bugs.

As always, have a great day, and keep testing your code.

Loading