[Performance] Add files to coverage whitelist instead of the whole directories when --filter or --git-diff-filter are used
#1543
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…he whole directories when `--filter` or `--git-diff-filter` are used
Currently, when we build the initial tests `phpunit.xml` config, we add (if doesn't exist) the following coverage whitelist:
```xml
<phpunit>
<coverage>
<include>
<directory>src/</directory>
<directory>example/</directory>
</include>
</coverage>
</phpunit>
```
It means that **all** the files from directories `src/` and `example/` will be processed by coverage driver (be it `xdebug`, `pcov` or `phpdbg`
).
Collecting code coverage costs a lot, that's why this change tries to reduce the number of files added to coverage whitelist, replacing `<directory>` tag with N `<file>` tags if possible.
And one of the case is when we use `--filter=src/path/to/File1.php,src/path/to/File2.php` option, or when we use `--git-diff-filter=AM` option, that results internally to `--filter=X,Y,Z`.
When the filter is used, we 100% know that we will mutate only particular files, so we need the code coverage only for them.
Result initial `phpunit.xml` file will be like this:
```
infection --filter=src/path/to/File1.php,src/path/to/File2.php
```
```xml
<phpunit>
<coverage>
<include>
<file>src/path/to/File1.php/</file>
<file>src/path/to/File2.php/</file>
</include>
</coverage>
</phpunit>
```
This will dramatically decrease the time needed for collecting coverage data since we reduce the number of processed files.
maks-rafalko
commented
Aug 4, 2021
| @@ -70,7 +70,6 @@ public function collectFiles( | |||
| $finder->notPath($excludeDirectory); | |||
| } | |||
|
|
|||
| // Generator here to make sure these files used only once | |||
| yield from $finder; | |||
There was a problem hiding this comment.
@sanmai FYI: I didn't find any test or case that will be broken by this change, I think we've added it just in case to catch the situation where Finder is used twice. But in this PR - this is exactly what is needed
There was a problem hiding this comment.
Might as well change the signature to @return array<...>
| @@ -48,7 +48,7 @@ | |||
| final class PathReplacer | |||
| { | |||
| private Filesystem $filesystem; | |||
| private ?string $phpUnitConfigDir = null; | |||
There was a problem hiding this comment.
useless default value since this property is initialized in constructor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Collecting coverage data is expensive operation.
Currently, when we build the initial tests
phpunit.xmlconfig, we add (if doesn't exist) the following coverage whitelist:It means that all the files from directories
src/andexample/will be processed by coverage driver (be itxdebug,pcovorphpdbg).
Collecting code coverage costs a lot, that's why this change tries to reduce the number of files added to coverage whitelist, replacing
<directory>tag with N<file>tags if possible.And one of such cases is when we use
--filter=src/path/to/File1.php,src/path/to/File2.phpoption, or when we use--git-diff-filter=AMoption, that results internally to--filter=X,Y,Z.When the filter is used, we 100% know that we will mutate only particular files, so we need the code coverage only for them.
After this change, the following command
creates the following
phpunit.xmlfor initial tests run that collects coverage:This will dramatically decrease the time needed for collecting coverage data since we reduce the number of processed files.
Some numbers
Tests (without Infection) with
srcfolder incoverage.include:............................................ 2423 / 2423 (100%) Time: 03:11.112, Memory: 587.62 MB OK (2423 tests, 9861 assertions) Generating code coverage report ... done [00:12.564]Tests (without Infection) with 5 files in
coverage.include:............................................ 2423 / 2423 (100%) Time: 01:12.771, Memory: 34.00 MB OK (2423 tests, 9861 assertions) Generating code coverage report ... done [00:00.041]Difference:
So, it saves 2 from 3 minutes.
Another case is with Infection and
--filteroption used for the real project.mastervs this PR:it is
4xfaster for the filtered file set.Notes:
This PR will benefit those developers, who use Infection with
--filteror--git-diff-filteroptions, running MT for the changed/added files (which I personally highly recommend, as it allows using Infection for the project regardless of its size).