Wednesday, August 25, 2010

The importance of unit tests II

Yesterday I had some fun spending some hours to introduce ajax in a legacy application (I call it legacy because it has a very small amount of automated tests). During the afternoon I was quite pleased with myself, partly due to the satisfaction associated with the work done, partly due to the "Steppenwolf" novel, partly due to the Beethoven Sonatas I was listening to, partly due to my full stomach. Being a very wise and intelllectual person, I am in favour of the latter.

All this abrupltly ended when I got an unexpected error from a web service called from a part of the codebase that I had modified in the morning, which seemed to go wrong when I submitted a foreign address.

First I checked the test that exercised the web service client, where I verified that all the parameters where correctly passed as expected. Then, I wrote some other tests trying to exercise the particular feature, narrowing the scope of my private investigations. Btw, this reminds me that I have not listened to the Dire Straits in a long time, which is a shame.

Everything seemed fine, so I spent some hours with the developers of the service trying to figure out what was going wrong with the service (if anything). After some head banging it emerged that he problem was that we were focusing on a particular parameter, while the error sneaked in another one. I hope they will not make me pay for the dents in the desk.

That would have been clear from the beginning if we checked all the parameters in the XML stream instead of focusing only on the ones that we thought were important for the specific call (the port in the service is only one, even if there should be many, and the behaviour is determined by which of the several zillions parameters are set and what their values are; I don't think this is very brilliant, but it cannot be changed, so complaining is useless... or at least it brings no business value).

So, everything went back to a wrong value, due to a "simple" setter method.

Now, you normally never write a test for a setter method. This is perfectly acceptable, as you should write tests for "everything that could possibily go wrong" and a normal setter is not included in the list.

Pity this setter was not a plain one, but contained an if:
public void setMyProperty(final String value) {
if (value != null || StringUtils.isEmpty(value.trim())) {
this.myProperty = "FIXED";
} else {
this.myProperty = value
}
}
This looks like a blunder... why that not-null check? is it to be able to trim the value or should it actually be a check the value is NOT null? If the former is true, why hasn't the author simply used the isBlank method instead? maybe because she was thinking of saving the trimmed property (which in the case she forgot to do) or because she didn't know the existence of the method?

Though strange this might sound, all this is not really important as I have access to the product owner and I could get all the answers I needed.

The point is: that method required a test, and it was nowhere to be found. You might argue that it's simple enough to avoid writing one, but all the time I wasted says something different. Still, you might add that it was my fault because I didn't check the setter in the first place, but this is only another arrow in my quiver: this is exactly the reason for which we need automated tests, as people do make mistakes and forget to check simple methods.

I'd like to think that if I were pair programming with the author of this code I'd never let her skip writing the failing tests first, at least not without fighting.

Note that I'm not pointing a blaming finger, I could have written that code myself (and sometimes I did, and I'm happy to say that I always regret it when I realize it).

I know I am fighting - and so far losing - a running battle, but tests are necessary, even if someone tells you they represent a cost. Actually they are not even a cost, they are an investment, they are one of the most important risk management tools software developers have.

It only took me some fifteen minutes to write four different tests that assessed the desired behaviour and rewrite the method from scratch. These fifteen minutes would spare three people some wasted hours. This also demonstrates how the cost of bug fixing dramatically increases with time.

Yet, for some unknown reasons and against all evidence, too many managers AND developers refuse to believe in practices like TDD or pair programming. Is it to mantain the illusion of control? is it fear of changes? is it lack of trust? is this evidence not so evident? let's try some math:
  • cost of writing the setter method: 5 minutes
  • perceived cost (usually coincides with the former): 5 minutes
  • cost of writing tests and the setter method: 15 minutes
  • perceived waste: cost of writing tests minus cost of writing the setter method = 10 minutes
  • cost of writing tests and the setter method, pair programming: 30 minutes
  • perceived as almost blasphemous waste: cost of writing tests pair programming minus cost of writing the setter method = 25 minutes
And this is where analysis normally end. On the other hand...
  • cost of finding the bug: 6 hours
  • actual cost: 6 hours plus 15 minutes PLUS 5 minutes = 6 hours and 20 minutes
  • actual waste: 6 hours PLUS 5 minutes
Also note that the time needed to write the code in the first place was completely wasted.

Still think that writing tests is too expensive? Well, the curious thing is that at this point everyone seem to agree on the reason behind the added cost: the code was sloppy. As a corollary, the blame is on the developer. Well, THIS IS NOT THE PROBLEM. The problem is that the way used to write the code was sloppy. And this does not depend entirely on the developer.

Think about it.

2 comments:

Steve & Miki said...

Nice article, couldn't agree more; I have also seen the massive benefit of having a suite of unit tests on our code. Couple this to a continuous integration server such as Hudson and you have a powerful regression tool.

I think something I learned (too late I think) is that the biggest value comes from writing tests right from the start. If you have a large code base that is largely untested, the best you can do is introduce unit tests gradually. I think we could have prevented a lot of bugs in production if we had had a policy of writing tests from the start.

- Steve Chamberlain

Unknown said...

Pity that sometimes adding tests gradually in an almost untested codebase can be a hard work, thanks to static classes and methods, references to concrete classes, inheritance all around and no composition anywhere, monster methods and so on.

Anyway, as you correctly said, it's the best thing you can do.

Every developer that has to face such a situation (which means every developer) should seriously study Working effectively with legacy code, which is a really great source of knowledge on the matter.