From d5c51692f59c8d5bd58c1f53c769ef19ec1c2b36 Mon Sep 17 00:00:00 2001 From: Maks Oleksyuk Date: Tue, 11 Aug 2026 22:34:33 +0300 Subject: [PATCH] feat: shard mutation testing by target instead of by test class Mutation runs are sharded by the files they mutate, so every mutation is generated and tested exactly once across the whole set of shards. Each shard's timeout window stays derived from the reference test suite duration recorded by the unsharded `--update-shards` run, since a shard's own initial run is shorter and would otherwise cut off mutations that a full run kills honestly. Co-Authored-By: Claude Opus 5 --- composer.json | 4 +- src/MutationTest.php | 38 ++++++ src/Options/LogJsonOption.php | 27 ++++ src/Plugins/Mutate.php | 5 + src/Repositories/ConfigurationRepository.php | 1 + src/Repositories/MutationRepository.php | 50 ++++++++ .../Configuration/AbstractConfiguration.php | 16 ++- .../Configuration/CliConfiguration.php | 6 + src/Support/Configuration/Configuration.php | 1 + src/Tester/MutationTestRunner.php | 118 +++++++++++++++++- tests/Unit/MutationTestTest.php | 27 ++++ .../Repositories/MutationRepositoryTest.php | 63 ++++++++++ 12 files changed, 350 insertions(+), 6 deletions(-) create mode 100644 src/Options/LogJsonOption.php create mode 100644 tests/Unit/MutationTestTest.php create mode 100644 tests/Unit/Repositories/MutationRepositoryTest.php diff --git a/composer.json b/composer.json index bdf1930..06c9ba7 100644 --- a/composer.json +++ b/composer.json @@ -31,12 +31,12 @@ "psr/simple-cache": "^3.0.0" }, "require-dev": { - "pestphp/pest": "^5.0.4", + "pestphp/pest": "^5.2.0", "pestphp/pest-dev-tools": "^5.0.0", "pestphp/pest-plugin-type-coverage": "^5.0.2" }, "conflict": { - "pestphp/pest": "<5.0.0" + "pestphp/pest": "<5.2.0" }, "autoload": { "psr-4": { diff --git a/src/MutationTest.php b/src/MutationTest.php index 7bf0918..b6d5152 100644 --- a/src/MutationTest.php +++ b/src/MutationTest.php @@ -22,6 +22,13 @@ class MutationTest private ?float $finish = null; + /** + * The test classes covering this mutation. + * + * @var array + */ + private array $coveringTestClasses = []; + private Process $process; public function __construct(public readonly Mutation $mutation) {} @@ -49,12 +56,15 @@ public function start(array $coveredLines, Configuration $configuration, array $ { // TODO: we should pass the tests to run in another way, maybe via cache, mutation or env variable $filters = []; + $coveringTestClasses = []; foreach (range($this->mutation->startLine, $this->mutation->endLine) as $lineNumber) { foreach ($coveredLines[$this->mutation->file->getRealPath()][$lineNumber] ?? [] as $test) { if (preg_match('/\\\\([a-zA-Z0-9]*)::(__pest_evaluable_)?([^#]*)"?/', $test, $matches) !== 1) { continue; } + $coveringTestClasses[] = $this->testClass($test); + if ($matches[2] === '__pest_evaluable_') { $filters[] = $matches[1].'::(.*)'.str_replace(['__', '_'], ['.{1,2}', '.'], $matches[3]); } else { @@ -64,6 +74,8 @@ public function start(array $coveredLines, Configuration $configuration, array $ } $filters = array_unique($filters); + $this->coveringTestClasses = array_values(array_unique($coveringTestClasses)); + if ($filters === []) { $this->updateResult(MutationTestResult::Uncovered); @@ -107,6 +119,32 @@ public function start(array $coveredLines, Configuration $configuration, array $ return true; } + /** + * Returns the test classes covering this mutation. + * + * @return array + */ + public function coveringTestClasses(): array + { + return $this->coveringTestClasses; + } + + /** + * Extracts the fully qualified class name from a code coverage test identifier. + * + * The filter above keeps only the last segment of the name, which is all a + * `--filter` pattern needs. Sharding matches against the fully qualified names + * `--list-tests` reports, so it needs the whole thing, minus Pest's `P\` prefix. + */ + private function testClass(string $test): string + { + $separator = strpos($test, '::'); + + $class = $separator === false ? $test : substr($test, 0, $separator); + + return preg_replace('/^P\\\\/', '', $class) ?? $class; + } + private function calculateTimeout(): int { /** @var TelemetryRepository $telemetryRepository */ diff --git a/src/Options/LogJsonOption.php b/src/Options/LogJsonOption.php new file mode 100644 index 0000000..6d30c1b --- /dev/null +++ b/src/Options/LogJsonOption.php @@ -0,0 +1,27 @@ +enable(); $this->ensurePrinterIsRegistered(); + // Mutation time does not correlate with test time, so sharded mutation runs get + // their own timings file rather than sharing the one regular runs write. + Shard::useTimingsFile('mutation-shards.json'); + $coverageRequired = array_filter($arguments, fn (string $argument): bool => str_starts_with($argument, '--coverage')) !== []; if ($coverageRequired) { $mutationTestRunner->doNotDisableCodeCoverage(); diff --git a/src/Repositories/ConfigurationRepository.php b/src/Repositories/ConfigurationRepository.php index f31e952..0135630 100644 --- a/src/Repositories/ConfigurationRepository.php +++ b/src/Repositories/ConfigurationRepository.php @@ -109,6 +109,7 @@ classes: $config['classes'] ?? [], mutationId: $config['mutation_id'] ?? null, retry: $config['retry'] ?? false, everything: $config['everything'] ?? false, + logJson: $config['log_json'] ?? null, ); } diff --git a/src/Repositories/MutationRepository.php b/src/Repositories/MutationRepository.php index 07fc90c..38f53d4 100644 --- a/src/Repositories/MutationRepository.php +++ b/src/Repositories/MutationRepository.php @@ -94,6 +94,56 @@ public function slowest(): array return array_slice($allTests, 0, 10); } + /** + * Returns the shard units, one per mutated file. + * + * A file is the unit of mutation work: every mutation belongs to exactly one, and + * the tests covering it must run in the same shard for the result to be honest. + * Sharding test classes instead would regenerate a file's mutations in every shard + * holding one of its covering tests, and report the ones killed elsewhere as escaped. + * + * @return array}> + */ + public function units(string $rootPath): array + { + $units = []; + + foreach ($this->tests as $file => $testCollection) { + $time = 0.0; + $tests = []; + + foreach ($testCollection->tests() as $test) { + $time += $test->duration(); + + foreach ($test->coveringTestClasses() as $class) { + $tests[$class] = true; + } + } + + if ($tests === []) { + continue; + } + + $units[$this->relativePath($file, $rootPath)] = [ + 'time' => round($time, 4), + 'tests' => array_keys($tests), + ]; + } + + return $units; + } + + /** + * Makes a mutated file's path relative to the root, so the units survive being + * written on one machine and read on another. + */ + private function relativePath(string $file, string $rootPath): string + { + $prefix = rtrim($rootPath, DIRECTORY_SEPARATOR).DIRECTORY_SEPARATOR; + + return str_starts_with($file, $prefix) ? substr($file, strlen($prefix)) : $file; + } + public function sortByEscapedFirst(): void { uasort($this->tests, fn (MutationTestCollection $a, MutationTestCollection $b): int => $b->hasLastRunEscapedMutation() <=> $a->hasLastRunEscapedMutation()); diff --git a/src/Support/Configuration/AbstractConfiguration.php b/src/Support/Configuration/AbstractConfiguration.php index e174b59..ce74b72 100644 --- a/src/Support/Configuration/AbstractConfiguration.php +++ b/src/Support/Configuration/AbstractConfiguration.php @@ -58,6 +58,8 @@ abstract class AbstractConfiguration implements ConfigurationContract private ?bool $everything = null; + private ?string $logJson = null; + /** * {@inheritDoc} */ @@ -144,6 +146,17 @@ public function profile(bool $profile = true): self return $this; } + /** + * Writes the results of this run to the given file, so that sharded runs can be + * added up into one report afterwards. + */ + public function logJson(string $path): self + { + $this->logJson = $path; + + return $this; + } + public function stopOnUntested(bool $stopOnUntested = true): self { $this->stopOnUntested = $stopOnUntested; @@ -194,7 +207,7 @@ public function everything(): self } /** - * @return array{covered_only?: bool, paths?: string[], paths_to_ignore?: string[], mutators?: class-string[], excluded_mutators?: class-string[], classes?: string[], parallel?: bool, processes?: int, profile?: bool, min_score?: float, ignore_min_score_on_zero_mutations?: bool, covered_only?: bool, stop_on_untested?: bool, stop_on_uncovered?: bool, mutation_id?: string, retry?: bool, everything?: bool} + * @return array{covered_only?: bool, paths?: string[], paths_to_ignore?: string[], mutators?: class-string[], excluded_mutators?: class-string[], classes?: string[], parallel?: bool, processes?: int, profile?: bool, min_score?: float, ignore_min_score_on_zero_mutations?: bool, covered_only?: bool, stop_on_untested?: bool, stop_on_uncovered?: bool, mutation_id?: string, retry?: bool, everything?: bool, log_json?: string} */ public function toArray(): array { @@ -215,6 +228,7 @@ public function toArray(): array 'mutation_id' => $this->mutationId, 'retry' => $this->retry, 'everything' => $this->everything, + 'log_json' => $this->logJson, ], fn (mixed $value): bool => ! is_null($value)); } diff --git a/src/Support/Configuration/CliConfiguration.php b/src/Support/Configuration/CliConfiguration.php index ef7052e..ee6e84c 100644 --- a/src/Support/Configuration/CliConfiguration.php +++ b/src/Support/Configuration/CliConfiguration.php @@ -13,6 +13,7 @@ use Pest\Mutate\Options\ExceptOption; use Pest\Mutate\Options\IgnoreMinScoreOnZeroMutationsOption; use Pest\Mutate\Options\IgnoreOption; +use Pest\Mutate\Options\LogJsonOption; use Pest\Mutate\Options\MinScoreOption; use Pest\Mutate\Options\MutateOption; use Pest\Mutate\Options\MutationIdOption; @@ -44,6 +45,7 @@ class CliConfiguration extends AbstractConfiguration ParallelOption::class, ProcessesOption::class, ProfileOption::class, + LogJsonOption::class, StopOnUntestedOption::class, StopOnUncoveredOption::class, BailOption::class, @@ -118,6 +120,10 @@ public function fromArguments(array $arguments): array $this->profile($input->getOption(ProfileOption::ARGUMENT) !== 'false'); } + if ($input->hasOption(LogJsonOption::ARGUMENT)) { + $this->logJson((string) $input->getOption(LogJsonOption::ARGUMENT)); // @phpstan-ignore-line + } + if ($_SERVER['COLLISION_PRINTER_PROFILE'] ?? false) { $this->profile(true); unset($_SERVER['COLLISION_PRINTER_PROFILE']); diff --git a/src/Support/Configuration/Configuration.php b/src/Support/Configuration/Configuration.php index 9e16b4f..badab2c 100644 --- a/src/Support/Configuration/Configuration.php +++ b/src/Support/Configuration/Configuration.php @@ -30,5 +30,6 @@ public function __construct( public readonly ?string $mutationId, public readonly bool $retry, public readonly bool $everything, + public readonly ?string $logJson, ) {} } diff --git a/src/Tester/MutationTestRunner.php b/src/Tester/MutationTestRunner.php index b451574..512c903 100644 --- a/src/Tester/MutationTestRunner.php +++ b/src/Tester/MutationTestRunner.php @@ -16,8 +16,10 @@ use Pest\Mutate\Support\Configuration\Configuration; use Pest\Mutate\Support\FileFinder; use Pest\Mutate\Support\MutationGenerator; +use Pest\Plugins\Shard; use Pest\Support\Container; use Pest\Support\Coverage; +use Pest\TestSuite; use Psr\SimpleCache\CacheInterface; use SebastianBergmann\CodeCoverage\CodeCoverage; use SebastianBergmann\CodeCoverage\Data\ProcessedCodeCoverageData; @@ -94,9 +96,10 @@ public function isCodeCoverageRequested(): bool public function run(): int { - Container::getInstance()->get(TelemetryRepository::class)->initialTestSuiteDuration( // @phpstan-ignore-line - microtime(true) - $this->startTime - ); + /** @var TelemetryRepository $telemetryRepository */ + $telemetryRepository = Container::getInstance()->get(TelemetryRepository::class); + + $telemetryRepository->initialTestSuiteDuration($this->initialTestSuiteDuration()); if (! Coverage::isAvailable() || ! file_exists($reportPath = Coverage::getPath())) { Container::getInstance()->get(Printer::class)->reportError('No coverage report found, aborting mutation testing.'); // @phpstan-ignore-line @@ -137,7 +140,13 @@ public function run(): int /** @var MutationGenerator $generator */ $generator = Container::getInstance()->get(MutationGenerator::class); + $shardFiles = $this->shardFiles(); + foreach ($files as $file) { + if ($shardFiles !== null && ! isset($shardFiles[$file->getRealPath()])) { + continue; + } + $linesToMutate = []; if ($this->getConfiguration()->coveredOnly) { @@ -186,6 +195,17 @@ classesToMutate: $this->getConfiguration()->everything ? [] : $this->getConfigur $mutationSuite->repository->saveResults(); + $this->writeJsonLog($mutationSuite); + + // A suite cut short by --bail or --stop-on-* holds partial durations, which would + // make for a badly balanced shards file. + if (! $this->stop) { + Shard::useTimings( + $mutationSuite->repository->units(TestSuite::getInstance()->rootPath), + ['suite_time' => round($telemetryRepository->getInitialTestSuiteDuration(), 4)], + ); + } + Facade::instance()->emitter()->finishMutationSuite($mutationSuite); return $this->isMinScoreIsReached($mutationSuite) ? 0 : 1; @@ -219,6 +239,98 @@ private function coveredLines(ProcessedCodeCoverageData $coverageData): array return $coveredLines; } + /** + * Returns the duration a mutation's test run is measured against. + * + * Each mutation is given a timeout derived from this, so it has to describe the same + * amount of work whether or not the suite is sharded. A shard only runs part of the + * suite, so its own initial run is shorter, and mutations that are killed honestly on + * a full run would be cut off as timeouts instead. The reference duration recorded by + * the unsharded `--update-shards` run is therefore preferred whenever it is longer. + * + * Only ever called once, at the start of the mutation suite: it measures from the + * start of the process, so calling it again later would fold the mutation suite's own + * duration into the reference. + */ + private function initialTestSuiteDuration(): float + { + $measured = microtime(true) - $this->startTime; + + $reference = Shard::metadata()['suite_time'] ?? null; + + if (is_numeric($reference) && (float) $reference > $measured) { + return (float) $reference; + } + + return $measured; + } + + /** + * Writes this run's results, so that sharded runs can be added up afterwards. + * + * Shards own disjoint sets of mutations, so summing the counters of every shard + * reproduces the score of a single unsharded run exactly. + */ + private function writeJsonLog(MutationSuite $mutationSuite): void + { + $path = $this->getConfiguration()->logJson; + + if ($path === null) { + return; + } + + $repository = $mutationSuite->repository; + + $directory = dirname($path); + + if (! is_dir($directory)) { + mkdir($directory, 0755, true); + } + + file_put_contents($path, json_encode([ + 'tested' => $repository->tested(), + 'untested' => $repository->untested(), + 'timeout' => $repository->timedOut(), + 'uncovered' => $repository->uncovered(), + 'not_run' => $repository->notRun(), + 'total' => $repository->total(), + 'score' => round($repository->score(), 2), + ], JSON_PRETTY_PRINT | JSON_THROW_ON_ERROR)."\n"); + } + + /** + * Returns the absolute paths of the mutated files this shard owns, or null when the + * run is not sharded. + * + * The restriction is applied here rather than through `--path` or `--class`, because + * passing either of those lifts the `__pest_mutate_only` group and makes every shard + * run the whole test suite instead of only the tests declaring `covers()`. + * + * @return array|null + */ + private function shardFiles(): ?array + { + $units = Shard::selectedUnits(); + + if ($units === []) { + return null; + } + + $root = rtrim(TestSuite::getInstance()->rootPath, DIRECTORY_SEPARATOR).DIRECTORY_SEPARATOR; + + $files = []; + + foreach ($units as $unit) { + $path = realpath(str_starts_with($unit, DIRECTORY_SEPARATOR) ? $unit : $root.$unit); + + if ($path !== false) { + $files[$path] = true; + } + } + + return $files; + } + private function getConfiguration(): Configuration { return Container::getInstance()->get(ConfigurationRepository::class)->mergedConfiguration(); // @phpstan-ignore-line diff --git a/tests/Unit/MutationTestTest.php b/tests/Unit/MutationTestTest.php new file mode 100644 index 0000000..8badc6d --- /dev/null +++ b/tests/Unit/MutationTestTest.php @@ -0,0 +1,27 @@ +getMethod('testClass'); + + expect($method->invoke($test, $identifier))->toBe($expected); +})->with([ + 'pest test' => ['P\Tests\Unit\FooTest::__pest_evaluable_it_works', 'Tests\Unit\FooTest'], + 'phpunit test' => ['Tests\Unit\FooTest::test_bar', 'Tests\Unit\FooTest'], + 'dataset' => ['P\Tests\Unit\FooTest::__pest_evaluable_it_works#0', 'Tests\Unit\FooTest'], + 'unnamespaced' => ['P\FooTest::test_bar', 'FooTest'], + 'no separator' => ['Tests\Unit\FooTest', 'Tests\Unit\FooTest'], +]); + +it('has no covering test classes before it starts', function (): void { + $test = new MutationTest(new Mutation(new SplFileInfo(__FILE__, '', ''), 'id', 'SomeMutator', 1, 2, '', '')); + + expect($test->coveringTestClasses())->toBe([]); +}); diff --git a/tests/Unit/Repositories/MutationRepositoryTest.php b/tests/Unit/Repositories/MutationRepositoryTest.php new file mode 100644 index 0000000..d59798d --- /dev/null +++ b/tests/Unit/Repositories/MutationRepositoryTest.php @@ -0,0 +1,63 @@ + $coveringTestClasses + */ +$addMutation = function (MutationRepository $repository, string $id, float $duration, array $coveringTestClasses): void { + $file = new SplFileInfo(__FILE__, '', ''); + + $repository->add(new Mutation($file, $id, 'SomeMutator', 1, 2, '', '')); + + $tests = $repository->all()[$file->getRealPath()]->tests(); + $test = end($tests); + + assert($test instanceof MutationTest); + + $reflection = new ReflectionClass($test); + $reflection->getProperty('start')->setValue($test, 0.0); + $reflection->getProperty('finish')->setValue($test, $duration); + $reflection->getProperty('coveringTestClasses')->setValue($test, $coveringTestClasses); +}; + +describe('units', function () use ($addMutation): void { + it('groups the mutations of a file into one unit, with every test covering it', function () use ($addMutation): void { + $repository = new MutationRepository; + + $addMutation($repository, 'a', 2.0, ['Tests\\Unit\\FooTest', 'Tests\\Unit\\BarTest']); + $addMutation($repository, 'b', 0.5, ['Tests\\Unit\\FooTest']); + + expect($repository->units(dirname(__DIR__, 3)))->toBe([ + 'tests/Unit/Repositories/MutationRepositoryTest.php' => [ + 'time' => 2.5, + 'tests' => ['Tests\\Unit\\FooTest', 'Tests\\Unit\\BarTest'], + ], + ]); + }); + + it('leaves out a file no test covers', function () use ($addMutation): void { + $repository = new MutationRepository; + + $addMutation($repository, 'a', 0.0, []); + + expect($repository->units(dirname(__DIR__, 3)))->toBe([]); + }); + + it('keeps an absolute path when the file is outside the root', function () use ($addMutation): void { + $repository = new MutationRepository; + + $addMutation($repository, 'a', 1.0, ['Tests\\Unit\\FooTest']); + + expect(array_keys($repository->units('/somewhere/else')))->toBe([__FILE__]); + }); + + it('returns nothing when no mutation ran', function (): void { + expect(new MutationRepository()->units('/'))->toBe([]); + }); +});