79

So today I had a talk with my teammate about unit testing. The whole thing started when he asked me "hey, where are the tests for that class, I see only one?". The whole class was a manager (or a service if you prefer to call it like that) and almost all the methods were simply delegating stuff to a DAO so it was similar to:

SomeClass getSomething(parameters) {
    return myDao.findSomethingBySomething(parameters);
}

A kind of boilerplate with no logic (or at least I do not consider such simple delegation as logic) but a useful boilerplate in most cases (layer separation etc.). And we had a rather lengthy discussion whether or not I should unit test it (I think that it is worth mentioning that I did fully unit test the DAO). His main arguments being that it was not TDD (obviously) and that someone might want to see the test to check what this method does (I do not know how it could be more obvious) or that in the future someone might want to change the implementation and add new (or more like "any") logic to it (in which case I guess someone should simply test that logic).

This made me think, though. Should we strive for the highest test coverage %? Or is it simply an art for art's sake then? I simply do not see any reason behind testing things like:

  • getters and setters (unless they actually have some logic in them)
  • "boilerplate" code

Obviously a test for such a method (with mocks) would take me less than a minute but I guess that is still time wasted and a millisecond longer for every CI.

Are there any rational/not "flammable" reasons to why one should test every single (or as many as he can) line of code?

Zenzen
  • 996

10 Answers10

63

I go by Kent Beck's rule of thumb:

Test everything that could possibly break.

Of course, that is subjective to some extent. To me, trivial getters/setters and one-liners like yours above usually aren't worth it. But then again, I spend most of my time writing unit tests for legacy code, only dreaming about a nice greenfield TDD project... On such projects, the rules are different. With legacy code, the main aim is to cover as much ground with as little effort as possible, so unit tests tend to be higher level and more complex, more like integration tests if one is pedantic about terminology. And when you are struggling to get overall code coverage up from 0%, or just managed to bump it over 25%, unit testing getters and setters is the least of your worries.

OTOH in a greenfield TDD project, it may be more matter-of-fact to write tests even for such methods. Especially as you have already written the test before you get the chance of starting to wonder "is this one line worth a dedicated test?". And at least these tests are trivial to write and fast to run, so it's not a big deal either way.

  • Ah I totally forgot that quote! Guess I will use it as my main argument because frankly - what can break here? Not really much. Only thing that can break is the method invocation and if that happens than it means something really bad happened. Thanks! – Zenzen Jan 19 '12 at 21:35
  • 6
    @Zenzen: "what can break here? Not really much." - So it can break. Just a small typo. Or somebody adds some code. Or messes up the dependency. I really think that Beck would claim that your main example qualifies as breakable. Getters and setters, less so, although I have caught myself out in a copy/paste error, even then. The real question is, if it's too trivial to write a test, why does it even exist? – pdr Jan 19 '12 at 21:44
  • I would not test the dependency because it would be an integration test. I would use a mock. 2a) if somebody adds some code then he adds actual logic WHICH should be tested then. 2b) Setters don't qualify then? Why? What if someone adds some code to them? Same thing. 3) Why is it there? Just to adhere to the common layered architecture of the application. Following that logic everything can break ;)
  • – Zenzen Jan 19 '12 at 21:48
  • 1
    The amount of time you spent thinking about it already you could have written the test. i say write the test, don't leave when not to write a test as a grey area, more broken windows will appear. – kett_chup Jan 20 '12 at 00:11
  • Assuming that EITHER you're looking at mocking out your DAO when you unit test this class OR you have separate tests for your DAO methods; don't bother testing the one-line methods that call the DAO. It doesn't make economic sense to spend a finite amount of time on a test that has a zero chance of finding any bugs. – Dawood ibn Kareem Jan 20 '12 at 03:22
  • 1
    I'll add that my general experience is that testing getters and setters is somewhat valuable in the long-term, but low-priority. The reason why is because why it has "zero" chance of finding a bug now, you can't guarantee that another developer won't add something in three months ("just a simple if statement") that will have a chance of breaking. Having a unit test in place guards against that. At the same time, it isn't really overly high priority, because you aren't going to find anything soon that way. – dclements Jan 20 '12 at 04:00
  • 8
    Blindly testing everything that could break doesn't make sense. There needs to be a strategy where high risk components get tested first. – CodeART Mar 27 '14 at 23:50