CodeCoverage depends on autoload order

[Slamdunk]

Q	A
php-code-coverage version	9.2.10
PHP version	8.1.2
Driver	Tested on both PCOV & Xdebug
Xdebug version	3.1.2
Installation Method	Composer
Usage Method	PHPUnit
PHPUnit version	9.5.13

Hi, currently CodeCoverage depends on code execution order.
I created a self contained repo that shows this behavior in https://github.com/Slamdunk/phpunit_codecoverage_bug#readme

Basically, given the following code:

/* 1 */ class A
/* 2 */ {
/* 3 */     public function inArray(): bool
/* 4 */     {
/* 5 */         return \in_array(1, [
/* 6 */             AType::OPT,
/* 7 */         ], true);
/* 8 */     }
/* 9 */ }

1. lines 6 and 7 are not considered LOC if AType was already loaded and parsed before the A is used in the test
2. otherwise if AType is unknown to PHP during inArray call, lines 6 and 7 are considered LOC.

This messes CodeCoverage merging in context like ParaTest where test execution is isolated but not deterministic.

Merging two CC reports coming from different tests run in different processes can result in something like this:

Which doesn't make much sense

I don't know if this library can do much about this, maybe @krakjoe and @derickr could help

[dvdoug]

Can you try changing processUncoveredFiles="true" to processUncoveredFiles="false" and see if that helps?

(not sure if it will in this example thinking it through, but it has helped in some other cases of inconsistencies)

[dvdoug]

Noting that it's class constants involved here which form part of the class definition, I'm thinking that for Xdebug this might relate to/be another example of https://bugs.xdebug.org/view.php?id=1798 which affects extends and implements?

[sebastianbergmann]

CC @krakjoe @derickr

[Slamdunk]

Can you try changing processUncoveredFiles="true" to processUncoveredFiles="false" and see if that helps?

This has no effect on the underlying issue

Noting that it's class constants involved here which form part of the class definition, I'm thinking that for Xdebug this might relate to/be another example of https://bugs.xdebug.org/view.php?id=1798 which affects extends and implements?

That's likely a similar issue: the solution I proposed in #892 should also cover your #775 indeed

[Slamdunk]

With the merge of #892, IIRC, the todo list now is:

1. Unify ParsingCoveredFileAnalyser and ParsingUncoveredFileAnalyser
2. Add memory level cache, both to speed things and and address Windows filesystem issues

[sebastianbergmann]

Agreed. Do you want to work on this, Filippo? Otherwise I'll try to work on this over the weekend.

[Slamdunk]

As soon as I'm able to code something, I'll open a PR on draft: if you don't see any it means I haven't started it yet so you can take that chance

[sebastianbergmann]

Cheers!

[sebastianbergmann]

@Slamdunk I just pushed 5698195 to the refactor-static-analysis branch. It should work, but one test currently fails.

[Slamdunk]

The failing diff relates to:

php-code-coverage/tests/_files/source_without_namespace.php

Lines 12 to 18 in 6882cdb

	function &foo($bar)
	{
	$baz = function () {};
	$a = true ? true : false;
	$b = "{$a}";
	$c = "${b}";
	}

And this:

php-code-coverage/src/StaticAnalysis/IgnoredLinesFindingVisitor.php

Lines 92 to 94 in 8c95e20

	if ($node instanceof ClassMethod || $node instanceof Function_) {
	return NodeTraverser::DONT_TRAVERSE_CHILDREN;
	}

Blocks all visitors, including ExecutableLinesFindingVisitor, to analyze that function's content, even thought it shouldn't.

I'm trying some fixes

[sebastianbergmann]

Ah, of course. Did not think about the usage of NodeTraverser::DONT_TRAVERSE_CHILDREN here. We probably can no longer use this optimization anymore.

[sebastianbergmann]

Should be taken care of in 115a62b.

[sebastianbergmann]

@Slamdunk I have pushed a67e775 to 9.2 and will merge it to master ASAP.

[Slamdunk]

With your pushes and #893 merged, I think this can be closed now.

Once released, I guess other ExecutableLinesFindingVisitor missing cases will emerge from userland code, but I like that we now have much more control over the reliability and determinability of what is and what isn't a Line of Code.

[mvorisek]

in

PHP 7.4.19 (cli) (built: May  4 2021 14:24:16) ( NTS Visual C++ 2017 x64 )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.19, Copyright (c), by Zend Technologies
    with Xdebug v3.0.4, Copyright (c) 2002-2021, by Derick Rethans

and test script:

<?php

$file = realpath(__DIR__ . '/code.php');

xdebug_start_code_coverage(XDEBUG_CC_UNUSED);
require $file;
print_r(xdebug_get_code_coverage()[$file]);

and code.php:

<?php

class AType {
    const OPT = 5;
}

class Foo
{
    public function xxx(): string
    {
        $v3
            =
            AType::OPT // line 13
            . ''
        ;

        return 'y';
    }
}

var_dump((new Foo())->xxx());

(if file is ended with one or more NL, an extra/last line is reported)
I get 1️⃣:

string(1) "y"
Array
(
    [14] => 1
    [17] => 1
    [18] => -1
    [21] => 1
)

but if I change the '' in concat to 'u' I get 2️⃣:

string(1) "y"
Array
(
    [13] => 1
    [14] => 1
    [17] => 1
    [21] => 1
)

if I change then (after the previous change) the return 'y'; to return $v3 I get 3️⃣:

string(2) "5u"
Array
(
    [14] => 1
    [17] => 1
    [18] => -1
    [21] => 1
)

NOTICE THE COVERAGE IS UNSTABLE: sometimes I get if [18] => -1, sometimes [18] => 1, for example when I change 'u' quickly to 'uu' and run test script quickly again I get 4️⃣:

string(3) "5uu"
Array
(
    [14] => 1
    [17] => 1
    [18] => -1
    [21] => 1
)

but after a short time I get 5️⃣:

string(3) "5uu"
Array
(
    [13] => 1
    [14] => 1
    [17] => 1
    [21] => 1
)

(which seems to be stable then, at least for several consequent runs)

---

and I confirm the reported problem, when I move the AType class declaration to bottom like:

<?php

class Foo
{
    public function xxx(): string
    {
        $v3
            =
            AType::OPT // line 9
            . 'u'
        ;

        return $v3;
    }
}

class AType {
    const OPT = 5;
}

var_dump((new Foo())->xxx());

I get 6️⃣:

string(2) "5u"
Array
(
    [9] => 1
    [10] => 1
    [13] => 1
    [14] => -1
    [21] => 1
)

and next run I get 7️⃣:

string(2) "5u"
Array
(
    [9] => 1
    [10] => 1
    [13] => 1
    [21] => 1
)

@Slamdunk can you please return the test scripts (and post results 1️⃣ - 7️⃣ here) on your side linux/PHP 8.0 with xdebug and PCOV? Maybe some problems can be addressed in PHP/xdebug directly.

[Slamdunk]

You can easily get the result on your own with the PHP Docker images, so you can speed this process up.
Btw, here are the result for 1, 2, 3 and 6, where the befores are your output, and the afters are mine with PHP 8.1:

1️⃣

 string(1) "y"
 Array
 (
-    [14] => 1
+    [11] => 1
     [17] => 1
     [18] => -1
     [21] => 1
+    [22] => 1
 )

2️⃣

 string(1) "y"
 Array
 (
-    [13] => 1
-    [14] => 1
+    [11] => 1
     [17] => 1
+    [18] => -1
     [21] => 1
+    [22] => 1
 )

3️⃣

 string(2) "5u"
 Array
 (
-    [14] => 1
+    [11] => 1
     [17] => 1
     [18] => -1
     [21] => 1
+    [22] => 1
 )

6️⃣

 string(2) "5u"
 Array
 (
+    [7] => 1
     [9] => 1
     [10] => 1
     [13] => 1
     [14] => -1
     [21] => 1
+    [22] => 1
 )

[mvorisek]

another "unstable" coverage output: #774