Code coverage: the unattainable 100%


27 August 2012, by

This probably feeds into Rowan’s recent series of blog posts, but rather than looking into the mechanics of writing unit tests, I’m going to discuss a little about the logistics of applying them, and how you can ensure that your tests are actually useful.

From my early days as a cynic, I’ve come full circle and am now a great fan of unit tests. Sometimes I even prefer writing them instead of “real” code. At times I have a tendency to write somewhat esoteric code, and a comprehensive unit test suite is one way to ensure that the customer has as much confidence in the code as I do – and the best way to make sure that you have a comprehensive unit test suite is to measure code coverage. That, however, is not without some pitfalls…

High coverage is good

This is kind of obvious, but worth stating. The more of your code that is covered, the more confident you can be if you’re refactoring, etc. Agile development holds that constant refactoring is a Good Thingtm, but you should always be confident that what you’re refactoring isn’t going to break something unexpected. A good unit test suite should give you some of that confidence.

High coverage can lie

Thinking back to some of my previous code, I have in the past written unit tests that cover lines but don’t actually make any valuable assertions. Consider the following method, for example:


public Long countFoos() {
Criteria fooCrit = session.createCriteria();
fooCrit.setProjection(Projections.rowCount());
return (Long) fooCrit.uniqueResult();
}

To maximise coverage, you could simply call the function and assert that your Criteria object has been asked for a single result. Obviously this is a fairly simple example, but I’ve been guilty of doing pretty much that in the past. All three lines are covered and your report looks happy. The thing that actually matters, however, is the middle line – we need to assert that the right projection was applied, otherwise our counting function won’t (necessarily) be counting. This can get a lot more convoluted once you have many parameters and multiple combinations of restrictions/aliases etc. – your tests should ensure that all of the right things are applied, not simply that the lines have been executed.

Branch coverage matters

It’s fairly easy to get good line coverage on just about anything – you just need to execute each line in your tests, after all. Arguably, it is more important to have good branch coverage, though. If your method is taking a different course of action depending on its inputs, then you have multiple branches. You can usually cover all the lines with a selection of carefully chosen parameters, but without high branch coverage, you have no guarantee that some other combination of parameters might not do something unexpected – particularly if you’re doing that comprehensive refactoring I mentioned before. If you really can’t cover all of your branches, then you should consider a) whether all of your conditions can exist simultaneously b) whether you should simplify your code to make the branch conditions more obvious.

100% coverage is nigh-on impossible

Consider this:


switch(compass) {
case NORTH:
return "up";
case SOUTH:
return "down";
case EAST:
return "right";
case WEST:
return "left";
default:
// There aren't any more but I have to have a default
throw new CompassException();
}

Compass demo

You cannot cover that default case (at least, not in java). More to the point, you probably can’t execute it in real code either. Your line and branch coverage have taken a hit. One way to fix this would be to add an extra enum value, but that carries the risk that you’re adding a future bug when your unneccessary enum value somehow gets used. You should probably just accept that your default case isn’t going to be covered, and your lines and branches will not be 100%.

Sometimes the coverage tool lies

I ran into an interesting thing with Cobertura a couple of days ago – I had a pair of entity classes which inherited everything from a shared superclass, and had some single-table inheritance going on. The superclass took a generic parameter. In effect:


@Entity
public class Foo extends Bar { }

You’d think that your coverage tool will spot that there’s nothing to actually cover, and hence not cause you any problems, but you may be wrong. My class above showed that I had 0/1 lines covered and 0/1 branches covered. But there aren’t any lines or branches. In fact, my coverage report said that the @Entity annotation wasn’t covered, which definitely isn’t an executable line, so what’s going on? My initial instinct was that it was a bug and that I (obviously) had covered the file fully, but it annoyed me as I wanted to have a per-file coverage threshold, which wasn’t going to be possible if I had a bunch of files at 0%, so I looked a little deeper.

Cobertura (which is our Java coverage tool of choice) handles its metrics based on the compiled code, not the written code – though the report at the end shows the uncompiled code, unsurprisingly. When your class extends a generic interface, the compiler makes additional functions (things like equals()) to handle the various intricacies of type erasure that are definitely beyond the remit of this post. These bridge functions (for that is what they are called) will almost certainly not be executed by your tests – unless you have an incredibly intricate knowledge of the JVM and can, as a result, predict what you will need to do to cover them. The upshot of this is that your surprisingly simple class comes out with 0% coverage.

Static methods get in the way

A lot of my fresh, clean, esoteric, obscure code is contained within static utility files, meaning that the main classes have a delightfully fresh look to them. As a rule, however, you can’t mock your static methods, and so you are back to the issue of manipulating your input parameters so as to get the desired result at each stage in your process. In many respects, this goes back to the same problem highlighted as regards branch coverage – unless your unit test also covers every line and branch of the utility class, then your coverage is not has high as it might seem at first glance. Furthermore, writing a set of tests that cover the method under test and a whole different utility method breaks the guiding principal that I was once told – your unit test should cover what you can see on the screen, no more and no less. The lines in a different file should be covered by a different test.

Too much of a dependence on static methods is usually an indication that you’re doing something wrong, and you should consider changing your approach to a more dependency-injected/IOC one that would enable appropriate mocking, but sometimes that’s just not possible. In those situations, there’s no easy answer – you can try to write all of the right tests to cover the static method, or you can accept that there’s a hidden risk in the system. The other alternative (at least for Java) is to use Powermock – which allows mocking of static methods. I have heard, however, that it does dastardly things to the JVM and hence is bad, so I won’t give it a full endorsement.

The elusive 100%

While I said before that 100% coverage is nigh-on impossible, it isn’t actually impossible. As an experiment, I decided to see whether I could get 100% line and branch coverage on a proper project (rather than a thirty-line, three file one). It was an interesting learning experience, but I wouldn’t recommend it – not only because it was time-consuming, but also because it can only be achieved by various dastardly means…

  • The most dastardly thing I did was to use some fairly evil Powermock functionality to add extra values to an enum during test execution. This was probably a bad thing.
  • I also used reflection to identify the hidden bridge functions for a class and to execute them all, catching and ignoring any exceptions that occur. This was probably a bad thing.
  • I also used reflection to grab hold of private methods, make them temporarily public and then execute them in isolation. This was not so much of a bad thing.

In summary

Unit tests are only as good as the person writing them, and coverage tools can distract you from the fact that your tests don’t necessarily make any valuable assertions. Use them widely, but wisely.

Tags: , , ,

Categories: Technical

«
»

4 Responses to “Code coverage: the unattainable 100%”

  1. Russ Jackson says:

    Nice article – thanks for taking the time to write.

    I like the switch(compass) example. I think it exposes dead code rather than the impossibility of 100% coverage, though. Just remove the default case and problem solved.

    An example that I am dealing with is where a 3rd party API throws a checked exception but I have taken steps to eliminate the possibility of that exception being thrown by complying with the API requirements. But, I am still forced to deal with that exception even though I can never get code coverage on my handler for that exception.

    Another example is that I have a utility class with only static methods. Therefore, I should hide the class constructor since I never intend to have the class instantiated and it fixes Sonar violations. But, since I never exercise this private empty constructor I can never get code coverage on it. And, investing any time doing so purely for the sake of code coverage numbers is wasted effort.

    I appreciate this article and I agree completely with the premise.

    Hey, also like the bot validation used to submit comments – very clever.

  2. Chris Harris says:

    Thanks Russ, these are some good examples.

    Glad you like the bot validation, it’s from here http://bestwebsoft.com/plugin/captcha-plugin/ – although unfortunately some bots seem to be able to crack it these days!

  3. Hi Hugh.

    I agree with Russ. The problem is that default block (in Compass demo) is your problem. This code will never be reached and it shouldn’t be there. Remove it and you will gain 100% coverage.

    Second thing is that it is possible to mock static methods e.g. PowerMock supports it.

    Kind regards,
    Patryk

  4. Hugh Greenish says:

    Hi Patryk,

    I’m not sure whether it’s still the case now, but certainly at the time of writing this article, Java wouldn’t let you *not* have a default block – so the only other option is to replace your last option with the default block. In a compass that’s fairly obvious – North, South, East, Other – but once you’re onto real options in real code, I’d argue that hiding one enum value is bad practice both from a readability perspective and in terms of what might happen if new values were added to that enum.

    Powermock does indeed allow mocking of static methods – I gave it a brief mention, although you’re right that I skipped over it somewhat!

    Hugh


Leave a Reply

* Mandatory fields


+ 6 = ten

Submit Comment