git-diff-filter ignored when no changes to src/

[ndench]

Question	Answer
Infection version	0.25.3
Test Framework version	PHPUnit 9.5.10
PHP version	8.0.5
Platform	Ubuntu
Github Repo	-

In our repo, we have PHP code in src/ and TypeScript in assets/. It's very common for branches to contain no changes to src/. For branches that contain src/ changes, the git-diff-filter works great, and cuts ~4m from our build time. However for branches that don't; the filter is ignored and all mutations are generated.

Looking at the GitDiffFileProvider and running the underlying command on the different branches we get something like this:

With src/ changes:

$ git diff origin/main --diff-filter=AM --name-only | grep src/ | paste -sd ","
src/App/Controller/ListController.php,src/App/Services/AppValidator.php

Without src/

$ git diff origin/main --diff-filter=AM --name-only | grep src/ | paste -sd ","

We run infection with the following command after manually running our test suite with coverage:

php vendor/bin/infection --min-covered-msi=100 --min-msi=100 --coverage=build/infection --threads=$(shell nproc) --show-mutations --skip-initial-tests  --git-diff-filter=AM --git-diff-base=origin/${GITHUB_BASE_REF} --ignore-msi-with-no-mutations --no-progress --logger-github

Looking at

infection/src/FileSystem/SourceFileFilter.php

Line 92 in 84140e6

if ($this->filters !== []) {

it appears that no src/ changes is treated the same as not specifying any filters. Is there a way we make the SourceFileFilter differentiate between these two cases?

[maks-rafalko]

Hello

but what do you expect from Infection when only typescript files are changed in assets?

1. do you expect Infection to be executed against typescript files? (resulting to 0 mutations and returning zero code thanks to --ignore-msi-with-no-mutations)
2. do you expect Infection to not be executed at all?
3. do you expect Infection to be executed for all the files?

As a workaround, if you choose the point 1, without any changes on Infection side, you can prepare a diff yourself and do not run Infection if only assets files are changed. See https://infection.github.io/guide/how-to.html#How-to-run-Infection-only-for-changed-files

other solution is to provide new option for Infection to grep not by hardcoded src/, but by any pattern. In your case it would by | grep -E 'src|assets'

for example:

--git-diff-source='src|assets'

[ndench]

I expected option 1 to occur, ie. it runs git diff sees that no files are changed and thus generates no mutations.
Although I'm not entirely sure what the practical difference is between option 1 and 2, the result is that infection does not mutations right?

Option 3 is what is currently happening, so that's not what I would have expected.

I think the suggestion of providing a git-diff-source is good, because it allows people to use the git diff filter if they're code is in a non-standard location. However, skipping all mutations if no files are modified is the best solution here IMO as this issue would also present if only README.md or some config files were modified as well.

[maks-rafalko]

This indeed requires more thinking, but I will put 2 ideas for future so they can be analyzed:

1. if we would introduce --dig-diff-source, the end user would pass '' (empty string there) so that changing README.md will be in the list of filtered files, and Infection will be executed under the hood with --filter=README.md, which will result to 0 mutations. This is what is requested/expected by the author of this issue
2. we can throw an exception (like NoCodeToMutate) if grep returns empty result and immediately stop Infection with 0 exit code, here

infection/src/Logger/GitHub/GitDiffFileProvider.php

Line 53 in 84140e6

return (string) shell_exec(

I like the second idea for the simplicity. It will be a BC break, but since we are on 0.x, I don't think it's an issue. The first idea will not breack BC.

[maks-rafalko]

See #1600 if it helps