Skip to content

Line Coverage: mark each line of the Branch as executable #959

Closed
@Slamdunk

Description

@Slamdunk

In the past year I had deluded myself that an ever curated ExecutableLinesFindingVisitor could eventually reach an acceptable quality over what to consider effectively an exec line or not.

That will never be the case: xDebug and PCOV are not AST-based, so neither ExecutableLinesFindingVisitor can be so.

This is even more problematic considering that PHP/xDebug/PCOV just lie.

Take this example, where no bugs of php-code-coverage is involved:
https://github.com/infection/infection/issues/1750

Lines 12, 13, 14, 15 and 16 have been executed for sure by the PHP engine, but the CC driver doesn't consider them as executed.

A solution can be to go the other way around of what we've done so far: consider each line of a branch as executable.

Disclaimer: I'm not talking about Branch/Patch coverage provided by xDebug with XDEBUG_CC_BRANCH_CHECK. That's a good feature, but pure line coverage is still the fastest and most supported CC methodology available everywhere, so that's where I want our attention to focus on.

The idea is: treat every line as executable, split them each into their branch of execution, and consider all lines of that branch as green if any of them is green according to xDebug/PCov.

Let's see this example:

$var
    =
    1
;

if ($var === 2) {
    $var
        .=
        2
    ;
}

var_dump(
    $var
);

A run with xdebug_start_code_coverage(XDEBUG_CC_UNUSED|XDEBUG_CC_DEAD_CODE) gives this result:

$var                 //  1
    =
    1
;

if ($var === 2) {    //  1
    $var
        .=
        2            // -1
    ;
}

var_dump(            //  1
    $var             //  1
);

The AST-based coverage is complex, but there are only 2 branches here, really easy to spot: outside and inside the if.
Knowing this, we can run this short checklist:

  1. Is there any line reported as 1 outside the if? Yes; then all the lines of that branch are executed
  2. Is there any line reported as 1 inside the if? No; then no lines of that branch are executed

The final report can be reported as:

$var                // green
    =               // green
    1               // green
;                   // green

if ($var === 2) {   // green
    $var            // RED
        .=          // RED
        2           // RED
    ;               // RED
}

var_dump(           // green
    $var            // green
);                  // green

The advantages of this approach are:

  1. Branch analysis is simplier than AST analysis
  2. Engine quirkiness gets hided to the user, and thus the report is more intuitive
  3. The concept of "multiline" doesn't interfere anymore in the CC analysis (all the bugs seen so far in ExecutableLinesFindingVisitor get workarounded)
  4. Downstream libraries have an easier life, for example Line CodeCoverage is not a reliable source of truth infection/infection#1750 is automatically fixed

/cc @krakjoe @derickr @mvorisek @theofidry

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions