Skip to content
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

Fix error handler to not silence error_get_last() result #5592

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Framework/TestRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -84,6 +85,8 @@ public function run(TestCase $test): void
$risky = false;
$skipped = false;

error_clear_last();

if ($this->shouldErrorHandlerBeUsed($test)) {
ErrorHandler::instance()->enable();
}
Expand Down
38 changes: 25 additions & 13 deletions src/Runner/ErrorHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,25 @@
*/
namespace PHPUnit\Runner;

use const E_COMPILE_ERROR;
use const E_COMPILE_WARNING;
use const E_CORE_ERROR;
use const E_CORE_WARNING;
use const E_DEPRECATED;
use const E_ERROR;
use const E_NOTICE;
use const E_PARSE;
use const E_RECOVERABLE_ERROR;
use const E_STRICT;
use const E_USER_DEPRECATED;
use const E_USER_ERROR;
use const E_USER_NOTICE;
use const E_USER_WARNING;
use const E_WARNING;
use function error_reporting;
use function restore_error_handler;
use function set_error_handler;
use ErrorException;
use PHPUnit\Event;
use PHPUnit\Event\Code\NoTestCaseObjectOnCallStackException;
use PHPUnit\Runner\Baseline\Baseline;
Expand All @@ -30,9 +39,12 @@
*/
final class ErrorHandler
{
private static ?self $instance = null;
private ?Baseline $baseline = null;
private bool $enabled = false;
private const UNHANDLEABLE_LEVELS = E_ERROR | E_PARSE | E_CORE_ERROR | E_CORE_WARNING | E_COMPILE_ERROR | E_COMPILE_WARNING;
private const UNSUPPRESSEABLE_LEVELS = E_ERROR | E_PARSE | E_CORE_ERROR | E_COMPILE_ERROR | E_USER_ERROR | E_RECOVERABLE_ERROR;
private static ?self $instance = null;
private ?Baseline $baseline = null;
private bool $enabled = false;
private ?int $originalErrorReportingLevel = null;

public static function instance(): self
{
Expand All @@ -44,7 +56,7 @@ public static function instance(): self
*/
public function __invoke(int $errorNumber, string $errorString, string $errorFile, int $errorLine): bool
{
$suppressed = !($errorNumber & error_reporting());
$suppressed = (error_reporting() & ~self::UNSUPPRESSEABLE_LEVELS) === 0;

if ($suppressed && (new ExcludeList)->isExcluded($errorFile)) {
return false;
Expand Down Expand Up @@ -140,13 +152,13 @@ public function __invoke(int $errorNumber, string $errorString, string $errorFil
$suppressed,
);

break;
throw new ErrorException('Any E_*_ERROR must abort execution');
mvorisek marked this conversation as resolved.
Show resolved Hide resolved

default:
return false;
}

return true;
return false;
}

public function enable(): void
Expand All @@ -155,15 +167,12 @@ public function enable(): void
return;
}

$oldErrorHandler = set_error_handler($this);

if ($oldErrorHandler !== null) {
restore_error_handler();

return;
}
set_error_handler($this);

$this->enabled = true;

$this->originalErrorReportingLevel = error_reporting();
error_reporting($this->originalErrorReportingLevel & self::UNHANDLEABLE_LEVELS);
}

public function disable(): void
Expand All @@ -175,6 +184,9 @@ public function disable(): void
restore_error_handler();

$this->enabled = false;

error_reporting(error_reporting() | $this->originalErrorReportingLevel);
$this->originalErrorReportingLevel = null;
}

public function use(Baseline $baseline): void
Expand Down
8 changes: 8 additions & 0 deletions tests/end-to-end/event/_files/DeprecatedFeatureTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
namespace PHPUnit\TestFixture\Event;

use const E_USER_DEPRECATED;
use function error_get_last;
use function trigger_error;
use PHPUnit\Framework\TestCase;

Expand All @@ -22,4 +23,11 @@ public function testDeprecatedFeature(): void

$this->assertTrue(true);
}

public function testDeprecatedSuppressedErrorGetLast(): void
{
$this->assertNull(error_get_last());
@trigger_error('message', E_USER_DEPRECATED);
$this->assertIsArray(error_get_last());
}
}
16 changes: 16 additions & 0 deletions tests/end-to-end/event/_files/IgnoreDeprecationsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/
namespace PHPUnit\TestFixture\Event;

use function error_get_last;
use function trigger_error;
use PHPUnit\Framework\Attributes\IgnoreDeprecations;
use PHPUnit\Framework\TestCase;
Expand All @@ -29,4 +30,19 @@ public function testTwo(): void

$this->assertTrue(true);
}

#[IgnoreDeprecations]
public function testOneErrorGetLast(): void
{
$this->assertNull(error_get_last());
trigger_error('message', E_USER_DEPRECATED);
$this->assertIsArray(error_get_last());
}

public function testTwoErrorGetLast(): void
{
$this->assertNull(error_get_last());
trigger_error('message', E_USER_DEPRECATED);
$this->assertIsArray(error_get_last());
}
}
8 changes: 8 additions & 0 deletions tests/end-to-end/event/_files/SuppressedUserNoticeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/
namespace PHPUnit\TestFixture\Event;

use function error_get_last;
use function trigger_error;
use PHPUnit\Framework\TestCase;

Expand All @@ -20,4 +21,11 @@ public function testSuppressedUserNotice(): void

@trigger_error('message', E_USER_NOTICE);
}

public function testSuppressedUserNoticeErrorGetLast(): void
{
$this->assertNull(error_get_last());
@trigger_error('message', E_USER_NOTICE);
$this->assertIsArray(error_get_last());
}
}
8 changes: 8 additions & 0 deletions tests/end-to-end/event/_files/SuppressedUserWarningTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/
namespace PHPUnit\TestFixture\Event;

use function error_get_last;
use function trigger_error;
use PHPUnit\Framework\TestCase;

Expand All @@ -20,4 +21,11 @@ public function testSuppressedUserWarning(): void

@trigger_error('message', E_USER_WARNING);
}

public function testSuppressedUserWarningErrorGetLast(): void
{
$this->assertNull(error_get_last());
@trigger_error('message', E_USER_WARNING);
$this->assertIsArray(error_get_last());
}
}
6 changes: 6 additions & 0 deletions tests/end-to-end/event/_files/UserErrorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,10 @@ public function testUserError(): void

trigger_error('message', E_USER_ERROR);
}

public function testUserErrorMustAbortExecution(): void
{
trigger_error('message', E_USER_ERROR);
$this->assertTrue(false);
}
}
8 changes: 8 additions & 0 deletions tests/end-to-end/event/_files/UserNoticeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/
namespace PHPUnit\TestFixture\Event;

use function error_get_last;
use function trigger_error;
use PHPUnit\Framework\TestCase;

Expand All @@ -20,4 +21,11 @@ public function testUserNotice(): void

trigger_error('message', E_USER_NOTICE);
}

public function testUserNoticeErrorGetLast(): void
{
$this->assertNull(error_get_last());
trigger_error('message', E_USER_NOTICE);
$this->assertIsArray(error_get_last());
}
}
8 changes: 8 additions & 0 deletions tests/end-to-end/event/_files/UserWarningTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/
namespace PHPUnit\TestFixture\Event;

use function error_get_last;
use function trigger_error;
use PHPUnit\Framework\TestCase;

Expand All @@ -20,4 +21,11 @@ public function testUserWarning(): void

trigger_error('message', E_USER_WARNING);
}

public function testUserWarningErrorGetLast(): void
{
$this->assertNull(error_get_last());
trigger_error('message', E_USER_WARNING);
$this->assertIsArray(error_get_last());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
*/
namespace PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled;

use function restore_error_handler;
use function set_error_handler;
use function sys_get_temp_dir;
use function tempnam;
use Exception;
Expand All @@ -33,4 +35,26 @@ public function testMethodB(): void
{
$this->assertSame('Triggering', (new Foo)->methodB()['message']);
}

public function testErrorHandlerSet(): void
{
$this->assertIsCallable($this->getErrorHandler());
}

#[WithoutErrorHandler]
public function testErrorHandlerIsNotSet(): void
{
$this->assertNull($this->getErrorHandler());
}

/**
* @return null|callable
*/
private function getErrorHandler()
{
$res = set_error_handler(static fn () => false);
restore_error_handler();

return $res;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ unlink($traceFile);
--EXPECTF--
PHPUnit Started (PHPUnit %s using %s)
Test Runner Configured
Test Suite Loaded (2 tests)
Test Suite Loaded (4 tests)
Event Facade Sealed
Test Runner Started
Test Suite Sorted
Test Runner Execution Started (2 tests)
Test Suite Started (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest, 2 tests)
Test Runner Execution Started (4 tests)
Test Suite Started (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest, 4 tests)
Test Preparation Started (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testOne)
Test Prepared (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testOne)
Test Triggered Test-Ignored Deprecation (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testOne)
Expand All @@ -41,7 +41,33 @@ message
Assertion Succeeded (Constraint: is true, Value: true)
Test Passed (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testTwo)
Test Finished (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testTwo)
Test Suite Finished (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest, 2 tests)
Test Preparation Started (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testOneErrorGetLast)
Test Prepared (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testOneErrorGetLast)
Assertion Succeeded (Constraint: is null, Value: null)
Test Triggered Test-Ignored Deprecation (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testOneErrorGetLast)
message
Assertion Succeeded (Constraint: is of type array, Value: Array &0 [
'type' => 16384,
'message' => 'message',
'file' => '%s%e_files%eIgnoreDeprecationsTest.php',
'line' => %d,
])
Test Passed (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testOneErrorGetLast)
Test Finished (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testOneErrorGetLast)
Test Preparation Started (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testTwoErrorGetLast)
Test Prepared (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testTwoErrorGetLast)
Assertion Succeeded (Constraint: is null, Value: null)
Test Triggered Deprecation (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testTwoErrorGetLast)
message
Assertion Succeeded (Constraint: is of type array, Value: Array &0 [
'type' => 16384,
'message' => 'message',
'file' => '%s%e_files%eIgnoreDeprecationsTest.php',
'line' => %d,
])
Test Passed (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testTwoErrorGetLast)
Test Finished (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest::testTwoErrorGetLast)
Test Suite Finished (PHPUnit\TestFixture\Event\IgnoreDeprecationsTest, 4 tests)
Test Runner Execution Finished
Test Runner Finished
PHPUnit Finished (Shell Exit Code: 0)
26 changes: 18 additions & 8 deletions tests/end-to-end/event/error-handler-can-be-disabled.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ unlink($traceFile);
PHPUnit Started (PHPUnit %s using %s)
Test Runner Configured
Bootstrap Finished (%s%esrc/Foo.php)
Test Suite Loaded (2 tests)
Test Suite Loaded (4 tests)
Event Facade Sealed
Test Runner Started
Test Suite Sorted
Test Runner Execution Started (2 tests)
Test Suite Started (%s%ephpunit.xml, 2 tests)
Test Suite Started (default, 2 tests)
Test Suite Started (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest, 2 tests)
Test Runner Execution Started (4 tests)
Test Suite Started (%s%ephpunit.xml, 4 tests)
Test Suite Started (default, 4 tests)
Test Suite Started (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest, 4 tests)
Test Preparation Started (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testMethodA)
Test Prepared (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testMethodA)
Assertion Succeeded (Constraint: exception of type "Exception", Value: {enable export of objects to see this value})
Expand All @@ -42,9 +42,19 @@ Test Prepared (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::test
Assertion Succeeded (Constraint: is identical to 'Triggering', Value: 'Triggering')
Test Passed (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testMethodB)
Test Finished (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testMethodB)
Test Suite Finished (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest, 2 tests)
Test Suite Finished (default, 2 tests)
Test Suite Finished (%s%ephpunit.xml, 2 tests)
Test Preparation Started (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testErrorHandlerSet)
Test Prepared (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testErrorHandlerSet)
Assertion Succeeded (Constraint: is of type callable, Value: {enable export of objects to see this value})
Test Passed (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testErrorHandlerSet)
Test Finished (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testErrorHandlerSet)
Test Preparation Started (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testErrorHandlerIsNotSet)
Test Prepared (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testErrorHandlerIsNotSet)
Assertion Succeeded (Constraint: is null, Value: null)
Test Passed (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testErrorHandlerIsNotSet)
Test Finished (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest::testErrorHandlerIsNotSet)
Test Suite Finished (PHPUnit\TestFixture\Event\ErrorHandlerCanBeDisabled\FooTest, 4 tests)
Test Suite Finished (default, 4 tests)
Test Suite Finished (%s%ephpunit.xml, 4 tests)
Test Runner Execution Finished
Test Runner Finished
PHPUnit Finished (Shell Exit Code: 0)
Loading