False Positive (not finding deprecated warning on Implicit conversion from float to int)

[navarr]

Question	Answer
Infection version	0.25.5 (infection.phar --version)
Test Framework version	PHPUnit 9.5.10
PHP version	8.1.0
Platform	Ubuntu LTS 20.04.3 (Github Actions)
Github Repo	navarr/binary-explode

Github Actions run: https://github.com/navarr/binary-explode/runs/4596804439?check_suite_focus=true

• Problem observed: On PHP 8.1.0, an implicit conversion from a float to an int raises a Deprecated warning. 3v4l
• This is covered in my logarithmic binary exploder by casting the output of log to an int Line
• My automated tests contains a test that would trigger this deprecation when run Line 3v4l
• Infection does not detect this deprecation and so the mutant removing the cast escapes

phpunit.xml

<?xml version="1.0" encoding="utf-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" colors="true" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
   <testsuite name="unit">
       <directory>tests</directory>
   </testsuite>
   <coverage cacheDirectory="/tmp">
       <include>
           <directory suffix=".php">src</directory>
       </include>
   </coverage>
</phpunit>

Output with issue

   ____      ____          __  _
  /  _/___  / __/__  _____/ /_(_)___  ____
  / // __ \/ /_/ _ \/ ___/ __/ / __ \/ __ \
_/ // / / / __/  __/ /__/ /_/ / /_/ / / / /
/___/_/ /_/_/  \___/\___/\__/_/\____/_/ /_/

Infection - PHP Mutation Testing Framework version 0.25.5


Running initial test suite...

PHPUnit version: 9.5.10

.: killed, M: escaped, U: uncovered, E: fatal error, X: syntax error, T: timed out, S: skipped, I: ignored


Generate mutants...

Processing source code files: 0
.EEEEE......E..ME.EE..T

23 mutations were generated:
     12 mutants were killed
      0 mutants were configured to be ignored
      0 mutants were not covered by tests
      1 covered mutants were not detected
      9 errors were encountered
      0 syntax errors were encountered
      1 time outs were encountered
      0 mutants required more time than configured

Metrics:
        Mutation Score Indicator (MSI): 95%
        Mutation Code Coverage: 100%
        Covered Code MSI: 95%

Please note that some mutants will inevitably be harmless (i.e. false positives).
Warning: Escaped Mutant for Mutator "CastInt":

--- Original
+++ New
@@ @@
    public function explode(int $value) : iterable
    {
        while ($value > 0) {
-            $power = (int) log($value, 2);
+            $power = log($value, 2);
            $part = 1 << $power;
            $value -= $part;
            (yield $part);


Time: 11s. Memory: 16.00MB
Error: ] The minimum required MSI percentage should be 100%, but actual is      
        95.65%. Improve your tests!                                            

Error: Process completed with exit code 1.

[maks-rafalko]

could you please try adding failOnWarning="true" to your phpunit.xml and run Infection again?

[navarr]

@maks-rafalko Thank you for the suggestion. That did not appear to have any effect. I had the smart idea of ensuring PHP is setup with error_reporting being set to -1, but the problem persists.

Commits: failOnWarning error_reporting=-1

[maks-rafalko]

Ok, the issue is much more complex than I thought.

First of all, let's state that PHPUnit does not fail on PHP 8.1 with implicit conversion deprecation. You can check it by removing (int) casting and running your test suit on PHP 8.1:

docker run --rm -v "$PWD":/opt -w /opt php:8.1-cli /opt/vendor/bin/phpunit                                                                                         

PHPUnit 9.5.10 by Sebastian Bergmann and contributors.

.........
[PHP Deprecated:  Implicit conversion from float 2.807354922057604 to int loses precision in /opt/src/LogarithmicBinaryExploder.php on line 25]
[PHP Deprecated:  Implicit conversion from float 1.584962500721156 to int loses precision in /opt/src/LogarithmicBinaryExploder.php on line 25]
.                                                        10 / 10 (100%)

Time: 00:00.003, Memory: 10.00 MB

OK (10 tests, 32 assertions)

and checking by echo $? - exit code is 0 (no errors).

Then there is a question - how to return non-zero error code for these deprecations? From what I know, only symfony/phpunit-bridge allows to do it.

I tried to install it and configure, and it fails PHPUnit execution with SYMFONY_DEPRECATIONS_HELPER=max[self]=0:

docker run --rm -v "$PWD":/opt -w /opt php:8.1-cli sh -c "SYMFONY_DEPRECATIONS_HELPER=max[self]=0 vendor/bin/simple-phpunit"        

                                          
PHPUnit 9.5.10 by Sebastian Bergmann and contributors.

Testing 
..........                                                        10 / 10 (100%)

Time: 00:00.004, Memory: 8.00 MB

OK (10 tests, 32 assertions)

Remaining self deprecation notices (2)

  1x: Implicit conversion from float 2.807354922057604 to int loses precision
    1x in LogarithmicBinaryExploderTest::testSeven from Navarr\BinaryExploder\Test

  1x: Implicit conversion from float 1.584962500721156 to int loses precision
    1x in LogarithmicBinaryExploderTest::testSeven from Navarr\BinaryExploder\Test

check by echo $? - return code is 1 (error).

Ok, but there is one more issue. Infection does not check return code 1 to detect PHPUnit failures. Instead, it parses PHPUnit output and returns tests pass for this case:

infection/src/TestFramework/PhpUnit/Adapter/PhpUnitAdapter.php

Lines 121 to 144 in c770354

	public function testsPass(string $output): bool
	{
	if (preg_match('/failures!/i', $output) === 1) {
	return false;
	}
	if (preg_match('/errors!/i', $output) === 1) {
	return false;
	}
	// OK (XX tests, YY assertions)
	$isOk = preg_match('/OK\s\(/', $output) === 1;
	// "OK, but incomplete, skipped, or risky tests!"
	$isOkWithInfo = preg_match('/OK\s?,/', $output) === 1;
	// "Warnings!" - e.g. when deprecated functions are used, but tests pass
	$isWarning = preg_match('/warnings!/i', $output) === 1;
	// "No tests executed!" - e.g. when --filter option contains too large regular expression
	$isNoTestsExecuted = preg_match('/No tests executed!/i', $output) === 1;
	return $isOk || $isOkWithInfo || $isWarning || $isNoTestsExecuted;
	}

Probably, we can solve this issue by updating this code

infection/src/Mutant/MutantExecutionResultFactory.php

Lines 102 to 105 in c770354

	if ($process->getExitCode() > 100) {
	// See \Symfony\Component\Process\Process::$exitCodes
	return DetectionStatus::ERROR;
	}

to the following:

- if ($process->getExitCode() > 100) {
+ if ($process->getExitCode() !== 0) {
    // See \Symfony\Component\Process\Process::$exitCodes
    return DetectionStatus::ERROR;
}

and in this case Infection will treat such execution as failure.

However, you will still need to use symfony/phpunit-bridge to disallow Deprecated: Implicit conversion from float 2.807354922057604 to int loses precision warning, because original PHPUnit can't do it.

@infection/core am I missing something here? what do you think about it?

[navarr]

After some time away from the computer, revisiting this, I found convertDeprecationsToExceptions for phpunit.xml. This was added as default true, but is currently default false.

Adding this to the phpunit.xml file and setting true on it has turned the result from M to . (in 0.25.5)

[maks-rafalko]

Nice, but I don't think Infection should set it automatically as many projects allow deprecations.

Is there anything we can do on our side from your point of view? Should we close it?

[maks-rafalko]

Feel free to reopen if needed