UnitTest.Musings.Random()
While I was doing some chores today, I began to have random musings about unit tests. It's strange how one's mind works while doing (mostly) manual labor.
Separating Code Coverage by Target
It's hard to get actual code coverage numbers for a specific target (i.e. class) without getting code coverage from others. Normally one writes unit tests to cover a single class. For example, I might have a class which represents a hand of cards, and in that class I might have a function which gets, as a string value, every card in the class :
public class Hand
{
private Card[] cards;
...
public override string ToString()
{
StringBuilder sb = new StringBuilder();
foreach (Card card in cards)
{
sb.Append(card.ToString());
}
return sb.ToString();
}
...
}
So if I wrote a unit test that covered the ToString function, I'd also cover a bit of the Card class. Once you amount a plethora of unit tests, you won't know what you explicitly have unit tests for and what you don't. You might say, well, as long as it's covered, I can see which lines of code aren't and write tests to explicitly cover those. That's not recommended for a few reasons -- like what happens if the code above changes and doesn't call Card::ToString() anymore? You might not notice that you lost a few percentage of coverage. And then what if Card::ToString gets a bug introduced that doesn't get caught, because the developers think that the ToString method is called from somewhere else in some other unit test? They may be caring developers and run all unit tests and inspect all of the code coverage numbers before and after, but odds are they won't.
What you really want is a way to tell the unit tests "Hey, from these set of tests only grab code coverage numbers from this target (i.e. function, class, etc.). I don't want to know about the other things it touches besides what I explicitly say." It's possible that I could get a large code coverage number simply by calling a simple button->click!
100% Code Coverage Myth
This is a common one, but I'll take a moment and rehash it. 100% code coverage is great, don't get me wrong. But what about the code you didn't write, the case you didn't handle, the data validation exception you didn't think to throw. I can easily get 100% code coverage of the Hand class above. But what happens if I do something easy to look for, like don't initialize cards? What if cards is initialized but the null is stored in the array at a few positions?
Again, 100% coverage is great, but it doesn't make me feel any better about the quality of the code. If somebody were to come to me and say that they hit 100% C.C., I'll tell them that their code probably isn't complete. And if it is (which is almost as hard as proving P==NP), then they either spent a lot of time on it, or it's not a whole lot of code.
Good(!!) Unit Tests
It's super important to write really good unit tests. By that I mean make sure you verify as much as possible without getting out of the scope of the tests. Don't check side effects in other target (as those might change), but DO check as much as you can in the current target.
I ran into this problem this week. I was looking to figure out why we had a bug and why we didn't catch it. So I looked at the unit tests that covered the feature that had the bug. I made sure that I wrote a unit test that covered the bug and ran the unit test. For the results, I used some built in functions that did the validation for me. "Interesting...it works! That's not right..." So I debugged it and followed it along. I could see that the data wasn't correct, so I continued tracing to see why it thought it was. I uncovered the following snippet of code (names changed to protect the innocent):
SomeObject actualItem = datastore.GetObjectById(expectedItem.Id);
Debug.WriteLine(expectedItem.ToString());
Debug.WriteLine(actualItem.ToString());
Assert.IsNotNull(actualItem);
Hm. He was checking that the item in the data store exists. But he didn't check that it was actually the correct data. This unit test went ALMOST all of the way, but it looks like the developer forgot to write the critical line :
Assert.AreEqual<SomeObject>(actualItem, expectedItem);
So I added the line to the verification, and found that a half of the unit tests, including my new one, failed. Wow! So I fixed the bug, went back and ran the unit tests again. All still failed! Why? It turns out that I had encountered two more bugs that hadn't been caught yet.
Hence why writing good(!!) unit tests is important.
A Unit Test's Brevity
A coworker and I often challenge each other on this point. I believe that shorter unit tests are better. My idea is to create some basic unit test runtime which you can use to create all of your objects in a few lines of code, which allows you to get quickly to what's really important. Additionally, you may even be able to write functions which simplify the validation of the data. Sure, you could use AreEqual, but maybe you want to spit out to the debug output what each object is before the AreEqual call? What if you have to read it from a service? He doesn't have an objection to this part at all.
However, he believes that writing longer unit tests are better (in general). "While I'm here checking Z, I might as well check W, X, and Y." That way I can pack it all into one unit test. My argument is that, while checking W, X, and Y while you're checking Z, this is only good if W, X, and Y never change. Plus, you really need separate tests for W, X, and Y. That way if W passes but X and Y don't, you're not debugging through large piles of code. You know "Hey, X didn't pass, so I better take a look at that piece of code first instead of W or Y." Beyond that, if you're duplicating these unit tests in others, when one validation changes, the others have to change as well -- unless you duplicate it, but then you're duplicating a bunch of hopefully verbose code all over the place. Even putting extra validation function calls often makes it confusing what exactly the unit test is supposed to be testing. What if the maintainer doesn't understand the unit test's point (due to commenting, naming of the test, etc.) and removes the key validation point that was the actual meat of the unit test? Unit tests being easy to understand is key for maintaining that code.
Another pretty unimportant thought I just had is that if the unit tests are super beefy, it's POSSIBLE that you'll run past the unit test timeout you've defined, if you follow the point that unit tests should check as much as possible for your target. If it takes 5 seconds to check each object and your unit test is 2 minutes per unit test, you may run out of time.
