Making Unit Testing Great Again. Part 3 : Unit tests that should have never been written…
In my previous post I showed you how, through refactoring we can split decision makers and “orchestrators”, thus removing external dependencies from our decision makers and making them much easier to unit test. If you got to the 3rd part of this post, I will assume that you agree with me and believe pure “orchestrators” do not need to be unit tested (if you don’t know what I mean by “orchestrator” please read part one of this series”. Today I wanted to show another unit testing trap we tend to fall into:
Writing unit tests that provide no value
They are written just for the sake of writing unit tests, which is worse then not writing unit tests at all. Writing unit tests that provide no value is doing work that adds negative value to the end result. It only adds more work. I stumbled upon this code while looking for a good example for part 2. You can safely skip trying to understand the following code gist for now, just skim over it:
The important thing here is that it is 80+ lines of code. It must have taken a few minutes to think about it and write this test class. So it should really provide us some value, right? So lets see what it is actually testing. Here is the implementation of InspectorContext the class these tests are testing (the original file contains about 40 lines of comments. I removed them from my gist because they were taking too much space)
Is this class really worth 80 lines of test code?
I think not. And here is why: There are no decisions or calculations that are the responsibility of this object! But there is logic in the constructor, you might say. All these “if” statements must be tested! Notice the “decisions or calculations that are the responsibility of this object” part of my statement. Guarding for null arguments should not be the responsibility of the InspectorContext. We can easily delegate that responsibility to a specialty “decision maker” called Guard object. Microsoft provides us with an implementation for this class Guard Class. We can safely assume Microsoft has tested it for us, so we don’t even need to test it, but just for fun, lets implement our own:
It is that simple, and here are the unit tests for this Guard class (I could argue that even these tests don’t provide much value, but will do them anyway):
So what would our InspectorContext look like with the introduction of our Guard class? Here:
And now we really have nothing worth unit testing in InspectorContext. It is pure “orchestrator” and we gained something actually useful: a specialty decision maker, that we can reuse throughout our code base.
We can now safely do what I love doing most: Delete useless code, and delete the 80+ lines of test code in the InspectorContextShould file.
