From b6fae72140d15e4c1387904872dedf2d8cd04af2 Mon Sep 17 00:00:00 2001 From: Tiago Santos Date: Tue, 29 Sep 2026 22:55:52 +0100 Subject: [PATCH 1/2] fix: stop counting a mutant whose subprocess never ran a test as killed A mutant's subprocess inherited two things it cannot use, and both made it exit before running a single test. MutationTest::hasFinished() reads any unsuccessful exit as a kill, so those mutants were reported as tested and the score read 100%. ParaTest-only options such as --processes and --passthru-php were forwarded verbatim. The mutant runs one PHPUnit process, which aborts with 'Unknown option "--processes"' and exit code 2. They are now stripped in MutantArguments, alongside the coverage options that were already dropped, so the original run keeps them for its own parallel execution. The subprocess also used the Pest script as its executable. That works wherever the shebang is honoured, but on Windows cmd.exe reads 'vendor/bin/pest' as the command 'vendor' and fails with exit code 1. The mutant now runs through PHP_BINARY, the same way the original run did. Closes pestphp/pest#1937 --- src/MutationTest.php | 9 +- src/Support/MutantArguments.php | 77 ++++++++++++++ tests/Unit/MutationTestTest.php | 117 +++++++++++++++++++++ tests/Unit/Support/MutantArgumentsTest.php | 69 ++++++++++++ 4 files changed, 269 insertions(+), 3 deletions(-) create mode 100644 src/Support/MutantArguments.php create mode 100644 tests/Unit/MutationTestTest.php create mode 100644 tests/Unit/Support/MutantArgumentsTest.php diff --git a/src/MutationTest.php b/src/MutationTest.php index 7bf0918..dcbde7f 100644 --- a/src/MutationTest.php +++ b/src/MutationTest.php @@ -9,6 +9,7 @@ use Pest\Mutate\Plugins\Mutate; use Pest\Mutate\Repositories\TelemetryRepository; use Pest\Mutate\Support\Configuration\Configuration; +use Pest\Mutate\Support\MutantArguments; use Pest\Mutate\Support\MutationTestResult; use Pest\Support\Container; use Symfony\Component\Process\Exception\ProcessTimedOutException; @@ -84,12 +85,14 @@ public function start(array $coveredLines, Configuration $configuration, array $ $envs['LARAVEL_PARALLEL_TESTING'] = '1'; } - // remove coverage arguments from the original arguments - $filteredArguments = array_filter($originalArguments, fn (string $argument): bool => ! str_starts_with($argument, '--coverage')); + $filteredArguments = MutantArguments::from($originalArguments); - // TODO: filter arguments to remove unnecessary stuff (Teamcity, Coverage, etc.) + // TODO: filter arguments to remove unnecessary stuff (Teamcity, etc.) $process = new Process( + // the first original argument is the Pest script, which is not executable on + // Windows, so the mutant is run the same way the original run was: through PHP command: [ + PHP_BINARY, ...$filteredArguments, '--bail', '--filter="'.implode('|', $filters).'"', diff --git a/src/Support/MutantArguments.php b/src/Support/MutantArguments.php new file mode 100644 index 0000000..5db86c4 --- /dev/null +++ b/src/Support/MutantArguments.php @@ -0,0 +1,77 @@ + + */ + private const array PARATEST_ONLY = [ + '--functional' => false, + '--max-batch-size' => true, + '--max-processes' => true, + '--no-test-tokens' => false, + '--passthru-php' => true, + '--processes' => true, + '--runner' => true, + '--shard-test-distribution' => true, + '--shard-test-distribution-seed' => true, + '--tmp-dir' => true, + '--verbose' => false, + '-m' => true, + ]; + + /** + * @param array $originalArguments + * @return array + */ + public static function from(array $originalArguments): array + { + $arguments = []; + $dropNextArgument = false; + + foreach ($originalArguments as $argument) { + if ($dropNextArgument) { + $dropNextArgument = false; + + continue; + } + + // the mutant reuses the coverage the original run already collected + if (str_starts_with($argument, '--coverage')) { + continue; + } + + $name = explode('=', $argument, 2)[0]; + + if (! array_key_exists($name, self::PARATEST_ONLY)) { + $arguments[] = $argument; + + continue; + } + + // `--processes 2` spends two arguments, and a stray `2` would be read as a path + $dropNextArgument = self::PARATEST_ONLY[$name] && ! str_contains($argument, '='); + } + + return $arguments; + } +} diff --git a/tests/Unit/MutationTestTest.php b/tests/Unit/MutationTestTest.php new file mode 100644 index 0000000..3b1ed9a --- /dev/null +++ b/tests/Unit/MutationTestTest.php @@ -0,0 +1,117 @@ +initialTestSuiteDuration(1); + + Container::getInstance()->add(TelemetryRepository::class, $telemetryRepository); + + $this->script = (string) tempnam(sys_get_temp_dir(), 'pest-mutate-script-'); + $this->argumentsSeen = (string) tempnam(sys_get_temp_dir(), 'pest-mutate-arguments-'); + + // stands in for the Pest script: records the arguments it was given, then passes + file_put_contents($this->script, sprintf( + 'argumentsSeen, true), + )); +}); + +afterEach(function (): void { + @unlink($this->script); + @unlink($this->argumentsSeen); +}); + +/** + * Runs one mutant whose original arguments are the given ones, and returns the arguments + * its subprocess actually received. + * + * @param array $originalArguments + * @return array + */ +function runMutantWith(array $originalArguments, string $argumentsSeen): array +{ + $file = new SplFileInfo(__DIR__.'/../Fixtures/Classes/SizeHelper.php', '', ''); + + $mutationTest = new MutationTest(new Mutation( + file: $file, + id: 'mutation-id', + mutator: GreaterOrEqualToGreater::class, + startLine: 15, + endLine: 15, + diff: '', + modifiedSourcePath: $file->getRealPath(), + )); + + $started = $mutationTest->start( + coveredLines: [ + $file->getRealPath() => [ + 15 => ['P\Tests\Unit\SizeHelperTest::__pest_evaluable_it_is_big'], + ], + ], + configuration: new Configuration( + coveredOnly: false, + paths: [], + pathsToIgnore: [], + mutators: [], + classes: [], + parallel: false, + processes: 1, + profile: false, + minScore: null, + ignoreMinScoreOnZeroMutations: false, + stopOnUntested: false, + stopOnUncovered: false, + mutationId: null, + retry: false, + everything: false, + ), + originalArguments: $originalArguments, + ); + + expect($started)->toBeTrue(); + + while (! $mutationTest->hasFinished()) { + usleep(10_000); + } + + // the subprocess exits successfully, so the mutant must be reported as escaped — + // anything else means it never ran the tests it was asked to run + expect($mutationTest->result())->toBe(MutationTestResult::Untested); + + /** @var array $arguments */ + $arguments = unserialize((string) file_get_contents($argumentsSeen)); + + return $arguments; +} + +it('runs the mutant through the php binary, rather than treating the pest script as executable', function (): void { + $arguments = runMutantWith([$this->script], $this->argumentsSeen); + + expect($arguments[0])->toBe($this->script) + ->and($arguments)->toContain('--bail'); +}); + +it('does not pass the paratest only options on to the mutant', function (): void { + $arguments = runMutantWith([ + $this->script, + '--processes=2', + '--passthru-php=-d pcov.enabled=1', + '--coverage-php='.sys_get_temp_dir().'/coverage.php', + ], $this->argumentsSeen); + + expect($arguments) + ->not->toContain('--processes=2') + ->not->toContain('--passthru-php=-d pcov.enabled=1') + ->not->toContain('--coverage-php='.sys_get_temp_dir().'/coverage.php'); +}); diff --git a/tests/Unit/Support/MutantArgumentsTest.php b/tests/Unit/Support/MutantArgumentsTest.php new file mode 100644 index 0000000..2c4e3f3 --- /dev/null +++ b/tests/Unit/Support/MutantArgumentsTest.php @@ -0,0 +1,69 @@ +toBe([ + 'vendor/bin/pest', + 'tests/Unit', + '--colors=never', + '--configuration=phpunit.xml', + ]); +}); + +it('drops the coverage arguments, since the mutant reuses the original coverage', function (): void { + expect(MutantArguments::from([ + 'vendor/bin/pest', + '--coverage-php=/tmp/coverage.php', + '--coverage', + ]))->toBe(['vendor/bin/pest']); +}); + +it('drops the paratest only options, which phpunit would reject before running a test', function (string $argument): void { + expect(MutantArguments::from(['vendor/bin/pest', $argument, 'tests/Unit'])) + ->toBe(['vendor/bin/pest', 'tests/Unit']); +})->with([ + '--functional', + '--max-batch-size=10', + '--max-processes=4', + '--no-test-tokens', + '--passthru-php=-d pcov.enabled=1', + '--processes=2', + '--runner=WrapperRunner', + '--shard-test-distribution=sequential', + '--shard-test-distribution-seed=0', + '--tmp-dir=/tmp', + '--verbose', + '-m=10', +]); + +it('drops the value of a paratest only option written as two arguments', function (): void { + expect(MutantArguments::from([ + 'vendor/bin/pest', + '--processes', '2', + '--passthru-php', '-d pcov.enabled=1', + 'tests/Unit', + ]))->toBe(['vendor/bin/pest', 'tests/Unit']); +}); + +it('keeps the argument after a paratest only option that takes no value', function (): void { + expect(MutantArguments::from(['vendor/bin/pest', '--functional', 'tests/Unit'])) + ->toBe(['vendor/bin/pest', 'tests/Unit']); +}); + +it('keeps an option whose name merely starts like a paratest only one', function (): void { + expect(MutantArguments::from(['vendor/bin/pest', '--process-isolation', '--runner-up'])) + ->toBe(['vendor/bin/pest', '--process-isolation', '--runner-up']); +}); + +it('returns a list even when it drops an argument from the middle', function (): void { + expect(array_keys(MutantArguments::from(['vendor/bin/pest', '--processes=2', 'tests/Unit']))) + ->toBe([0, 1]); +}); From 40684ae758940aa250657df782a6f036b03610f5 Mon Sep 17 00:00:00 2001 From: Tiago Santos Date: Mon, 19 Oct 2026 09:00:30 +0100 Subject: [PATCH 2/2] fix: keep -p and --shard out of the mutant -p is Pest's parallel flag, so the subprocess started another parallel run. --shard was not in the ParaTest list. A coverage option split across two arguments left its path as a test path. --- src/Support/MutantArguments.php | 52 ++++++++++++++++++---- tests/Unit/Support/MutantArgumentsTest.php | 17 ++++++- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/Support/MutantArguments.php b/src/Support/MutantArguments.php index 5db86c4..8ed3826 100644 --- a/src/Support/MutantArguments.php +++ b/src/Support/MutantArguments.php @@ -29,14 +29,19 @@ final class MutantArguments '--max-batch-size' => true, '--max-processes' => true, '--no-test-tokens' => false, + '--parallel' => false, '--passthru-php' => true, '--processes' => true, '--runner' => true, + '--shard' => false, '--shard-test-distribution' => true, '--shard-test-distribution-seed' => true, '--tmp-dir' => true, '--verbose' => false, '-m' => true, + // Pest uses -p as the parallel flag, so it must not swallow the next argument. + // ParaTest's own -p is --processes, which Pest spells out as --processes. + '-p' => false, ]; /** @@ -45,18 +50,19 @@ final class MutantArguments */ public static function from(array $originalArguments): array { + $originalArguments = array_values($originalArguments); $arguments = []; - $dropNextArgument = false; + $count = count($originalArguments); - foreach ($originalArguments as $argument) { - if ($dropNextArgument) { - $dropNextArgument = false; - - continue; - } + for ($index = 0; $index < $count; $index++) { + $argument = $originalArguments[$index]; // the mutant reuses the coverage the original run already collected if (str_starts_with($argument, '--coverage')) { + if (self::valueFollows($originalArguments, $index, $argument)) { + $index++; + } + continue; } @@ -68,10 +74,40 @@ public static function from(array $originalArguments): array continue; } + if ($name === '--shard') { + $next = $originalArguments[$index + 1] ?? null; + + // `--shard 1/2` is the value. `--shard tests/Unit` is not: Pest rejects that form. + if (! str_contains($argument, '=') && is_string($next) && preg_match('/^\d+\/\d+$/', $next) === 1) { + $index++; + } + + continue; + } + // `--processes 2` spends two arguments, and a stray `2` would be read as a path - $dropNextArgument = self::PARATEST_ONLY[$name] && ! str_contains($argument, '='); + if (self::PARATEST_ONLY[$name] && ! str_contains($argument, '=')) { + $index++; + } } return $arguments; } + + /** + * `--coverage-php /tmp/coverage.php` spends two arguments. `--coverage` itself does not, + * and neither does an option that already carries its value after `=`. + * + * @param array $originalArguments + */ + private static function valueFollows(array $originalArguments, int $index, string $argument): bool + { + if ($argument === '--coverage' || str_contains($argument, '=')) { + return false; + } + + $next = $originalArguments[$index + 1] ?? null; + + return is_string($next) && ! str_starts_with($next, '-'); + } } diff --git a/tests/Unit/Support/MutantArgumentsTest.php b/tests/Unit/Support/MutantArgumentsTest.php index 2c4e3f3..f7ce0bc 100644 --- a/tests/Unit/Support/MutantArgumentsTest.php +++ b/tests/Unit/Support/MutantArgumentsTest.php @@ -23,7 +23,17 @@ 'vendor/bin/pest', '--coverage-php=/tmp/coverage.php', '--coverage', - ]))->toBe(['vendor/bin/pest']); + 'tests/Unit', + ]))->toBe(['vendor/bin/pest', 'tests/Unit']); +}); + +it('drops the path of a coverage option written as two arguments', function (): void { + expect(MutantArguments::from([ + 'vendor/bin/pest', + '--coverage-php', '/tmp/coverage.php', + '--coverage-text', '--colors=never', + 'tests/Unit', + ]))->toBe(['vendor/bin/pest', '--colors=never', 'tests/Unit']); }); it('drops the paratest only options, which phpunit would reject before running a test', function (string $argument): void { @@ -34,14 +44,18 @@ '--max-batch-size=10', '--max-processes=4', '--no-test-tokens', + '--parallel', '--passthru-php=-d pcov.enabled=1', '--processes=2', '--runner=WrapperRunner', + '--shard', + '--shard=1/2', '--shard-test-distribution=sequential', '--shard-test-distribution-seed=0', '--tmp-dir=/tmp', '--verbose', '-m=10', + '-p', ]); it('drops the value of a paratest only option written as two arguments', function (): void { @@ -49,6 +63,7 @@ 'vendor/bin/pest', '--processes', '2', '--passthru-php', '-d pcov.enabled=1', + '--shard', '1/4', 'tests/Unit', ]))->toBe(['vendor/bin/pest', 'tests/Unit']); });