Skip to content

Conversation

@gpn273
Copy link
Contributor

@gpn273 gpn273 commented Jan 6, 2026

Problem

When a child process crashes unexpectedly (e.g., due to out-of-memory errors) after executing tests but before writing result/coverage files, Paratest would silently ignore the missing files and exit with code 0. This causes false-positive CI pipeline passes despite test failures.

Scenario

  1. Test executes and reports error (E) in output
  2. Status file is written (worker marked as "free")
  3. Process crashes due to OOM before application->end() completes
  4. Result/coverage files are never written
  5. Paratest exits successfully (exit code 0)

Solution

Added validation to ensure all workers that executed tests have written their result and coverage files. If files are missing, throw MissingResultsException with a clear error message.

Changes

  • Added WrapperWorker::hasExecutedTests() to track which workers ran tests
  • Added WrapperRunner::$requiredTestResultFiles and $requiredCoverageFiles to track file paths that must exist
  • Validate test result files only for workers that executed tests (not idle workers)
  • Validate coverage files only for workers that executed tests and have coverage enabled
  • Throw MissingResultsException with actionable error message when files are missing

Testing

  • New MissingResultsException unit tests verify exception behaviour
  • Validated fixes in a code base which replicates the issue which this pull request solves

Fixes

Addresses the issue where Paratest would pass CI despite worker crashes when using --passthru-php='-dmemory_limit=1G' or similar memory constraints.

Screenshot

Screenshot demonstrating the exception raising when test result files are missing for a given worker:

image

@gpn273 gpn273 marked this pull request as draft January 6, 2026 19:47
@gpn273 gpn273 marked this pull request as ready for review January 6, 2026 20:14
@Slamdunk
Copy link
Member

Slamdunk commented Jan 7, 2026

This is a welcome contribution, thank you for the explanation and the fix.

I'm wondering: can we also add a test that reproduce the issue?

An idea could be to create a test that registers a shutdown function to delete the result file and the coverage file, they can be easeily gathered within the test by copy-pasting the getopt line of bin/phpunit-wrapper.php: would you be so kind to give it a try please?

@Jean85
Copy link
Collaborator

Jean85 commented Jan 7, 2026

An idea could be to create a test that registers a shutdown function to delete the result file and the coverage file, they can be easeily gathered within the test by copy-pasting the getopt line of bin/phpunit-wrapper.php: would you be so kind to give it a try please?

Stealing from my own ideas, you could also:

@gpn273
Copy link
Contributor Author

gpn273 commented Jan 7, 2026

Thank you both (@Slamdunk & @Jean85) for your feedback.

I've added test cases that reproduce the issue using the approach you suggested.

The test fixtures register shutdown functions that find file paths from $_SERVER['argv'] (for test result files) and unserialize --phpunit-argv (for coverage files), wait for the files to be written, then delete them to simulate an incomplete application->end() call. I've added three test cases covering: missing test result files, missing both files, and missing only coverage files.

All 128 tests now pass, and the tests verify that MissingResultsException is thrown with appropriate error messages for each scenario.

Copy link
Member

@Slamdunk Slamdunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this very well written PR 💪

@Slamdunk Slamdunk added the bug label Jan 8, 2026
@Slamdunk Slamdunk merged commit f0fdfd8 into paratestphp:7.x Jan 8, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants