The Hidden Trap of Code Coverage

.

Tags:

Code which never gets tested is an accident waiting to happen. The coverage of the code as it is exercised by the suite becomes an important metric. It is not a surprise if everyone aims to hit that magical 100% coverage. This applies also to the world of front-end web development with JavaScript.

I’ve written previously on the use of coverage tool to ensure Esprima code quality (with a minor twist that the coverage tool is also based on Esprima). This allows us to claim that we reach the maximum possible statement coverage since the set of 500 unit tests touches every meaningful statement in the code.

While statement coverage is important and you should definitely try to hit its peak, care must be taken to ensure that this is not the only coverage analysis being considered. As an illustration, let’s have a look at the following function:

function inc(p, q) {
    if (q == undefined) q = 1;
    return p + q/q;
}

The task of this function is very simple. I’m sure you can quickly notice the glaring bug in that function. This is intentional to demonstrate the point. Please do realize that in real-word scenarios, functions tend to be bigger and a logic mistake is often hidden in multiple layer of complexity.

To test that function, a simple assertion should be enough:

assert("inc(4) must give 5", inc(4) == 5);

The test reporter will not complain since it will pass with flying colors. If the test library supports checking the code coverage of the function, it will also say that both the if statement (including its body containing the assignment to q) and the return statement are executed. From the statement coverage point of view, such a reporter will say Congrats, 100% covered!.

At this point, if you stop writing more tests because you are confident of the achievement, you will never expose the bug in the function. Only if you add another test, e.g checking for inc(4, 2), you would realize that something is fishy with the implementation.

Looking at the code, how is the situation different if it would have been written like the following?

function inc(p, q) {
    if (q == undefined) {
        return p + 1;
    } else {
        return p + q/q;
    }
}

Running the same test will still mark it as pass but now the coverage analyzer loudly complains that you only hit one of the return statements. It’s definitely not 100% covered. You are then forced to continue writing more tests which hit another return statement and eventually reveal the bug.

Why is this possible? Mainly because typical statement coverage analysis does not reveal the actual code sequence. In the above example, there are two code paths to the end of the function and the unit test only follows one of them. The two code paths are unfortunately not obvious in the first version of the code.

As I always said, we still need to push JavaScript tools to the next level. I hope someone out there is working on JavaScript LCSAJ.

Update: Now you can use Istanbul to track JavaScript branch coverage. Very useful!

My Twitter profileMy Google+ profileMy LinkedIn profile

Ariya Hidayat

Software Provocateur
These days, I promote software craftsmanship around web technologies. If you like this article, read also other similar blog posts and follow me @ariyahidayat.
My LinkedIn profile

Latest posts by Ariya Hidayat (see all)

  • David Johnson

    Your tests have to examine the boundary conditions of your logic, and code coverage simply doesn’t address that. I think the best use of code coverage tools is to simply show you what code has and has not been addressed by your tests.

    • http://ariya.ofilabs.com/ Ariya Hidayat

      It’s the case for the statement coverage. The LCSAJ analysis on the other hand is supposed to give you the program flow coverage.

  • Frerich Raabe

    I think what you’re looking for is a tool which does decision coverage or (in your case) branch coverage. As it happens, the company I work for has such a product. ;-)

    In either case, your code coverage tool wouldn’t have caught that calling your function like inc(x, 0) would try to divide-by-zero. You could either add a new conditional for that, or assert() (some code coverage tools will take assertions into account when deciding whether the control flow can reach a certain point or not).

  • http://profiles.google.com/florin.jurcovici Florin Jurcovici

    If I understand you correctly, you’re saying that 100% code coverage does not provide a guarantee that you properly test your code. Am I right? If yes, that’s no different from any other language in which unit tests are possible. Proper testing is a matter of semantics, and tools are still very bad at this, and coverage specifically has very little to do with semantics. IMO.

    • http://ariya.ofilabs.com/ Ariya Hidayat

      Note that a different type of coverage analysis, e.g. linear code sequence and jump, relates better to the semantics. There’s also symbolic execution for more assisted semantic-based test construction.

      • http://profiles.google.com/florin.jurcovici Florin Jurcovici

        As a human, you can only understand what the code does, not what it’s supposed to do, by just reading it. The same with tools. In order for tools to be able to semantically analyze code, they’d need to develop an understanding of the problem domain. Most often such tools don’t even get a description of the problem domain. If tools were able to understand problem domains, we’d have programs writing code for us already.

        • http://ariya.ofilabs.com/ Ariya Hidayat

          The goal of tooling is not that ambitious. It’s only to go as far as helping the developers on a best-effort basis.

  • PC

    Isn’t q/q always going to be 1?

    • DG

      The code in this example is horrible, isn’t it? I suggest reading Clean Code.

      • http://ariya.ofilabs.com/ Ariya Hidayat

        Don’t read too much into the example as is, it’s only to illustrate the point.

    • http://ariya.ofilabs.com/ Ariya Hidayat

      Yes, that’s intentional. I’m sure you understand what I mean after reading to the end.

  • darren

    I look at code coverage like it is a barometric gauge, in general I mainly care if the code coverage is dropping, not what the actual value is.

  • Jon Peterson

    I don’t see why one would think their code was covered if they only use one parameter to test a function that accepts two parameters. The solution to something like this would not be an automated tool (edge cases, man… the bane of automation) but rather better code documentation and review processes. The individual writing that function should have documented that it accepted two parameters, and that the second one was optional and defaulted to 1.

    Relying on tools to guess at the intentions of programmers and attempt to figure out all possible paths data can take through the code only results in more hidden bugs as the automation tools gleefully fail to recognize unusual situations.

    • http://ariya.ofilabs.com/ Ariya Hidayat

      While this is true in this particular heavily simplified example, the real-world code is often different and it is hard to ensure that there are still stones left unturned. Automatic code sequence and jump coverage analysis can help.

  • Krishnan Anantheswaran

    I thought I left a message here but apparently it did not stick.

    I wrote yet another code coverage tool:
    https://github.com/gotwarlost/istanbul

    based on esprima, of course (thanks for the lovely library!)

    It computes “branch” coverage in addition to statement and function coverage.
    While it doesn’t solve world hunger, this tool would have done better in this particular example since it would have shown that the `else` path for the `if` was not taken.

    Would love to get your feedback on this coverage library.

    Thanks,
    Krishnan

    • http://ariya.ofilabs.com/ Ariya Hidayat

      I spotted Istanbul the other day (via YUI), looks like an excellent tool indeed! I will give it a try soon-ish!

  • Gabriel Pettier

    I though the real bug was to find with something like `inc(x, 0)`

    however, yes, coverage should reflect branch coverage, not line coverage, one way is to define in your style guide that the result is always on a different line, and i guess you need to always define an “else” branch too.