Escaped Mutant does not escape when mutation is tested manually #1273

sebastianbergmann · 2020-06-18T04:48:09Z

I started to use Infection in one of my projects, code-unit, and noticed something weird. Could be a bug, or I may be confused -- in which case this might be a support issue.

This is my infection.json configuration file:

{
    "source": {
        "directories": [
            "src"
        ]
    },
    "mutators": {
        "@default": true
    },
    "minMsi": 100,
    "minCoveredMsi": 100
}

Here is the output I get:

./tools/infection --show-mutations
You are running Infection with PCOV enabled.

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

Infection - PHP Mutation Testing Framework 0.16.3@52d597af80429d52dd6218fcb9766fc7653ec88c

Running initial test suite...

PHPUnit version: 9.2.3

   52 [============================] < 1 sec

Generate mutants...

Processing source code files: 15/15
.: killed, M: escaped, S: uncovered, E: fatal error, T: timed out

.......................................SS....SS..S   ( 50 / 127)
SSSSSSSSS............MM.M.........M...MM.......M..   (100 / 127)
......M.......S.SS.SS......                          (127 / 127)
Escaped mutants:
================


1) /usr/local/src/code-unit/src/CodeUnitCollectionIterator.php:31    [M] DecrementInteger

--- Original
+++ New
@@ @@
     }
     public function rewind() : void
     {
-        $this->position = 0;
+        $this->position = -1;
     }
     public function valid() : bool
     {


2) /usr/local/src/code-unit/src/CodeUnitCollectionIterator.php:31    [M] DecrementInteger

--- Original
+++ New
@@ @@
     }
     public function rewind() : void
     {
-        $this->position = 0;
+        $this->position = -1;
     }
     public function valid() : bool
     {


3) /usr/local/src/code-unit/src/CodeUnitCollectionIterator.php:51    [M] Increment

--- Original
+++ New
@@ @@
     }
     public function next() : void
     {
-        $this->position++;
+        $this->position--;
     }
 }


4) /usr/local/src/code-unit/src/Mapper.php:28    [M] UnwrapArrayMerge

--- Original
+++ New
@@ @@
             if (!isset($result[$sourceFileName])) {
                 $result[$sourceFileName] = [];
             }
-            $result[$sourceFileName] = \array_merge($result[$sourceFileName], $codeUnit->sourceLines());
+            $result[$sourceFileName] = $codeUnit->sourceLines();
         }
         foreach (\array_keys($result) as $sourceFileName) {
             $result[$sourceFileName] = \array_values(\array_unique($result[$sourceFileName]));


5) /usr/local/src/code-unit/src/Mapper.php:32    [M] UnwrapArrayValues

--- Original
+++ New
@@ @@
             $result[$sourceFileName] = \array_merge($result[$sourceFileName], $codeUnit->sourceLines());
         }
         foreach (\array_keys($result) as $sourceFileName) {
-            $result[$sourceFileName] = \array_values(\array_unique($result[$sourceFileName]));
+            $result[$sourceFileName] = \array_unique($result[$sourceFileName]);
             \sort($result[$sourceFileName]);
         }
         \ksort($result);


6) /usr/local/src/code-unit/src/Mapper.php:34    [M] FunctionCallRemoval

--- Original
+++ New
@@ @@
         }
         foreach (\array_keys($result) as $sourceFileName) {
             $result[$sourceFileName] = \array_values(\array_unique($result[$sourceFileName]));
-            \sort($result[$sourceFileName]);
+            
         }
         \ksort($result);
         return $result;


7) /usr/local/src/code-unit/src/Mapper.php:51    [M] LogicalAnd

--- Original
+++ New
@@ @@
     {
         if (\strpos($unit, '::') !== false) {
             [$firstPart, $secondPart] = \explode('::', $unit);
-            if (empty($firstPart) && \function_exists($secondPart)) {
+            if (empty($firstPart) || \function_exists($secondPart)) {
                 return CodeUnitCollection::fromList(CodeUnit::forFunction($secondPart));
             }
             if (\class_exists($firstPart)) {


8) /usr/local/src/code-unit/src/Mapper.php:115    [M] FalseValue

--- Original
+++ New
@@ @@
             if (\function_exists($unit)) {
                 return CodeUnitCollection::fromList(CodeUnit::forFunction($unit));
             }
-            if (\strpos($unit, '<extended>') !== false) {
+            if (\strpos($unit, '<extended>') !== true) {
                 $unit = \str_replace('<extended>', '', $unit);
                 if (\class_exists($unit)) {
                     return $this->classAndParentClassesAndTraits($unit);



127 mutations were generated:
     100 mutants were killed
      19 mutants were not covered by tests
       8 covered mutants were not detected
       0 errors were encountered
       0 time outs were encountered

Metrics:
         Mutation Score Indicator (MSI): 78%
         Mutation Code Coverage: 85%
         Covered Code MSI: 92%

Please note that some mutants will inevitably be harmless (i.e. false positives).
                                                                                                                        
 [ERROR] The minimum required MSI percentage should be 100%, but actual is 78.740157480315%. Improve your tests!

The first escaped mutant mutated the code like so:

--- Original
+++ New
@@ @@
     }
     public function rewind() : void
     {
-        $this->position = 0;
+        $this->position = -1;

"Escaped mutant" means that the tests still pass with the change introduced by the mutant. However, that is not the case when I make the change manually and run PHPUnit manually without Infection:

diff --git a/src/CodeUnitCollectionIterator.php b/src/CodeUnitCollectionIterator.php
index e42e368..677d841 100644
--- a/src/CodeUnitCollectionIterator.php
+++ b/src/CodeUnitCollectionIterator.php
@@ -28,7 +28,7 @@ public function __construct(CodeUnitCollection $collection)
 
     public function rewind(): void
     {
-        $this->position = 0;
+        $this->position = -1;

PHPUnit 9.2.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.4.6 with PCOV 1.0.6
Configuration: /usr/local/src/code-unit/phpunit.xml

...........R............................F......                   47 / 47 (100%)

Time: 00:00.015, Memory: 4.00 MB

There was 1 failure:

1) SebastianBergmann\CodeUnit\MapperTest::testCanMapCodeUnitObjectsToArrayWithSourceLines
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
-Array &0 (
-    '/usr/local/src/code-unit/tests/_fixture/FixtureClass.php' => Array &1 (
-        0 => 12
-        1 => 13
-        2 => 14
-        3 => 15
-        4 => 16
-        5 => 17
-        6 => 18
-        7 => 19
-        8 => 20
-        9 => 21
-        10 => 22
-        11 => 23
-        12 => 24
-        13 => 25
-        14 => 26
-        15 => 27
-        16 => 28
-    )
-    '/usr/local/src/code-unit/tests/_fixture/FixtureInterface.php' => Array &2 (
-        0 => 12
-        1 => 13
-        2 => 14
-        3 => 15
-    )
-)
+Array &0 ()

/usr/local/src/code-unit/tests/unit/MapperTest.php:226

--

There was 1 risky test:

1) SebastianBergmann\CodeUnit\CodeUnitCollectionTest::testCanBeIterated
This test did not perform any assertions

/usr/local/src/code-unit/tests/unit/CodeUnitCollectionTest.php:71

FAILURES!
Tests: 47, Assertions: 126, Failures: 1, Risky: 1.

By the way, the escaped mutants 1 and 2 look identical to me.

The text was updated successfully, but these errors were encountered:

sanmai · 2020-06-18T05:07:54Z

MapperTest declared to cover only CodeUnit\Mapper therefore Infection won't consider this test when testing against a mutation in CodeUnitCollectionIterator.

You can see this better from the coverage report:

If MapperTest does actually cover CodeUnitCollectionIterator, it should be declared as such. Then this mutation will be caught. Likewise, CodeUnitCollectionIteratorTest may require some improvements.

sebastianbergmann · 2020-06-18T05:14:44Z

You are correct about SebastianBergmann\CodeUnit\MapperTest::testCanMapCodeUnitObjectsToArrayWithSourceLines(). But what about SebastianBergmann\CodeUnit\CodeUnitCollectionTest::testCanBeIterated()? This test has @covers \SebastianBergmann\CodeUnit\CodeUnitCollectionIterator and is marked as risky with the mutation (and not marked as risky without it).

It looks to me that if a test becomes risky due to a mutation then this is interpreted by Infection as "passing". That would be a bug, IMO.

sanmai · 2020-06-18T05:22:51Z

What Infection looks at is the exit code. If there's a funny test, but PHPUnit exits with a zero code, then Infection thinks that all tests are passing.

And indeed in this case of a risky test Infection will see a non-error exit code. PHPUnit says it's OK to have a risky test:

OK, but incomplete, skipped, or risky tests!
Tests: 47, Assertions: 126, Risky: 1.

sebastianbergmann · 2020-06-18T05:26:47Z

I think Infection should call PHPUnit with --fail-on-risky to ensure that PHPUnit exits with a non-zero exit code for tests that become risky due to mutations.

sanmai · 2020-06-18T05:29:31Z

Infection can't know if risky tests are OK or not, all the while you are, as any other user, in a perfect position to make this decision. What you need is to add failOnRisky="true" to your phpunit.xml, and then Infection will respect this directive passing it along.

sebastianbergmann · 2020-06-18T05:33:38Z

I disagree. If a test is not risky before a mutation but risky after a mutation then Infection should not treat the mutant as escaped.

sanmai · 2020-06-18T05:49:52Z

There's nothing to disagree. It will do as you want it, if you add failOnRisky="true" to your PHPUnit. It is your decision to make.

If you want Infection to track changes in the number of tests, there are two problems:

Infection is a general-purpose mutation testing tool. So far it entrusts the decision to determine if tests are failing or not, to the testing framework, be it PHPUnit, PhpSpec, or Codeception. It might be possible to make Infection more deeply integrated with PHPUnit, but I don't think this will be as straightforward without at least some work towards this direction done on PHPUnit's side. Say, if PHPUnit could write a JSON or XML with numbers of passing-failing-risky-etc tests, then Infection could be made to track changes to the file.
Even if Infection would have been able to track these changes, this would mean that Infection had to run exact same set of tests twice, before and after applying a mutation. Worst-case Infection would run twice as long as it is now.

Consider that we have users where running Infection takes tens of minutes or even hours. This time will have to be doubled, just because a user can't decide if they want failOnRisky="true" or not. Do you think this will worth it?

sebastianbergmann · 2020-06-18T06:14:17Z

if PHPUnit could write a JSON or XML with numbers of passing-failing-risky-etc tests

This information should be available in the XML logfile that can be generated with --log-junit.

sanmai · 2020-06-18T06:21:46Z

True, there's a counter for warnings, but, again, with the second point argument, and with incomparable ease to add something like this, I don't think there will be any movement unless there's a very needy volunteer.

…tercepted in the current process by Infection's stream wrapper see infection/infection#1273

…ready set Fixes #1274 Since PHPUnit won't treat new risky tests as failures by default, and since this behavior causes an unnecessary confusion even for experienced users (see #1273), we add failOnRisky="true" and failOnWarning="true" by default unless a conflicting directive is present, just like we do with executionOrder="random". That is, if there's already failOnRisky="false" then it should not be changed.

maks-rafalko · 2020-07-06T20:28:58Z

By the way, the escaped mutants 1 and 2 look identical to me.

can't reproduce it on any of the commits before and after introducing infection to code-unit

php -v
PHP 7.3.15 (cli) (built: Feb 20 2020 10:38:38) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.3.15, Copyright (c) 1998-2018 Zend Technologies
    with Zend OPcache v7.3.15, Copyright (c) 1999-2018, by Zend Technologies

Infection version: master 33ba2d3

@sebastianbergmann if you will face this issue again, please open an issue

…ready set (#1280) Fixes #1274 Since PHPUnit won't treat new risky tests as failures by default, and since this behavior causes an unnecessary confusion even for experienced users (see #1273), we add failOnRisky="true" and failOnWarning="true" by default unless a conflicting directive is present, just like we do with executionOrder="random". That is, if there's already failOnRisky="false" then it should not be changed.

sebastianbergmann added Bug Not Confirmed labels Jun 18, 2020

sanmai added Question and removed Bug Not Confirmed labels Jun 18, 2020

sebastianbergmann closed this as completed Jun 18, 2020

sanmai mentioned this issue Jun 18, 2020

Update phpunit.xml to fail on risky tests by default #1274

Closed

maks-rafalko added a commit to infection/include-interceptor that referenced this issue Jun 21, 2020

Register shutdown function and throw exception if the file was not in…

9a84a1f

…tercepted in the current process by Infection's stream wrapper see infection/infection#1273

maks-rafalko mentioned this issue Jun 21, 2020

Register shutdown function and throw exception if the file was not intercepted infection/include-interceptor#13

Closed

maks-rafalko mentioned this issue Jul 6, 2020

Set failOnRisky, failOnWarning to true if parameters are not already set #1280

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Escaped Mutant does not escape when mutation is tested manually #1273

Escaped Mutant does not escape when mutation is tested manually #1273

sebastianbergmann commented Jun 18, 2020

sanmai commented Jun 18, 2020

sebastianbergmann commented Jun 18, 2020

sanmai commented Jun 18, 2020

sebastianbergmann commented Jun 18, 2020

sanmai commented Jun 18, 2020 •

edited

sebastianbergmann commented Jun 18, 2020

sanmai commented Jun 18, 2020 •

edited

sebastianbergmann commented Jun 18, 2020

sanmai commented Jun 18, 2020 •

edited

maks-rafalko commented Jul 6, 2020

Escaped Mutant does not escape when mutation is tested manually #1273

Escaped Mutant does not escape when mutation is tested manually #1273

Comments

sebastianbergmann commented Jun 18, 2020

sanmai commented Jun 18, 2020

sebastianbergmann commented Jun 18, 2020

sanmai commented Jun 18, 2020

sebastianbergmann commented Jun 18, 2020

sanmai commented Jun 18, 2020 • edited

sebastianbergmann commented Jun 18, 2020

sanmai commented Jun 18, 2020 • edited

sebastianbergmann commented Jun 18, 2020

sanmai commented Jun 18, 2020 • edited

maks-rafalko commented Jul 6, 2020

sanmai commented Jun 18, 2020 •

edited

sanmai commented Jun 18, 2020 •

edited

sanmai commented Jun 18, 2020 •

edited