true

I've been in the process of creating a test suite for a project, and while I realize getting 100% coverage isn't the metric one should strive to, there is a strange bit in the code coverage report to which I would like some clarification.

See screenshot:

enter image description here

Because the last line of the method being tested is a return, the final line (which is just a closing bracket) shows up as never being executed, and as a consequence the whole method is flagged as not executed in the overview. (Either that, or I'm not reading the report correctly.)

The complete method:

static public function &getDomain($domain = null) {
    $domain = $domain ?: self::domain();

    if (! array_key_exists($domain, self::$domains)) {
        self::$domains[$domain] = new Config();
    }

    return self::$domains[$domain];
}

Is there a reason for this, or is it a glitch?

(Yes, I read through How to get 100% Code Coverage with PHPUnit, different case although similar.)

Edit:

Trudging on through the report, I noticed the same is true for a switch statement elsewhere in the code. So this behaviour is at least to some extent consistent, but baffling to me none the less.

Edit2:

I'm running on: PHPUnit 3.6.7, PHP 5.4.0RC5, XDebug 2.2.0-dev on a OS X

upvote
  flag
@Lieven The return is the last line of a method, but I posted the complete method as well. – nikc.org
upvote
  flag
The only strange bit I see is the & b to return a reference. In PHP 5.x all objects are references. I don't think this would trip up Xdebug, and it works for edorian, but you may want to try removing it. – David Harkness
upvote
  flag
@DavidHarkness Perhaps, but as I mention in my edit, the same occurs for a switch statement as well. All cases end in a break so the closing bracket is supposedly skipped. Just seems odd to me that the bracket is considered a line of runnable code at all. – nikc.org
upvote
  flag
It's not the closing brace that is runnable per se but rather that the block is exited by falling through. Use of return, continue, break, throw, etc. alter the regular flow of execution. However, in this case Xdebug appears to be incorrectly detecting that it's impossible to fall through because the return is unconditional. – David Harkness
upvote
  flag
I wouldn't say return in a function, or break inside a switch is so irregular or such a strange occurrence that it should break a code coverage analyzer. But it was already established as being a bug, see the comments in Edorian's answer. – nikc.org
upvote
  flag
They shouldn't break the CC analyzer. I'm only saying that if they are conditional then there are two ways out of the block that each must be counted by the analyzer separately. – David Harkness

4 Answers 11

up vote 35 down vote accepted

First off: 100% code coverage is a great metric to strive for. It's just not always achievable with a sane amount of effort and it's not always important to do so :)

The issue comes from xDebug telling PHPUnit that this line is executable but not covered.

For simple cases xDebug can tell that the line is NOT reachable so you get 100% code coverage there.

See the simple example below.


2nd Update

The issue is now fixed xDebug bugtracker so building a new version of xDebug will solve those issues :)

Update (see below for issues with php 5.3.x)

Since you are running PHP 5.4 and the DEV version of xDebug I've installed those and tested it. I run into the same issues as you with the same output you've commented on.

I'm not a 100% sure if the issue comes from php-code-coverage (the phpunit module) for xDebug. It might also be an issue with xDebug dev.

I've filed a bug with php-code-coverage and we'll figure out where the issue comes from.


For PHP 5.3.x issues:

For more complex cases this CAN fail.

For the code you showed all I can say is that "It works for me" (complex sample below).

Maybe update xDebug and PHPUnit Versions and try again.

I've seen it fail with current versions but it depends on how the whole class looks sometimes.

Removing ?: operators and other single-line multi-statement things might also help out.

There is ongoing refactoring in xDebug to avoid more of those cases as far as I'm aware. xDebug once wants to be able to provide "statement coverage" and that should fix a lot of those cases. For now there is not much one can do here

While //@codeCoverageIgnoreStart and //@codeCoverageIgnoreEnd will get this line "covered" it looks really ugly and is usually doing more bad than good.

For another case where this happens see the question and answers from:

what-to-do-when-project-coding-standards-conflicts-with-unit-test-code-coverage


Simple example:

<?php
class FooTest extends PHPUnit_Framework_TestCase {
    public function testBar() {
        $x = new Foo();
        $this->assertSame(1, $x->bar());
    }
}

<?php
class Foo {
    public function bar() {
        return 1;
    }
}

produces:

phpunit --coverage-text mep.php 
PHPUnit 3.6.7 by Sebastian Bergmann.

.

Time: 0 seconds, Memory: 3.50Mb

OK (1 test, 1 assertion)

Generating textual code coverage report, this may take a moment.

Code Coverage Report 
  2012-01-10 15:54:56

 Summary: 
  Classes: 100.00% (2/2)
  Methods: 100.00% (1/1)
  Lines:   100.00% (1/1)

Foo
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  1/  1)

Complex example:

<?php

require __DIR__ . '/foo.php';

class FooTest extends PHPUnit_Framework_TestCase {

    public function testBar() {
        $this->assertSame('b', Foo::getDomain('a'));
        $this->assertInstanceOf('Config', Foo::getDomain('foo'));
    }
}

<?php

class Foo {
    static $domains = array('a' => 'b');

    static public function &getDomain($domain = null) {
        $domain = $domain ?: self::domain();
        if (! array_key_exists($domain, self::$domains)) {
            self::$domains[$domain] = new Config();
        }
        return self::$domains[$domain];
    }
}

class Config {}

produces:

PHPUnit 3.6.7 by Sebastian Bergmann.

.

Time: 0 seconds, Memory: 3.50Mb

OK (1 test, 2 assertions)

Generating textual code coverage report, this may take a moment.

Code Coverage Report 
  2012-01-10 15:55:55

 Summary: 
  Classes: 100.00% (2/2)
  Methods: 100.00% (1/1)
  Lines:   100.00% (5/5)

Foo
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  5/  5)
upvote
  flag
For the simple case, we have a match, but when I run your complex example, I get Classes 50%, Methods: 0.00%, Lines: 83.33%. I'll update my version information to my question. PS. Your test is accessing getDomain as an instance method while the method is declared static. – nikc.org
upvote
  flag
5.4RC and dev version of xDebug. I only tested it with 5.3.8 & current stable xDebug. I'll see if i get the same numbers for those versions – edorian
1 upvote
  flag
@nikc.org Could reproduce. Works with 5.3.8 and breaks with 5.4.0RC5 regardless when both are using xDebug 2.2.0-dev. I've filed a bug and edited information in the answer – edorian
upvote
  flag
Next to xdebug from my own experience, I think there are some flaws in the code-coverage package as well that is not able to cover semicolons on the next line and such stuff you might want to use to precisely style the code. – hakre
upvote
  flag
@hakre The semicolon edge case is actually xDebug related :) Maybe it should be added to the edge cases here: phpunit.de/manual/current/en/… – edorian
upvote
  flag
@edorian Way cool, thanks a lot! – nikc.org
1 upvote
  flag
@nikc.org This issue was fixed in git (thanks @Derick) and it now works like expected :) – edorian
1 upvote
  flag
@edorian Wow, that was awesome. Thanks Derick too! I think I'll have to donate now :-) – nikc.org

With regards to your switch statement code coverage issue, simply add a "default" case which doesn't do anything and you'll get full coverage.

Much of the problem here is the insistence on getting 100% execution coverage of "lines". (Managers like this idea; it is a simple model they can understand). Many lines aren't "executable" (whitespace, gaps between function declarations, comments, declarations, "pure syntax" e.g., the closing "}" of a switch or class declaration, or complex statements split across multiple source lines).

What you really want to know is, "is all the executable code covered?" This distinction seems silly yet leads to a solution. XDebug tracks what gets executed, well, by line number and your XDebug-based scheme thus reports ranges of executed lines. And you get the troubles discussed in this thread, including the klunky solutions of having to annotate the code with "don't count me" comments, putting "}" on the same line as the last executable statement, etc. No programmer is really willing to do that let alone maintain it.

If one defines executable code as that code which which can be called or is controlled by a conditional (what the compiler people call "basic blocks"), and the coverage tracking is done that way, then the layout of the code and the silly cases simply disappear. A test coverage tool of this type collects what is called "branch coverage", and you can get or not get 100% "branch coverage" literally by executing all the executable code. In addition, it will pick up those funny cases where you have a conditional within a line (using "x?y:z") or in which you have two conventional statements in a line (e.g.,

 if  (...)  {   if  (...)  stmt1; else stmt2; stmt3 }

Since XDebug tracks by line, I beleive it treats this as one statment, and considers it coverage if control gets to the line, when in fact there are 5 parts to actually test.

Our PHP Test Coverage tool implements these ideas. In particular, it understands that code following a return statement isn't executable, and it will tell you that you haven't executed it, if it is non-empty. That makes the OP's original problem just vanish. No more playing games to get "real" coverage numbers.

As with all choices, sometimes there is a downside. Our tool has a code instrument component that only runs under Windows; instrumented PHP code can run anywhere and the processing/display is done by a platform independent Java program. So this might be awkward for OP's OSX system. The instrumenter works fine across NFS-capable file systems, so he could arguably run the instrumenter on a PC and instrument his OSX files.

This particular problem was raised by someone trying to push his coverage numbers up; the problem was IMHO artificial and can be cured by stepping around the artificiality. There's another way to push up your numbers without writing more tests, and that's finding and removing duplicate code. If you remove duplicates, there's less code to test and testing one (non)copy in effects tests the (now nonexistent other copy) so it is easier to get higher numbers. You can read more about this here.

1 upvote
  flag
I disagree. It is bad practice to write 2 statements on a single line. Especially the example you wrote is gross. I think the exceptions are the "x?y:z" conditions. – klodoma
upvote
  flag
It may be bad practice, but people write code like that and it as to be tested. Most test organizations aren't allow to change to source just to make it convenient or match their opinions of style. – Ira Baxter

Here is what to do to get switch statement 100% coverage:

Ensure there is at least one test that sends a case that doesn't exist.

So, if you have:

switch ($name) {
    case 'terry':
        return 'blah';
    case 'lucky':
        return 'blahblah';
    case 'gerard':
        return 'blahblah';
}

ensure at least one of your tests sends a name that is neither terry nor lucky nor gerard.

Not the answer you're looking for? Browse other questions tagged or ask your own question.