pestphp / pestphp/pest

Exit code 1 on a fully green `--parallel` run combined with `--filter`/`--group`

Open
#1,774 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
PHP
Stars
11.7k
Forks
538
Avg merge
4d 11h
Merged PRs (30d)
8

Description

What happened

--parallel combined with --filter (or --group) can exit 1 while every test that ran passed and nothing failed, errored, was skipped or risky.

Minimal deterministic reproduction, two test files and one process:

tests/AaaTest.php  ->  it('alpha matches the filter', fn () => expect(1)->toBe(1));
tests/ZzzTest.php  ->  it('zulu does not match the filter', fn () => expect(2)->toBe(2));
$ php vendor/bin/pest --parallel --processes=1 --filter=alpha; echo "exit=$?"

  .

  Tests:    1 passed (1 assertions)
  Duration: 0.16s

  Parallel: 1 process

exit=1

$ php vendor/bin/pest --filter=alpha; echo "exit=$?"                # serial
  Tests:    1 passed (1 assertions)
exit=0

$ php vendor/bin/pest --parallel --processes=1; echo "exit=$?"      # no filter
exit=0

Only --parallel and a selective flag together reproduce it. Note the exit code must be read bare — piping the run through tail/grep reports the pipe's status and hides this entirely, which is presumably part of why it goes unnoticed.

Why it happens

Two things combine. The second one is the actual defect.

1. A worker's serialized numberOfTests reflects only its last assigned file.
ApplicationForWrapperWorker::bootstrap() calls TestResultFacade::init() once per worker, and PHPUnit's Runner\TestResult\Collector assigns rather than accumulates:

// vendor/phpunit/phpunit/src/Runner/TestResult/Collector.php:241
$this->numberOfTests = $event->testSuite()->count();

numberOfTestsRun accumulates (+=), numberOfTests does not. So a worker whose last assigned file matches zero tests under the filter serializes a TestResult with numberOfTests === 0, hence hasTests() === false, even though it ran and passed tests from earlier files. In the reproduction above the single worker takes AaaTest.php (1 match) then ZzzTest.php (0 matches), and ends on 0.

2. WrapperRunner::complete() sums booleans into the test-count slot.
src/Plugins/Parallel/Paratest/WrapperRunner.php, in the per-worker merge loop:

$testResultSum = new TestResult(
    (int) $testResultSum->hasTests() + (int) $testResult->hasTests(),   // <- $numberOfTests
    $testResultSum->numberOfTestsRun() + $testResult->numberOfTestsRun(),
    ...

TestResult::__construct()'s first parameter is int $numberOfTests. The expression being passed is a count of workers that happened to report hasTests(), not a count of tests. Run 500 tests across 8 workers and the merged numberOfTests is at most 8; every worker reporting hasTests() === false makes it exactly 0.

hasTests() is numberOfTests > 0, and ShellExitCodeCalculator then does:

if ($failOnEmptyTestSuite && !$result->hasTests()) {
    $returnCode = self::FAILURE_EXIT;
}

so the run is graded as an empty suite despite numberOfTestsRun() > 0.

Whether a real project trips this is pure file-distribution luck: it depends on which file each worker happens to process last. Two of our repositories run the same selective command; one is red and one is green.

Suggested fix

The boolean sum looks like it was meant as an "any worker had tests" flag but landed in the count slot. Either accumulate the real count:

$testResultSum->numberOfTests() + $testResult->numberOfTests(),

or, given defect 1 makes the per-worker numberOfTests unreliable under filters, derive it from what actually ran:

$testResultSum->numberOfTestsRun() + $testResult->numberOfTestsRun(),

The second is robust to the collector's assign-not-accumulate behaviour. Happy to open a PR for whichever you prefer.

Versions
  • pestphp/pest v4.7.5
  • brianium/paratest v7.20.0
  • PHP 8.4, macOS

Workaround in the meantime: force selective runs serial, i.e. drop --parallel whenever --filter/--group is present.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/Plugins/Parallel/Paratest/WrapperRunner.php, especially complete(), and inspect the TestResult values merged from workers. Reproduce the issue with --parallel, --processes=1, and --filter=alpha using the two-file example, then run the relevant parallel test suite. Done means a fully passing selective parallel run exits 0 while empty suites still receive the correct failure status.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.