Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Since v10 some lines are ignores #1022

Open
mvorisek opened this issue Nov 26, 2023 · 25 comments
Open

Since v10 some lines are ignores #1022

mvorisek opened this issue Nov 26, 2023 · 25 comments

Comments

@mvorisek
Copy link
Contributor

mvorisek commented Nov 26, 2023 •
edited
Loading

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
Copy link
Owner

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
Copy link
Contributor Author

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

@sebastianbergmann
Copy link
Owner

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

@Slamdunk
Copy link
Contributor

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
Copy link
Contributor Author

mvorisek commented Jan 17, 2024 •
edited
Loading

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor

@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
Copy link
Contributor Author

#1022 (comment) this few lines should be enough - no other repo/files should be needed

@Slamdunk
Copy link
Contributor

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

image

🤷

@mvorisek
Copy link
Contributor Author

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

@Slamdunk
Copy link
Contributor

Slamdunk commented Jan 17, 2024 •
edited
Loading

I get L38 counted and uncovered

@sebastianbergmann
Copy link
Owner

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

@mvorisek
Copy link
Contributor Author

@Slamdunk thank you - one more check - l37 must be of course covered (in #1022 (comment) screenshot), ie. please run the test and confirm the previous.

@Slamdunk
Copy link
Contributor

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

@mvorisek
Copy link
Contributor Author

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
Copy link
Contributor

Slamdunk commented Jan 17, 2024 •
edited
Loading

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
Copy link
Contributor Author

mvorisek commented Jan 17, 2024 •
edited
Loading

image

deps of atk4/ui@cb8e2d0 (after upgrade): https://github.com/atk4/ui/actions/runs/6996994492/job/19033470295#step:7:20 [1]
deps of atk4/ui@15f5d10 (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
Copy link
Contributor

Slamdunk commented Jan 18, 2024 •
edited
Loading

Aaaaand... how do I reproduce this locally?

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

@mvorisek
Copy link
Contributor Author

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

@sebastianbergmann
Copy link
Owner

Is it relevant?

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

@Slamdunk
Copy link
Contributor

As far as I can tell, this issue is not linked to #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 👍

@mvorisek
Copy link
Contributor Author

In #1022 (comment) 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
Copy link
Contributor

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@sebastianbergmann @Slamdunk @mvorisek