Friday, September 12, 2008

Can unit testing be a waste?

Picture courtesy of Kecko@flickr
It is sometimes hard to start implementing new feature (or fix a bug) by writing an unit test even for a very agile and experienced developer, that's for sure. But it pays off and it usually turns out to be the easiest and the most efficient way. Besides this, developers have to decide what to test. "You should test everything!" some of you may say, but what does it mean?

Should I test all my JavaBeans (as Java is the language I mostly use I will give examples in this technology)? Should I test all my Data Access Objects? Should I test all toString(), equals(), hashCode(), etc. methods? Or maybe I should test only stuff where I "integrate" all those "low-level" components/classes?

I'll try to answer the question of what level of abstraction should be tested by your unit tests and why some unit tests may be considered a waste in this post.

Testing everything (literally) is a waste
You should start developing new features from unit tests. More so, you should start by writing tests from the highest possible level of abstraction. I mean that if you want to test adding new users to the database feature you should start from the service class that takes input parameters, creates user object and then stores it using some kind of DAO object into the database. You should not start from testing and implementing User class or UserDao class.

Why? Well, if you start thinking of objects you will need for implementing your feature you may invent some overkill architecture, many unnecessary classes that will eventually be unused. With this approach you will start from implementing User class, then UserDao class and eventually you will implement the service class that will use these both classes and expose some public methods to the outer world.
With this approach, if you are still agile and start from writing an unit test, you should end up with at least three unit tests (one for each class). And assuming that User class is stupid JavaBean with only getters and setters you will have to write unit test for those methods, right?

This is a waste (especially unit testing getters and setters) because you could have only one unit test class with less code and have 100% code coverage.
Agile methodologies focus on eliminating waste - that's why you should not test everything if you want to be really agile.

The only way is to start from the highest possible level of abstraction and then adding necessary classes just-in-time. And the example follows...

What should you test
I will not give you an example of adding users to the database but instead an example of Struts2 action that retrieves announcements from the database and creates RssChannel object (that is processed by the View layer later on and returned as an application/rss+xml content type that is readable by the RSS readers). Take a look at the attached RssAction.java file. This action uses other classes like:

  • AnnouncementDao

  • RssChannel

  • Announcement

  • ...


Now take a look at the test for this action: RssActionTest.java. At the first sight this unit test tests only com.bielu.annoboard.action.rss.RssAction but in reality I also tests all classes mentioned above.

The effect? Cobertura tells me that:

  • RssAction is covered at 100%

  • RssChannelHelper is covered at 92%

  • RssChannel is covered at 95%

  • AnnouncementDaoImpl (which is an implementation of the AnnouncementDao) is covered at 100%

  • Announcement is covered at 100%


Voila! No waste and everything (or almost everything) is tested!

Use your common sense
As always there are some pitfalls lurking - my previous opinions are not always true. For sure you should NOT test getters and setters in your JavaBeans. But what if you implement for example toString(), equals() or compareTo() method? Should you test them or not?

I generally test classes from the highest possible abstraction level but I sometimes also test lower-level classes. Here are some pieces of advice you should consider following:

  • it is a very good idea to test toString() method

  • it is an absolute MUST to test (to death) equals() and hashCode() methods and contract relations between them (refer to "Effective Java" book) - if you use Sets, Maps and collections in general this is a critical part - I even remember the very small bug there in one of my previous projects and two seasoned Java developers looking for the problem for couple of hours - this is my coding horror

  • it is a good idea to also test compareTo() and contract relations to equals() method

  • check with your code coverage tool what is still not tested after the high-level tests

  • consider removing the code that is not tested and see whether it was used at all (see this post)

  • if it is necessary but not tested then test it from the lower level


Use these pieces of advice as well as your commons sense wisely and remember to eliminate waste every time you can (yes! writing unnecessary unit tests is a waste).

I'm really curious about your opinions about writing unnecessary unit tests. Do you think writing unnecessary tests can be even dangerous? Do you consider unnecessary and overlapping unit tests a waste? Share your opinions here.

Originally published on AgileSoftwareDevelopment.com

1 comment:

Kenneth Xu said...

You assume yourself writes every line of the application.

What if different layers are written by different developers. Don't you need to test each component as one single unit?

Actually, I felt the same about the "waste code" and this is what brought me to your blog. But on the other hand, it seems to me that you are talking about some mix between UNIT and INTEGRATION test.