Skip to content

Commit 93aa92b

Browse files
mvoriseksebastianbergmann
mvorisek
authored and
sebastianbergmann
committed
Check and restore error/exception global handlers
1 parent 549f23c commit 93aa92b

File tree

5 files changed

+272
-12
lines changed

5 files changed

+272
-12
lines changed

‎.psalm/baseline.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,10 @@
510510
<MissingThrowsDocblock>
511511
<code>getMethod</code>
512512
</MissingThrowsDocblock>
513+
<PossiblyNullArgument>
514+
<code><![CDATA[$this->backupGlobalErrorHandlers]]></code>
515+
<code><![CDATA[$this->backupGlobalExceptionHandlers]]></code>
516+
</PossiblyNullArgument>
513517
<PropertyNotSetInConstructor>
514518
<code>$outputBufferingLevel</code>
515519
</PropertyNotSetInConstructor>

‎src/Framework/TestCase.php

Lines changed: 142 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use const PHP_URL_PATH;
2121
use function array_keys;
2222
use function array_merge;
23+
use function array_reverse;
2324
use function array_values;
2425
use function assert;
2526
use function basename;
@@ -28,6 +29,7 @@
2829
use function clearstatcache;
2930
use function count;
3031
use function defined;
32+
use function error_clear_last;
3133
use function explode;
3234
use function getcwd;
3335
use function implode;
@@ -48,6 +50,10 @@
4850
use function parse_url;
4951
use function pathinfo;
5052
use function preg_replace;
53+
use function restore_error_handler;
54+
use function restore_exception_handler;
55+
use function set_error_handler;
56+
use function set_exception_handler;
5157
use function setlocale;
5258
use function sprintf;
5359
use function str_contains;
@@ -126,14 +132,24 @@ abstract class TestCase extends Assert implements Reorderable, SelfDescribing, T
126132
*/
127133
private array $backupStaticPropertiesExcludeList = [];
128134
private ?Snapshot $snapshot = null;
129-
private ?bool $runClassInSeparateProcess = null;
130-
private ?bool $runTestInSeparateProcess = null;
131-
private bool $preserveGlobalState = false;
132-
private bool $inIsolation = false;
133-
private ?string $expectedException = null;
134-
private ?string $expectedExceptionMessage = null;
135-
private ?string $expectedExceptionMessageRegExp = null;
136-
private null|int|string $expectedExceptionCode = null;
135+
136+
/**
137+
* @psalm-var list<callable>
138+
*/
139+
private ?array $backupGlobalErrorHandlers = null;
140+
141+
/**
142+
* @psalm-var list<callable>
143+
*/
144+
private ?array $backupGlobalExceptionHandlers = null;
145+
private ?bool $runClassInSeparateProcess = null;
146+
private ?bool $runTestInSeparateProcess = null;
147+
private bool $preserveGlobalState = false;
148+
private bool $inIsolation = false;
149+
private ?string $expectedException = null;
150+
private ?string $expectedExceptionMessage = null;
151+
private ?string $expectedExceptionMessageRegExp = null;
152+
private null|int|string $expectedExceptionCode = null;
137153

138154
/**
139155
* @psalm-var list<ExecutionOrderDependency>
@@ -618,13 +634,16 @@ final public function runBare(): void
618634
{
619635
$emitter = Event\Facade::emitter();
620636

637+
error_clear_last();
638+
clearstatcache();
639+
621640
$emitter->testPreparationStarted(
622641
$this->valueObjectForEvents(),
623642
);
624643

625644
$this->snapshotGlobalState();
645+
$this->snapshotGlobalErrorExceptionHandlers();
626646
$this->startOutputBuffering();
627-
clearstatcache();
628647

629648
$hookMethods = (new HookMethods)->hookMethods(static::class);
630649
$hasMetRequirements = false;
@@ -776,6 +795,7 @@ final public function runBare(): void
776795
chdir($currentWorkingDirectory);
777796
}
778797

798+
$this->restoreGlobalErrorExceptionHandlers();
779799
$this->restoreGlobalState();
780800
$this->unregisterCustomComparators();
781801
$this->cleanupIniSettings();
@@ -1683,6 +1703,119 @@ private function stopOutputBuffering(): bool
16831703
return true;
16841704
}
16851705

1706+
private function snapshotGlobalErrorExceptionHandlers(): void
1707+
{
1708+
$this->backupGlobalErrorHandlers = $this->getActiveErrorHandlers();
1709+
$this->backupGlobalExceptionHandlers = $this->getActiveExceptionHandlers();
1710+
}
1711+
1712+
/**
1713+
* @throws MoreThanOneDataSetFromDataProviderException
1714+
*/
1715+
private function restoreGlobalErrorExceptionHandlers(): void
1716+
{
1717+
$activeErrorHandlers = $this->getActiveErrorHandlers();
1718+
$activeExceptionHandlers = $this->getActiveExceptionHandlers();
1719+
1720+
$message = null;
1721+
1722+
if ($activeErrorHandlers !== $this->backupGlobalErrorHandlers) {
1723+
if (count($activeErrorHandlers) > count($this->backupGlobalErrorHandlers)) {
1724+
$message = 'Test code or tested code did not remove its own error handlers';
1725+
} else {
1726+
$message = 'Test code or tested code removed error handlers other than its own';
1727+
}
1728+
1729+
foreach ($activeErrorHandlers as $handler) {
1730+
restore_error_handler();
1731+
}
1732+
1733+
foreach ($this->backupGlobalErrorHandlers as $handler) {
1734+
set_error_handler($handler);
1735+
}
1736+
}
1737+
1738+
if ($activeExceptionHandlers !== $this->backupGlobalExceptionHandlers) {
1739+
if (count($activeExceptionHandlers) > count($this->backupGlobalExceptionHandlers)) {
1740+
$message = 'Test code or tested code did not remove its own exception handlers';
1741+
} else {
1742+
$message = 'Test code or tested code removed exception handlers other than its own';
1743+
}
1744+
1745+
foreach ($activeExceptionHandlers as $handler) {
1746+
restore_exception_handler();
1747+
}
1748+
1749+
foreach ($this->backupGlobalExceptionHandlers as $handler) {
1750+
set_exception_handler($handler);
1751+
}
1752+
}
1753+
1754+
$this->backupGlobalErrorHandlers = null;
1755+
$this->backupGlobalExceptionHandlers = null;
1756+
1757+
if ($message !== null) {
1758+
Event\Facade::emitter()->testConsideredRisky(
1759+
$this->valueObjectForEvents(),
1760+
$message,
1761+
);
1762+
1763+
$this->status = TestStatus::risky($message);
1764+
}
1765+
}
1766+
1767+
/**
1768+
* @return list<callable>
1769+
*/
1770+
private function getActiveErrorHandlers(): array
1771+
{
1772+
$res = [];
1773+
1774+
while (true) {
1775+
$previousHandler = set_error_handler(static fn () => false);
1776+
restore_error_handler();
1777+
1778+
if ($previousHandler === null) {
1779+
break;
1780+
}
1781+
$res[] = $previousHandler;
1782+
restore_error_handler();
1783+
}
1784+
$res = array_reverse($res);
1785+
1786+
foreach ($res as $handler) {
1787+
set_error_handler($handler);
1788+
}
1789+
1790+
return $res;
1791+
}
1792+
1793+
/**
1794+
* @return list<callable>
1795+
*/
1796+
private function getActiveExceptionHandlers(): array
1797+
{
1798+
$res = [];
1799+
1800+
while (true) {
1801+
$previousHandler = set_exception_handler(static fn () => null);
1802+
restore_exception_handler();
1803+
1804+
if ($previousHandler === null) {
1805+
break;
1806+
}
1807+
$res[] = $previousHandler;
1808+
restore_exception_handler();
1809+
}
1810+
$res = array_reverse($res);
1811+
1812+
foreach ($res as $handler) {
1813+
set_exception_handler($handler);
1814+
}
1815+
1816+
return $res;
1817+
}
1818+
16861819
private function snapshotGlobalState(): void
16871820
{
16881821
if ($this->runTestInSeparateProcess || $this->inIsolation ||

‎src/Framework/TestRunner.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
use function assert;
1414
use function class_exists;
1515
use function defined;
16-
use function error_clear_last;
1716
use function extension_loaded;
1817
use function get_include_path;
1918
use function hrtime;
@@ -85,8 +84,6 @@ public function run(TestCase $test): void
8584
$risky = false;
8685
$skipped = false;
8786

88-
error_clear_last();
89-
9087
if ($this->shouldErrorHandlerBeUsed($test)) {
9188
ErrorHandler::instance()->enable();
9289
}

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
--TEST--
2+
https://github.com/sebastianbergmann/phpunit/pull/5592
3+
--FILE--
4+
<?php declare(strict_types=1);
5+
$_SERVER['argv'][] = '--do-not-cache-result';
6+
$_SERVER['argv'][] = '--no-configuration';
7+
$_SERVER['argv'][] = __DIR__ . '/5592/Issue5592Test.php';
8+
9+
set_exception_handler(static fn () => null);
10+
11+
require_once __DIR__ . '/../../bootstrap.php';
12+
(new PHPUnit\TextUI\Application)->run($_SERVER['argv']);
13+
--EXPECTF--
14+
PHPUnit %s by Sebastian Bergmann and contributors.
15+
16+
Runtime: %s
17+
18+
.FF.FF 6 / 6 (100%)
19+
20+
Time: %s, Memory: %s
21+
22+
There were 4 failures:
23+
24+
1) PHPUnit\TestFixture\Issue5592Test::testAddedErrorHandler
25+
Failed asserting that false is true.
26+
27+
%sIssue5592Test.php:%i
28+
29+
2) PHPUnit\TestFixture\Issue5592Test::testRemovedErrorHandler
30+
Failed asserting that false is true.
31+
32+
%sIssue5592Test.php:%i
33+
34+
3) PHPUnit\TestFixture\Issue5592Test::testAddedExceptionHandler
35+
Failed asserting that false is true.
36+
37+
%sIssue5592Test.php:%i
38+
39+
4) PHPUnit\TestFixture\Issue5592Test::testRemovedExceptionHandler
40+
Failed asserting that false is true.
41+
42+
%sIssue5592Test.php:%i
43+
44+
--
45+
46+
There were 4 risky tests:
47+
48+
1) PHPUnit\TestFixture\Issue5592Test::testAddedErrorHandler
49+
Test code or tested code did not remove its own error handlers
50+
51+
%sIssue5592Test.php:%i
52+
53+
2) PHPUnit\TestFixture\Issue5592Test::testRemovedErrorHandler
54+
Test code or tested code removed error handlers other than its own
55+
56+
%sIssue5592Test.php:%i
57+
58+
3) PHPUnit\TestFixture\Issue5592Test::testAddedExceptionHandler
59+
Test code or tested code did not remove its own exception handlers
60+
61+
%sIssue5592Test.php:%i
62+
63+
4) PHPUnit\TestFixture\Issue5592Test::testRemovedExceptionHandler
64+
Test code or tested code removed exception handlers other than its own
65+
66+
%sIssue5592Test.php:%i
67+
68+
FAILURES!
69+
Tests: 6, Assertions: 6, Failures: 4, Risky: 4.

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php declare(strict_types=1);
2+
/*
3+
* This file is part of PHPUnit.
4+
*
5+
* (c) Sebastian Bergmann <[email protected]>
6+
*
7+
* For the full copyright and license information, please view the LICENSE
8+
* file that was distributed with this source code.
9+
*/
10+
namespace PHPUnit\TestFixture;
11+
12+
use function restore_error_handler;
13+
use function restore_exception_handler;
14+
use function set_error_handler;
15+
use function set_exception_handler;
16+
use PHPUnit\Framework\TestCase;
17+
18+
class Issue5592Test extends TestCase
19+
{
20+
public function testAddedAndRemovedErrorHandler(): void
21+
{
22+
set_error_handler(static fn () => false);
23+
restore_error_handler();
24+
$this->assertTrue(true);
25+
}
26+
27+
public function testAddedErrorHandler(): void
28+
{
29+
set_error_handler(static fn () => false);
30+
$this->assertTrue(false);
31+
}
32+
33+
public function testRemovedErrorHandler(): void
34+
{
35+
restore_error_handler();
36+
$this->assertTrue(false);
37+
}
38+
39+
public function testAddedAndRemovedExceptionHandler(): void
40+
{
41+
set_exception_handler(static fn () => null);
42+
restore_exception_handler();
43+
$this->assertTrue(true);
44+
}
45+
46+
public function testAddedExceptionHandler(): void
47+
{
48+
set_exception_handler(static fn () => null);
49+
$this->assertTrue(false);
50+
}
51+
52+
public function testRemovedExceptionHandler(): void
53+
{
54+
restore_exception_handler();
55+
$this->assertTrue(false);
56+
}
57+
}

0 commit comments

Comments
 (0)