Getters, Setters and the Great Coverage Conspiracy
A frequent topic of Java-related blogs is whether it is worthwhile to write unit tests for simple getters and setters. This posting that I came across today proposes a reflection-based trick for eliminating much of the work in writing these tests. Maybe this is an improvement over other approaches, but what bothers me most is the motivation for wanting to test getters and setters in the first place.
It seems that many of those advocating unit testing simple getters and setters are driven by a desire to improve their coverage scores with the actual utility of the tests a secondary concern.
Firstly, I should state that I am absolutely in favour of measuring coverage for test suites. In fact, I think it’s pretty much essential. If you are writing automated tests but not measuring code coverage then you are just scratching around in the dark. What’s great about coverage reports, particularly those that show branch coverage as well as line coverage, is that you get to see exactly where your tests are neglecting certain scenarios. Coverage reports can also be useful in highlighting code that is not used and can be removed.
The problem with code coverage is that it only shows where your tests are weak. It does not prove that your tests are good, even if the coverage is 100%. So writing tests with the sole aim of improving the coverage score is merely an exercise in self-deception. It’s the tail wagging the dog.
If you need to add tests for all your getters and setters in order to achieve x% code coverage, where x is some mandated target, there are two questions you need to ask:
- Do you have too many getters and setters?
- Are you avoiding testing difficult code?
I could go on for pages about the first point. There are far too many getters and setters in most Java code. Too many developers think encapsulation is simply a case of making fields private and providing access to them with getters and setters. It would be better to aim for making fields private and not providing access to them with getters and setters. Favouring constructor-based dependency injection over setter-based DI is something else to consider (although that’s a whole other article in the making…).
How do you know if you have too many getters and setters? Well your coverage reports are a good starting point. If the getters and setters are essential to your application, it will be just about impossible to avoid exercising them indirectly from other tests. If you have good coverage elsewhere but the getters and setters aren’t touched, chances are they aren’t needed. Adding more tests is not the only way of improving your test coverage. Another way is to remove code so that you have less to test.
The second question above is also important. If you require your team to achieve a rigid 75% test coverage target then you are almost guaranteeing that you will get tests for the 75% of the application that is easiest to test. Writing tests for getters and setters helps to fulfil the 75% requirement without needing to think about how to test the difficult bits of the system. Unfortunately, the other 25% is probably the code that really needs testing/refactoring.
For me it’s pretty clear. Don’t write unit tests for getters and setters. Better still, don’t write getters and setters (except where necessary). And don’t confuse test-driven development with coverage-driven development.
on April 2nd, 2008 at 8:32 pm
I agree with most of the premises, but I draw a slightly different conclusion. Right because of what you said about the 75% coverage, I think that the actual coverage should be around 90%-95% (on the other hand I’m not so strict about the fact that the coverage is only achieved by unit tests, a mix of unit and integration tests would do as well).
Now, covering getters and setters becomes important so you don’ run into the opposite mistake: ah, well, there’s a 25% missing coverage, but it’s getters and setters, while maybe the lack of coverage is in another place. This could be addressed in a different way if we had an annotation such as @IgnoreCoverage that prevents coverage tools to take some method into account, but somebody could misuse that.
At last, you’re right about having too many getters and setters, but here the problem is with specific technologies: e.g. if you have JavaBeans based stuff (which is a huge set of things, from Swing to JSF to JPA) you need to have a good number of getters and setters (and for Swing this means that you need also property change listeners, which indeed is a good thing to test).
on April 2nd, 2008 at 8:48 pm
Fabrizio, good points. The 75% figure was plucked out of the air (it just happens to be the figure mentioned in the post I linked to). My point is not to just aim for a particular figure (whether 75% or 95%) because it’s too easy to fall into the trap of chasing the numbers rather than adding value.
You are right about Swing. Even without the getters and setters, UI code is hard to test well. Using something like FEST (http://fest.easytesting.org/swing/wiki/pmwiki.php) gets you some of the way there but I’m never really satisifed with my UI tests.
As for the Java Beans conventions (no-arg constructors and getters/setters). I’ve been thinking for a while that they are outdated. They made sense 10 years ago, but a lot of libraries have moved beyond the need for such clumsy conventions by using bytecode instrumentation to avoid having to compromise on encapsulation and invariants in your classes.
on April 3rd, 2008 at 1:10 am
I also agree with premise but have a different conclusion.
Developers shouldn’t test getters/setters: unit-test frameworks should (insofar as possible), via a configuration option or annotation. It should be as easy as possible and, indeed, not given much merit as a metric.
The unit-test frameworks (or code-coverage etc) should report on the number of getters/setters tested and the total number. This posts asks “do you have too many getters/setters?”. Good question: why not have the ability to report on it, in its own right? Again, this is not a report for the CTO.
Finally, code-coverage should be able to report coverage percentages that either include or exclude getters and setters. Then one would know the wheat from the chaff.
on April 4th, 2008 at 7:12 pm
If you cannot be bothered testing a method, you have to ask yourself whether you need it.
My suggestion is, don’t add a getter/setter unless it actually is useful. Don’t forget you can have public final or public mutable fields and dump the extra code which doesn’t add any value.
Certainly, 90%+ of getter/setter just bloat the code and the only real difference removing them would have is to make the code shorter.
on April 9th, 2008 at 7:47 pm
“As for the Java Beans conventions (no-arg constructors and getters/setters). I’ve been thinking for a while that they are outdated. They made sense 10 years ago, but a lot of libraries have moved beyond the need for such clumsy conventions by using bytecode instrumentation to avoid having to compromise on encapsulation and invariants in your classes.”
Oh, yeah, that’s going to raise my confidence in my code. Code I can neither test nor read. Bytecode instrumentation IS breaking ALL encapsulation, by definition. Bytecode instrumentation is the universal pick to every encapsulation you thought you locked… a hacker’s dream… a black hat’s a-bomb.
I mean, come on. Let’s be rational.
on April 9th, 2008 at 8:50 pm
Well it’s a matter of priorities. You’re concerns are valid, but I am more interested in the maintainability of the source.
Using something like JiBX (instrumentation) instead of Castor (refelction) makes the code cleaner because I don’t have to compromise on encapsulation at coding time. A developer can’t call the instrumented code directly because its not there. It doesn’t exist until after compilation. This means developers are less likely to (lazily or inadvertently) introduce inappropriate dependencies and coupling.
It’s not a perfect solution for the reasons you point out, and I wouldn’t use it for absolutely everything, but giving up on final fields and encapsulation (my definition of encapsulation rather than yours
) is even less ideal in my opinion.
It can be tested but, just as for getters and setters that don’t have their own tests, you’d have to test it indirectly by testing the code that uses it.
The instrumentation approach also often comes with a big performance advantage over reflection-based techniques.
on April 9th, 2008 at 9:27 pm
Well to each his own- obviously there are two valid sides to this and I see not reason to pretend that’s not so.
Just to drop a bomb on this convo- I never, ever ever unit test. Not one line, not one class. Why? The historical reason is, I’m a UI developer, most of my code is UI based so when unit testing hit, I went *sniff* … *sniff*… hmmm….not for me…
Non-historically, I *really* think it’s useless, though, as I say I’ve never actually DONE it. I actually take it as a virtue that I could *sniff* … *sniff* and decide it’s silly pretty quickly and not look back (much.. or hard) .
For me, it’s one of those technologies that came along in a huff and frenzy, proclaimed the world was its stomping grounds, and gained a lot of adherents who spend a lot of time worshiping at its altar. A lot.
Here’s my argument:
You’re not going to pump water through all conceivable pipes because you can’t. If you don’t understand that (but yo probably do) you probably have some foundational CS still ahead of you, (just like most everyone else).
In fact, you’re only testing for use cases you can think of, which is what you programmed for in the first place. You’re not *really* testing for anything outside of what you expect. In fact, if your code is clear, short simple well-named methods and coherent objects, you can go farther towards proving that your code will work as expected- farther than your unit test busy work will get you.
Here’s my Modus Operandi.. …ready?
I look at each and every line of code and ask myself- why would this not do what’s expected? Yep I sit there and and contemplate each line of code, and i say “hmmm… what could possibly go wrong that doesn’t represent a catastrophic failure on the part of either an outside library or the computer system?”
And I leave little proof-notes to myself in front of my methods which signify the reason why it can’t go wrong. I have about 30 or so so far. I have the typing of them automated as macros in my IDE.
I am never wrong. Ever. Over the course of 8 years, three since I’ve been doing this, (before I was just really really careful) three bugs have escaped me and gotten up to QA and one has shipped. And, no, I am not writing scripts for web pages or anything of that sort. My code is in Fortune 500s company-critical project offerings and the DOD.
I never unit test because it’s not the best way to ensure my code doesn’t have bugs and it’s not the best use of my brain power. Your intelligence and the understanding of the deep semantics of your program which only you can posses is far more able to detect bugs and many times more sensitive to bugs caused by an error in logic and unstated assumptions than unit tests are or are ever likely to be.
I never unit test.
Anything.
Ever.
So …. there.
on April 11th, 2008 at 10:32 am
Great post, Dan!
I’ve found bugs in my own code (and test cases) by seeing that certain setters or getters were not being called during testing. Had a unit test been written purely to cover those getters and setters, I most likely would not have found those bugs.
Unit tests which exist purely for the sake of increasing your code coverage can often do more harm than good.
@Fabrizio: Clover, (http://www.atlassian.com/software/clover/) provides Context Filters to have certain blocks of code excluded from your coverage figures. You can even define your own so you can exclude all getters/setters if you wish.
@Michael:
“Finally, code-coverage should be able to report coverage percentages that either include or exclude getters and setters. Then one would know the wheat from the chaff.”
The latest release of Clover (2.2.1) now reports on the percentage of your code that is excluded via a Context Filter, thus making it possible to do what you want. Have a look at the text below the stats in the header of this sample report
http://downloads.atlassian.com/software/clover/samples/checkstyle/index.html