current challenge: figure out a way to kill mutants across github action jobs - I will think about that

Some comments posted by me disappeared. I linked the --git-diff-filter / --git-diff-base / --git-diff-lines options

Some comments posted by me disappeared. I linked the --git-diff-filter / --git-diff-base / --git-diff-lines options

yeah, I did not yet activate these options, because then there would be no results reported in this PR.

The PHAR distribution is the recommended one for installing Infection. I think we could just add infection to tools in setup-php action.

I think this only make sense in case we don't want run infection locally. the more we use GitHub Actions only stuff, the harder it is to run the very same setup on a local computer.

The test is failing sporadically after introducing the random order. It's probably because of:

I did not yet see a test, which started failling more often since random order was enabled.
which particular test do you mean?

the build on the master branch is broken long before random order with the same errors

I think this only make sense in case we don't want run infection locally.

Yeah, I think we don't. Given the support for many PHP versions (and that we only typically have a single PHP version running locally), I think it's unlikely we'd run Infection locally.

Also I'm thinking we'll be passing GitHub Actions-specific env variables to the CLI options anyway.

Why is the build green even when there are escaped mutants?

Why is the build green even when there are escaped mutants?

because we did not yet define a "min MSI", meaning there is no minimum threshold yet.
if we want the test to fail when at least a single escaped mutant occurs, we need min-msi=100

Yeah, I want that, alongside the Git-filtering options.

We can demonstrate the failing by adding some new change here in this PR temporarily.

Also I don't see PHPStan configured as mutant killer here.

I think, as long as we only mutate with TrinaryLogicMutator its pretty unlikely that the PHPStan killer will help us.
in addition the process is already slow and adding the PHPStan killer to the mix will make it a even slower

I really want it though. I don't want to write tests for things that would error in PHPStan. Also we'd have more incentives to make it faster 😊

Make sure to carry over PHPStan's result cache, probably with upload and download-artifact actions. It should speed up the build significantly.

@maks-rafalko I wonder why infection treats this PHPStan result as a killed mutant?

I would debug the exit code. Here is the logic how Infection determines if it's killed or escaped for PHPStan process output.

https://github.com/infection/infection/blob/466255c58cdc6dbe307012cc077b9088ff3cfaa5/src/StaticAnalysis/PHPStan/Mutant/PHPStanMutantExecutionResultFactory.php#L94

We discussed with @ondrejmirtes that checking non-zero exit code is not reliable, but AFAIK nothing has been implemented on PHPStan side to improve it, so we still check (non-)zero exit code.

local debugging reveals: we get a exit code of 2, because infection internally passes --fail-without-result-cache and the result cache is not used as the PHPStan output describes:

Result cache not used because extension file /Users/m.staab/dvl/phpstan-doctrine/src/Rules/Doctrine/ORM/QueryBuilderDqlRule.php hash does not match.
Result cache was not saved because of --tmp-file and --instead-of CLI options passed (editor mode).

Yeah, that's tricky. I suppose we should have a special exit code for when the result is green but result cache was not used (so that Infection does not use it to kill mutants).

Or maybe Infection can stop passing --fail-without-result-cache altogether.

Or maybe Infection can stop passing --fail-without-result-cache altogether.

I think thats the way to go. not everyone has the tools to use a result cache in CI.

Or maybe Infection can stop passing --fail-without-result-cache altogether.

I remember I've added it by @staabm's suggestion to avoid the cases when for some (unexpected) reason we miss the cache, so for me this option did make sense so far.

Can I ask you why do we have here

Result cache not used because extension file /src/Rules/Doctrine/ORM/QueryBuilderDqlRule.php hash does not match.

Is it the issue that can be fixed here? Or is it expected?

Can I ask you why do we have here

Result cache not used because extension file /src/Rules/Doctrine/ORM/QueryBuilderDqlRule.php hash does not match.

Is it the issue that can be fixed here? Or is it expected?

my guess is that this problem occurs because phpstan-doctrine is a phpstan extension repository (regular non phpstan extension projects would not have this case if I got it right)

I remember I've added it by @staabm's suggestion to avoid the cases when for some (unexpected) reason we miss the cache, so for me this option did make sense so far.

ohh I remember that.. while running infection, we actually have a primed cache (caused by the initial test-run). so we don't need CI tooling for the cache to work.

This can happen in any project with custom rules or other extensions. When Infection actually decides to mutate an extension file, it's inevitable the result cache won't be used.

This can happen in any project with custom rules or other extensions. When Infection actually decides to mutate an extension file, it's inevitable the result cache won't be used.

1. so do you say here we have a cache miss issue because Infection mutates the extension's file?
2. does it mean we need to remove passing --fail-without-result-cache by Infection?

I'm not sure I understand all the cases to make a decision.

I think what we're facing here is that we want to make sure the user primed the PHPStan cache before running Infection. But it can still be invalidated in these cases.

--fail-without-result-cache can sometimes fill this purpose but it's not perfect.

@ondrejmirtes I think we are ready to land this.

in case you are fine with the current forced-error I would revert it and we can do the first steps with Infection on PHPStan.

As described above, we are likely seeing every mutation 3 times now (once for each PHP Version).
in case we don't want to put a new aggregation/evaluation job after mutations runs, we could start with a single PHP Version only.

--fail--without-result-cache

nope, I fixed it in infection and updated this PR to the latest release

see CI results:

Awesome, thank you!

Do you want me to create phpstan/build-infection repository where the mutators could live? 😊

Do you want me to create phpstan/build-infection repository where the mutators could live?

sure. we need a plan how to scale it over all repos (or what steps next)

I imagine it could similarly to build-cs:

Does the infection "json5" config file format support includes? The only thing I'm looking to DRY is probably the mutators implementations and the mutators config key, everything else can be copy-pasted across repositories (because I suspect there will have to be some adjustments).

So I imagine:

1. Let's do the new repository
2. Let's try to reuse it in phpstan-doctrine
3. Then we can use it in more places, I would aggresively put it in phpstan-src probably first