Infection wrongly claims "continue to break" mutation would be uncovered

[theseer]

Question	Answer
Infection version	0.26.21 (phar)
Test Framework version	10.2.6 (phar)
PHP version	8.2.8 / 8.3.0beta1
Platform	Fedora 38
Github Repo	https://github.com/theseer/infection-bug

The above repository should be self-contained - except for PHP itself.

When running ./tools/infection, the report (see infection.txt) claims, the mutation to change the continue in the sample code to break 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.

[maks-rafalko]

Hello, thanks for creating repository.

I've cloned it, and on my PC it kills all the mutants. In the infection.txt, when I run as ./tools/infection --debug --log-verbosity=all I see the following:

) /tmp/remove/infection-bug/src/Bug.php:9    [M] Continue_

--- Original
+++ New
@@ @@
         $result = [];
         foreach ($data as $value) {
             if (!$this->isOkay($value)) {
-                continue;
+                break;
             }
             $result[] = $value;
         }

$ '/tmp/remove/infection-bug/tools/phpunit' '--configuration' '/tmp/infection/phpunitConfiguration.70414d51cf4372d744dde90073794da4.infection.xml'
  PHPUnit 10.2.6 by Sebastian Bergmann and contributors.
  
  Runtime:       PHP 8.2.8
  Configuration: /tmp/infection/phpunitConfiguration.70414d51cf4372d744dde90073794da4.infection.xml
  
  F                                                                   1 / 1 (100%)
  
  Time: 00:00, Memory: 22.41 MB
  
  There was 1 failure:
  
  1) BugTest::testResult
  Failed asserting that two arrays are equal.
  --- Expected
  +++ Actual
  @@ @@
   Array (
       0 => 'a'
  -    1 => 'c'
   )
  
  /tmp/remove/infection-bug/tests/BugTest.php:10
  
  FAILURES!
  Tests: 1, Assertions: 1, Failures: 1.

please note that test fails. My PHP is also 8.2.8:

PHP 8.2.8 (cli) (built: Jul  5 2023 18:47:24) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.2.8, Copyright (c) Zend Technologies
    with Xdebug v3.2.1, Copyright (c) 2002-2023, by Derick Rethans

The only one idea I have is that you use pcov instead of xdebug? If so, could you please generate html coverage for the tests and see if the continue line is covered?

This is what I have:

XDEBUG_MODE=coverage ./tools/phpunit --coverage-html=coverage-html

[theseer]

Coverage Information with pcov:

Indeed that line is not marked as covered. I can confirm it is marked as covered as well as killed when using xdebug.

While that clearly is a difference in coverage information and likely the culprit, I'm not sure I understand the complaint infection has: The mutation makes the test fail, thus it is technically killed.

Isn't it? Shouldn't the coverage info only be of secondary concern? Or is that exactly what "uncovered" tries to tell me?

Update:
Looking at the screenshot again, with PCOV, it also is not marked as uncovered but rather none executable.

[maks-rafalko]

"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 pcov.

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

• Line CodeCoverage is not a reliable source of truth #1750
• Group line code-coverage by branch sebastianbergmann/php-code-coverage#964
• False positive: not covered mutant for arrow function #1747

Please create a ticket in the sebastianbergmann/php-code-coverage instead, but IMO we can do nothing on the Infection side.

[theseer]

"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 pcov.

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).

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.

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.

Please create a ticket in the sebastianbergmann/php-code-coverage instead, but IMO we can do nothing on the Infection side.

@sebastianbergmann: Shall I?

[sebastianbergmann]

@sebastianbergmann: Shall I?

Yes, please. And here's hoping that @Slamdunk is willing / has time to fix yet another issue :-)

[Slamdunk]

Already on it 😉

[theseer]

@Slamdunk So, no ticket needed?

[maks-rafalko]

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.

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.

[Slamdunk]

@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 sebastianbergmann/php-code-coverage:

https://github.com/sebastianbergmann/php-code-coverage/blob/723fc3165bd366d7c3325e817ed686ae81b484d6/tests/_files/source_for_branched_exec_lines.php#L268-L270

Not sure where the bug is, but definitely not in Infection, so we need to continue the discussion either in php-code-coverage or pcov repo.

[theseer]

@Slamdunk Interesting. I have literally no idea how that can be:

theseer@nyda ~/Desktop/bug main $ 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/theseer/Desktop/bug/phpunit.xml

.                                                                   1 / 1 (100%)

Time: 00:00.001, Memory: 14.41 MB

OK (1 test, 1 assertion)

Generating code coverage report in Clover XML format ... <?xml version="1.0" encoding="UTF-8"?>
<coverage generated="1690364458">
  <project timestamp="1690364458">
    <file name="/home/theseer/Desktop/bug/src/Bug.php">
      <class name="Bug" namespace="global">
        <metrics complexity="4" methods="2" coveredmethods="2" conditionals="0" coveredconditionals="0" statements="6" coveredstatements="6" elements="8" coveredelements="8"/>
      </class>
      <line num="5" type="method" name="test" visibility="public" complexity="3" crap="3" count="1"/>
      <line num="6" type="stmt" count="1"/>
      <line num="7" type="stmt" count="1"/>
      <line num="8" type="stmt" count="1"/>
      <line num="12" type="stmt" count="1"/>
      <line num="15" type="stmt" count="1"/>
      <line num="18" type="method" name="isOkay" visibility="private" complexity="1" crap="1" count="1"/>
      <line num="19" type="stmt" count="1"/>
      <metrics loc="23" ncloc="23" classes="1" methods="2" coveredmethods="2" conditionals="0" coveredconditionals="0" statements="6" coveredstatements="6" elements="8" coveredelements="8"/>
    </file>
    <metrics files="1" loc="23" ncloc="23" classes="1" methods="2" coveredmethods="2" conditionals="0" coveredconditionals="0" statements="6" coveredstatements="6" elements="8" coveredelements="8"/>
  </project>
</coverage>
done [00:00]

I can try to make a fedora based container for you? I agree that aspect is getting off topic for this repository.
(@Slamdunk How do we continue with this?)

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 #[UsesClass] a collaborator without it having explicit tests, any behavior changing mutation of that class will break the test of the unit under test. I totally agree that that's rather implicit but nevertheless the mutation didn't survive the test run.

But this also goes off-topic for this issue and maybe we should discuss this in person or using a different means of communication.

[Slamdunk]

I can try to make a fedora based container for you?

Yes please

How do we continue with this?

A new issue on https://github.com/sebastianbergmann/php-code-coverage

[theseer]

Found it.

It's an opcache issue that apparently throws of pcov:

$ php ./tools/phpunit --coverage-clover=php://stdout | grep -c 'num="9"'
0

$ php -d opache.enable=0 -d opcache.enable_cli=0 ./tools/phpunit --coverage-clover=php://stdout | grep -c 'num="9"'
1

I guess that qualifies as a bug in pcov?

[Slamdunk]

I guess that qualifies as a bug in pcov?

I don't have the necessary knowledge to answer this question

[theseer]

I'll open on there and we'll see what happens ;)

[theseer]

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.

[maks-rafalko]

but at least the infection playground shows a phpunit run for every mutation.

No, please look at this example: https://infection-php.dev/r/8w2k

as you can see, method notCovered is really not covered by tests, and if you click to Uncovered mutants - there is no output from PHPUnit. Infection does not run any tests for such code, because we know in advance that this is useless. The case from this issue is because of bug in underlying coverage driver, not a normal behavior.

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

	// It's a proxy call to Mutation, can be done one stage up
	if ($mutant->isCoveredByTest()) {
	return true;
	}
	$this->eventDispatcher->dispatch(new MutantProcessWasFinished(
	MutantExecutionResult::createFromNonCoveredMutant($mutant)
	));
	return false;

On the other side, if the line is covered (from coverage driver) - Infection runs tests to see if they kill or not mutants.

[theseer]

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.