-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Detect premature end of PHPUnit's main PHP process #6319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Detect premature end of PHPUnit's main PHP process #6319
Conversation
src/Framework/TestCase.php
Outdated
@@ -352,7 +352,12 @@ final public function run(): void | |||
} | |||
|
|||
if (!$this->shouldRunInSeparateProcess() || $this->requirementsNotSatisfied()) { | |||
(new TestRunner)->run($this); | |||
try { | |||
ShutdownHandler::setMessage('Fatal error: Premature end of PHP process.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure whether the error is precise enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added the test-name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm maybe even PHPUnit process
instead of PHP process
?
the test itself might not call exit() directly but it might be called from code down the line
@@ -39,6 +39,7 @@ private static function register(): void | |||
return; | |||
} | |||
|
|||
self::$registered = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixes failling end-to-end test:
https://github.com/sebastianbergmann/phpunit/actions/runs/16960222681/job/48071048701?pr=6319
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PHPStan should have detected this problem: phpstan/phpstan#13384
$this->assertTrue(true); | ||
$this->assertTrue(true); | ||
|
||
exit(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just realized there is a even more critical case, namely a test calling exit(0)
, from non-isolated tests.
this would make PHPUnit return a 0 exit-code and noone would notice (besides the newly added error message), that tests did not run successfully.
stuff for a separate issue though. maybe even stuff for phpstan-phpunit as I can't think of how we can make phpunit return a non-zero exit code, when a test itself calls already exit(0)
.
when a test calls
exit()
phpunit main process halted and printed nothing.since PHPUnit 12.3 we have a
ShutdownHandler
which we utilize with this PR to handle this situation and print a more usefull error message.before this PR:
after this PR:
similar to #6234
-> in a separate PR I will add expectation handling for subprocesses.