mirror of
https://github.com/pestphp/pest.git
synced 2026-06-15 07:28:23 +02:00
[5.x] Make time-based sharding namespace-agnostic and forward --test-directory (#1677)
* refactor(shard): extract parseListTestsOutput for testability * test(shard): characterize parseListTestsOutput current behavior * fix(shard): parse any PHP FQCN namespace from --list-tests * fix(shard): forward --test-directory to list-tests subprocess * chore: lint + snapshot fixups Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(shard): make extracted helpers private, test via reflection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert visual_snapshot_of_help_command_output * revert visual_snapshot_of_help_command_output * keep function removeParallelArguments * strip --processes argument when building list-tests command The removeParallelArguments method was not filtering --processes flags, causing the list-tests subprocess to fail when parallel execution was enabled. This prevented time-based sharding from working correctly with the --parallel option. Now both --parallel/-p and --processes arguments are removed from the command used to enumerate tests, ensuring the subprocess runs successfully. * test: re-add namespace-agnostic sharding tests - 5.x merge kept describe()-style test file, dropped PR #1677 tests for parseListTestsOutput + buildListTestsCommand. Re-add them in matching style. - Also fix removeParallelArguments test broken by merge: source array_values() + strips --processes, so expects ['bin/pest','tests/']. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@ -199,15 +199,14 @@ final class Shard implements AddsOutput, HandlesArguments, Terminable
|
|||||||
*/
|
*/
|
||||||
private function allTests(array $arguments): array
|
private function allTests(array $arguments): array
|
||||||
{
|
{
|
||||||
$output = new Process([
|
$command = $this->buildListTestsCommand(
|
||||||
'php',
|
$arguments,
|
||||||
...$this->removeParallelArguments($arguments),
|
TestSuite::getInstance()->testPath,
|
||||||
'--list-tests',
|
);
|
||||||
])->setTimeout(120)->mustRun()->getOutput();
|
|
||||||
|
|
||||||
preg_match_all('/ - (?:P\\\\)?(Tests\\\\[^:]+)::/', $output, $matches);
|
$output = (new Process($command))->setTimeout(120)->mustRun()->getOutput();
|
||||||
|
|
||||||
return array_values(array_unique($matches[1]));
|
return $this->parseListTestsOutput($output);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -216,7 +215,36 @@ final class Shard implements AddsOutput, HandlesArguments, Terminable
|
|||||||
*/
|
*/
|
||||||
private function removeParallelArguments(array $arguments): array
|
private function removeParallelArguments(array $arguments): array
|
||||||
{
|
{
|
||||||
return array_filter($arguments, fn (string $argument): bool => ! in_array($argument, ['--parallel', '-p'], strict: true));
|
return array_values(array_filter(
|
||||||
|
$arguments,
|
||||||
|
fn (string $argument): bool => ! in_array($argument, ['--parallel', '-p'], strict: true)
|
||||||
|
&& ! str_starts_with($argument, '--processes'),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Builds the subprocess command used to enumerate tests via `--list-tests`.
|
||||||
|
*
|
||||||
|
* @param list<string> $arguments
|
||||||
|
* @return list<string>
|
||||||
|
*/
|
||||||
|
private function buildListTestsCommand(array $arguments, string $testPath): array
|
||||||
|
{
|
||||||
|
$filtered = $this->removeParallelArguments($arguments);
|
||||||
|
|
||||||
|
return ['php', ...$filtered, '--test-directory='.$testPath, '--list-tests'];
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Parses `--list-tests` output into a unique list of test class FQCNs.
|
||||||
|
*
|
||||||
|
* @return list<string>
|
||||||
|
*/
|
||||||
|
private function parseListTestsOutput(string $output): array
|
||||||
|
{
|
||||||
|
preg_match_all('/ - (?:P\\\\)?([A-Za-z_]\w*(?:\\\\[A-Za-z_]\w*)*)::/', $output, $matches);
|
||||||
|
|
||||||
|
return array_values(array_unique($matches[1]));
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@ -232,11 +232,209 @@ describe('handleArguments', function () {
|
|||||||
$reflection = new ReflectionClass($shard);
|
$reflection = new ReflectionClass($shard);
|
||||||
$method = $reflection->getMethod('removeParallelArguments');
|
$method = $reflection->getMethod('removeParallelArguments');
|
||||||
|
|
||||||
$arguments = ['bin/pest', '--parallel', 'tests/', '-p'];
|
$arguments = ['bin/pest', '--parallel', '--processes=4', 'tests/', '-p'];
|
||||||
|
|
||||||
$result = $method->invoke($shard, $arguments);
|
$result = $method->invoke($shard, $arguments);
|
||||||
|
|
||||||
expect($result)->toBe([0 => 'bin/pest', 2 => 'tests/']);
|
expect($result)->toBe(['bin/pest', 'tests/']);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('parseListTestsOutput', function () {
|
||||||
|
it('parses Tests\\ namespaced classes from --list-tests output', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('parseListTestsOutput');
|
||||||
|
|
||||||
|
$listOutput = <<<'OUT'
|
||||||
|
INFO Available tests:
|
||||||
|
|
||||||
|
- P\Tests\Features\After::__pest_evaluable_it_runs
|
||||||
|
- P\Tests\Features\After::__pest_evaluable_it_runs_twice
|
||||||
|
- P\Tests\Unit\Foo::test_bar
|
||||||
|
OUT;
|
||||||
|
|
||||||
|
expect($method->invoke($shard, $listOutput))->toBe([
|
||||||
|
'Tests\\Features\\After',
|
||||||
|
'Tests\\Unit\\Foo',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('deduplicates repeated class names from multiple test methods', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('parseListTestsOutput');
|
||||||
|
|
||||||
|
$listOutput = <<<'OUT'
|
||||||
|
- P\Tests\Same::method_a
|
||||||
|
- P\Tests\Same::method_b
|
||||||
|
- P\Tests\Same::method_c
|
||||||
|
OUT;
|
||||||
|
|
||||||
|
expect($method->invoke($shard, $listOutput))->toBe(['Tests\\Same']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns an empty list for output with no matching lines', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('parseListTestsOutput');
|
||||||
|
|
||||||
|
expect($method->invoke($shard, ''))->toBe([])
|
||||||
|
->and($method->invoke($shard, 'some random text'))->toBe([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('parses non-Tests namespaced classes', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('parseListTestsOutput');
|
||||||
|
|
||||||
|
$listOutput = <<<'OUT'
|
||||||
|
- P\Acme\Sharding\OneTest::test_foo
|
||||||
|
- P\Acme\Sharding\TwoTest::test_bar
|
||||||
|
- App\Suite\BazTest::test_qux
|
||||||
|
OUT;
|
||||||
|
|
||||||
|
expect($method->invoke($shard, $listOutput))->toBe([
|
||||||
|
'Acme\\Sharding\\OneTest',
|
||||||
|
'Acme\\Sharding\\TwoTest',
|
||||||
|
'App\\Suite\\BazTest',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('parses unnamespaced top-level classes', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('parseListTestsOutput');
|
||||||
|
|
||||||
|
expect($method->invoke($shard, ' - P\FooTest::test_bar'))->toBe(['FooTest']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('strips the P\\ Pest prefix but keeps the rest of the FQCN', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('parseListTestsOutput');
|
||||||
|
|
||||||
|
$listOutput = <<<'OUT'
|
||||||
|
- P\Acme\OneTest::a
|
||||||
|
- Acme\TwoTest::b
|
||||||
|
OUT;
|
||||||
|
|
||||||
|
expect($method->invoke($shard, $listOutput))->toBe([
|
||||||
|
'Acme\\OneTest',
|
||||||
|
'Acme\\TwoTest',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ignores junk lines that lack the " - …::" framing', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('parseListTestsOutput');
|
||||||
|
|
||||||
|
$listOutput = <<<'OUT'
|
||||||
|
INFO Available tests:
|
||||||
|
|
||||||
|
There were errors:
|
||||||
|
garbage ::: not a test
|
||||||
|
- P\Acme\RealTest::method
|
||||||
|
OUT;
|
||||||
|
|
||||||
|
expect($method->invoke($shard, $listOutput))->toBe(['Acme\\RealTest']);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('buildListTestsCommand', function () {
|
||||||
|
it('builds the list-tests command with the forwarded --test-directory', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('buildListTestsCommand');
|
||||||
|
|
||||||
|
$command = $method->invoke($shard, ['bin/pest', '--update-shards'], 'custom/suite');
|
||||||
|
|
||||||
|
expect($command)->toBe([
|
||||||
|
'php',
|
||||||
|
'bin/pest',
|
||||||
|
'--update-shards',
|
||||||
|
'--test-directory=custom/suite',
|
||||||
|
'--list-tests',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('strips --parallel and -p when building the list-tests command', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('buildListTestsCommand');
|
||||||
|
|
||||||
|
$command = $method->invoke($shard, ['bin/pest', '--parallel', '--update-shards', '-p'], 'tests');
|
||||||
|
|
||||||
|
expect($command)->toBe([
|
||||||
|
'php',
|
||||||
|
'bin/pest',
|
||||||
|
'--update-shards',
|
||||||
|
'--test-directory=tests',
|
||||||
|
'--list-tests',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('forwards --test-directory even when input arguments include one', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('buildListTestsCommand');
|
||||||
|
|
||||||
|
$command = $method->invoke($shard, ['bin/pest'], 'suites');
|
||||||
|
|
||||||
|
expect($command)->toContain('--test-directory=suites');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('strips --processes=N when building the list-tests command', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('buildListTestsCommand');
|
||||||
|
|
||||||
|
$command = $method->invoke($shard, ['bin/pest', '--parallel', '--processes=4', '--update-shards'], 'tests');
|
||||||
|
|
||||||
|
expect($command)->toBe([
|
||||||
|
'php',
|
||||||
|
'bin/pest',
|
||||||
|
'--update-shards',
|
||||||
|
'--test-directory=tests',
|
||||||
|
'--list-tests',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('strips --processes N (space-separated) when building the list-tests command', function () {
|
||||||
|
$output = new BufferedOutput;
|
||||||
|
$shard = new Shard($output);
|
||||||
|
|
||||||
|
$reflection = new ReflectionClass($shard);
|
||||||
|
$method = $reflection->getMethod('buildListTestsCommand');
|
||||||
|
|
||||||
|
$command = $method->invoke($shard, ['bin/pest', '--parallel', '--processes', '4', '--update-shards'], 'tests');
|
||||||
|
|
||||||
|
expect($command)->not->toContain('--processes')
|
||||||
|
->and($command)->toContain('--update-shards')
|
||||||
|
->and($command)->toContain('--test-directory=tests');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user