From 9709c0239fcd73d0b498220494a58b18dfe21d39 Mon Sep 17 00:00:00 2001 From: adamjsturge Date: Mon, 25 Aug 2025 14:40:12 -0700 Subject: [PATCH 1/2] Fix Processes on Sharding --- src/Plugins/Shard.php | 28 ++++++++++++- tests/Unit/Plugins/Shard.php | 76 ++++++++++++++++++++++++++++++++++++ tests/Visual/Shard.php | 48 +++++++++++++++++++++++ 3 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/Plugins/Shard.php create mode 100644 tests/Visual/Shard.php diff --git a/src/Plugins/Shard.php b/src/Plugins/Shard.php index f48260bb5..3945ed28d 100644 --- a/src/Plugins/Shard.php +++ b/src/Plugins/Shard.php @@ -100,7 +100,33 @@ private function allTests(array $arguments): array */ private function removeParallelArguments(array $arguments): array { - return array_filter($arguments, fn (string $argument): bool => ! in_array($argument, ['--parallel', '-p'], strict: true)); + $filtered = []; + $skipNext = false; + + foreach ($arguments as $argument) { + if ($skipNext) { + $skipNext = false; + + continue; + } + + if (in_array($argument, ['--parallel', '-p'], strict: true)) { + continue; + } + + if (str_starts_with($argument, '--processes')) { + if (str_contains($argument, '=')) { + continue; + } + $skipNext = true; + + continue; + } + + $filtered[] = $argument; + } + + return $filtered; } /** diff --git a/tests/Unit/Plugins/Shard.php b/tests/Unit/Plugins/Shard.php new file mode 100644 index 000000000..ca88d4403 --- /dev/null +++ b/tests/Unit/Plugins/Shard.php @@ -0,0 +1,76 @@ +getMethod('removeParallelArguments'); + $method->setAccessible(true); + + $arguments = [ + 'php', + 'bin/pest', + '--processes=12', + '--parallel', + '--shard=1/4', + '--verbose', + ]; + + $result = $method->invoke($shard, $arguments); + + expect($result)->toBe(['php', 'bin/pest', '--shard=1/4', '--verbose']) + ->and($result)->not->toContain('--processes=12') + ->and($result)->not->toContain('--parallel'); +}); + +it('removes space-separated processes arguments', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('removeParallelArguments'); + $method->setAccessible(true); + + $arguments = [ + 'php', + 'bin/pest', + '--processes', + '8', + '--parallel', + '--other-flag', + ]; + + $result = $method->invoke($shard, $arguments); + + expect($result)->toBe(['php', 'bin/pest', '--other-flag']) + ->and($result)->not->toContain('--processes') + ->and($result)->not->toContain('8') + ->and($result)->not->toContain('--parallel'); +}); + +it('preserves non-parallel arguments when filtering', function () { + $output = new BufferedOutput; + $shard = new Shard($output); + + $reflection = new ReflectionClass($shard); + $method = $reflection->getMethod('removeParallelArguments'); + $method->setAccessible(true); + + $arguments = [ + 'php', + 'bin/pest', + '--filter', + 'UserTest', + '--verbose', + '--stop-on-failure', + ]; + + $result = $method->invoke($shard, $arguments); + + expect($result)->toBe($arguments); // Should be unchanged +}); diff --git a/tests/Visual/Shard.php b/tests/Visual/Shard.php new file mode 100644 index 000000000..6d01266d3 --- /dev/null +++ b/tests/Visual/Shard.php @@ -0,0 +1,48 @@ + 'DefaultPrinter', + 'COLLISION_IGNORE_DURATION' => 'true', + ]); + + $process->run(); + + expect($process->getExitCode())->toBe(0) + ->and($process->getOutput()) + ->not->toContain('Unknown option "--processes"') + ->toContain('Tests:') + ->toContain('Shard:'); +})->skipOnWindows(); + +test('shard removes processes from list-tests subprocess call', function () { + // This test verifies that --processes doesn't get passed to the --list-tests call + $process = new Process([ + 'php', + 'bin/pest', + '--processes=2', + '--shard=1/1', + '--parallel', + 'tests/Fixtures/ExampleTest.php', + ], dirname(__DIR__, 2), [ + 'COLLISION_PRINTER' => 'DefaultPrinter', + 'COLLISION_IGNORE_DURATION' => 'true', + ]); + + $process->run(); + + expect($process->getExitCode())->toBe(0) + ->and($process->getOutput()) + ->not->toContain('Unknown option "--processes"') + ->not->toContain('The "--processes" option requires a value') + ->toContain('Shard:'); // Just check that shard output exists, don't be too specific +})->skipOnWindows(); From 9afb4816a355b2efbc1c9afa58aa861e002aa5a7 Mon Sep 17 00:00:00 2001 From: adamjsturge Date: Tue, 9 Sep 2025 10:28:28 -0700 Subject: [PATCH 2/2] Cleaner Function Names for Shard Argument Stripping --- src/Plugins/Shard.php | 50 +++++++++++++++++++++++------------- tests/Unit/Plugins/Shard.php | 8 +++--- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/Plugins/Shard.php b/src/Plugins/Shard.php index 3945ed28d..80e6740d2 100644 --- a/src/Plugins/Shard.php +++ b/src/Plugins/Shard.php @@ -85,7 +85,7 @@ private function allTests(array $arguments): array { $output = (new Process([ 'php', - ...$this->removeParallelArguments($arguments), + ...$this->removeParallelizationArguments($arguments), '--list-tests', ]))->mustRun()->getOutput(); @@ -94,39 +94,53 @@ private function allTests(array $arguments): array return array_values(array_unique($matches[1])); } + /** + * Removes both parallel and processes arguments from the arguments array. + * This is useful when running commands that don't support parallel execution. + * + * @param array $arguments + * @return array + */ + private function removeParallelizationArguments(array $arguments): array + { + return $this->removeProcessesArguments($this->removeParallelArguments($arguments)); + } + /** * @param array $arguments * @return array */ private function removeParallelArguments(array $arguments): array { - $filtered = []; - $skipNext = false; + return array_filter($arguments, fn (string $argument): bool => ! in_array($argument, ['--parallel', '-p'], strict: true)); + } + + /** + * @param array $arguments + * @return array + */ + private function removeProcessesArguments(array $arguments): array + { + return array_values(array_filter($arguments, function (string $argument) { + if (str_starts_with($argument, '--processes') && str_contains($argument, '=')) { + return false; + } - foreach ($arguments as $argument) { + static $skipNext = false; if ($skipNext) { $skipNext = false; - continue; - } - - if (in_array($argument, ['--parallel', '-p'], strict: true)) { - continue; + return false; } - if (str_starts_with($argument, '--processes')) { - if (str_contains($argument, '=')) { - continue; - } + if ($argument === '--processes') { $skipNext = true; - continue; + return false; } - $filtered[] = $argument; - } - - return $filtered; + return true; + })); } /** diff --git a/tests/Unit/Plugins/Shard.php b/tests/Unit/Plugins/Shard.php index ca88d4403..911658f34 100644 --- a/tests/Unit/Plugins/Shard.php +++ b/tests/Unit/Plugins/Shard.php @@ -7,9 +7,9 @@ $output = new BufferedOutput; $shard = new Shard($output); - // Use reflection to test the private removeParallelArguments method + // Use reflection to test the private removeParallelizationArguments method $reflection = new ReflectionClass($shard); - $method = $reflection->getMethod('removeParallelArguments'); + $method = $reflection->getMethod('removeParallelizationArguments'); $method->setAccessible(true); $arguments = [ @@ -33,7 +33,7 @@ $shard = new Shard($output); $reflection = new ReflectionClass($shard); - $method = $reflection->getMethod('removeParallelArguments'); + $method = $reflection->getMethod('removeParallelizationArguments'); $method->setAccessible(true); $arguments = [ @@ -58,7 +58,7 @@ $shard = new Shard($output); $reflection = new ReflectionClass($shard); - $method = $reflection->getMethod('removeParallelArguments'); + $method = $reflection->getMethod('removeParallelizationArguments'); $method->setAccessible(true); $arguments = [