From d6b898628451bdb5fb0ca482457829b76f00ea7e Mon Sep 17 00:00:00 2001 From: Maks Oleksyuk Date: Fri, 7 Aug 2026 15:01:25 +0300 Subject: [PATCH] feat: let a shard unit bundle several test classes Co-Authored-By: Claude Opus 5 --- src/Plugins/Help.php | 2 +- src/Plugins/Shard.php | 255 ++++++++++++++---- ...isual_snapshot_of_help_command_output.snap | 2 +- tests/.snapshots/success.txt | 11 +- tests/Unit/Plugins/Shard.php | 186 +++++++++++++ tests/Visual/Parallel.php | 4 +- 6 files changed, 403 insertions(+), 57 deletions(-) diff --git a/src/Plugins/Help.php b/src/Plugins/Help.php index 9615fa599..339582aae 100644 --- a/src/Plugins/Help.php +++ b/src/Plugins/Help.php @@ -154,7 +154,7 @@ private function getContent(): array ], [ 'arg' => '--update-shards', - 'desc' => 'Update shards.json with test timing data for time-balanced sharding', + 'desc' => 'Update the shards file with timing data for time-balanced sharding', ], ], ...$content['Execution']]; diff --git a/src/Plugins/Shard.php b/src/Plugins/Shard.php index 7c7a29cc6..69f5b0b9a 100644 --- a/src/Plugins/Shard.php +++ b/src/Plugins/Shard.php @@ -41,6 +41,23 @@ final class Shard implements AddsOutput, HandlesArguments, Terminable private static bool $updateShards = false; + private static ?string $timingsFilename = null; + + /** + * @var array}>|null + */ + private static ?array $externalTimings = null; + + /** + * @var list + */ + private static array $selectedUnits = []; + + /** + * @var array + */ + private static array $metadata = []; + private static bool $timeBalanced = false; private static bool $shardsOutdated = false; @@ -48,7 +65,7 @@ final class Shard implements AddsOutput, HandlesArguments, Terminable private static bool $passed = false; /** - * @var array|null + * @var array}>|null */ private static ?array $collectedTimings = null; @@ -63,6 +80,41 @@ public function __construct( // } + public static function useTimingsFile(string $filename): void + { + self::$timingsFilename = $filename; + } + + /** + * @param array}> $units + * @param array $metadata Stored alongside the units, and read back + * by {@see self::metadata()} on sharded runs. + */ + public static function useTimings(array $units, array $metadata = []): void + { + self::$externalTimings = $units; + + if ($metadata !== []) { + self::$metadata = $metadata; + } + } + + /** + * @return array + */ + public static function metadata(): array + { + return self::$metadata; + } + + /** + * @return list + */ + public static function selectedUnits(): array + { + return self::$selectedUnits; + } + /** * {@inheritDoc} */ @@ -98,20 +150,25 @@ public function handleArguments(array $arguments): array /** @phpstan-ignore-next-line */ $tests = $this->allTests($arguments); - $timings = $this->loadShardsFile(); - if ($timings !== null) { - $knownTests = array_values(array_filter($tests, fn (string $test): bool => isset($timings[$test]))); - $newTests = array_values(array_diff($tests, $knownTests)); + $units = $this->loadShardsFile(); + if ($units !== null) { + $newTests = array_values(array_diff($tests, $this->testsOf($units))); + + $partitions = $this->partitionByTime($units, $total); - $partitions = $this->partitionByTime($knownTests, $timings, $total); + $median = $this->medianTime($units); foreach ($newTests as $i => $test) { - $partitions[$i % $total][] = $test; + $partitions[$i % $total][$test] = ['time' => $median, 'tests' => [$test]]; } - $testsToRun = $partitions[$index - 1] ?? []; + $selected = $partitions[$index - 1] ?? []; + + self::$selectedUnits = array_keys($selected); self::$timeBalanced = true; self::$shardsOutdated = $newTests !== []; + + $testsToRun = array_values(array_intersect($tests, $this->testsOf($selected))); } else { $isInCurrentShard = fn (int $key): bool => $key % $total === ($index - 1); $testsToRun = array_values(array_filter($tests, $isInCurrentShard, ARRAY_FILTER_USE_KEY)); @@ -198,9 +255,12 @@ private function removeParallelArguments(array $arguments): array */ private function buildListTestsCommand(array $arguments, string $testPath): array { - $filtered = $this->removeParallelArguments($arguments); + $filtered = array_filter( + $this->removeParallelArguments($arguments), + fn (string $argument): bool => ! str_starts_with($argument, '--coverage'), + ); - return ['php', ...$filtered, '--test-directory='.$testPath, '--list-tests']; + return ['php', ...array_values($filtered), '--test-directory='.$testPath, '--list-tests']; } /** @@ -274,15 +334,14 @@ public function addOutput(int $exitCode): int { self::$passed = $exitCode === 0; - if (self::$updateShards && self::$passed && ! Parallel::isWorker()) { + if (self::$updateShards && (self::$passed || self::$externalTimings !== null) && ! Parallel::isWorker()) { self::$collectedTimings = $this->collectTimings(); - $count = self::$knownTests !== null - ? count(array_intersect_key(self::$collectedTimings, array_flip(self::$knownTests))) - : count(self::$collectedTimings); + $count = count($this->unitsToWrite(self::$collectedTimings)); $this->output->writeln(sprintf( - ' Shards: shards.json updated with timings for %d test class%s.', + ' Shards: %s updated with timings for %d test class%s.', + $this->timingsFilename(), $count, $count === 1 ? '' : 'es', )); @@ -310,7 +369,10 @@ public function addOutput(int $exitCode): int )); if (self::$shardsOutdated) { - $this->output->writeln(' WARN The [tests/.pest/shards.json] file is out of date. Run [--update-shards] to update it.'); + $this->output->writeln(sprintf( + ' WARN The [%s] file is out of date. Run [--update-shards] to update it.', + $this->relativeTimingsPath(), + )); } return $exitCode; @@ -328,7 +390,7 @@ public function terminate(): void return; } - if (! self::$passed) { + if (! self::$passed && self::$externalTimings === null) { return; } @@ -342,10 +404,14 @@ public function terminate(): void } /** - * @return array + * @return array}> */ private function collectTimings(): array { + if (self::$externalTimings !== null) { + return self::$externalTimings; + } + $runId = Parallel::getGlobal('SHARD_RUN_ID'); if (is_string($runId)) { @@ -407,15 +473,46 @@ private function readWorkerTimings(string $runId): array return $merged; } + private function timingsFilename(): string + { + return self::$timingsFilename ?? 'shards.json'; + } + + private function relativeTimingsPath(): string + { + return implode(DIRECTORY_SEPARATOR, [TestSuite::getInstance()->testPath, '.pest', $this->timingsFilename()]); + } + private function shardsPath(): string { - $testSuite = TestSuite::getInstance(); + return TestSuite::getInstance()->rootPath.DIRECTORY_SEPARATOR.$this->relativeTimingsPath(); + } - return implode(DIRECTORY_SEPARATOR, [$testSuite->rootPath, $testSuite->testPath, '.pest', 'shards.json']); + /** + * @param array $units + * @return array}> + */ + private function normaliseUnits(array $units): array + { + $normalised = []; + + foreach ($units as $key => $unit) { + if (is_array($unit) && isset($unit['time']) && isset($unit['tests']) && is_array($unit['tests'])) { + $normalised[$key] = ['time' => (float) $unit['time'], 'tests' => array_values(array_map(strval(...), $unit['tests']))]; + + continue; + } + + if (is_float($unit) || is_int($unit)) { + $normalised[$key] = ['time' => (float) $unit, 'tests' => [$key]]; + } + } + + return $normalised; } /** - * @return array|null + * @return array}>|null */ private function loadShardsFile(): ?array { @@ -428,50 +525,74 @@ private function loadShardsFile(): ?array $contents = file_get_contents($path); if ($contents === false) { - throw new InvalidOption('The [tests/.pest/shards.json] file could not be read. Delete it or run [--update-shards] to regenerate.'); + throw new InvalidOption(sprintf('The [%s] file could not be read. Delete it or run [--update-shards] to regenerate.', $this->relativeTimingsPath())); } $data = json_decode($contents, true); - if (! is_array($data) || ! isset($data['timings']) || ! is_array($data['timings'])) { - throw new InvalidOption('The [tests/.pest/shards.json] file is corrupted. Delete it or run [--update-shards] to regenerate.'); + $units = null; + + if (is_array($data)) { + $units = $data['units'] ?? $data['timings'] ?? null; + } + + if (! is_array($units)) { + throw new InvalidOption(sprintf('The [%s] file is corrupted. Delete it or run [--update-shards] to regenerate.', $this->relativeTimingsPath())); + } + + if (is_array($data) && isset($data['metadata']) && is_array($data['metadata'])) { + self::$metadata = array_filter($data['metadata'], is_scalar(...)); } - return $data['timings']; + return $this->normaliseUnits($units); } /** - * @param list $tests - * @param array $timings - * @return list> + * @param array}> $units + * @return list */ - private function partitionByTime(array $tests, array $timings, int $total): array + private function testsOf(array $units): array { - $knownTimings = array_filter( - array_map(fn (string $test): ?float => $timings[$test] ?? null, $tests), - fn (?float $t): bool => $t !== null, - ); + $tests = []; - $median = $knownTimings !== [] ? $this->median(array_values($knownTimings)) : 1.0; + foreach ($units as $unit) { + foreach ($unit['tests'] as $test) { + $tests[$test] = true; + } + } - $testsWithTimings = array_map( - fn (string $test): array => ['test' => $test, 'time' => $timings[$test] ?? $median], - $tests, - ); + return array_keys($tests); + } - usort($testsWithTimings, fn (array $a, array $b): int => $b['time'] <=> $a['time']); + /** + * @param array}> $units + */ + private function medianTime(array $units): float + { + $times = array_column($units, 'time'); - /** @var list> */ + return $times === [] ? 1.0 : $this->median($times); + } + + /** + * @param array}> $units + * @return list}>> + */ + private function partitionByTime(array $units, int $total): array + { + uasort($units, fn (array $a, array $b): int => $b['time'] <=> $a['time']); + + /** @var list}>> */ $bins = array_fill(0, $total, []); /** @var non-empty-list */ $binTimes = array_fill(0, $total, 0.0); - foreach ($testsWithTimings as $item) { + foreach ($units as $key => $unit) { $minIndex = array_search(min($binTimes), $binTimes, strict: true); assert(is_int($minIndex)); - $bins[$minIndex][] = $item['test']; - $binTimes[$minIndex] += $item['time']; + $bins[$minIndex][$key] = $unit; + $binTimes[$minIndex] += $unit['time']; } return $bins; @@ -495,9 +616,32 @@ private function median(array $values): float } /** - * @param array $timings + * @param array}> $units + * @return array}> */ - private function writeTimings(array $timings): void + private function unitsToWrite(array $units): array + { + $units = $this->normaliseUnits($units); + + if (self::$knownTests === null) { + return $units; + } + + $known = self::$knownTests; + + $units = array_filter($units, fn (array $unit): bool => array_intersect($unit['tests'], $known) !== []); + + foreach (array_diff($known, $this->testsOf($units)) as $test) { + $units[$test] = ['time' => 0.0, 'tests' => [$test]]; + } + + return $units; + } + + /** + * @param array}> $units + */ + private function writeTimings(array $units): void { $path = $this->shardsPath(); @@ -506,18 +650,25 @@ private function writeTimings(array $timings): void mkdir($directory, 0755, true); } - if (self::$knownTests !== null) { - $knownSet = array_flip(self::$knownTests); - $timings = array_intersect_key($timings, $knownSet); - } + $bundled = self::$externalTimings !== null && array_filter($units, is_array(...)) !== []; + + $units = $this->unitsToWrite($units); - ksort($timings); + ksort($units); - $canonical = self::$knownTests ?? array_keys($timings); + $canonical = self::$knownTests ?? $this->testsOf($units); sort($canonical); + $payload = $bundled + ? ['units' => $units] + : ['timings' => array_map(fn (array $unit): float => $unit['time'], $units)]; + + if (self::$metadata !== []) { + $payload['metadata'] = self::$metadata; + } + file_put_contents($path, json_encode([ - 'timings' => $timings, + ...$payload, 'checksum' => md5(implode("\n", $canonical)), 'updated_at' => date('c'), ], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)."\n"); diff --git a/tests/.pest/snapshots/Visual/Help/visual_snapshot_of_help_command_output.snap b/tests/.pest/snapshots/Visual/Help/visual_snapshot_of_help_command_output.snap index f39892d88..198ae2718 100644 --- a/tests/.pest/snapshots/Visual/Help/visual_snapshot_of_help_command_output.snap +++ b/tests/.pest/snapshots/Visual/Help/visual_snapshot_of_help_command_output.snap @@ -54,7 +54,7 @@ EXECUTION OPTIONS: --parallel ........................................... Run tests in parallel --update-snapshots Update snapshots for tests using the "toMatchSnapshot" expectation - --update-shards Update shards.json with test timing data for time-balanced sharding + --update-shards Update the shards file with timing data for time-balanced sharding --globals-backup ................. Backup and restore $GLOBALS for each test --static-backup ......... Backup and restore static properties for each test --strict-coverage ................... Be strict about code coverage metadata diff --git a/tests/.snapshots/success.txt b/tests/.snapshots/success.txt index c6bc6212b..9f0ca0819 100644 --- a/tests/.snapshots/success.txt +++ b/tests/.snapshots/success.txt @@ -1836,6 +1836,15 @@ ✓ addOutput → it displays shard information after test execution ✓ addOutput → it uses singular form for single test file ✓ addOutput → it returns original exit code when shard is not set + ✓ timings file → it reads and writes shards.json until a plugin overrides the filename + ✓ timings file → it prefers timings supplied by a plugin over the ones collected from the test run + ✓ timings file → it records known tests without supplied timings as zero + ✓ timings file → it keeps timings supplied by a plugin even when the test suite did not pass + ✓ timings file → it strips coverage arguments when building the list-tests command + ✓ units → it treats a bare timing as a unit bundling only its own test class + ✓ units → it keeps the test classes a unit bundles together in one shard + ✓ units → it collects every test class the given units bundle + ✓ units → it writes bundled units, and reads back both shapes PASS Tests\Unit\Plugins\Tia\ContentHash ✓ of() → it returns false when file does not exist @@ -2221,4 +2230,4 @@ ✓ pass with dataset with ('my-datas-set-value') ✓ within describe → pass with dataset with ('my-datas-set-value') - Tests: 2 deprecated, 4 warnings, 5 incomplete, 2 notices, 40 todos, 35 skipped, 1570 passed (3410 assertions) \ No newline at end of file + Tests: 2 deprecated, 4 warnings, 5 incomplete, 2 notices, 40 todos, 35 skipped, 1579 passed (3423 assertions) \ No newline at end of file diff --git a/tests/Unit/Plugins/Shard.php b/tests/Unit/Plugins/Shard.php index f31b2cc1d..a14bfb0da 100644 --- a/tests/Unit/Plugins/Shard.php +++ b/tests/Unit/Plugins/Shard.php @@ -2,6 +2,8 @@ use Pest\Exceptions\InvalidOption; use Pest\Plugins\Shard; +use Pest\Subscribers\EnsureShardTimingsAreCollected; +use Pest\Support\Arr; use Symfony\Component\Console\Input\ArgvInput; use Symfony\Component\Console\Output\BufferedOutput; @@ -497,3 +499,187 @@ ->and($outputText)->not->toContain('Shard:'); }); }); + +describe('timings file', function (): void { + afterEach(function (): void { + $reflection = new ReflectionClass(Shard::class); + $reflection->getProperty('timingsFilename')->setValue(null, null); + $reflection->getProperty('externalTimings')->setValue(null, null); + $reflection->getProperty('collectedTimings')->setValue(null, null); + $reflection->getProperty('knownTests')->setValue(null, null); + $reflection->getProperty('updateShards')->setValue(null, false); + $reflection->getProperty('shard')->setValue(null, null); + + new ReflectionClass(EnsureShardTimingsAreCollected::class) + ->getProperty('timings') + ->setValue(null, []); + }); + + it('reads and writes shards.json until a plugin overrides the filename', function (): void { + $output = new BufferedOutput; + $shard = new Shard($output); + + $method = new ReflectionClass($shard)->getMethod('shardsPath'); + + expect($method->invoke($shard))->toEndWith('.pest'.DIRECTORY_SEPARATOR.'shards.json'); + + Shard::useTimingsFile('mutation-shards.json'); + + expect($method->invoke($shard))->toEndWith('.pest'.DIRECTORY_SEPARATOR.'mutation-shards.json'); + }); + + it('prefers timings supplied by a plugin over the ones collected from the test run', function (): void { + $output = new BufferedOutput; + $shard = new Shard($output); + + new ReflectionClass(EnsureShardTimingsAreCollected::class) + ->getProperty('timings') + ->setValue(null, ['Tests\\Unit\\CollectedTest' => 9.0]); + + Shard::useTimings(['Tests\\Unit\\SuppliedTest' => 1.5]); + + $method = new ReflectionClass($shard)->getMethod('collectTimings'); + + expect($method->invoke($shard))->toBe(['Tests\\Unit\\SuppliedTest' => 1.5]); + }); + + it('records known tests without supplied timings as zero', function (): void { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $reflection->getProperty('knownTests')->setValue(null, ['Tests\\Unit\\MutatedTest', 'Tests\\Unit\\PlainTest']); + + Shard::useTimingsFile('shards-fixture.json'); + Shard::useTimings($timings = ['Tests\\Unit\\MutatedTest' => 4.5]); + + $path = $reflection->getMethod('shardsPath')->invoke($shard); + + try { + $reflection->getMethod('writeTimings')->invoke($shard, $timings); + + expect(json_decode((string) file_get_contents($path), true)['timings'])->toEqual([ + 'Tests\\Unit\\MutatedTest' => 4.5, + 'Tests\\Unit\\PlainTest' => 0.0, + ]); + } finally { + @unlink($path); + } + }); + + it('keeps timings supplied by a plugin even when the test suite did not pass', function (): void { + $output = new BufferedOutput; + $shard = new Shard($output); + + new ReflectionClass($shard)->getProperty('updateShards')->setValue(null, true); + + Shard::useTimingsFile('mutation-shards.json'); + Shard::useTimings(['Tests\\Unit\\SuppliedTest' => 1.5]); + + $paratest = Arr::get($_SERVER, 'PARATEST'); + unset($_SERVER['PARATEST']); + + try { + expect($shard->addOutput(1))->toBe(1) + ->and($output->fetch())->toContain('mutation-shards.json updated with timings for 1 test class.'); + } finally { + if ($paratest !== null) { + $_SERVER['PARATEST'] = $paratest; + } + } + }); + + it('strips coverage arguments when building the list-tests command', function (): void { + $output = new BufferedOutput; + $shard = new Shard($output); + + $method = new ReflectionClass($shard)->getMethod('buildListTestsCommand'); + + $command = $method->invoke($shard, ['bin/pest', '--coverage-php=/tmp/coverage.php', '--update-shards'], 'tests'); + + expect($command)->toBe([ + 'php', + 'bin/pest', + '--update-shards', + '--test-directory=tests', + '--list-tests', + ]); + }); +}); + +describe('units', function (): void { + afterEach(function (): void { + $reflection = new ReflectionClass(Shard::class); + $reflection->getProperty('timingsFilename')->setValue(null, null); + $reflection->getProperty('externalTimings')->setValue(null, null); + $reflection->getProperty('collectedTimings')->setValue(null, null); + $reflection->getProperty('knownTests')->setValue(null, null); + $reflection->getProperty('selectedUnits')->setValue(null, []); + }); + + it('treats a bare timing as a unit bundling only its own test class', function (): void { + $output = new BufferedOutput; + $shard = new Shard($output); + + $method = new ReflectionClass($shard)->getMethod('normaliseUnits'); + + expect($method->invoke($shard, ['Tests\\Unit\\FooTest' => 1.5]))->toBe([ + 'Tests\\Unit\\FooTest' => ['time' => 1.5, 'tests' => ['Tests\\Unit\\FooTest']], + ]); + }); + + it('keeps the test classes a unit bundles together in one shard', function (): void { + $output = new BufferedOutput; + $shard = new Shard($output); + + $method = new ReflectionClass($shard)->getMethod('partitionByTime'); + + $partitions = $method->invoke($shard, [ + 'app/Heavy.php' => ['time' => 10.0, 'tests' => ['Tests\\Unit\\OneTest', 'Tests\\Unit\\TwoTest']], + 'app/Light.php' => ['time' => 1.0, 'tests' => ['Tests\\Unit\\ThreeTest']], + 'app/Medium.php' => ['time' => 4.0, 'tests' => ['Tests\\Unit\\FourTest']], + ], 2); + + expect(array_keys($partitions[0]))->toBe(['app/Heavy.php']) + ->and(array_keys($partitions[1]))->toBe(['app/Medium.php', 'app/Light.php']); + }); + + it('collects every test class the given units bundle', function (): void { + $output = new BufferedOutput; + $shard = new Shard($output); + + $method = new ReflectionClass($shard)->getMethod('testsOf'); + + expect($method->invoke($shard, [ + 'app/One.php' => ['time' => 1.0, 'tests' => ['Tests\\Unit\\FooTest', 'Tests\\Unit\\BarTest']], + 'app/Two.php' => ['time' => 1.0, 'tests' => ['Tests\\Unit\\BarTest']], + ]))->toBe(['Tests\\Unit\\FooTest', 'Tests\\Unit\\BarTest']); + }); + + it('writes bundled units, and reads back both shapes', function (): void { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $reflection->getProperty('knownTests')->setValue(null, ['Tests\\Unit\\FooTest', 'Tests\\Unit\\BarTest']); + + Shard::useTimingsFile('units-fixture.json'); + Shard::useTimings($units = [ + 'app/One.php' => ['time' => 4.5, 'tests' => ['Tests\\Unit\\FooTest']], + ]); + + $path = $reflection->getMethod('shardsPath')->invoke($shard); + + try { + $reflection->getMethod('writeTimings')->invoke($shard, $units); + + expect(json_decode((string) file_get_contents($path), true))->toHaveKey('units') + ->and($reflection->getMethod('loadShardsFile')->invoke($shard))->toEqual([ + 'app/One.php' => ['time' => 4.5, 'tests' => ['Tests\\Unit\\FooTest']], + 'Tests\\Unit\\BarTest' => ['time' => 0.0, 'tests' => ['Tests\\Unit\\BarTest']], + ]); + } finally { + @unlink($path); + } + }); +}); diff --git a/tests/Visual/Parallel.php b/tests/Visual/Parallel.php index 6bb90a0ae..fee632d64 100644 --- a/tests/Visual/Parallel.php +++ b/tests/Visual/Parallel.php @@ -26,13 +26,13 @@ $file = file_get_contents(__FILE__); $file = preg_replace( '/\$expected = \'.*?\';/', - "\$expected = '2 deprecated, 4 warnings, 5 incomplete, 3 notices, 40 todos, 27 skipped, 1552 passed (3355 assertions)';", + "\$expected = '2 deprecated, 4 warnings, 5 incomplete, 3 notices, 40 todos, 27 skipped, 1561 passed (3368 assertions)';", $file, ); file_put_contents(__FILE__, $file); } - $expected = '2 deprecated, 4 warnings, 5 incomplete, 3 notices, 40 todos, 27 skipped, 1552 passed (3355 assertions)'; + $expected = '2 deprecated, 4 warnings, 5 incomplete, 3 notices, 40 todos, 27 skipped, 1561 passed (3368 assertions)'; expect($output) ->toContain("Tests: {$expected}")