Friday, July 3, 2015

Unit Testing Asserts: Skip the Message Parameter

The Assert methods that we get with our unit testing frameworks have an optional message parameter. Under normal conditions, we should *not* include this message. This is opposite of what I used to do. But I'm rarely perfect at something when I'm first starting out.

Getting Started with Testing
When I first came across the Assert object (and all of the methods that go with it), I saw that most of the methods had an optional message object. I figured that it was a best practice to fill in this message to let the developer know what went wrong in a test failure.

To me, this was helpful. It tells us that the SelectedPeople property is not empty. But this turns out to be unnecessary.

One of my problems when I was first starting out is that I didn't have a good naming scheme for tests. So, this is what a typical test looked like:

Even with the bad naming, the Assert message is not necessary, but let's explore a couple of good testing practices before looking specifically at the message.

Good Naming
The first step in helping the developer when tests fail is to make sure that we have good names for our tests. Roy Osherove recommends a 3-part naming scheme.

The idea is that by just looking at the names of the tests, we can figure out what went wrong without needing to look at the test code or the test failure details.

The methodology that I use is a bit of mutation from this. Here's the test from above with a better name:

In this case, we're testing the "SelectedPeople" property of the "Model" object in our view model that we're working with. When we call the "ClearSelection" method, we expect that the "SelectedPeople" property will be empty.

When this test fails, we see it in the test results:

And this tells us that the SelectedPeople property is *not* empty just by looking at the failing test name.

Note: I can think of a couple of different ways to name this method based on Osherove's scheme. The level of detail would depend on the types/number of tests that we have as well as how we are organizing our tests.

Additional Note: This test was written a couple of years ago, and I can think of a few ways to enhance the readability based on my experience since then. But that's true of pretty much any code that I've written more than 6 months ago.

So if we have a good naming scheme, the test name tells us what went wrong.

Assert Message is Just a Comment
The Assert Message is really just a comment. And just like we want to avoid unnecessary comments in our code, we want to avoid unnecessary comments in our tests.

And if we feel the need to explain code in comments, we need to look at rewriting the code. (Check out slides 18 - 26 in my Clean Code presentation or watch a few minutes of the presentation starting at 21:04.)

Let's look at the test run details to see if this message is really necessary:

The standard Assert.AreEqual method gives us a useful message. We expected a "0" value for the count of our collection, but the actual value was "2". And the "SelectedPeople count is not zero" message does not add any useful information.

So, let's remove it. Here's our updated test:

And the failure details:

This is still perfectly readable. We already know from the test name that we expect "SelectedPeople" to be empty. And the standard Assert.AreEqual message tells us that we have 2 actual items.

And it's easy to see that we don't need the additional message.

Minimized, but not Eliminated
So, I've modified my testing behavior a bit. By ensuring good names, the Assert messages become unnecessary. But I find them useful in a couple of situations.

Notice from the test above that I have an Assert (with message) in the Arrange section of the test:

There are situations where I want to validate the arrangement before testing. In this case, since I'm clearing out a collection, I want to make sure that the collection is actually populated first. If I don't populate the collection, then I don't know whether "Clear" actually worked, or if the collection was empty when I started.

This arrangement populates the collection with 2 items, and then uses an Assert as a sanity check to make sure that we have to 2 items we expect to be there. In this case, the message tells us that the Arrangement went bad (rather than the "Act" part of our test).

Is this a good way to do things? Well, I've been re-thinking this as well. I've used this pattern in the past, and it has worked. But I'm thinking about ways that this can be improved.

Multiple Asserts
The problem is that we really should not have multiple "Asserts" in a single test. In the test above, the first assert is really a sanity check, and the second assert is the actual one that we want for the test.

But I have written tests that have multiple asserts. Let's take a look at the application:

The test with the multiple asserts has to do with the checkbox filters that are under the list box. Here's one of the tests:

This test is a bit more complicated that I'd like it to be -- primarily because this is testing an asynchronous service call that uses the APM (Asynchronous Programming Model). So, you'll notice the "tracker.WaitForChange" method. This is a helper class that waits for a property to be changed (by hooking up to the PropertyChanged event).

Note: The "tracker" object is actually very useful when we're testing OO code. I'll show that in a future article.

And rather than having a separate test for each filter, I've combined them into a single test.

This has a Big Downside
The big downside to this is that if the first "Assert.IsTrue" fails (the one checking "Include70s"), then none of the other Asserts will run. This is because the Assert method throws an exception which prevents the rest of the test from running.

So if we get one failure, we don't know whether the other items would have failed as well. This may change the approach we take to fixing the bug. If we fix the first problem, we may find that the second Assert fails, and we have to go through the process again.

Because we're testing multiple things in 1 test, our test name needs to be generic enough to be relevant to all of the cases. This is why our test name says "Filters" rather than "Include70sFilter". Since the test name does not give us the details we need when it fails, we need to look at the Message that we're sending back from the Assert.

This is why it's recommended that we only have 1 Assert per test. Ideally, this test should be broken up into 4 different tests. That would enable us to give each test a more specific name, and we could eliminate the Assert messages.

As a side note, there are testing frameworks that are designed for multiple asserts on a single arrangement. One of these frameworks is MSpec. I haven't used it myself, but when I was talking to David Batten (Pluralsight and Twitter) at the Denver Dev Day, he said he's had good experiences with it.

Constant Improvement
Programming is a constant learning process. By examining our existing code, we can determine which things work well and which things can be improved. And by looking at other people's code, we come across ideas that we would not have thought of ourselves.

Tips for Good Tests
  • Useful Test Names
  • One Assert per Test
  • Avoid Assert Messages
In this case, I definitely see the benefit of avoiding Assert Messages. If I feel the need to put in the Message, then I can treat that as a warning flag that I probably want to rewrite the test.

Happy Coding!

No comments:

Post a Comment