diff --git a/.psalm/baseline.xml b/.psalm/baseline.xml index 2d54a60dc36..e2ecadbdfcf 100644 --- a/.psalm/baseline.xml +++ b/.psalm/baseline.xml @@ -510,6 +510,10 @@ getMethod + + backupGlobalErrorHandlers]]> + backupGlobalExceptionHandlers]]> + $outputBufferingLevel diff --git a/src/Framework/TestCase.php b/src/Framework/TestCase.php index 51333014894..8a0433e79e9 100644 --- a/src/Framework/TestCase.php +++ b/src/Framework/TestCase.php @@ -20,6 +20,7 @@ use const PHP_URL_PATH; use function array_keys; use function array_merge; +use function array_reverse; use function array_values; use function assert; use function basename; @@ -28,6 +29,7 @@ use function clearstatcache; use function count; use function defined; +use function error_clear_last; use function explode; use function getcwd; use function implode; @@ -48,6 +50,10 @@ use function parse_url; use function pathinfo; use function preg_replace; +use function restore_error_handler; +use function restore_exception_handler; +use function set_error_handler; +use function set_exception_handler; use function setlocale; use function sprintf; use function str_contains; @@ -126,14 +132,24 @@ abstract class TestCase extends Assert implements Reorderable, SelfDescribing, T */ private array $backupStaticPropertiesExcludeList = []; private ?Snapshot $snapshot = null; - private ?bool $runClassInSeparateProcess = null; - private ?bool $runTestInSeparateProcess = null; - private bool $preserveGlobalState = false; - private bool $inIsolation = false; - private ?string $expectedException = null; - private ?string $expectedExceptionMessage = null; - private ?string $expectedExceptionMessageRegExp = null; - private null|int|string $expectedExceptionCode = null; + + /** + * @psalm-var list + */ + private ?array $backupGlobalErrorHandlers = null; + + /** + * @psalm-var list + */ + private ?array $backupGlobalExceptionHandlers = null; + private ?bool $runClassInSeparateProcess = null; + private ?bool $runTestInSeparateProcess = null; + private bool $preserveGlobalState = false; + private bool $inIsolation = false; + private ?string $expectedException = null; + private ?string $expectedExceptionMessage = null; + private ?string $expectedExceptionMessageRegExp = null; + private null|int|string $expectedExceptionCode = null; /** * @psalm-var list @@ -618,13 +634,16 @@ final public function runBare(): void { $emitter = Event\Facade::emitter(); + error_clear_last(); + clearstatcache(); + $emitter->testPreparationStarted( $this->valueObjectForEvents(), ); $this->snapshotGlobalState(); + $this->snapshotGlobalErrorExceptionHandlers(); $this->startOutputBuffering(); - clearstatcache(); $hookMethods = (new HookMethods)->hookMethods(static::class); $hasMetRequirements = false; @@ -776,6 +795,7 @@ final public function runBare(): void chdir($currentWorkingDirectory); } + $this->restoreGlobalErrorExceptionHandlers(); $this->restoreGlobalState(); $this->unregisterCustomComparators(); $this->cleanupIniSettings(); @@ -1683,6 +1703,119 @@ private function stopOutputBuffering(): bool return true; } + private function snapshotGlobalErrorExceptionHandlers(): void + { + $this->backupGlobalErrorHandlers = $this->getActiveErrorHandlers(); + $this->backupGlobalExceptionHandlers = $this->getActiveExceptionHandlers(); + } + + /** + * @throws MoreThanOneDataSetFromDataProviderException + */ + private function restoreGlobalErrorExceptionHandlers(): void + { + $activeErrorHandlers = $this->getActiveErrorHandlers(); + $activeExceptionHandlers = $this->getActiveExceptionHandlers(); + + $message = null; + + if ($activeErrorHandlers !== $this->backupGlobalErrorHandlers) { + if (count($activeErrorHandlers) > count($this->backupGlobalErrorHandlers)) { + $message = 'Test code or tested code did not remove its own error handlers'; + } else { + $message = 'Test code or tested code removed error handlers other than its own'; + } + + foreach ($activeErrorHandlers as $handler) { + restore_error_handler(); + } + + foreach ($this->backupGlobalErrorHandlers as $handler) { + set_error_handler($handler); + } + } + + if ($activeExceptionHandlers !== $this->backupGlobalExceptionHandlers) { + if (count($activeExceptionHandlers) > count($this->backupGlobalExceptionHandlers)) { + $message = 'Test code or tested code did not remove its own exception handlers'; + } else { + $message = 'Test code or tested code removed exception handlers other than its own'; + } + + foreach ($activeExceptionHandlers as $handler) { + restore_exception_handler(); + } + + foreach ($this->backupGlobalExceptionHandlers as $handler) { + set_exception_handler($handler); + } + } + + $this->backupGlobalErrorHandlers = null; + $this->backupGlobalExceptionHandlers = null; + + if ($message !== null) { + Event\Facade::emitter()->testConsideredRisky( + $this->valueObjectForEvents(), + $message, + ); + + $this->status = TestStatus::risky($message); + } + } + + /** + * @return list + */ + private function getActiveErrorHandlers(): array + { + $res = []; + + while (true) { + $previousHandler = set_error_handler(static fn () => false); + restore_error_handler(); + + if ($previousHandler === null) { + break; + } + $res[] = $previousHandler; + restore_error_handler(); + } + $res = array_reverse($res); + + foreach ($res as $handler) { + set_error_handler($handler); + } + + return $res; + } + + /** + * @return list + */ + private function getActiveExceptionHandlers(): array + { + $res = []; + + while (true) { + $previousHandler = set_exception_handler(static fn () => null); + restore_exception_handler(); + + if ($previousHandler === null) { + break; + } + $res[] = $previousHandler; + restore_exception_handler(); + } + $res = array_reverse($res); + + foreach ($res as $handler) { + set_exception_handler($handler); + } + + return $res; + } + private function snapshotGlobalState(): void { if ($this->runTestInSeparateProcess || $this->inIsolation || diff --git a/src/Framework/TestRunner.php b/src/Framework/TestRunner.php index c03647f0494..e5f5b033bf8 100644 --- a/src/Framework/TestRunner.php +++ b/src/Framework/TestRunner.php @@ -13,7 +13,6 @@ use function assert; use function class_exists; use function defined; -use function error_clear_last; use function extension_loaded; use function get_include_path; use function hrtime; @@ -85,8 +84,6 @@ public function run(TestCase $test): void $risky = false; $skipped = false; - error_clear_last(); - if ($this->shouldErrorHandlerBeUsed($test)) { ErrorHandler::instance()->enable(); } diff --git a/tests/end-to-end/regression/5592.phpt b/tests/end-to-end/regression/5592.phpt new file mode 100644 index 00000000000..2c3d50bdc09 --- /dev/null +++ b/tests/end-to-end/regression/5592.phpt @@ -0,0 +1,69 @@ +--TEST-- +https://github.com/sebastianbergmann/phpunit/pull/5592 +--FILE-- + null); + +require_once __DIR__ . '/../../bootstrap.php'; +(new PHPUnit\TextUI\Application)->run($_SERVER['argv']); +--EXPECTF-- +PHPUnit %s by Sebastian Bergmann and contributors. + +Runtime: %s + +.FF.FF 6 / 6 (100%) + +Time: %s, Memory: %s + +There were 4 failures: + +1) PHPUnit\TestFixture\Issue5592Test::testAddedErrorHandler +Failed asserting that false is true. + +%sIssue5592Test.php:%i + +2) PHPUnit\TestFixture\Issue5592Test::testRemovedErrorHandler +Failed asserting that false is true. + +%sIssue5592Test.php:%i + +3) PHPUnit\TestFixture\Issue5592Test::testAddedExceptionHandler +Failed asserting that false is true. + +%sIssue5592Test.php:%i + +4) PHPUnit\TestFixture\Issue5592Test::testRemovedExceptionHandler +Failed asserting that false is true. + +%sIssue5592Test.php:%i + +-- + +There were 4 risky tests: + +1) PHPUnit\TestFixture\Issue5592Test::testAddedErrorHandler +Test code or tested code did not remove its own error handlers + +%sIssue5592Test.php:%i + +2) PHPUnit\TestFixture\Issue5592Test::testRemovedErrorHandler +Test code or tested code removed error handlers other than its own + +%sIssue5592Test.php:%i + +3) PHPUnit\TestFixture\Issue5592Test::testAddedExceptionHandler +Test code or tested code did not remove its own exception handlers + +%sIssue5592Test.php:%i + +4) PHPUnit\TestFixture\Issue5592Test::testRemovedExceptionHandler +Test code or tested code removed exception handlers other than its own + +%sIssue5592Test.php:%i + +FAILURES! +Tests: 6, Assertions: 6, Failures: 4, Risky: 4. diff --git a/tests/end-to-end/regression/5592/Issue5592Test.php b/tests/end-to-end/regression/5592/Issue5592Test.php new file mode 100644 index 00000000000..7c5cb81dc12 --- /dev/null +++ b/tests/end-to-end/regression/5592/Issue5592Test.php @@ -0,0 +1,57 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace PHPUnit\TestFixture; + +use function restore_error_handler; +use function restore_exception_handler; +use function set_error_handler; +use function set_exception_handler; +use PHPUnit\Framework\TestCase; + +class Issue5592Test extends TestCase +{ + public function testAddedAndRemovedErrorHandler(): void + { + set_error_handler(static fn () => false); + restore_error_handler(); + $this->assertTrue(true); + } + + public function testAddedErrorHandler(): void + { + set_error_handler(static fn () => false); + $this->assertTrue(false); + } + + public function testRemovedErrorHandler(): void + { + restore_error_handler(); + $this->assertTrue(false); + } + + public function testAddedAndRemovedExceptionHandler(): void + { + set_exception_handler(static fn () => null); + restore_exception_handler(); + $this->assertTrue(true); + } + + public function testAddedExceptionHandler(): void + { + set_exception_handler(static fn () => null); + $this->assertTrue(false); + } + + public function testRemovedExceptionHandler(): void + { + restore_exception_handler(); + $this->assertTrue(false); + } +}