search

sebastianbergmann / php-code-coverage

Library that provides collection, processing, and rendering functionality for PHP code coverage information.
BSD 3-Clause "New" or "Revised" License
8.81k stars 373 forks source link

Since v10 some lines are ignores #1022

Open mvorisek opened 10 months ago

mvorisek commented 10 months ago
Q A
php-code-coverage version 10.1.9
PHP version 8.2.13
Driver Xdebug
PCOV version (if used) n/a
Xdebug version (if used) 3.2.2
Installation Method Composer
Usage Method PHPUnit / other
PHPUnit version (if used) 10.4.2

source: https://github.com/atk4/ui/blob/5.0.0/tests/JsTest.php#L38

image

Started to happen when PHPUnit was upgraded to v10 and php-code-coverage from v9.2.29 to v10.1.9 on the same PHP version and xdebug version. See the screenshot, l38 went from uncovered to ignored.

sebastianbergmann commented 10 months ago

Thank you for your report.

Please provide a minimal, self-contained, reproducing test case that shows the problem you are reporting.

Without such a minimal, self-contained, reproducing test case I will not be able to investigate this issue.

mvorisek commented 10 months ago

The source is linked, ie. lines l37 - l39, it seems the issue is when the if expression is CTE (compile time evalueable).

sebastianbergmann commented 8 months ago

@dvdoug @Slamdunk Do you have an idea what to do here? Thanks!

Slamdunk commented 8 months ago

I see that the source is linked, but it's very cumbersome to reproduce locally, I didn't manage to achieve the result.

@mvorisek can you provide the least amount of files that reproduce the issue in a separate repo please?

mvorisek commented 8 months ago

I belive the reason you was not able to reproduce it is you did not run phpunit /w opcache. I belive so as PHP_INT_SIZE === 4 is always false (/w 64-bit php binary) and the condition body is eliminated as dead code only when opcache is enabled.

Please run phpunit /w opcache like: php -d opcache.enable_cli=1 vendor/bin/phpunit ....

The following code should be then enough to reproduce:

    public function testNumbers(): void
    {
        if (\PHP_INT_SIZE === 4) {
            self::markTestIncomplete('Test is not supported on 32bit php');
        }

        $v = 'a';
    }
sebastianbergmann commented 8 months ago

I did not realize that this is related to bytecode optimization before.

I would argue that code coverage data should not be collected/processed when OpCache is active. We should probably error out when code coverage reporting is requested and OpCache is active.

mvorisek commented 8 months ago

Thankfully, opcache is generally supported and we need it for other testing (memory leaks testing). I belive @Slamdunk can fix this as in v9 it was working.

Slamdunk commented 8 months ago

@mvorisek the repo doesn't provide all the necessary requirements for the test to run locally, I get many unrelated errors like:

1) Atk4\Ui\Tests\DemosTest::testDemosStatusAndHtmlResponse with data set #0 ('index.php')                                                                                                                                                       
Atk4\Ui\Exception: Database error: Sqlite database does not exist, create it first                                                                                                                                                              

/home/tessarotto/repos/ui/demos/init-app.php:98                                                                                                                                                                                                 
/home/tessarotto/repos/ui/tests/DemosTest.php:75                                                                                                                                                                                                
/home/tessarotto/repos/ui/vendor/bin/phpunit:122                                                                                                                                                                                                

2) Atk4\Ui\Tests\DemosTest::testDemosStatusAndHtmlResponse with data set #1 ('_unit-test/callback-nested.php')                                                                                                                                  
Error: Attempt to assign property "runCalled" on string                                                                                                                                                                                         

/home/tessarotto/repos/ui/tests/DemosTest.php:85                                                                                                                                                                                                
/home/tessarotto/repos/ui/vendor/bin/phpunit:122

Can you provide some Docker based reproducible environment please?

mvorisek commented 8 months ago

https://github.com/sebastianbergmann/php-code-coverage/issues/1022#issuecomment-1895822629 this few lines should be enough - no other repo/files should be needed

Slamdunk commented 8 months ago

I honestly have no idea what that's supposed to mean, but in the previous run I get this:

image

:shrug:

mvorisek commented 8 months ago

Please confim me you get l38 uncovered - ie. not uncounted/unmeasured - with v10 and opcache enabled, if so, I will provide a repro.

Slamdunk commented 8 months ago

I get L38 counted and uncovered

sebastianbergmann commented 8 months ago

@mvorisek Can you share your OpCache settings, please? Thanks!

mvorisek commented 8 months ago

@Slamdunk thank you - one more check - l37 must be of course covered (in https://github.com/sebastianbergmann/php-code-coverage/issues/1022#issuecomment-1895845117 screenshot), ie. please run the test and confirm the previous.

Slamdunk commented 8 months ago

L37 is uncovered because I am not able to run the test suite locally, so...

mvorisek commented 8 months ago

Here is a full repro:

https://github.com/atk4/ui/tree/repro_coverage_1022

https://github.com/atk4/ui/blob/repro_coverage_1022/src/Repro/Repro.php and https://github.com/atk4/ui/blob/repro_coverage_1022/tests/JsTest.php files are enough to reproduce

opcache config: nothing special, no JIT, just enabled opcache for CLI

https://app.codecov.io/gh/atk4/ui/commit/4c95b7fbdaee358232897bb9c4c85aed67eb9f1c/blob/src/Repro/Repro.php

image

Slamdunk commented 8 months ago

Running that branch locally I found:

  1. xDebug always scans the code regardless of the opcache active or not, and the CC is always as expected (if line green, code inside if red); in the issue you wrote Xdebug version (if used) => 3.2.2 but the only way I have been able to reproduce this is without xDebug
  2. The issue is very old: I forced the installation of "phpunit/phpunit": "9.2.0" and "phpunit/php-code-coverage": "9.0.0" and the issue was already present: image I couldn't go further back in versions history because many components started conflicting each other.

But @mvorisek you stated that it was working on php-code-coverage:v9.2.29: can you create a reproducible build on that version that demonstrates the bug wasn't there? I am unable to.

mvorisek commented 8 months ago

image

deps of https://github.com/atk4/ui/commit/cb8e2d02e92242f1affafaa4810691fe3da502c8 (after upgrade): https://github.com/atk4/ui/actions/runs/6996994492/job/19033470295#step:7:20 [1] deps of https://github.com/atk4/ui/commit/15f5d101a2e8d613b033094302ee08288e67c476 (before upgrade): https://github.com/atk4/ui/actions/runs/6971004868/job/18970098703#step:7:20 [2]

coverage after upgrade: https://app.codecov.io/gh/atk4/ui/commit/cb8e2d02e92242f1affafaa4810691fe3da502c8/blob/tests/JsTest.php#L38 coverage before upgrade: https://app.codecov.io/gh/atk4/ui/commit/15f5d101a2e8d613b033094302ee08288e67c476/blob/tests/JsTest.php#L38

in the coverage pages, there is possible to download the uploaded data: image

[1]

  - Locking atk4/behat-mink-selenium2-driver (1.6.2)
  - Locking atk4/core (dev-develop aa92f8e)
  - Locking atk4/data (dev-develop 0a015af)
  - Locking atk4/ergebnis-phpunit-slow-test-detector (2.4.0)
  - Locking behat/mink (v1.10.0)
  - Locking benmorel/weakmap-polyfill (0.4.0)
  - Locking doctrine/cache (2.2.0)
  - Locking doctrine/dbal (3.6.7)
  - Locking doctrine/deprecations (1.1.2)
  - Locking doctrine/event-manager (2.0.0)
  - Locking fzaninotto/faker (dev-master 5ffe7db)
  - Locking guzzlehttp/guzzle (7.8.0)
  - Locking guzzlehttp/promises (2.0.1)
  - Locking guzzlehttp/psr7 (2.6.1)
  - Locking instaclick/php-webdriver (1.4.16)
  - Locking mvorisek/atk4-hintable (1.9.9)
  - Locking myclabs/deep-copy (1.11.1)
  - Locking nikic/php-parser (v4.17.1)
  - Locking nyholm/psr7 (1.8.1)
  - Locking nyholm/psr7-server (1.1.0)
  - Locking phar-io/manifest (2.0.3)
  - Locking phar-io/version (3.2.1)
  - Locking phpunit/php-code-coverage (10.1.9)
  - Locking phpunit/php-file-iterator (4.1.0)
  - Locking phpunit/php-invoker (4.0.0)
  - Locking phpunit/php-text-template (3.0.1)
  - Locking phpunit/php-timer (6.0.0)
  - Locking phpunit/phpcov (9.0.2)
  - Locking phpunit/phpunit (10.4.2)
  - Locking psr/cache (3.0.0)
  - Locking psr/http-client (1.0.3)
  - Locking psr/http-factory (1.0.2)
  - Locking psr/http-message (2.0)
  - Locking psr/log (3.0.0)
  - Locking ralouphie/getallheaders (3.0.3)
  - Locking sebastian/cli-parser (2.0.0)
  - Locking sebastian/code-unit (2.0.0)
  - Locking sebastian/code-unit-reverse-lookup (3.0.0)
  - Locking sebastian/comparator (5.0.1)
  - Locking sebastian/complexity (3.1.0)
  - Locking sebastian/diff (5.0.3)
  - Locking sebastian/environment (6.0.1)
  - Locking sebastian/exporter (5.1.1)
  - Locking sebastian/global-state (6.0.1)
  - Locking sebastian/lines-of-code (2.0.1)
  - Locking sebastian/object-enumerator (5.0.0)
  - Locking sebastian/object-reflector (3.0.0)
  - Locking sebastian/recursion-context (5.0.0)
  - Locking sebastian/type (4.0.0)
  - Locking sebastian/version (4.0.1)
  - Locking symfony/css-selector (v6.3.2)
  - Locking symfony/deprecation-contracts (v3.4.0)
  - Locking symfony/filesystem (v6.3.1)
  - Locking symfony/http-foundation (v6.3.8)
  - Locking symfony/polyfill-ctype (v1.28.0)
  - Locking symfony/polyfill-mbstring (v1.28.0)
  - Locking symfony/polyfill-php80 (v1.28.0)
  - Locking symfony/polyfill-php81 (v1.28.0)
  - Locking symfony/polyfill-php82 (v1.28.0)
  - Locking symfony/polyfill-php83 (v1.28.0)
  - Locking symfony/process (v6.3.4)
  - Locking symfony/yaml (v6.3.8)
  - Locking theseer/tokenizer (1.2.2)

[2]

  - Locking atk4/behat-mink-selenium2-driver (1.6.2)
  - Locking atk4/core (dev-develop 9bc7010)
  - Locking atk4/data (dev-develop 2a7e312)
  - Locking behat/mink (v1.10.0)
  - Locking benmorel/weakmap-polyfill (0.4.0)
  - Locking doctrine/cache (2.2.0)
  - Locking doctrine/dbal (3.6.7)
  - Locking doctrine/deprecations (1.1.2)
  - Locking doctrine/event-manager (2.0.0)
  - Locking doctrine/instantiator (2.0.0)
  - Locking ergebnis/composer-normalize (2.39.0)
  - Locking ergebnis/json (1.1.0)
  - Locking ergebnis/json-normalizer (4.3.0)
  - Locking ergebnis/json-pointer (3.3.0)
  - Locking ergebnis/json-printer (3.4.0)
  - Locking ergebnis/json-schema-validator (4.1.0)
  - Locking fzaninotto/faker (dev-master 5ffe7db)
  - Locking guzzlehttp/guzzle (7.8.0)
  - Locking guzzlehttp/promises (2.0.1)
  - Locking guzzlehttp/psr7 (2.6.1)
  - Locking instaclick/php-webdriver (1.4.16)
  - Locking johnkary/phpunit-speedtrap (v3.3.0)
  - Locking justinrainbow/json-schema (v5.2.13)
  - Locking localheinz/diff (1.1.1)
  - Locking mvorisek/atk4-hintable (1.9.8)
  - Locking myclabs/deep-copy (1.11.1)
  - Locking nikic/php-parser (v4.17.1)
  - Locking nyholm/psr7 (1.8.1)
  - Locking nyholm/psr7-server (1.1.0)
  - Locking phar-io/manifest (2.0.3)
  - Locking phar-io/version (3.2.1)
  - Locking phpunit/php-code-coverage (9.2.29)
  - Locking phpunit/php-file-iterator (3.0.6)
  - Locking phpunit/php-invoker (3.1.1)
  - Locking phpunit/php-text-template (2.0.4)
  - Locking phpunit/php-timer (5.0.3)
  - Locking phpunit/phpcov (8.2.1)
  - Locking phpunit/phpunit (9.6.13)
  - Locking psr/cache (3.0.0)
  - Locking psr/http-client (1.0.3)
  - Locking psr/http-factory (1.0.2)
  - Locking psr/http-message (2.0)
  - Locking psr/log (3.0.0)
  - Locking ralouphie/getallheaders (3.0.3)
  - Locking sebastian/cli-parser (1.0.1)
  - Locking sebastian/code-unit (1.0.8)
  - Locking sebastian/code-unit-reverse-lookup (2.0.3)
  - Locking sebastian/comparator (4.0.8)
  - Locking sebastian/complexity (2.0.2)
  - Locking sebastian/diff (4.0.5)
  - Locking sebastian/environment (5.1.5)
  - Locking sebastian/exporter (4.0.5)
  - Locking sebastian/global-state (5.0.6)
  - Locking sebastian/lines-of-code (1.0.3)
  - Locking sebastian/object-enumerator (4.0.4)
  - Locking sebastian/object-reflector (2.0.4)
  - Locking sebastian/recursion-context (4.0.5)
  - Locking sebastian/resource-operations (3.0.3)
  - Locking sebastian/type (3.2.1)
  - Locking sebastian/version (3.0.2)
  - Locking symfony/css-selector (v6.3.2)
  - Locking symfony/deprecation-contracts (v3.4.0)
  - Locking symfony/filesystem (v6.3.1)
  - Locking symfony/http-foundation (v6.3.8)
  - Locking symfony/polyfill-ctype (v1.28.0)
  - Locking symfony/polyfill-mbstring (v1.28.0)
  - Locking symfony/polyfill-php80 (v1.28.0)
  - Locking symfony/polyfill-php81 (v1.28.0)
  - Locking symfony/polyfill-php82 (v1.28.0)
  - Locking symfony/polyfill-php83 (v1.28.0)
  - Locking symfony/process (v6.3.4)
  - Locking symfony/yaml (v6.3.8)
  - Locking theseer/tokenizer (1.2.2)
Slamdunk commented 8 months ago

Aaaaand... how do I reproduce this locally?

I hope may come the day that I don't ask this question ever again :pray:

mvorisek commented 8 months ago

Is it relevant? The issue is l38 is uncounted and you was able to reproduce it. Let's focus on the fix.

sebastianbergmann commented 8 months ago

Is it relevant?

Of course it is relevant. How is @Slamdunk supposed to fix a bug that he cannot reproduce locally?

Slamdunk commented 8 months ago

As far as I can tell, this issue is not linked to https://github.com/sebastianbergmann/php-code-coverage/pull/964

I was trying to help you looking for when the issue was introduced, but I can only search for it if I can reproduce it locally.

I'm happy though that for you this is not relevant: ping us when you fix it by yourself then :+1:

mvorisek commented 8 months ago

In https://github.com/sebastianbergmann/php-code-coverage/issues/1022#issuecomment-1895978140 you have written:

xDebug always scans the code regardless of the opcache active or not, and the CC is always as expected (if line green, code inside if red)

so what else input/repro you need?

This is what is expected - if line green, code inside if red (not blank/not uncovered).

If lines generated by #964 are missing in coverage output in general, they must be red/uncovered, not ignored.

Slamdunk commented 8 months ago

Those 2 lines have always been missing from CC with opcache active. Your bug only appeared recently. I am unable to match this two informations and thus I am unable to find any meaningful source for your bug.

If you want your bug to be solved, I'm afraid I am unable to. If you want opcache to be irrelevant for CC generation, I re-quote Sebastian:

I would argue that code coverage data should not be collected/processed when OpCache is active. We should probably error out when code coverage reporting is requested and OpCache is active.

I am sorry I can't help any further; I have already spent all the free time I am willing to on this topic. If you think my contribution can still be valuable to you, I can give you a quote for inspecting it until it's solved.

mvorisek commented 8 months ago

I tried to help with the information in v9 it was working. However, I do not know myself why (even after a few experiments), but it seems the issue was also in v9. Let's please do not insist on this. Let's please acknowledge the issue is present and let's focus on the fix.