March 19, 2010

Best practices for Assert statements in unit tests

Some may find this trivial, but I find myself going over this in almost every code review: You’ve got to pay attention to your unit test assert statements:

Don’t hide the comparison in your assert
Consider this assert statement:

Assert.IsTrue(myObj.Name == "Expected name");

Why is this bad? Take a look at the test runner output when the test fails:

NUnit.Framework.AssertionException:   Expected: True
But was:  False

The comparison is hidden, so the outcome of the test is always a non-informative true/false. Change the statement to

Assert.AreEqual("Expected name", myObj.Name);

This will give the following output when the test fails:

NUnit.Framework.AssertionException:   Expected string length 13 but was 12. Strings differ at index 0.
Expected: "Expected name"
But was:  "Another name"
-----------^

Much more informative, right?

I personally like the alternate fluent assert constructs you can do with NUnit, because it makes you write the assert statement right almost without thinking about it:

Assert.That(myObj.Name, Is.EqualTo("Expected name"));

The assert message should add information or be omitted

How about:

Assert.AreEqual(myObj.Name, "Peter", "Expected name to be Peter.");

The assert message is a waste of typing efforts, because the output of the test runner will tell you anyway. Use the assertion messages only to provide extra information, or omit them:

myObj.DoSomething();
Assert.That(myObj.HasError, Is.True, "Expected HasError to be set because DoSomething produces an error.");

Multiple assert statements

When completing a complex arrange part of a test, it is tempting to make a lot of assert statements (after all this trouble, it’s time to assert to heck out of it, right?) Well not really…

The assert statement is the test. When you look at a test that has multiple assert statements, the test will most likely violate the SRP principle (yes, it also applies to tests), meaning that the test is testing to many things at once. Why is this bad? Because the test will most likely be big and difficult to understand. What will you name the test if it tests 5 different things? Have you seen each of the assert statements fail? If you haven’t, how do you know that the test is working?

Resolution is simply to split it up. You can extract any complex arrange/setup part to a private method and reuse this from your split up tests.

The only case where I see myself diverting from this practice is when doing progressive assertions against the same object. Again it is about making the test informative. Here my expected outcome is that a person with the expected name is added to Persons:

Assert.That(myObj.Persons[0].Name, Is.EqualTo("Expected name"));

Will potentially throw a NullReferenceException or an IndexOutOfRangeException, which doesn’t give a clear clue to what the problem is. Here I would use:

Assert.That(myObj.Persons, Is.Not.Null);

Assert.That(myObj.Persons, Is.EqualTo(1));

Assert.That(myObj.Persons[0].Is.EqualTo("Expected name"));

This will be more informative in case the Persons list is not instantiated or populated.

Conclusion

The general issue here is, that when you are constructing your test, you know exactly what is going on, at that precise moment. It is very easy to be blinded by focusing only on making this test go green. But try to look ahead – in 3 months, this test may fail. Imagine your trusted coworker having to find out what went wrong – fast. That’s why your tests should be easy to understand and informative when failing. And that, my friend, is achieved by paying attention to the assert statement.

No comments:

Post a Comment