From bdf046f09b74933693a14195b76aec49f73c2010 Mon Sep 17 00:00:00 2001 From: jim Date: Tue, 6 Jan 2026 16:43:16 +0000 Subject: [PATCH 1/5] Compact shard filter argument --- src/Plugins/Shard.php | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/Plugins/Shard.php b/src/Plugins/Shard.php index f48260bb..cad555f2 100644 --- a/src/Plugins/Shard.php +++ b/src/Plugins/Shard.php @@ -105,10 +105,42 @@ private function removeParallelArguments(array $arguments): array /** * Builds the filter argument for the given tests to run. + * + * @param array $testsToRun */ - private function buildFilterArgument(mixed $testsToRun): string + private function buildFilterArgument(array $testsToRun): string { - return addslashes(implode('|', $testsToRun)); + if (empty($testsToRun)) { + return ''; + } + + $tree = []; + foreach ($testsToRun as $class) { + $parts = explode('\\', $class); + $current = &$tree; + foreach ($parts as $part) { + if (! isset($current[$part])) { + $current[$part] = []; + } + $current = &$current[$part]; + } + } + + $buildRegex = function (array $tree) use (&$buildRegex): string { + $parts = []; + foreach ($tree as $key => $sub) { + $subRegex = $buildRegex($sub); + if ($subRegex === '') { + $parts[] = preg_quote($key, '/'); + } else { + $parts[] = preg_quote($key, '/').'\\\\'.(count($sub) > 1 ? '('.$subRegex.')' : $subRegex); + } + } + + return implode('|', $parts); + }; + + return $buildRegex($tree); } /** From 949dc46145d5c56f1bd0e0fa47b518e418126f5b Mon Sep 17 00:00:00 2001 From: jim Date: Tue, 6 Jan 2026 16:57:55 +0000 Subject: [PATCH 2/5] Enforce maximum filter length --- src/Plugins/Shard.php | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/Plugins/Shard.php b/src/Plugins/Shard.php index cad555f2..ae8ae858 100644 --- a/src/Plugins/Shard.php +++ b/src/Plugins/Shard.php @@ -21,6 +21,13 @@ final class Shard implements AddsOutput, HandlesArguments private const string SHARD_OPTION = 'shard'; + /** + * The maximum length allowed for the filter argument. + * While ARG_MAX can be 2MB, individual arguments are often limited to 128KB (MAX_ARG_STRLEN). + * Practical limits in CI environments (like Docker or pipeline runners) can be even lower. + */ + private const int MAX_FILTER_LENGTH = 32768; + /** * The shard index and total number of shards. * @@ -72,7 +79,11 @@ public function handleArguments(array $arguments): array 'testsCount' => count($tests), ]; - return [...$arguments, '--filter', $this->buildFilterArgument($testsToRun)]; + $filter = $this->buildFilterArgument($testsToRun); + + $this->ensureFilterLengthIsSafe($filter); + + return [...$arguments, '--filter', $filter]; } /** @@ -140,7 +151,31 @@ private function buildFilterArgument(array $testsToRun): string return implode('|', $parts); }; - return $buildRegex($tree); + $filter = $buildRegex($tree); + + $this->ensureFilterLengthIsSafe($filter); + + return $filter; + } + + /** + * Ensures that the filter length is safe for the current environment. + * + * @throws InvalidOption + */ + private function ensureFilterLengthIsSafe(string $filter): void + { + $maxLength = (int) (getenv('PEST_SHARD_MAX_FILTER_LENGTH') ?: self::MAX_FILTER_LENGTH); + + if (strlen($filter) > $maxLength) { + throw new InvalidOption(sprintf( + 'The generated filter for this shard is too long (%d characters). '. + 'This can cause issues with some environments (limit is %d characters). '. + 'Please increase the number of shards (e.g., use 1/4 instead of 1/2) to reduce the filter length.', + strlen($filter), + $maxLength + )); + } } /** From 44a6efd3b6edd65004d3e9a865a05f7d5ae685ac Mon Sep 17 00:00:00 2001 From: jim Date: Tue, 6 Jan 2026 19:40:09 +0000 Subject: [PATCH 3/5] Fix phpstan issue in `buildFilterArgument` --- src/Plugins/Shard.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Plugins/Shard.php b/src/Plugins/Shard.php index ae8ae858..59e01bab 100644 --- a/src/Plugins/Shard.php +++ b/src/Plugins/Shard.php @@ -121,10 +121,11 @@ private function removeParallelArguments(array $arguments): array */ private function buildFilterArgument(array $testsToRun): string { - if (empty($testsToRun)) { + if ($testsToRun === []) { return ''; } + /** @var array $tree */ $tree = []; foreach ($testsToRun as $class) { $parts = explode('\\', $class); From 5f54b9035ad6dedf4b264dba3c84ec4d08e8d4db Mon Sep 17 00:00:00 2001 From: oddvalue Date: Wed, 7 Jan 2026 06:55:41 +0000 Subject: [PATCH 4/5] Update Shard.php --- src/Plugins/Shard.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Plugins/Shard.php b/src/Plugins/Shard.php index 59e01bab..027ba0d9 100644 --- a/src/Plugins/Shard.php +++ b/src/Plugins/Shard.php @@ -152,11 +152,7 @@ private function buildFilterArgument(array $testsToRun): string return implode('|', $parts); }; - $filter = $buildRegex($tree); - - $this->ensureFilterLengthIsSafe($filter); - - return $filter; + return $buildRegex($tree); } /** From d2ff347d2e37636d34fd8c7d17a1d17196a643eb Mon Sep 17 00:00:00 2001 From: jim Date: Wed, 7 Jan 2026 10:30:54 +0000 Subject: [PATCH 5/5] Add unit tests for `Shard` plugin --- tests/Unit/Plugins/Shard.php | 325 +++++++++++++++++++++++++++++++++++ 1 file changed, 325 insertions(+) create mode 100644 tests/Unit/Plugins/Shard.php diff --git a/tests/Unit/Plugins/Shard.php b/tests/Unit/Plugins/Shard.php new file mode 100644 index 00000000..92f2049a --- /dev/null +++ b/tests/Unit/Plugins/Shard.php @@ -0,0 +1,325 @@ +toBe([ + 'index' => $expectedIndex, + 'total' => $expectedTotal, + ]); + })->with([ + ['1/2', 1, 2], + ['2/2', 2, 2], + ['1/4', 1, 4], + ['4/4', 4, 4], + ['1/10', 1, 10], + ['10/10', 10, 10], + ['5/100', 5, 100], + ]); + + it('throws exception for invalid format', function (array $arguments, string $errorPattern) { + $input = new ArgvInput($arguments); + + Shard::getShard($input); + })->with([ + [['test', '--shard', 'invalid'], 'must be in the format "index/total"'], + [['test', '--shard', '1'], 'must be in the format "index/total"'], + [['test', '--shard', '1/'], 'must be in the format "index/total"'], + [['test', '--shard', '/2'], 'must be in the format "index/total"'], + [['test', '--shard', 'a/b'], 'must be in the format "index/total"'], + [['test', '--shard', '1.5/2'], 'must be in the format "index/total"'], + ])->throws(InvalidOption::class); + + it('throws exception for invalid index or total values', function (array $arguments, string $errorPattern) { + $input = new ArgvInput($arguments); + + Shard::getShard($input); + })->with([ + [['test', '--shard', '0/2'], 'must be a non-negative integer'], + [['test', '--shard', '1/0'], 'must be a non-negative integer'], + [['test', '--shard', '-1/2'], 'must be a non-negative integer'], + [['test', '--shard', '1/-2'], 'must be a non-negative integer'], + [['test', '--shard', '3/2'], 'must be a non-negative integer less than the total'], + [['test', '--shard', '5/4'], 'must be a non-negative integer less than the total'], + ])->throws(InvalidOption::class); +}); + +describe('buildFilterArgument', function () { + it('generates compact filter for single test', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('buildFilterArgument'); + + $filter = $method->invoke($shard, ['Tests\\Unit\\ExampleTest']); + + expect($filter)->toBe('Tests\\\\Unit\\\\ExampleTest'); + }); + + it('generates compact filter for multiple tests with common prefix', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('buildFilterArgument'); + + $filter = $method->invoke($shard, [ + 'Tests\\Unit\\Foo\\BarTest', + 'Tests\\Unit\\Foo\\BazTest', + ]); + + // Should create a compact regex using tree structure + expect($filter)->toBe('Tests\\\\Unit\\\\Foo\\\\(BarTest|BazTest)'); + }); + + it('generates compact filter for tests with different namespaces', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('buildFilterArgument'); + + $filter = $method->invoke($shard, [ + 'Tests\\Unit\\FooTest', + 'Tests\\Feature\\BarTest', + ]); + + // Should create a compact regex with branches for Unit and Feature + expect($filter)->toBe('Tests\\\\(Unit\\\\FooTest|Feature\\\\BarTest)'); + }); + + it('returns empty string for empty test list', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('buildFilterArgument'); + + $filter = $method->invoke($shard, []); + + expect($filter)->toBe(''); + }); + + it('generates compact filter for deeply nested namespaces', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('buildFilterArgument'); + + $filter = $method->invoke($shard, [ + 'Tests\\Unit\\Plugins\\Concerns\\Foo', + 'Tests\\Unit\\Plugins\\Concerns\\Bar', + 'Tests\\Unit\\Plugins\\Concerns\\Baz', + ]); + + // Should optimize common prefix + expect($filter)->toBe('Tests\\\\Unit\\\\Plugins\\\\Concerns\\\\(Foo|Bar|Baz)'); + }); + + it('handles mix of nested and flat namespaces', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('buildFilterArgument'); + + $tests = [ + 'Tests\\Unit\\SimpleTest', + 'Tests\\Unit\\Plugins\\Concerns\\HandleArguments', + 'Tests\\Unit\\Plugins\\Concerns\\Validation', + 'Tests\\Unit\\Another\\Deep\\Nested\\Test', + ]; + + $filter = $method->invoke($shard, $tests); + + expect($filter) + ->toBe(addslashes('Tests\\Unit\\(SimpleTest|Plugins\\Concerns\\(HandleArguments|Validation)|Another\\Deep\\Nested\\Test)')); + }); +}); + +describe('ensureFilterLengthIsSafe', function () { + it('accepts filter within length limit', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('ensureFilterLengthIsSafe'); + + $filter = str_repeat('a', 1000); + + $method->invoke($shard, $filter); + + expect(true)->toBeTrue(); // Should not throw + }); + + it('throws exception when filter exceeds default limit', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('ensureFilterLengthIsSafe'); + + // Create a filter that exceeds 32KB + $filter = str_repeat('a', 32769); + + $method->invoke($shard, $filter); + })->throws(InvalidOption::class, 'The generated filter for this shard is too long'); + + it('respects custom limit from environment variable', function () { + putenv('PEST_SHARD_MAX_FILTER_LENGTH=1000'); + + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('ensureFilterLengthIsSafe'); + + $filter = str_repeat('a', 1001); + + try { + $method->invoke($shard, $filter); + expect(false)->toBeTrue('Should have thrown exception'); + } catch (InvalidOption $e) { + expect($e->getMessage())->toContain('1001 characters') + ->and($e->getMessage())->toContain('limit is 1000 characters'); + } finally { + putenv('PEST_SHARD_MAX_FILTER_LENGTH'); + } + }); + + it('accepts filter within custom limit', function () { + putenv('PEST_SHARD_MAX_FILTER_LENGTH=1000'); + + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('ensureFilterLengthIsSafe'); + + $filter = str_repeat('a', 999); + + try { + $method->invoke($shard, $filter); + expect(true)->toBeTrue(); // Should not throw + } catch (InvalidOption) { + expect(false)->toBeTrue('Should not have thrown exception'); + } finally { + putenv('PEST_SHARD_MAX_FILTER_LENGTH'); + } + }); +}); + +describe('handleArguments', function () { + it('returns original arguments when shard option is not present', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $arguments = ['bin/pest', 'tests/', '--parallel']; + + $result = $shard->handleArguments($arguments); + + expect($result)->toBe($arguments); + }); + + it('generates shard filter when shard option is present', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $arguments = ['bin/pest', 'tests/', '--parallel', '--shard=1/2']; + + $result = $shard->handleArguments($arguments); + + expect($result) + ->toContain('bin/pest') + ->toContain('tests/') + ->toContain('--parallel') + ->toContain('--filter'); + }); + + it('removes parallel arguments from test discovery', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('removeParallelArguments'); + + $arguments = ['bin/pest', '--parallel', 'tests/', '-p']; + + $result = $method->invoke($shard, $arguments); + + expect($result)->toBe([0 => 'bin/pest', 2 => 'tests/']); + }); +}); + +describe('addOutput', function () { + it('displays shard information after test execution', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + // Simulate setting shard info + $reflection = new ReflectionClass($shard); + $property = $reflection->getProperty('shard'); + $property->setValue(null, [ + 'index' => 2, + 'total' => 4, + 'testsRan' => 25, + 'testsCount' => 100, + ]); + + $exitCode = $shard->addOutput(0); + $outputText = $output->fetch(); + + expect($exitCode)->toBe(0) + ->and($outputText)->toContain('Shard:') + ->and($outputText)->toContain('2 of 4') + ->and($outputText)->toContain('25 files ran') + ->and($outputText)->toContain('out of 100'); + }); + + it('uses singular form for single test file', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + // Simulate setting shard info + $reflection = new ReflectionClass($shard); + $property = $reflection->getProperty('shard'); + $property->setValue(null, [ + 'index' => 1, + 'total' => 4, + 'testsRan' => 1, + 'testsCount' => 100, + ]); + + $shard->addOutput(0); + $outputText = $output->fetch(); + + expect($outputText)->toContain('1 file ran') + ->and($outputText)->not->toContain('1 files'); + }); + + it('returns original exit code when shard is not set', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + // Reset static shard property to ensure it's null + $reflection = new ReflectionClass($shard); + $property = $reflection->getProperty('shard'); + $property->setValue(null, null); + + $exitCode = $shard->addOutput(1); + $outputText = $output->fetch(); + + expect($exitCode)->toBe(1) + ->and($outputText)->toBe(''); + }); +});