Check and restore error/exception global handlers

.psalm/baseline.xml

@@ -510,6 +510,10 @@
     <MissingThrowsDocblock>
       <code>getMethod</code>
     </MissingThrowsDocblock>
+    <PossiblyNullArgument>
+      <code><![CDATA[$this->backupGlobalErrorHandlers]]></code>
+      <code><![CDATA[$this->backupGlobalExceptionHandlers]]></code>
+    </PossiblyNullArgument>
     <PropertyNotSetInConstructor>
       <code>$outputBufferingLevel</code>
     </PropertyNotSetInConstructor>

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 @@
      */
     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<callable>
+     */
+    private ?array $backupGlobalErrorHandlers = null;
+
+    /**
+     * @psalm-var list<callable>
+     */
+    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<ExecutionOrderDependency>
@@ -584,7 +600,7 @@
     }
 
     /**
      * @internal This method is not covered by the backward compatibility promise for PHPUnit
      */
     final public function doesNotPerformAssertions(): bool
     {
@@ -618,13 +634,16 @@
     {
         $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 @@
             chdir($currentWorkingDirectory);
         }
 
+        $this->restoreGlobalErrorExceptionHandlers();
         $this->restoreGlobalState();
         $this->unregisterCustomComparators();
         $this->cleanupIniSettings();
@@ -1683,13 +1703,126 @@
         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<callable>
+     */
+    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<callable>
+     */
+    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 ||
             (!$this->backupGlobals && !$this->backupStaticProperties)) {
             return;
         }
 
         $snapshot = $this->createGlobalStateSnapshot($this->backupGlobals === true);
 
         $this->snapshot = $snapshot;

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();
         }

tests/end-to-end/regression/5592.phpt

@@ -0,0 +1,69 @@
+--TEST--
+https://github.com/sebastianbergmann/phpunit/pull/5592
+--FILE--
+<?php declare(strict_types=1);
+$_SERVER['argv'][] = '--do-not-cache-result';
+$_SERVER['argv'][] = '--no-configuration';
+$_SERVER['argv'][] = __DIR__ . '/5592/Issue5592Test.php';
+
+set_exception_handler(static fn () => 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.

tests/end-to-end/regression/5592/Issue5592Test.php

@@ -0,0 +1,57 @@
+<?php declare(strict_types=1);
+/*
+ * This file is part of PHPUnit.
+ *
+ * (c) Sebastian Bergmann <[email protected]>
+ *
+ * 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);
+    }
+}