-
-
Notifications
You must be signed in to change notification settings - Fork 160
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.
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
Infection wrongly claims "continue to break" mutation would be uncovered #1874
Comments
Hello, thanks for creating repository. I've cloned it, and on my PC it kills all the mutants. In the
please note that test fails. My PHP is also 8.2.8:
The only one idea I have is that you use This is what I have: XDEBUG_MODE=coverage ./tools/phpunit --coverage-html=coverage-html |
"uncovered" is the Infection's term, not PHPUnit's. It means that the line of code, on which mutation is done, is not covered by any test based on the information of coverage driver. In your case it's So when Infection "knows" the line is not covered, then mutant can not be killed in any way. Infection relies on coverage in the first place: this is source of truth. Though in the reality coverage driver may produce incorrect coverage and result to false-positives. We had many such cases before, and @Slamdunk has a great job improving PHPUnit's coverage package, see
Please create a ticket in the |
That's only arguably correct: According to the (potentially wrong) output from pcov, the line in question can not be covered - as it's considered none executable. The coverage information for the code in general says 100% (of coverable lines).
I'd challenge that decision - at least for the sake of arguing ;): The mutation makes tests fail that passed before. Thus, the mutation is covered by a test.
@sebastianbergmann: Shall I? |
Yes, please. And here's hoping that @Slamdunk is willing / has time to fix yet another issue :-) |
Already on it 😉 |
@Slamdunk So, no ticket needed? |
this is a trade-off of course, but it's a way cheaper (=faster) to rely on code coverage information rather than run the tests to check if they fail when the line is marked as not covered by the coverage driver. If Infection would do this, it would be slower, IMO. |
@theseer I cannot reproduce the issue (Ubuntu 23.04 here, binaries from https://deb.sury.org/), both xDebug and PCOV report 9th line as expected: $ php -d xdebug.mode=coverage -d pcov.enabled=0 ./tools/phpunit --coverage-clover=php://stdout
PHPUnit 10.2.6 by Sebastian Bergmann and contributors.
Runtime: PHP 8.2.8 with Xdebug 3.2.1
Configuration: /home/tessarotto/repos/infection-bug/phpunit.xml
. 1 / 1 (100%)
Time: 00:00.065, Memory: 26.41 MB
OK (1 test, 1 assertion)
Generating code coverage report in Clover XML format ...
[...]
<line num="9" type="stmt" count="1"/>
[...]
$ php -d xdebug.mode=off -d pcov.enabled=1 ./tools/phpunit --coverage-clover=php://stdout
PHPUnit 10.2.6 by Sebastian Bergmann and contributors.
Runtime: PHP 8.2.8 with PCOV 1.0.11
Configuration: /home/tessarotto/repos/infection-bug/phpunit.xml
. 1 / 1 (100%)
Time: 00:00.001, Memory: 22.41 MB
OK (1 test, 1 assertion)
Generating code coverage report in Clover XML format ...
[...]
<line num="9" type="stmt" count="1"/>
[...] And case is already tested in Not sure where the bug is, but definitely not in Infection, so we need to continue the discussion either in |
@Slamdunk Interesting. I have literally no idea how that can be:
I can try to make a fedora based container for you? I agree that aspect is getting off topic for this repository. On the other hand, I'd still argue that infection is not fully correct here: The line according to pcov (at least on my box) is marked as uncoverable, thus complaining it's not covered is at least questionable. I understand that's an edge case that might also be a bug in pcov only triggered on my box for whatever bizarre reason. I have to admit that what @maks-rafalko wrote regarding not actually running the tests surprised me. I didn't look into the code, but at least the infection playground shows a phpunit run for every mutation. I also don't understand how one could reliably detect the effectiveness of a mutation by merely looking at coverage data. If I were to But this also goes off-topic for this issue and maybe we should discuss this in person or using a different means of communication. |
Yes please
A new issue on https://github.com/sebastianbergmann/php-code-coverage |
Found it. It's an opcache issue that apparently throws of pcov:
I guess that qualifies as a bug in pcov? |
I don't have the necessary knowledge to answer this question |
I'll open on there and we'll see what happens ;) |
For reference: krakjoe/pcov#101 Given the last release is from 2021, I'm not having high hopes there's anything going to happen any time soon. |
No, please look at this example: https://infection-php.dev/r/8w2k as you can see, method In a normal scenario, if Infection sees uncovered code, it doesn't run tests. This is how it was intentionally designed :) Code that does this is here: infection/src/Process/Runner/MutationTestingRunner.php Lines 91 to 100 in 3e0387b
On the other side, if the line is covered (from coverage driver) - Infection runs tests to see if they kill or not mutants. |
I totally agree with you for not covered executable lines of code. I just didn't know infection is that smart :) The only thing I do not agree with is that a mutation on a proclaimed non-executable-line (falsely, i know) can ever be covered. But given the situation only arose because of a bug / optimizer side effect, this certainly qualifies as an edge case. I wouldn't even know what the correct behavior would be in that case, e.g. skip the mutation or require to run tests. So, yes, let's just keep this issue closed because it's not worth the effort of considering a change. |
The above repository should be self-contained - except for PHP itself.
When running
./tools/infection
, the report (seeinfection.txt
) claims, the mutation to change thecontinue
in the sample code tobreak
would be uncovered.That's wrong, as manually changing the Code and running PHPUnit shows.
Interestingly enough, when extracting the example and pasting it into the Infection Playground (
https://infection-php.dev/r/4evk) this same change is claimed as covered.
The text was updated successfully, but these errors were encountered: