From 23c16c1ce51fbaf05569321094532b428bf5fbd8 Mon Sep 17 00:00:00 2001 From: Sebastian Bergmann Date: Wed, 27 Dec 2023 16:08:13 +0100 Subject: [PATCH] Revert "Check and restore error/exception global handlers" This reverts commit 93aa92b9a1c77476e97ac1832e58907017f7ee28. --- .psalm/baseline.xml | 4 - src/Framework/TestCase.php | 151 ++---------------- src/Framework/TestRunner.php | 3 + tests/end-to-end/regression/5592.phpt | 69 -------- .../regression/5592/Issue5592Test.php | 57 ------- 5 files changed, 12 insertions(+), 272 deletions(-) delete mode 100644 tests/end-to-end/regression/5592.phpt delete mode 100644 tests/end-to-end/regression/5592/Issue5592Test.php diff --git a/.psalm/baseline.xml b/.psalm/baseline.xml index 8790893338b..9bf62d54447 100644 --- a/.psalm/baseline.xml +++ b/.psalm/baseline.xml @@ -514,10 +514,6 @@ getMethod - - backupGlobalErrorHandlers]]> - backupGlobalExceptionHandlers]]> - $outputBufferingLevel diff --git a/src/Framework/TestCase.php b/src/Framework/TestCase.php index cf8f66beba0..9f9c0b0dcdb 100644 --- a/src/Framework/TestCase.php +++ b/src/Framework/TestCase.php @@ -20,7 +20,6 @@ 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; @@ -29,7 +28,6 @@ use function clearstatcache; use function count; use function defined; -use function error_clear_last; use function explode; use function getcwd; use function implode; @@ -50,10 +48,6 @@ 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; @@ -132,24 +126,14 @@ abstract class TestCase extends Assert implements Reorderable, SelfDescribing, T */ private array $backupStaticPropertiesExcludeList = []; private ?Snapshot $snapshot = 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; + 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 @@ -633,16 +617,13 @@ 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; @@ -794,7 +775,6 @@ final public function runBare(): void chdir($currentWorkingDirectory); } - $this->restoreGlobalErrorExceptionHandlers(); $this->restoreGlobalState(); $this->unregisterCustomComparators(); $this->cleanupIniSettings(); @@ -1702,119 +1682,6 @@ 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 6af3c64bd43..c7f31d7c1d1 100644 --- a/src/Framework/TestRunner.php +++ b/src/Framework/TestRunner.php @@ -13,6 +13,7 @@ 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; @@ -82,6 +83,8 @@ 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 deleted file mode 100644 index 2c3d50bdc09..00000000000 --- a/tests/end-to-end/regression/5592.phpt +++ /dev/null @@ -1,69 +0,0 @@ ---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 deleted file mode 100644 index 7c5cb81dc12..00000000000 --- a/tests/end-to-end/regression/5592/Issue5592Test.php +++ /dev/null @@ -1,57 +0,0 @@ - - * - * 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); - } -}