Infection's --only-covering-test-cases flag does not work correctly on PHPUnit 10

[acelaya]

Question	Answer
Infection version	0.26.19
Test Framework version	PHPUnit 10.0.4
PHP version	8.2.2
Platform	Ubuntu
Github Repo	https://github.com/shlinkio/shlink-installer

I'm updating a project to PHPUnit 10, and with that version, infection reports 42% of killed mutants, while it reports 90% with PHPUnit 9.6

I have even manually applied some of the mutants reported as "not killed", and then the tests no longer pass, so these are false positives.

• Project: https://github.com/shlinkio/shlink-installer
• Pull request updating to PHPUnit 10: Feature/phpunit 10 shlinkio/shlink-installer#181
• Build: https://github.com/shlinkio/shlink-installer/actions/runs/4107812419/jobs/7087757162

[acelaya]

Same here shlinkio/shlink-config#25.

This one is a farely smaller project, but the score has dropped from around 90% to 70% and they are also false positives.

[sanmai]

I could not reproduce the problem. On PHPUnit 9.6.3 I'm looking at:

27 mutations were generated:
      23 mutants were killed
       0 mutants were configured to be ignored
       0 mutants were not covered by tests
       4 covered mutants were not detected

After running composer require -W --dev phpunit/phpunit:^10 followed by php vendor/bin/phpunit --migrate-configuration I get:

27 mutations were generated:
      23 mutants were killed
       0 mutants were configured to be ignored
       0 mutants were not covered by tests
       4 covered mutants were not detected

There is no difference whatsoever.

If not like this, how can we reproduce the issue?

[acelaya]

What I'm doing in order to avoid running the tests twice is first run phpunit like this:

vendor/bin/phpunit --order-by=random --testdox --colors=always --coverage-clover=build/clover.xml --coverage-xml=build/coverage-xml --log-junit=build/junit.xml

And then run infection like this:

vendor/bin/infection --threads=4 --min-msi=85 --log-verbosity=default --only-covered --only-covering-test-cases --coverage=build --skip-initial-tests

The code coverage is generated with pcov.

[sanmai]

Well that's peculiar indeed. If I do this after checking out https://github.com/shlinkio/shlink-config:

composer require -W --dev phpunit/phpunit:^9
XDEBUG_MODE=coverage vendor/bin/phpunit --order-by=random --coverage-clover=build/clover.xml --coverage-xml=build/coverage-xml --log-junit=build/junit.xml
vendor/bin/infection --threads=4 --min-msi=85 --log-verbosity=default --only-covered --only-covering-test-cases --coverage=build --skip-initial-tests

I get:

27 mutations were generated:
      23 mutants were killed

And if I do this afterwards:

composer require -W --dev phpunit/phpunit:^10
XDEBUG_MODE=coverage vendor/bin/phpunit --order-by=random --coverage-clover=build/clover.xml --coverage-xml=build/coverage-xml --log-junit=build/junit.xml
vendor/bin/infection --threads=4 --min-msi=85 --log-verbosity=default --only-covered --only-covering-test-cases --coverage=build --skip-initial-tests

I get:

27 mutations were generated:
      19 mutants were killed

[acelaya]

@sanmai how did you run infection in the first try, when you were getting the same results both with PHPUnit 9 and 10?

[sanmai]

I run the following script:

composer require -W --dev phpunit/phpunit:^9
./vendor/bin/infection -j4

composer require -W --dev phpunit/phpunit:^10
./vendor/bin/infection -j4

In both cases I got the same result.

Infections makes a bunch of configuration tweaks to make PHPUnit work best (for example), and I suspect some of them changed the default values between PHPUnit 9 and 10. I'd say you can fix this issue if you can figure what was changed.

[acelaya]

Ok, I found the reason. If I remove the --only-covering-test-cases flag from the infection command, then I get the same results both with PHPUnit 9 and 10.

[sanmai]

Sound like it's a bug after all.

[sanmai]

Script to reproduce:

git clone git@github.com:shlinkio/shlink-config.git && cd shlink-config

composer require -W --dev phpunit/phpunit:^9
./vendor/bin/infection -j4 --only-covering-test-cases

composer require -W --dev phpunit/phpunit:^10
php vendor/bin/phpunit --migrate-configuration
./vendor/bin/infection -j4 --only-covering-test-cases

--- PHPUnit 9
+++ PHPUnit 10
@@ -1,14 +1,14 @@
 27 mutations were generated:
-      23 mutants were killed
+      19 mutants were killed
        0 mutants were configured to be ignored
        0 mutants were not covered by tests
-       4 covered mutants were not detected
+       8 covered mutants were not detected
        0 errors were encountered
        0 syntax errors were encountered
        0 time outs were encountered
        0 mutants required more time than configured
 
 Metrics:
-         Mutation Score Indicator (MSI): 85%
+         Mutation Score Indicator (MSI): 70%
          Mutation Code Coverage: 100%
-         Covered Code MSI: 85%
+         Covered Code MSI: 70%

This flag was added in #1539

@maks-rafalko, can you see if you can make sense of this problem?

[maks-rafalko]

Unfortunately, I can look into it not earlier than the end of the week, so if it's urgent, feel free to help ;)

I'm pretty sure something has changed in PHPUnit 10 in the way how it treats --filter option, because it's excaclty what is added to Mutant's PHPUnit process by this --only-covering-test-cases option:

infection/src/TestFramework/PhpUnit/CommandLine/ArgumentsAndOptionsBuilder.php

Lines 84 to 111 in aaae66a

	if ($this->executeOnlyCoveringTestCases && count($tests) > 0) {
	$filterString = '/';
	$usedTestCases = [];
	foreach ($tests as $testLocation) {
	$testCaseString = $testLocation->getMethod();
	$partsDelimitedByColons = explode('::', $testCaseString, 2);
	if (count($partsDelimitedByColons) > 1) {
	$parts = explode('\\', $partsDelimitedByColons[0]);
	$testCaseString = sprintf('%s::%s', end($parts), $partsDelimitedByColons[1]);
	}
	if (array_key_exists($testCaseString, $usedTestCases)) {
	continue;
	}
	$usedTestCases[$testCaseString] = true;
	$filterString .= preg_quote($testCaseString, '/') . '|';
	}
	$filterString = rtrim($filterString, '|') . '/';
	$options[] = '--filter';
	$options[] = $filterString;
	}

I would (or will) start debugging from this place.

What we can do is to generate infection.log file with --debug --log-verbosity=all and compare generated command line, then run it for PHPUnit 9 and 10

[acelaya]

There's no such thing as "urgent" in the open source world. I'm not expecting this to be done quickly just because I reported it.

Now that I know what it is, the workaround is very easy. I can afford the performance impact of removing that flag for now, and I could also stick with PHPUnit 9 for a bit longer.

In any case, running the debug command is something I can definitely do to provide a bit more clarity.

[acelaya]

I have run infection with both PHPUnit 9 and 10, in https://github.com/shlinkio/shlink-config, including --debug --log-verbosity=all --only-covering-test-cases (among others).

These are the generated logs:

infection-log-phpunit-9.txt
infection-log-phpunit-10.txt

[maks-rafalko]

for the provided logs, we can see that for the ecaped mutant:

1) /app/src/Factory/DottedAccessConfigAbstractFactory.php:44    [M] InstanceOf_

-        if (!is_array($array) && !$array instanceof ArrayAccess) {
+        if (!is_array($array) && !false) {

there are different number of tests executed.

PHPUnit 9:

$ '/app/vendor/bin/phpunit' '--configuration' '/app/build/infection/temp/infection/phpunitConfiguration.b8b37dcdb254e9780851c18427557625.infection.xml' '--filter' '/DottedAccessConfigAbstractFactoryTest\:\:throwsExceptionWhenFirstPartOfTheServiceDoesNotResultInAnArray with data set "object"|DottedAccessConfigAbstractFactoryTest\:\:throwsExceptionWhenFirstPartOfTheServiceDoesNotResultInAnArray with data set "true"|DottedAccessConfigAbstractFactoryTest\:\:throwsExceptionWhenFirstPartOfTheServiceDoesNotResultInAnArray with data set "string"|DottedAccessConfigAbstractFactoryTest\:\:throwsExceptionWhenFirstPartOfTheServiceDoesNotResultInAnArray with data set "number"|DottedAccessConfigAbstractFactoryTest\:\:throwsExceptionWhenFirstPartOfTheServiceDoesNotResultInAnArray with data set "false"|DottedAccessConfigAbstractFactoryTest\:\:dottedNotationIsRecursivelyResolvedUntilLastValueIsFoundAndReturned|DottedAccessConfigAbstractFactoryTest\:\:exceptionIsThrownIfAnyStepCannotBeResolved/'
  PHPUnit 9.6.3 by Sebastian Bergmann and contributors.
  
  .......                                                             7 / 7 (100%)
  
  Time: 00:00.005, Memory: 8.00 MB
  
  OK (7 tests, 14 assertions)

and PHPUnit 10

$ '/app/vendor/bin/phpunit' '--configuration' '/app/build/infection/temp/infection/phpunitConfiguration.40370b47b96a7a84fca47f3598427ff3.infection.xml' '--filter' '/DottedAccessConfigAbstractFactoryTest\:\:exceptionIsThrownIfAnyStepCannotBeResolved|DottedAccessConfigAbstractFactoryTest\:\:throwsExceptionWhenFirstPartOfTheServiceDoesNotResultInAnArray\#object|DottedAccessConfigAbstractFactoryTest\:\:throwsExceptionWhenFirstPartOfTheServiceDoesNotResultInAnArray\#true|DottedAccessConfigAbstractFactoryTest\:\:throwsExceptionWhenFirstPartOfTheServiceDoesNotResultInAnArray\#string|DottedAccessConfigAbstractFactoryTest\:\:throwsExceptionWhenFirstPartOfTheServiceDoesNotResultInAnArray\#number|DottedAccessConfigAbstractFactoryTest\:\:throwsExceptionWhenFirstPartOfTheServiceDoesNotResultInAnArray\#false|DottedAccessConfigAbstractFactoryTest\:\:dottedNotationIsRecursivelyResolvedUntilLastValueIsFoundAndReturned/'
  PHPUnit 10.0.7 by Sebastian Bergmann and contributors.
  
  Runtime:       PHP 8.2.2
  Configuration: /app/build/infection/temp/infection/phpunitConfiguration.40370b47b96a7a84fca47f3598427ff3.infection.xml
  
  ..                                                                  2 / 2 (100%)
  
  Time: 00:00.039, Memory: 10.00 MB
  
  OK (2 tests, 4 assertions)

From what I see, it happens because --filter is build differently. Please see (the first command line is for PHPUnit 9, the second is for PHPUnit 10)

I assume that the command line, generated for PHPUnit 10 is not recognized by PHPUnit 10, that's why it executes less tests (and as a result kill less mutants).

We generate this --filter regexes based on XML reports, generated in the code I mentioned above. This is a place I would continue to investigate - compare XML reports and understand why PHPUnit 9 and 10 generate them differently (in a non-backward compatible way)

[maks-rafalko]

<coverage>
  <line nr="9">
    <covered by="ExampleTest\Test\SourceClassTest::test_hello#First"/>
    <covered by="ExampleTest\Test\SourceClassTest::test_hello#Second"/>
  </line>
</coverage>

I can confirm PHPUnit 10 is now differently generates xml coverage (BC break). So we need to update infection to understand both formats (old and new one).

PHPUnit 10 itself does not work with this format, so this doesn't work

vendor/bin/phpunit --filter='ExampleTest\\Test\\SourceClassTest::test_hello#First'

while the following old format works

vendor/bin/phpunit --filter='ExampleTest\\Test\\SourceClassTest::test_hello with data set "First"'

Or, it can be a bug of PHPUnit 10? As I can see with data set substrings in the source code still

[maks-rafalko]

@acelaya please use --only-covering-test-cases again, should be fixed now in 0.27.0

[acelaya]

Thanks!

[acelaya]

I have just tried it, and even thought it's not as bad as it was, I'm afraid there's still a drop in percentage of killed mutants when using that flag.

On the project I have first tried:

• PHPUnit 10.1 + infection 0.27 and no flag: 83% killed mutants.
• PHPUnit 10.1 + infection 0.27 + --only-covering-test-cases: 74% killed mutants.

I can share reports this afternoon if needed.

[maks-rafalko]

yes please, probably there is another bug

[maks-rafalko]

@acelaya is it in https://github.com/shlinkio/shlink-installer repository?

[acelaya]

No, sorry. I was testing it on https://github.com/shlinkio/shlink, which is a larger repository.

These are the logs when running infection with --debug --log-verbosity=all --only-covering-test-cases: infection-log.txt

[maks-rafalko]

@acelaya is this issue still relevant with the latest Infection?