Friday, 15 June 2007
The First Law of Unit Testing
A few years ago, I moved from doing mainly web UI development in classic ASP and Visual Basic to ASP.NET in C#. As I was writing code one day, I thought to myself, “How can I make sure my code is working before it gets to QA.” I had heard rumblings of this technique called unit testing, and did some investigation. After finding NUnit and integrating it into my workflow, I never looked back.
I try to be a great unit tester. A wise man once said that a software engineer is a master of quality assurance. I strive for 100% code coverage, and really robust tests.
However, as I dig through code, I see things like this:1
public class Spell
{
public string[] Ingredients = null;
public int ID = 0;
private void ValidateIngredients()
{
if( spell.Ingredients == null )
{
throw new ValidationException("Spell ingredient list cannot be null.");
}
if( spell.Ingredients.Length == 0 )
{
throw new ValidationException("Spell ingredient list cannot be empty.");
}
}
private void ValidateID(bool isNew)
{
if( spell.ID != 0 )
{
throw new ValidationException("Spell has an ID greater than 0. Has it already been saved?");
}
}
public void EnsureValid(bool isNew)
{
ValidateIngredients();
ValidateID(isNew);
}
}
public class Magic
{
private const bool SHOULD_BE_NEW = true;
public bool AddSpell(Spell spell)
{
spell.EnsureValid( SHOULD_BE_NEW );
int spellID = DataLayer.CreateSpell(spell)
return return spellID;
}
}
internal class DataLayer()
{
public static int CreateSpell(Spell spell)
{
// code to save snipped for brevity
}
}
[TestFixture]
public class MagicLogicTest
{
[Test]
[ExpectedException(typeof(ValiationException))]
public void SpellWithNullIngredientsDoesNotGetSaved()
{
Spell spell = new Spell();
spell.Ingredients = null;
Magic.AddSpell( spell );
}
[Test]
[ExpectedException(typeof(ValiationException))]
public void SpellWithEmptyIngredientsDoesNotGetSaved()
{
Spell spell = new Spell();
spell.Ingredients = new string[0];
Magic.AddSpell( spell );
}
[Test]
[ExpectedException(typeof(ValiationException))]
public void SpellWithInvalidIDDoesNotGetSaved()
{
Spell spell = new Spell();
spell.ID = 1;
Magic.AddSpell( spell );
}
}
So, we have four classes here:
- One representing a spell.
- A MagicLogic class, which holds all our business logic for working with magic.
- A data layer for persisting our spell to the database of our choosing.
- A test fixture for our MagicLogic business logic class.2
How does this look to you? What changes would you make (from a unit testing perspective)?
Go ahead. I’ll wait while you think.
Ready?
The MagicLogicTest test fixture is testing the validation routines of the Spell class which is Not Good©™® because it breaks Aaron’s First Law of Unit Testing3:
A unit test fixture should exercise and test functionality of one class.
When testing the AddSpell method, all we care about is that it calls the EnsureValid method on the spell, not that all the validations take place because that is all the AddSpell method does: calls the EnsureValid method. We will save testing the actual validation for the Spell test fixture, because that is where the validation code actually resides. So, our three unit tests for the MagicLogic class can actually be replaced with one test. Here is the new MagicLogicTest test fixture:
[TestFixture]
public class MagicLogicTest
{
[Test]
[ExpectedException(typeof(ValidationException))]
public class AddSpellValidatesSpell()
{
Spell spell = new Spell();
spell.ID = -1;
MagicLogic.AddSpell( spell );
}
}
Boom!
Yes, that is the smallest amount of code I could write to illustrate my point. Trust me.
And, yes, there is a test fixture class for the
Spellclass, but did you really want me to show it, too?I am the first to admit that the only hard and fast rule is that there are no hard a fast rules, but the First Guideline of Unit Testing doesn’t sound as good. Yes, there are times when a law can and should be broken, but it doesn’t happen very often, and you usually have to be a cop, so don’t do it.
