From dd94477804b80d3139dc5428aa0f20f0af5f4aad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20J=C3=A4ger?= Date: Thu, 16 Jul 2026 20:10:32 +0200 Subject: [PATCH] fix: parallel runs with group filters no longer exit 1 despite green tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In parallel mode each wrapper worker's serialized TestResult only carries the `numberOfTests` of the *last* file it executed — PHPUnit's Collector overwrites the counter on every `executionStarted` event. When a worker's final file has all of its tests excluded by `--group`/`--exclude-group`, that worker reports `hasTests() === false` even though it ran plenty of tests. If every worker happens to end on such a fully-filtered file, the merged TestResult claims the whole suite was empty. Because PHPUnit 12 enables `failOnEmptyTestSuite` implicitly whenever an explicit test selection is present (TextUI\Configuration\Merger), the run then exits 1 — with a fully green summary and no error output whatsoever. Which worker ends on which file is a timing lottery, so this manifests as a flaky, silent CI failure (likely the root cause of #1483). Fix: while merging worker results, treat a worker that actually ran tests (`numberOfTestsRun() > 0`) as having tests. Genuinely empty selections (no worker ran anything) still exit non-zero. Deterministic reproduction (before this fix): php bin/pest --parallel --processes=1 --group=filtered-group \ tests/.tests/ParallelGroupFilteredLastFile # Tests: 1 passed (1 assertions) — exit code 1 With `--processes=1` the single worker deterministically ends on ZTest.php, whose only test is outside the filtered group. --- .../Parallel/Paratest/WrapperRunner.php | 12 ++++++- .../ParallelGroupFilteredLastFile/ATest.php | 5 +++ .../ParallelGroupFilteredLastFile/ZTest.php | 5 +++ tests/Visual/Parallel.php | 35 +++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 tests/.tests/ParallelGroupFilteredLastFile/ATest.php create mode 100644 tests/.tests/ParallelGroupFilteredLastFile/ZTest.php diff --git a/src/Plugins/Parallel/Paratest/WrapperRunner.php b/src/Plugins/Parallel/Paratest/WrapperRunner.php index 19ce3d1a4..b7c356678 100644 --- a/src/Plugins/Parallel/Paratest/WrapperRunner.php +++ b/src/Plugins/Parallel/Paratest/WrapperRunner.php @@ -323,8 +323,18 @@ private function complete(TestResult $testResultSum): int /** @var list $failedEvents */ $failedEvents = array_merge_recursive($testResultSum->testFailedEvents(), $testResult->testFailedEvents()); + // A worker's `hasTests()` only reflects the *last* file it executed: + // PHPUnit's Collector overwrites `numberOfTests` on every + // `executionStarted` event. When a worker's final file has all of + // its tests excluded by `--group`/`--exclude-group`, that worker + // reports `hasTests() === false` even though it ran plenty of + // tests. If every worker happens to end on such a file, the merged + // result claims the whole suite was empty and the run exits 1 + // (PHPUnit enables `failOnEmptyTestSuite` implicitly for explicit + // test selections) — despite a fully green summary. Treating a + // worker with executed tests as "has tests" restores the truth. $testResultSum = new TestResult( - (int) $testResultSum->hasTests() + (int) $testResult->hasTests(), + (int) ($testResultSum->hasTests() || $testResult->hasTests() || $testResult->numberOfTestsRun() > 0), $testResultSum->numberOfTestsRun() + $testResult->numberOfTestsRun(), $testResultSum->numberOfAssertions() + $testResult->numberOfAssertions(), array_merge_recursive($testResultSum->testErroredEvents(), $testResult->testErroredEvents()), diff --git a/tests/.tests/ParallelGroupFilteredLastFile/ATest.php b/tests/.tests/ParallelGroupFilteredLastFile/ATest.php new file mode 100644 index 000000000..38ece0d06 --- /dev/null +++ b/tests/.tests/ParallelGroupFilteredLastFile/ATest.php @@ -0,0 +1,5 @@ +toBeTrue(); +})->group('filtered-group'); diff --git a/tests/.tests/ParallelGroupFilteredLastFile/ZTest.php b/tests/.tests/ParallelGroupFilteredLastFile/ZTest.php new file mode 100644 index 000000000..259551f9d --- /dev/null +++ b/tests/.tests/ParallelGroupFilteredLastFile/ZTest.php @@ -0,0 +1,5 @@ +toBeTrue(); +}); diff --git a/tests/Visual/Parallel.php b/tests/Visual/Parallel.php index d497eef3c..38cfb69aa 100644 --- a/tests/Visual/Parallel.php +++ b/tests/Visual/Parallel.php @@ -47,3 +47,38 @@ ->toContain('Tests: 1 failed, 1 passed (1 assertions)') ->toContain('Parallel: 3 processes'); })->skipOnWindows(); + +test('parallel group-filtered run is not considered empty when workers end on fully filtered files', function () { + // Regression: a worker's TestResult only carries `numberOfTests` of its + // LAST executed file (PHPUnit's Collector overwrites it per execution). + // With `--processes=1` the single worker deterministically ends on + // ZTest.php, whose only test is excluded by the group filter — before the + // fix the merged result claimed "no tests" and exited 1 despite the green + // summary (PHPUnit enables failOnEmptyTestSuite implicitly for --group). + $process = new Process( + ['php', 'bin/pest', '--parallel', '--processes=1', '--group=filtered-group', 'tests/.tests/ParallelGroupFilteredLastFile'], + dirname(__DIR__, 2), + ['COLLISION_PRINTER' => 'DefaultPrinter', 'COLLISION_IGNORE_DURATION' => 'true'], + ); + + $process->run(); + + expect(removeAnsiEscapeSequences($process->getOutput())) + ->toContain('Tests: 1 passed (1 assertions)'); + + expect($process->getExitCode())->toBe(0); +})->skipOnWindows(); + +test('parallel group-filtered run still fails when the selection matches no tests at all', function () { + // Companion guard: a genuinely empty selection must keep exiting non-zero + // (failOnEmptyTestSuite) — the fix must not mask real "0 tests" runs. + $process = new Process( + ['php', 'bin/pest', '--parallel', '--processes=3', '--group=group-that-does-not-exist', 'tests/.tests/ParallelGroupFilteredLastFile'], + dirname(__DIR__, 2), + ['COLLISION_PRINTER' => 'DefaultPrinter', 'COLLISION_IGNORE_DURATION' => 'true'], + ); + + $process->run(); + + expect($process->getExitCode())->not->toBe(0); +})->skipOnWindows();