[Performance] Improve Infection performance executed against slow test suites #1539

maks-rafalko · 2021-07-19T21:20:16Z

This PR dramatically improves performance for Infection executed against functional (read - slow) tests.

- Time: 4m 18s. Memory: 0.05GB
+ Time: 1m 19s. Memory: 0.06GB

for one of the PR in my daily job's project.

Problem

As you know, we always said that Infection runs only those tests that cover mutated line, not the all tests from an application.

It is partially true, but not exactly. PHPUnit's XML file allows us to only filter original set of tests by <directory> or <file> .

And this is what we do starting from 2017.

Example of a `phpunit.xml` file generated by Infection for one of the Mutant (click me)

<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="./vendor/phpunit/phpunit/schema/9.2.xsd" bootstrap="/infection/tests/e2e/PestTestFramework/./infection/interceptor.autoload.740af9ed4589f3494f0c14ed544eec29.infection.php" colors="false" stopOnFailure="true" cacheResult="false" stderr="false">
  <testsuites>
    <testsuite name="Infection testsuite with filtered tests">
      <file>/infection/tests/e2e/PestTestFramework/tests/CalculatorPhpUnitTest.php</file>
    </testsuite>
  </testsuites>
  <filter>
    <whitelist>
      <directory>/infection/tests/e2e/PestTestFramework/src/</directory>
    </whitelist>
  </filter>
</phpunit>

Notice in the example above, that we tell PHPUnit to run tests only from CalculatorPhpUnitTest.php. But this file can contain 1 test that covers mutated line, and hundreds of other tests that still will be executed for Mutant.

It wasn't always noticeable because we (ok, at least I am) used Infection for unit tests, which are fast enough.

However, since I'm currently working on a project with 2000+ functional tests and run Infection against this huge functional test suite, this became a bottleneck.

Imagine there is a file ProductApiTest.php that covers all the API endpoints with functional tests (Symfony WebTestCase).

In this file, there is only 1 test case that covers mutated line. This test case takes 1s
In this file, there are additionally other 100 test cases that together take 15s

So, with Infection 0.23.0 (current master), instead of running 1 test case for 1s, we run all the tests from ProductApiTest.php for 1+15=16s.

Solution

With this PR, for the example explained above, we run only 1 test case for 1s, saving 15 from 16 seconds.

Technical details

Since PHPUnit doesn't allow us to provide test cases on phpunit.xml file level, we can only do it on a command line level, using --filter option

Command line on master: 'phpunit' '--configuration' '/path-to/phpunitConfiguration.d3b360aa95bfe11dadbe87dbd49b70c4.infection.xml'
Command line on this PR: 'phpunit' '--configuration' '/path-to/phpunitConfiguration.d3b360aa95bfe11dadbe87dbd49b70c4.infection.xml' '--filter' '/App\\Test::test_case1|App\\Test::test_case2/'

Instead of 1000 words, here is an example of the logs for the escaped mutant:

master:

3) /srv/api/src/DataPersister/TableViewDataPersister.php:32    [M] InstanceOf_

--- Original
+++ New
@@ @@
     }
     public function persist($data, array $context = [])
     {
-        if ($data instanceof TableView && ($context['collection_operation_name'] ?? null) === 'post' && $this->security->getUser() instanceof Employee) {
+        if ($data instanceof TableView && ($context['collection_operation_name'] ?? null) === 'post' && true) {
             /** @var Employee $employee */
             $employee = $this->security->getUser();
             $data->setCreatedBy($employee);

$ '/srv/api/vendor/phpunit/phpunit/phpunit' '--configuration' '/tmp/infection/phpunitConfiguration.1aadd4bc46a4b6ae51fb8e3a0d295e89.infection.xml'
  Test cache cleared
  PHPUnit 9.5.6 by Sebastian Bergmann and contributors.
  
  Random Seed:   1625689971
  
  Testing 
  .............................................................   61 / 1166 (  5%)
  .............................................................  122 / 1166 ( 10%)
  .............................................................  183 / 1166 ( 15%)
  .............................................................  244 / 1166 ( 20%)
  .............................................................  305 / 1166 ( 26%)
  .............................................................  366 / 1166 ( 31%)
  .............................................................  427 / 1166 ( 36%)
  .............................................................  488 / 1166 ( 41%)
  .............................................................  549 / 1166 ( 47%)
  .............................................................  610 / 1166 ( 52%)
  .............................................................  671 / 1166 ( 57%)
  ..............................

and this PR:

3) /srv/api/src/DataPersister/TableViewDataPersister.php:32    [M] InstanceOf_

--- Original
+++ New
@@ @@
     }
     public function persist($data, array $context = [])
     {
-        if ($data instanceof TableView && ($context['collection_operation_name'] ?? null) === 'post' && $this->security->getUser() instanceof Employee) {
+        if ($data instanceof TableView && ($context['collection_operation_name'] ?? null) === 'post' && true) {
             /** @var Employee $employee */
             $employee = $this->security->getUser();
             $data->setCreatedBy($employee);

$ '/srv/api/vendor/phpunit/phpunit/phpunit' '--configuration' '/tmp/infection/phpunitConfiguration.5b66e9f56127cb2a42531283cf8dc120.infection.xml' '--filter' '/App\\Tests\\[...very long filter]/'
  Test cache cleared
  PHPUnit 9.5.6 by Sebastian Bergmann and contributors.
  
  Random Seed:   1625689648
  
  Testing 
  ...............................................................  63 / 192 ( 32%)
  ............................................................... 126 / 192 ( 65%)
  ............................................................... 189 / 192 ( 98%)
  ...                                                             192 / 192 (100%)

So it is 192 executed tests (in this branch) instead of 1166 (in master).

How it impacts unit (fast) test suites

I tried to understand the impact for fast tests, running master branch and this PR against Infection itsetlf:

make test-infection-xdebug-80-docker

From my tests, running Infection against Infection becomes slower with this PR.

master:
  2m 52s. Memory: 0.17GB
  2m 47s. Memory: 0.17GB
  2m 46s. Memory: 0.17GB

PR:
  3m 8s. Memory: 0.19GB
  3m 6s. Memory: 0.19GB
  3m 9s. Memory: 0.19GB

Why was memory usage increased (I don't think it's a problem though)? Because now we need to store all those `--filter '/the long list of test cases/' options
Why was the execution time increased? From my tests, there are 2 reasons
- Using --filter in PHPUnit itself adds some penalty as I understand, because this option is matched using preg_match() inside PHPUnit. But this adds only 5 extra seconds with the "Infection for Infection" example.
- Preparing a unique set of test cases and build `--filter '/the long list of test cases/' option takes the most extra time

Keeping that in mind, probably it's better to enable this feature by some option? If so, what the name of this new option you can recommend?

--only-covering-test-cases
--slow-tests (probably it will be some common option to enable not just this feature)
...

I don't know.

Conclusion

The slower the tests, the more benefit / performance boost this PR will give.

Covered by tests
Doc PR: Add --only-covering-test-cases option docs site#221

sanmai · 2021-07-19T21:25:10Z

Suggestion: if we can know a certain test suite contains this much tests (a thousand) which take this much seconds to run, we can make an educated guess as to whenever to enable this new option.

maks-rafalko · 2021-07-19T21:29:46Z

Suggestion: if we can know a certain test suite contains this much tests (a thousand) which take this much seconds to run, we can make an educated guess as to whenever to enable this new option.

thousand of tests does not mean they are slow, to be honest. 1000 unit tests can take less than a second, while 1000 functional tests can take minutes.

Probably we need to calculate the total time of the covering tests and compare with some constant (precalculated value). I tried it, and enabled --filter option dynamically, but my results were very strange or inaccurate. Probably will give it one more try

sanmai · 2021-07-19T21:32:31Z

src/TestFramework/PhpUnit/CommandLine/ArgumentsAndOptionsBuilder.php

+    {
+        $options = $this->buildForInitialTestsRun($configPath, $extraOptions);
+
+        if (count($tests) > 0) {


I don't think we can take just the number of tests, and test suite time wouldn't necessary give the whole picture. I'd say median test time multiplied on the number of tests will give a better estimate to the time measure because median naturally filters outliers. Or we can go full with three standard deviations of the mean, which should give possibly even better estimate.

sanmai · 2021-07-19T21:34:46Z

src/TestFramework/PhpUnit/CommandLine/ArgumentsAndOptionsBuilder.php

+                $filterString .= escapeshellcmd($testCaseString) . '|';
+            }
+
+            $filterString = rtrim($filterString, '|') . '/';


Are we sure we absolutely need this computation? I mean, is there a chance we won't run this mutant ever? And what about mutants on the same line of the same file?

didn't get you, could you please clarify what do you mean?

This code just builds '/App\\\\Test::test_case1|App\\\\Test::test_case2/' string, meaning to run only 2 test cases of X. Those tests cases - is 100% of all the test cases that cover particular mutated line (Mutant)

Ocramius · 2021-07-20T15:29:06Z

@maks-rafalko I'm re-reading the description of this issue, and I'm trying to make sense of it.

Just because I'm trying to understand, if I have a test like following:

class MyTest ... {
   function test1() { ... }
   function test2() { ... }
}

Does this patch make sure that only either of test1 or test2 are run for a specific mutation, if they cover separate code section? Did infection run all of MyTest before this patch?

maks-rafalko · 2021-07-20T15:33:04Z

Does this patch make sure that only either of test1 or test2 are run for a specific mutation, if they cover separate code section? Did infection run all of MyTest before this patch?

@Ocramius Exactly. Currently (in master), Infection creates a custom phpunit.xml for each mutant, and adds test files that cover mutated line, not particular test cases inside those files:

<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="./vendor/phpunit/phpunit/schema/9.2.xsd" bootstrap="/infection/tests/e2e/PestTestFramework/./infection/interceptor.autoload.740af9ed4589f3494f0c14ed544eec29.infection.php" colors="false" stopOnFailure="true" cacheResult="false" stderr="false">
  <testsuites>
    <testsuite name="Infection testsuite with filtered tests">
      <file>/infection/tests/e2e/PestTestFramework/tests/CalculatorPhpUnitTest.php</file>
    </testsuite>
  </testsuites>
  <filter>
    <whitelist>
      <directory>/infection/tests/e2e/PestTestFramework/src/</directory>
    </whitelist>
  </filter>
</phpunit>

^ as I described in the description, CalculatorPhpUnitTest.php test file can have 1000 tests cases, but only 10 cover mutated line. So, we need --filter 'test_case_1|test_case_2...|test_case_10 to reduce the number of executed tests.

PHPunit has no ability to configure which test cases to run in phpunit.xml, this is a not merged feature - sebastianbergmann/phpunit#4449

Ocramius · 2021-07-20T15:35:42Z

Excellent++! This is gonna be literal HOURS of mutation test runs saved. For me, the largest mutation test suite (integration tests) takes 2h :D

maks-rafalko · 2021-07-20T15:36:30Z

Yes, this is what I'm experiencing on my test suite as well with 2000+ functional tests ;)

sanmai · 2021-07-21T00:00:35Z

I tried it, and enabled --filter option dynamically, but my results were very strange or inaccurate. Probably will give it one more try

Yes, it is all not that simple, unfortunately. I think this is worthy a dedicated research project, and an option toggle like --only-covering-test-cases should be the best course of action now. We can add an auto toggle later.

Previously, we ran **files** that contains tests that cover mutated line. In `phpunit.xml` this is all we can do. To run **test cases** that cover mutated line from that files, we can only use `--filter` options. This is what this change does

… tests run and for mutant

…on-unique test cases and using `foreach`

…y covering test cases By default, Infection runs all tests cases from **files** that contain at least 1 test case that cover mutated line Using `--only-covering-test-cases`, we can enable filtering and run only those test cases from files, that cover mutated line. For very fast test suites this decreases performance (see #1539). For slow test suites (like functional / integration tests) - this can dramatically improve performance of Mutation Testing / Infection

…e a regular expression

maks-rafalko · 2021-07-22T21:48:31Z

@sanmai ready for review. I've added --only-covering-test-cases option for now as we discussed above, so this new feature will be disabled by default and can be enabled manually

sanmai

LGTM

…y covering test cases By default, Infection runs all tests cases from **files** that contain at least 1 test case that cover mutated line Using `--only-covering-test-cases`, we can enable filtering and run only those test cases from files, that cover mutated line. For very fast test suites this decreases performance (see #1539). For slow test suites (like functional / integration tests) - this can dramatically improve performance of Mutation Testing / Infection

maks-rafalko · 2021-07-25T13:00:26Z

Thank you @sanmai for the review

nicojs · 2021-08-02T03:42:50Z

Q: can the performance degradation on the "infection on infection" usecase also be influenced by simply having more tests in this branch? Or did you correct for that in some way?

maks-rafalko · 2021-08-02T11:09:46Z

I avoided the impact of mutating the new code and tests by adding @infection-ignore-all on the new code, and commenting out the tests.

The most impact is really because of building the very long strings for each mutant where running all unit tests is faster than creating this filter

maks-rafalko added RFC DX Developer Experience Performance labels Jul 19, 2021

maks-rafalko added this to the 0.24.0 milestone Jul 19, 2021

sanmai self-requested a review July 19, 2021 21:25

sanmai reviewed Jul 19, 2021

View reviewed changes

maks-rafalko added 5 commits July 22, 2021 22:51

Split ArgumentsAndOptionsBuilder::build into 2 methods: for initial…

e0cc2b1

… tests run and for mutant

Add more tests, remove redundant .* from regular expression

7ae27ac

Add special char for Pest e2e test to make sure it works as expected

a47acff

Use more performant way of building --filter option, not escaping n…

bd4316c

…on-unique test cases and using `foreach`

maks-rafalko force-pushed the feature/run-covering-tests branch from a4c5b0f to e81e0de Compare July 22, 2021 20:58

maks-rafalko force-pushed the feature/run-covering-tests branch from e81e0de to b185ad9 Compare July 22, 2021 21:02

maks-rafalko added 2 commits July 23, 2021 00:31

Use preg_quote() instead of escapeshellcmd to prepare string to b…

7a23d0b

…e a regular expression

Fix CS

64c1591

sanmai self-requested a review July 23, 2021 07:09

sanmai approved these changes Jul 23, 2021

View reviewed changes

maks-rafalko merged commit fd58f99 into master Jul 25, 2021

maks-rafalko deleted the feature/run-covering-tests branch July 25, 2021 13:00

maks-rafalko mentioned this pull request Aug 7, 2021

Infection does not run any tests if --filter is too big #1545

Open

sanmai mentioned this pull request Feb 8, 2023

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

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Performance] Improve Infection performance executed against slow test suites #1539

[Performance] Improve Infection performance executed against slow test suites #1539

maks-rafalko commented Jul 19, 2021 •

edited

sanmai commented Jul 19, 2021

maks-rafalko commented Jul 19, 2021 •

edited

sanmai Jul 19, 2021

sanmai Jul 19, 2021

maks-rafalko Jul 19, 2021 •

edited

Ocramius commented Jul 20, 2021

maks-rafalko commented Jul 20, 2021 •

edited

Ocramius commented Jul 20, 2021

maks-rafalko commented Jul 20, 2021

sanmai commented Jul 21, 2021

maks-rafalko commented Jul 22, 2021

sanmai left a comment

maks-rafalko commented Jul 25, 2021

nicojs commented Aug 2, 2021

maks-rafalko commented Aug 2, 2021

[Performance] Improve Infection performance executed against slow test suites #1539

[Performance] Improve Infection performance executed against slow test suites #1539

Conversation

maks-rafalko commented Jul 19, 2021 • edited

Problem

Solution

Technical details

How it impacts unit (fast) test suites

Conclusion

sanmai commented Jul 19, 2021

maks-rafalko commented Jul 19, 2021 • edited

sanmai Jul 19, 2021

Choose a reason for hiding this comment

sanmai Jul 19, 2021

Choose a reason for hiding this comment

maks-rafalko Jul 19, 2021 • edited

Choose a reason for hiding this comment

Ocramius commented Jul 20, 2021

maks-rafalko commented Jul 20, 2021 • edited

Ocramius commented Jul 20, 2021

maks-rafalko commented Jul 20, 2021

sanmai commented Jul 21, 2021

maks-rafalko commented Jul 22, 2021

sanmai left a comment

Choose a reason for hiding this comment

maks-rafalko commented Jul 25, 2021

nicojs commented Aug 2, 2021

maks-rafalko commented Aug 2, 2021

maks-rafalko commented Jul 19, 2021 •

edited

maks-rafalko commented Jul 19, 2021 •

edited

maks-rafalko Jul 19, 2021 •

edited

maks-rafalko commented Jul 20, 2021 •

edited