TestLocator returns non-unique tests, but Mutation didn't know that (i…

…nfection#1310)

* TestLocator returns non-unique tests, but Mutation didn't know that

Fixes infection#1304

TestLocator returns non-unique tests, and JUnitTestCaseSorter works around that; we have to do that too.

* Extend test case to include summation

* Timings are per test suite, not per test, therefore we have to unique by test suite name

* Fix PHPStan warning

* Extract adding logic into JUnitTestCaseTimeAdder

src/Mutation/Mutation.php

@@ -37,12 +37,11 @@
 
 use function array_intersect_key;
 use function array_keys;
-use function array_map;
-use function array_sum;
 use function implode;
 use Infection\AbstractTestFramework\Coverage\TestLocation;
 use Infection\Mutator\ProfileList;
 use Infection\PhpParser\MutatedNode;
+use Infection\TestFramework\Coverage\JUnit\JUnitTestCaseTimeAdder;
 use function md5;
 use PhpParser\Node;
 use function Safe\array_flip;
@@ -165,12 +164,8 @@ public function getAllTests(): array
      */
     public function getNominalTestExecutionTime(): float
     {
-        return $this->nominalTimeToTest ?? $this->nominalTimeToTest = array_sum(array_map(
-            static function (TestLocation $testLocation) {
-                return $testLocation->getExecutionTime();
-            },
-            $this->tests
-        ));
+        // TestLocator returns non-unique tests, and JUnitTestCaseSorter works around that; we have to do that too.
+        return $this->nominalTimeToTest ?? $this->nominalTimeToTest = (new JUnitTestCaseTimeAdder($this->tests))->getTotalTestTime();
     }
 
     public function getHash(): string

src/TestFramework/Coverage/JUnit/JUnitTestCaseTimeAdder.php

@@ -0,0 +1,91 @@
+<?php
+/**
+ * This code is licensed under the BSD 3-Clause License.
+ *
+ * Copyright (c) 2017, Maks Rafalko
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice, this
+ *   list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ *   this list of conditions and the following disclaimer in the documentation
+ *   and/or other materials provided with the distribution.
+ *
+ * * Neither the name of the copyright holder nor the names of its
+ *   contributors may be used to endorse or promote products derived from
+ *   this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+declare(strict_types=1);
+
+namespace Infection\TestFramework\Coverage\JUnit;
+
+use function array_sum;
+use Infection\AbstractTestFramework\Coverage\TestLocation;
+use function Safe\substr;
+use Traversable;
+
+/**
+ * @internal
+ */
+final class JUnitTestCaseTimeAdder
+{
+    /**
+     * @var TestLocation[]
+     */
+    private $tests;
+
+    /**
+     * @param TestLocation[] $tests
+     */
+    public function __construct(array $tests)
+    {
+        $this->tests = $tests;
+    }
+
+    public function getTotalTestTime(): float
+    {
+        return array_sum(
+            iterator_to_array(
+                $this->uniqueTestLocations(),
+                true // Duplicate keys must be overwritten.
+            )
+        );
+    }
+
+    /**
+     * Returns unique'd test cases with timings. Timings are per test suite, not per test, therefore we have to unique by test suite name.
+     *
+     * @return Traversable<string, float|null>
+     */
+    private function uniqueTestLocations(): Traversable
+    {
+        foreach ($this->tests as $testLocation) {
+            $methodName = $testLocation->getMethod();
+            $methodSeparatorPos = strpos($methodName, '::');
+
+            if ($methodSeparatorPos === false) {
+                // Just for the off case where we have rubbish in the test method name
+                continue;
+            }
+
+            // For each test we discard method name, and return a single timing for an entire suite
+            yield substr($methodName, 0, $methodSeparatorPos) => $testLocation->getExecutionTime();
+        }
+    }
+}

tests/e2e/TimeoutSkipped/composer.json

@@ -0,0 +1,15 @@
+{
+    "require-dev": {
+        "phpunit/phpunit": "^8"
+    },
+    "autoload": {
+        "psr-4": {
+            "TimeoutSkipped\\": "src/"
+        }
+    },
+    "autoload-dev": {
+        "psr-4": {
+            "TimeoutSkipped\\Test\\": "tests/"
+        }
+    }
+}

tests/e2e/TimeoutSkipped/expected-output.txt

@@ -0,0 +1,8 @@
+Total: 5
+
+Killed: 2
+Errored: 0
+Escaped: 2
+Timed Out: 1
+Skipped: 0
+Not Covered: 0

tests/e2e/TimeoutSkipped/infection.json

@@ -0,0 +1,12 @@
+{
+    "timeout": 2,
+    "source": {
+        "directories": [
+            "src"
+        ]
+    },
+    "logs": {
+        "summary": "infection.log"
+    },
+    "tmpDir": "."
+}

tests/e2e/TimeoutSkipped/phpunit.xml.dist

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:noNamespaceSchemaLocation="./vendor/phpunit/phpunit/phpunit.xsd"
+         bootstrap="./vendor/autoload.php"
+         colors="true"
+>
+    <testsuites>
+        <testsuite name="Test Suite">
+            <directory>./tests/</directory>
+        </testsuite>
+    </testsuites>
+
+    <filter>
+        <whitelist>
+            <directory>./src/</directory>
+        </whitelist>
+    </filter>
+</phpunit>

tests/e2e/TimeoutSkipped/src/SourceClass.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace TimeoutSkipped;
+
+class SourceClass
+{
+    public function add(int $a, int $b): int
+    {
+        sleep(1);
+        return $a + $b;
+    }
+}

tests/e2e/TimeoutSkipped/tests/SourceClassTest.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace TimeoutSkipped\Test;
+
+use TimeoutSkipped\SourceClass;
+use PHPUnit\Framework\TestCase;
+
+class SourceClassTest extends TestCase
+{
+    public function test_it_adds_2_numbers(): void
+    {
+        $source = new SourceClass();
+
+        $result = $source->add(1, 2);
+
+        self::assertSame(3, $result);
+    }
+}

tests/phpunit/Mutation/MutationTest.php

@@ -64,6 +64,7 @@ public function test_it_can_be_instantiated(
         MutatedNode $mutatedNode,
         int $mutationByMutatorIndex,
         array $tests,
+        float $timeToTest,
         array $expectedAttributes,
         int $expectedOriginalStartingLine,
         bool $expectedCoveredByTests,
@@ -88,6 +89,7 @@ public function test_it_can_be_instantiated(
         $this->assertSame($mutatedNodeClass, $mutation->getMutatedNodeClass());
         $this->assertSame($mutatedNode, $mutation->getMutatedNode());
         $this->assertSame($tests, $mutation->getAllTests());
+        $this->assertSame($timeToTest, $mutation->getNominalTestExecutionTime());
         $this->assertSame($expectedCoveredByTests, $mutation->isCoveredByTest());
         $this->assertSame($expectedHash, $mutation->getHash());
     }
@@ -112,6 +114,7 @@ public function valuesProvider(): iterable
             MutatedNode::wrap(new Node\Scalar\LNumber(1)),
             -1,
             [],
+            0.0,
             $nominalAttributes,
             $originalStartingLine,
             false,
@@ -136,6 +139,7 @@ public function valuesProvider(): iterable
                     0.01
                 ),
             ],
+            0.01,
             $nominalAttributes,
             $originalStartingLine,
             true,
@@ -160,6 +164,7 @@ public function valuesProvider(): iterable
                     0.01
                 ),
             ],
+            0.01,
             $nominalAttributes,
             $originalStartingLine,
             true,
@@ -181,9 +186,15 @@ public function valuesProvider(): iterable
                 new TestLocation(
                     'FooTest::test_it_can_instantiate',
                     '/path/to/acme/FooTest.php',
-                    0.01
+                    1.1
+                ),
+                new TestLocation(
+                    'FooTest::test_it_can_do_something',
+                    '/path/to/acme/FooTest.php',
+                    1.1
                 ),
             ],
+            1.1,
             $nominalAttributes,
             $originalStartingLine,
             true,
@@ -202,6 +213,7 @@ public function valuesProvider(): iterable
             MutatedNode::wrap(new Node\Scalar\LNumber(1)),
             0,
             [],
+            0.0,
             $nominalAttributes,
             $originalStartingLine,
             false,
@@ -228,7 +240,13 @@ public function valuesProvider(): iterable
                     '/path/to/acme/FooTest.php',
                     0.01
                 ),
+                new TestLocation(
+                    'BarTest::test_it_just_works',
+                    '/path/to/acme/FooTest.php',
+                    0.02
+                ),
             ],
+            0.03,
             $nominalAttributes,
             $originalStartingLine,
             true,

tests/phpunit/TestFramework/Coverage/JUnit/JUnitTestCaseTimeAdderTest.php

@@ -0,0 +1,109 @@
+<?php
+/**
+ * This code is licensed under the BSD 3-Clause License.
+ *
+ * Copyright (c) 2017, Maks Rafalko
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice, this
+ *   list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ *   this list of conditions and the following disclaimer in the documentation
+ *   and/or other materials provided with the distribution.
+ *
+ * * Neither the name of the copyright holder nor the names of its
+ *   contributors may be used to endorse or promote products derived from
+ *   this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+declare(strict_types=1);
+
+namespace Infection\Tests\TestFramework\Coverage\JUnit;
+
+use Infection\AbstractTestFramework\Coverage\TestLocation;
+use Infection\TestFramework\Coverage\JUnit\JUnitTestCaseTimeAdder;
+use PHPUnit\Framework\TestCase;
+
+final class JUnitTestCaseTimeAdderTest extends TestCase
+{
+    public function test_it_returns_time_for_the_only_test(): void
+    {
+        $coverageTestCases = [
+            new TestLocation(
+                'Test::testMethod1',
+                '/path/to/test-file-1',
+                0.000234
+            ),
+        ];
+
+        $adder = new JUnitTestCaseTimeAdder($coverageTestCases);
+
+        $total = $adder->getTotalTestTime();
+
+        $this->assertSame(0.000234, $total);
+    }
+
+    public function test_it_ignores_tests_without_valid_suite_name(): void
+    {
+        $coverageTestCases = [
+            new TestLocation(
+                'rubbish',
+                '/path/to/test-file-1',
+                0.000234
+            ),
+        ];
+
+        $adder = new JUnitTestCaseTimeAdder($coverageTestCases);
+
+        $total = $adder->getTotalTestTime();
+
+        $this->assertSame(0.0, $total);
+    }
+
+    public function test_it_returns_sum_for_uniqued_test_cases(): void
+    {
+        $coverageTestCases = [
+            new TestLocation(
+                'FooTest::testMethod1',
+                '/path/to/test-file-1',
+                0.9
+            ),
+            new TestLocation(
+                'FooTest::testMethod2',
+                '/path/to/test-file-2',
+                0.9
+            ),
+            new TestLocation(
+                'BarTest::testMethod3_1',
+                '/path/to/test-file-3',
+                0.2
+            ),
+            new TestLocation(
+                'BarTest::testMethod3_2',
+                '/path/to/test-file-4',
+                0.2
+            ),
+        ];
+
+        $adder = new JUnitTestCaseTimeAdder($coverageTestCases);
+
+        $total = $adder->getTotalTestTime();
+
+        $this->assertSame(0.9 + 0.2, $total);
+    }
+}