fix: stop counting a mutant whose subprocess never ran a test as killed

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).'"',

src/Support/MutantArguments.php

@@ -0,0 +1,113 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Pest\Mutate\Support;
+
+/**
+ * The arguments of the original run, reduced to what a mutant's subprocess can accept.
+ *
+ * A mutant runs one PHPUnit process, never ParaTest, so every ParaTest-only option has to
+ * go. Left in, PHPUnit aborts with `Unknown option "--processes"` before running a single
+ * test, and that exit code is indistinguishable from a killed mutant — so the mutant is
+ * counted as tested and the score reads 100%.
+ *
+ * @see https://github.com/pestphp/pest/issues/1937
+ */
+final class MutantArguments
+{
+    /**
+     * ParaTest's own options, mapped to whether they take a value.
+     *
+     * Only the options ParaTest adds on top of PHPUnit are listed: the ones it merely
+     * forwards are valid for the mutant too.
+     *
+     * @var array<string, bool>
+     */
+    private const array PARATEST_ONLY = [
+        '--functional' => false,
+        '--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,
+    ];
+
+    /**
+     * @param  array<int, string>  $originalArguments
+     * @return array<int, string>
+     */
+    public static function from(array $originalArguments): array
+    {
+        $originalArguments = array_values($originalArguments);
+        $arguments = [];
+        $count = count($originalArguments);
+
+        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;
+            }
+
+            $name = explode('=', $argument, 2)[0];
+
+            if (! array_key_exists($name, self::PARATEST_ONLY)) {
+                $arguments[] = $argument;
+
+                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
+            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<int, string>  $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, '-');
+    }
+}

tests/Unit/MutationTestTest.php

@@ -0,0 +1,117 @@
+<?php
+
+declare(strict_types=1);
+
+use Pest\Mutate\Mutation;
+use Pest\Mutate\MutationTest;
+use Pest\Mutate\Mutators\Equality\GreaterOrEqualToGreater;
+use Pest\Mutate\Repositories\TelemetryRepository;
+use Pest\Mutate\Support\Configuration\Configuration;
+use Pest\Mutate\Support\MutationTestResult;
+use Pest\Support\Container;
+use Symfony\Component\Finder\SplFileInfo;
+
+beforeEach(function (): void {
+    $telemetryRepository = new TelemetryRepository;
+    $telemetryRepository->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(
+        '<?php file_put_contents(%s, serialize($argv)); exit(0);',
+        var_export($this->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<int, string>  $originalArguments
+ * @return array<int, string>
+ */
+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<int, string> $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');
+});

tests/Unit/Support/MutantArgumentsTest.php

@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+use Pest\Mutate\Support\MutantArguments;
+
+it('keeps the arguments a mutant subprocess can accept', function (): void {
+    expect(MutantArguments::from([
+        'vendor/bin/pest',
+        'tests/Unit',
+        '--colors=never',
+        '--configuration=phpunit.xml',
+    ]))->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',
+        '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 {
+    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',
+    '--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 {
+    expect(MutantArguments::from([
+        'vendor/bin/pest',
+        '--processes', '2',
+        '--passthru-php', '-d pcov.enabled=1',
+        '--shard', '1/4',
+        '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]);
+});