From 5e0a0ecc84eda0e4b550becb7ad229488e199359 Mon Sep 17 00:00:00 2001 From: Ross Addison Date: Mon, 6 Jul 2026 13:48:44 +0100 Subject: [PATCH 01/16] fix(phpunit-mirror): add .placeholder.php so EmptyRun stub directory is mirrored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tests/Application/Stub/EmptyRun/ is intentionally empty — it is the test fixture for EmptyRunTest, which asserts that a Testo run over an empty directory yields Status::Risky with zero tests collected. Git does not track empty directories, and bin/build-phpunit.php only copies *.php files when populating the tests/PhpUnit/ mirror, so the mirror never contained tests/PhpUnit/Application/Stub/EmptyRun/. The mirrored EmptyRunTest resolved __DIR__ . '/../../Stub/EmptyRun' to that missing path and threw InvalidArgumentException: File or directory not found — aborting Infection's initial PHPUnit test run on every CI push to 1.x. Add .placeholder.php (no namespace, no classes, no tests) to the source directory. The build script copies it verbatim into the mirror, which creates the required directory. Testo's FinderConfig still discovers zero tests there, so Status::Risky is reported and the assertion holds. Co-Authored-By: Claude Sonnet 4.6 --- tests/Application/Stub/EmptyRun/.placeholder.php | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 tests/Application/Stub/EmptyRun/.placeholder.php diff --git a/tests/Application/Stub/EmptyRun/.placeholder.php b/tests/Application/Stub/EmptyRun/.placeholder.php new file mode 100644 index 00000000..72680edf --- /dev/null +++ b/tests/Application/Stub/EmptyRun/.placeholder.php @@ -0,0 +1,10 @@ + Date: Mon, 6 Jul 2026 14:47:56 +0100 Subject: [PATCH 02/16] feat(error-handler): add ErrorHandlerInterceptor plugin (#73) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the error handler interceptor described in issue #73. The plugin wraps each test in set_error_handler() / restore_error_handler() and accumulates any PHP errors triggered during the test into a CapturedErrors attribute on the returned TestResult. Behaviour: - Default (failOnError: false): errors are collected and stored as a CapturedErrors attribute; the test result status is unchanged. - failOnError: true: a captured error upgrades a passing test to Status::Failed and wraps the first error in an ErrorException as the failure, preserving any pre-existing failure from the next() chain. Includes 10 unit tests covering collect mode, fail mode, multiple errors, first-error-wins semantics, and handler restoration (both normal and throw paths). All tests use zero-param closures for set_error_handler callbacks to avoid SonarQube S1172 (unused parameter) — PHP silently discards extra arguments when a callable declares fewer params than the caller passes. Also wires the plugin into the monorepo: composer.json (require + autoload-dev + path-repository version), testo.php (src exclusion + suites), and split-publish.yml (error-handler-[0-9]* tag). Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/split-publish.yml | 1 + composer.json | 13 + plugin/error-handler/composer.json | 39 +++ plugin/error-handler/src/CapturedError.php | 20 ++ .../error-handler/src/ErrorHandlerPlugin.php | 37 +++ .../src/Internal/CapturedErrors.php | 29 +++ .../src/Internal/ErrorHandlerInterceptor.php | 70 ++++++ .../Unit/ErrorHandlerInterceptorTest.php | 223 ++++++++++++++++++ plugin/error-handler/tests/suites.php | 15 ++ testo.php | 2 + 10 files changed, 449 insertions(+) create mode 100644 plugin/error-handler/composer.json create mode 100644 plugin/error-handler/src/CapturedError.php create mode 100644 plugin/error-handler/src/ErrorHandlerPlugin.php create mode 100644 plugin/error-handler/src/Internal/CapturedErrors.php create mode 100644 plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php create mode 100644 plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php create mode 100644 plugin/error-handler/tests/suites.php diff --git a/.github/workflows/split-publish.yml b/.github/workflows/split-publish.yml index d6bdea0d..ff65833f 100644 --- a/.github/workflows/split-publish.yml +++ b/.github/workflows/split-publish.yml @@ -29,6 +29,7 @@ on: # yamllint disable-line rule:truthy - 'convention-[0-9]*' - 'data-[0-9]*' - 'facade-[0-9]*' + - 'error-handler-[0-9]*' - 'filter-[0-9]*' - 'inline-[0-9]*' - 'lifecycle-[0-9]*' diff --git a/composer.json b/composer.json index 48529c38..a4fbbc2e 100644 --- a/composer.json +++ b/composer.json @@ -48,6 +48,17 @@ "testo/filter": "^0.1.7", "testo/inline": "^0.1.9", "testo/lifecycle": "^0.1.6", + "testo/assert": "^0.1.12", + "testo/fiber": "^0.1.2", + "testo/bench": "^0.1.8", + "testo/bridge-symfony-console": "^0.1.8", + "testo/codecov": "^0.1.12", + "testo/convention": "^0.1.4", + "testo/data": "^0.1.7", + "testo/error-handler": "^0.1", + "testo/filter": "^0.1.6", + "testo/inline": "^0.1.8", + "testo/lifecycle": "^0.1.5", "testo/repeat": "^0.1.9", "testo/retry": "^0.1.5", "testo/test": "^0.1.7", @@ -101,6 +112,7 @@ "Tests\\Convention\\": "plugin/convention/tests/", "Tests\\Data\\": "plugin/data/tests/", "Tests\\Facade\\": "plugin/facade/tests/", + "Tests\\ErrorHandler\\": "plugin/error-handler/tests/", "Tests\\Filter\\": "plugin/filter/tests/", "Tests\\Lifecycle\\": "plugin/lifecycle/tests/", "Tests\\Repeat\\": "plugin/repeat/tests/", @@ -126,6 +138,7 @@ "testo/convention": "0.1.x-dev", "testo/data": "0.1.x-dev", "testo/facade": "0.1.x-dev", + "testo/error-handler": "0.1.x-dev", "testo/filter": "0.1.x-dev", "testo/inline": "0.1.x-dev", "testo/lifecycle": "0.1.x-dev", diff --git a/plugin/error-handler/composer.json b/plugin/error-handler/composer.json new file mode 100644 index 00000000..36edb2f0 --- /dev/null +++ b/plugin/error-handler/composer.json @@ -0,0 +1,39 @@ +{ + "name": "testo/error-handler", + "description": "Error handler interceptor plugin for the Testo testing framework.", + "license": "BSD-3-Clause", + "type": "library", + "keywords": [ + "testo", + "error-handler", + "test" + ], + "authors": [ + { + "name": "Aleksei Gagarin (roxblnfk)", + "homepage": "https://github.com/roxblnfk" + } + ], + "funding": [ + { + "type": "boosty", + "url": "https://boosty.to/roxblnfk" + } + ], + "require": { + "php": ">=8.2", + "testo/testo": "0.10.34 - 1" + }, + "autoload": { + "psr-4": { + "Testo\\ErrorHandler\\": "src/" + } + }, + "minimum-stability": "dev", + "prefer-stable": true, + "extra": { + "branch-alias": { + "dev-1.x": "1.x-dev" + } + } +} diff --git a/plugin/error-handler/src/CapturedError.php b/plugin/error-handler/src/CapturedError.php new file mode 100644 index 00000000..4c1526d7 --- /dev/null +++ b/plugin/error-handler/src/CapturedError.php @@ -0,0 +1,20 @@ +get(InterceptorCollector::class) + ->addInterceptor(new ErrorHandlerInterceptor($this->failOnError)); + } +} diff --git a/plugin/error-handler/src/Internal/CapturedErrors.php b/plugin/error-handler/src/Internal/CapturedErrors.php new file mode 100644 index 00000000..ce7304ea --- /dev/null +++ b/plugin/error-handler/src/Internal/CapturedErrors.php @@ -0,0 +1,29 @@ + $errors */ + public function __construct( + public array $errors, + ) {} + + public function isEmpty(): bool + { + return $this->errors === []; + } +} diff --git a/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php b/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php new file mode 100644 index 00000000..3886f294 --- /dev/null +++ b/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php @@ -0,0 +1,70 @@ + $errors */ + $errors = []; + + \set_error_handler( + static function (int $severity, string $message, string $file, int $line) use (&$errors): bool { + $errors[] = new CapturedError($severity, $message, $file, $line); + return true; + }, + ); + + try { + $result = $next($info); + } finally { + \restore_error_handler(); + } + + if ($errors === []) { + return $result; + } + + $result = $result->withAttribute(CapturedErrors::class, new CapturedErrors($errors)); + + if ($this->failOnError && !$result->status->isFailure()) { + $first = $errors[0]; + $result = $result + ->with(status: Status::Failed) + ->withFailure(new \ErrorException($first->message, 0, $first->severity, $first->file, $first->line)); + } + + return $result; + } +} diff --git a/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php b/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php new file mode 100644 index 00000000..fec8d021 --- /dev/null +++ b/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php @@ -0,0 +1,223 @@ + new TestResult(info: $info, status: Status::Passed); + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Passed); + Assert::null($result->getAttribute(CapturedErrors::class)); + } + + public function capturedErrorIsStoredAsAttribute(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(); + $next = static function (TestInfo $info): TestResult { + \trigger_error('test warning', \E_USER_WARNING); + return new TestResult(info: $info, status: Status::Passed); + }; + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Passed); + $errors = $result->getAttribute(CapturedErrors::class); + Assert::instanceOf($errors, CapturedErrors::class); + Assert::false($errors->isEmpty()); + Assert::same(\count($errors->errors), 1); + Assert::same($errors->errors[0]->message, 'test warning'); + Assert::same($errors->errors[0]->severity, \E_USER_WARNING); + } + + public function multipleErrorsAreAllCaptured(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(); + $next = static function (TestInfo $info): TestResult { + \trigger_error('first', \E_USER_NOTICE); + \trigger_error('second', \E_USER_WARNING); + \trigger_error('third', \E_USER_DEPRECATED); + return new TestResult(info: $info, status: Status::Passed); + }; + + $result = $interceptor->runTest($info, $next); + + $errors = $result->getAttribute(CapturedErrors::class); + Assert::instanceOf($errors, CapturedErrors::class); + Assert::same(\count($errors->errors), 3); + Assert::same($errors->errors[0]->message, 'first'); + Assert::same($errors->errors[1]->message, 'second'); + Assert::same($errors->errors[2]->message, 'third'); + } + + public function collectModePreservesPassingStatus(): void + { + $interceptor = new ErrorHandlerInterceptor(failOnError: false); + $info = self::createTestInfo(); + $next = static function (TestInfo $info): TestResult { + \trigger_error('deprecated usage', \E_USER_DEPRECATED); + return new TestResult(info: $info, status: Status::Passed); + }; + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Passed); + Assert::notNull($result->getAttribute(CapturedErrors::class)); + } + + public function failModeUpgradesPassingTestToFailed(): void + { + $interceptor = new ErrorHandlerInterceptor(failOnError: true); + $info = self::createTestInfo(); + $next = static function (TestInfo $info): TestResult { + \trigger_error('user warning', \E_USER_WARNING); + return new TestResult(info: $info, status: Status::Passed); + }; + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Failed); + Assert::instanceOf($result->failure, \ErrorException::class); + Assert::same($result->failure->getMessage(), 'user warning'); + Assert::same($result->failure->getSeverity(), \E_USER_WARNING); + } + + public function failModeUsesFirstErrorAsFailure(): void + { + $interceptor = new ErrorHandlerInterceptor(failOnError: true); + $info = self::createTestInfo(); + $next = static function (TestInfo $info): TestResult { + \trigger_error('first error', \E_USER_WARNING); + \trigger_error('second error', \E_USER_NOTICE); + return new TestResult(info: $info, status: Status::Passed); + }; + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Failed); + Assert::instanceOf($result->failure, \ErrorException::class); + Assert::same($result->failure->getMessage(), 'first error'); + } + + public function failModeDoesNotOverrideAlreadyFailedTest(): void + { + $interceptor = new ErrorHandlerInterceptor(failOnError: true); + $info = self::createTestInfo(); + $originalFailure = new \RuntimeException('assertion failure'); + $next = static function (TestInfo $info) use ($originalFailure): TestResult { + \trigger_error('also an error', \E_USER_WARNING); + return new TestResult(info: $info, status: Status::Failed, failure: $originalFailure); + }; + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Failed); + Assert::same($result->failure, $originalFailure); + } + + public function failModeDoesNotOverrideErrorStatus(): void + { + $interceptor = new ErrorHandlerInterceptor(failOnError: true); + $info = self::createTestInfo(); + $originalFailure = new \RuntimeException('unexpected throw'); + $next = static function (TestInfo $info) use ($originalFailure): TestResult { + \trigger_error('also triggered', \E_USER_WARNING); + return new TestResult(info: $info, status: Status::Error, failure: $originalFailure); + }; + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Error); + Assert::same($result->failure, $originalFailure); + } + + public function handlerIsRestoredAfterTestCompletes(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(); + $next = static fn(TestInfo $info): TestResult => new TestResult(info: $info, status: Status::Passed); + + // Zero-param closure: PHP discards extra arguments silently, avoiding S1172. + $count = 0; + \set_error_handler(static function () use (&$count): bool { + $count++; + return true; + }); + + try { + $interceptor->runTest($info, $next); + \trigger_error('after test', \E_USER_NOTICE); + } finally { + \restore_error_handler(); + } + + Assert::same($count, 1); + } + + public function handlerIsRestoredEvenWhenTestThrows(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(); + // Arrow function with no params: throw is a valid expression in PHP 8+. + $next = static fn(): TestResult => throw new \RuntimeException('unexpected throw'); + + $count = 0; + \set_error_handler(static function () use (&$count): bool { + $count++; + return true; + }); + + try { + try { + $interceptor->runTest($info, $next); + } catch (\RuntimeException) { + // expected + } + \trigger_error('after throw', \E_USER_NOTICE); + } finally { + \restore_error_handler(); + } + + Assert::same($count, 1); + } + + private static function createTestInfo(): TestInfo + { + $reflection = new \ReflectionMethod(self::class, 'createTestInfo'); + $caseDefinition = new CaseDefinition(name: 'TestCase', type: 'test'); + $caseInfo = new CaseInfo(definition: $caseDefinition); + $testDefinition = new TestDefinition(reflection: $reflection); + + return new TestInfo( + name: 'testMethod', + caseInfo: $caseInfo, + testDefinition: $testDefinition, + ); + } +} diff --git a/plugin/error-handler/tests/suites.php b/plugin/error-handler/tests/suites.php new file mode 100644 index 00000000..cf7146f9 --- /dev/null +++ b/plugin/error-handler/tests/suites.php @@ -0,0 +1,15 @@ + Date: Thu, 13 Aug 2026 16:08:22 +0100 Subject: [PATCH 03/16] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php b/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php index 3886f294..7d868295 100644 --- a/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php +++ b/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php @@ -58,7 +58,7 @@ static function (int $severity, string $message, string $file, int $line) use (& $result = $result->withAttribute(CapturedErrors::class, new CapturedErrors($errors)); - if ($this->failOnError && !$result->status->isFailure()) { + if ($this->failOnError && $result->status === Status::Passed) { $first = $errors[0]; $result = $result ->with(status: Status::Failed) From 745e2273c73cfc488eed1f5e0496fc671f2870a3 Mon Sep 17 00:00:00 2001 From: Ross Addison Date: Thu, 13 Aug 2026 17:49:23 +0100 Subject: [PATCH 04/16] fix(error-handler): make ErrorHandlerInterceptor fiber-safe; promote CapturedErrors to public MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses roxblnfk's review on #262: - set_error_handler()/restore_error_handler() operate on one process-global stack. The old code installed its handler once before $next() and restored once after — but $next() can suspend a fiber mid-test while a sibling test interleaves, so the handler stayed installed (and, on an interleaved resume, restore_error_handler() could pop a sibling's frame instead of its own). Same defect as #254. Fixed by wrapping $next() in its own fiber and swapping the handler on every suspend/resume — restore (native stack pop) on suspend, reinstall on resume — mirroring the already-reviewed pattern in MockeryInterceptor::run() and MessengerHub::scope(). Regression test added (restoresTheOuterHandlerWhileSuspendedAndReinstallsItsOwnOnResume): confirmed it fails against the old code (an error fired while suspended was wrongly captured by this test's own handler instead of reaching the outer one) and passes against the fix. - Promoted Internal\CapturedErrors to a public Testo\ErrorHandler\CapturedErrors class (Copilot review comment): the plugin's own docs already tell consumers to read this attribute off TestResult, so it was never really internal — it just wasn't marked as such. Fixes the @api-marked ErrorHandlerPlugin's docblock referencing an internal type. - The other Copilot comment (restrict the failOnError status upgrade to Status::Passed) was already fixed in a prior commit on this branch — no change needed. Also rebased onto current 1.x (26 commits behind), resolving one real conflict in composer.json (version bumps landed upstream since this PR opened) and the same CaseDefinition/CaseInfo required-argument fix already applied on #264. The second question from review — what should happen if a test changes the error handler itself mid-run — is intentionally left open; per roxblnfk's own comment it needs research/discussion before implementing, not a quick fix. Verified: - composer rector:ci: clean, 0 files - Full Testo suite: 1682 passed, 6 failed/7 error (same pre-existing Bench/Self baseline as the current rector/* PR series, unrelated) - ErrorHandlerInterceptorTest: 11/11 passed, including the new fiber-safety regression test (confirmed it fails against the old code) - Psalm: this repo's Psalm CI only covers core/ (confirmed via psalm.xml and psalm.yml's trigger paths) — plugin/error-handler/ was never in scope, unchanged by this fix Co-Authored-By: Claude Sonnet 5 --- .../src/{Internal => }/CapturedErrors.php | 7 +- .../error-handler/src/ErrorHandlerPlugin.php | 2 +- .../src/Internal/ErrorHandlerInterceptor.php | 64 +++++++++++++++---- .../Unit/ErrorHandlerInterceptorTest.php | 59 ++++++++++++++++- 4 files changed, 112 insertions(+), 20 deletions(-) rename plugin/error-handler/src/{Internal => }/CapturedErrors.php (80%) diff --git a/plugin/error-handler/src/Internal/CapturedErrors.php b/plugin/error-handler/src/CapturedErrors.php similarity index 80% rename from plugin/error-handler/src/Internal/CapturedErrors.php rename to plugin/error-handler/src/CapturedErrors.php index ce7304ea..982dc5d9 100644 --- a/plugin/error-handler/src/Internal/CapturedErrors.php +++ b/plugin/error-handler/src/CapturedErrors.php @@ -2,9 +2,7 @@ declare(strict_types=1); -namespace Testo\ErrorHandler\Internal; - -use Testo\ErrorHandler\CapturedError; +namespace Testo\ErrorHandler; /** * Collection of PHP errors accumulated during test execution. @@ -12,8 +10,7 @@ * Stored as a {@see \Testo\Core\Context\TestResult} attribute under the key {@see CapturedErrors::class}. * Renderers that wish to display collected errors should retrieve it from the result. * - * @internal - * @psalm-internal Testo\ErrorHandler + * @api */ final readonly class CapturedErrors { diff --git a/plugin/error-handler/src/ErrorHandlerPlugin.php b/plugin/error-handler/src/ErrorHandlerPlugin.php index 3ac1c06d..9c40ba38 100644 --- a/plugin/error-handler/src/ErrorHandlerPlugin.php +++ b/plugin/error-handler/src/ErrorHandlerPlugin.php @@ -12,7 +12,7 @@ /** * Plugin that captures PHP errors raised during test execution. * - * By default errors are collected and stored as a {@see Internal\CapturedErrors} attribute + * By default errors are collected and stored as a {@see CapturedErrors} attribute * on the {@see \Testo\Core\Context\TestResult}, but the test still passes. Pass * {@see $failOnError}: true to make any captured error fail the test instead. * diff --git a/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php b/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php index 7d868295..8e056c27 100644 --- a/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php +++ b/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php @@ -8,6 +8,7 @@ use Testo\Core\Context\TestResult; use Testo\Core\Value\Status; use Testo\ErrorHandler\CapturedError; +use Testo\ErrorHandler\CapturedErrors; use Testo\Pipeline\Attribute\InterceptorOptions; use Testo\Pipeline\Middleware\TestRunInterceptor; @@ -39,18 +40,12 @@ public function runTest(TestInfo $info, callable $next): TestResult /** @var list $errors */ $errors = []; - \set_error_handler( - static function (int $severity, string $message, string $file, int $line) use (&$errors): bool { - $errors[] = new CapturedError($severity, $message, $file, $line); - return true; - }, - ); + $handler = static function (int $severity, string $message, string $file, int $line) use (&$errors): bool { + $errors[] = new CapturedError($severity, $message, $file, $line); + return true; + }; - try { - $result = $next($info); - } finally { - \restore_error_handler(); - } + $result = $this->run($info, $next, $handler); if ($errors === []) { return $result; @@ -67,4 +62,51 @@ static function (int $severity, string $message, string $file, int $line) use (& return $result; } + + /** + * Runs the test with {@see $handler} installed via {@see \set_error_handler()}, keeping it + * bound to this test across fiber suspensions. + * + * set_error_handler()/restore_error_handler() operate on one process-global stack, so under + * concurrent (fiber-based) execution — where sibling tests interleave with this one — a plain + * install-before/restore-after around $next() would leak errors into the wrong test's + * CapturedErrors, and an interleaved resume could pop a sibling's handler instead of ours. On + * every suspension we restore whichever handler was active before this test installed its own + * (the native stack does that for free); on resumption we re-install this test's handler. + * Mirrors {@see \Testo\Bridge\Mockery\Internal\MockeryInterceptor::run()} and + * {@see \Testo\Application\Internal\MessengerHub::scope()}. + * + * @param callable(TestInfo): TestResult $next + */ + private function run(TestInfo $info, callable $next, \Closure $handler): TestResult + { + \set_error_handler($handler); + try { + if (\Fiber::getCurrent() === null) { + return $next($info); + } + + $fiber = new \Fiber(static fn(): TestResult => $next($info)); + $value = $fiber->start(); + while (!$fiber->isTerminated()) { + \restore_error_handler(); + try { + $resume = \Fiber::suspend($value); + } catch (\Throwable $e) { + \set_error_handler($handler); + $value = $fiber->throw($e); + continue; + } + + \set_error_handler($handler); + $value = $fiber->resume($resume); + } + + /** @var TestResult $result */ + $result = $fiber->getReturn(); + return $result; + } finally { + \restore_error_handler(); + } + } } diff --git a/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php b/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php index fec8d021..c3e1072b 100644 --- a/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php +++ b/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php @@ -4,16 +4,18 @@ namespace Tests\ErrorHandler\Unit; +use Internal\Path; use Testo\Assert; use Testo\Codecov\Covers; use Testo\Core\Context\CaseInfo; +use Testo\Core\Context\Identity\SuiteIdentity; use Testo\Core\Context\TestInfo; use Testo\Core\Context\TestResult; use Testo\Core\Definition\CaseDefinition; use Testo\Core\Definition\TestDefinition; use Testo\Core\Value\Status; use Testo\ErrorHandler\CapturedError; -use Testo\ErrorHandler\Internal\CapturedErrors; +use Testo\ErrorHandler\CapturedErrors; use Testo\ErrorHandler\Internal\ErrorHandlerInterceptor; use Testo\Test; @@ -207,11 +209,62 @@ public function handlerIsRestoredEvenWhenTestThrows(): void Assert::same($count, 1); } + /** + * The error-handler stack is process-global, and Testo can run tests inside fibers with + * sibling tests interleaving on suspend/resume. A plain install-before/restore-after around + * $next() would leave this test's handler installed for the entire suspension window, so a + * sibling's error fired while this test is suspended would wrongly be captured here instead + * of reaching whatever was active before this test started. + */ + public function restoresTheOuterHandlerWhileSuspendedAndReinstallsItsOwnOnResume(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(); + + $outerCount = 0; + \set_error_handler(static function () use (&$outerCount): bool { + $outerCount++; + return true; + }); + + try { + $next = static function (TestInfo $info): TestResult { + \trigger_error('before suspend', \E_USER_NOTICE); + \Fiber::suspend(); + \trigger_error('after resume', \E_USER_NOTICE); + return new TestResult(info: $info, status: Status::Passed); + }; + + // runTest() only takes the fiber-aware branch when a fiber is already active, so + // drive it inside our own fiber here — exactly how Testo's scheduler runs a test. + $fiber = new \Fiber(static fn(): TestResult => $interceptor->runTest($info, $next)); + $fiber->start(); + + // While this test is suspended, an error fired by anything else running in the + // process (a sibling test interleaving via the scheduler) must fall through to + // whatever was active before this test installed its own handler. + \trigger_error('fired while suspended', \E_USER_NOTICE); + Assert::same($outerCount, 1); + + $fiber->resume(); + Assert::true($fiber->isTerminated()); + + $result = $fiber->getReturn(); + $errors = $result->getAttribute(CapturedErrors::class); + Assert::instanceOf($errors, CapturedErrors::class); + Assert::same(\count($errors->errors), 2); + Assert::same($errors->errors[0]->message, 'before suspend'); + Assert::same($errors->errors[1]->message, 'after resume'); + } finally { + \restore_error_handler(); + } + } + private static function createTestInfo(): TestInfo { $reflection = new \ReflectionMethod(self::class, 'createTestInfo'); - $caseDefinition = new CaseDefinition(name: 'TestCase', type: 'test'); - $caseInfo = new CaseInfo(definition: $caseDefinition); + $caseDefinition = new CaseDefinition(name: 'TestCase', type: 'test', file: Path::create(__FILE__)); + $caseInfo = new CaseInfo(definition: $caseDefinition, suiteIdentity: new SuiteIdentity('ErrorHandler/Unit')); $testDefinition = new TestDefinition(reflection: $reflection); return new TestInfo( From 731a6ed680f68820148360430242b75af4969e4e Mon Sep 17 00:00:00 2001 From: Ross Addison Date: Sat, 12 Sep 2026 23:58:40 +0100 Subject: [PATCH 05/16] chore: fix duplicate composer.json keys left by rebasing onto 1.x Rebasing feat/73-error-handler-interceptor onto the current 1.x tip (21 commits ahead) also dropped this branch's two unrelated infection.json commits (f3bb577, baa0449 -- neither is on 1.x; per roxblnfk's own review comment, they belong to a different concern and don't belong in this PR). That rebase's automatic 3-way merge for composer.json's `require` block produced literal duplicate keys instead of a real conflict: this branch's own commit had pinned older testo/* version constraints than 1.x has since moved to, and git treated the two blocks as independent additions rather than the same keys needing resolution, since nothing else nearby differed enough to force a conflict marker. The file was syntactically valid JSON throughout (PHP's json_decode silently keeps only the last occurrence of a duplicate key), but genuinely contained two require blocks side by side -- confirmed by grepping the working tree, not assumed from the diff alone. Fixed by keeping the single already-current-on-1.x block and inserting only the one genuinely new line this PR adds ("testo/error-handler": "^0.1") into it, in its original relative position. Verified: valid JSON, no duplicate keys (checked programmatically), composer install succeeds, and the ErrorHandler/Unit suite still passes 11/11 (including the fiber-safety regression test) against the rebased tree. Co-Authored-By: Claude Sonnet 5 --- composer.json | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/composer.json b/composer.json index a4fbbc2e..b9cf1603 100644 --- a/composer.json +++ b/composer.json @@ -45,20 +45,10 @@ "testo/codecov": "^0.2.1", "testo/convention": "^0.1.5", "testo/data": "^0.1.9", + "testo/error-handler": "^0.1", "testo/filter": "^0.1.7", "testo/inline": "^0.1.9", "testo/lifecycle": "^0.1.6", - "testo/assert": "^0.1.12", - "testo/fiber": "^0.1.2", - "testo/bench": "^0.1.8", - "testo/bridge-symfony-console": "^0.1.8", - "testo/codecov": "^0.1.12", - "testo/convention": "^0.1.4", - "testo/data": "^0.1.7", - "testo/error-handler": "^0.1", - "testo/filter": "^0.1.6", - "testo/inline": "^0.1.8", - "testo/lifecycle": "^0.1.5", "testo/repeat": "^0.1.9", "testo/retry": "^0.1.5", "testo/test": "^0.1.7", From 6dbc8e7aabbd45304c62a7ce0c4400475c23e3e3 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 23:45:32 +0000 Subject: [PATCH 06/16] style(cs): apply php-cs-fixer --- plugin/error-handler/src/CapturedErrors.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugin/error-handler/src/CapturedErrors.php b/plugin/error-handler/src/CapturedErrors.php index 982dc5d9..68a55a9a 100644 --- a/plugin/error-handler/src/CapturedErrors.php +++ b/plugin/error-handler/src/CapturedErrors.php @@ -14,7 +14,9 @@ */ final readonly class CapturedErrors { - /** @param list $errors */ + /** + * @param list $errors + */ public function __construct( public array $errors, ) {} From 56f46c7942abc8fc2ffa9ed32da23a4bcc2b1fd8 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Sun, 13 Sep 2026 20:06:32 +0400 Subject: [PATCH 07/16] chore(error-handler): drop the EmptyRun placeholder and trim comments to constraints The PHPUnit mirror already keeps the empty stub directory alive through its .gitkeep, so the placeholder duplicated that fix and put a PHP file into a fixture meant to be empty. The remaining comments now state only the fiber constraint on the handler stack; tooling notes and the history of the fix are gone. Assisted-By: Claude Fable 5.1 --- .../src/Internal/ErrorHandlerInterceptor.php | 13 ++----------- .../tests/Unit/ErrorHandlerInterceptorTest.php | 16 +--------------- tests/Application/Stub/EmptyRun/.placeholder.php | 10 ---------- 3 files changed, 3 insertions(+), 36 deletions(-) delete mode 100644 tests/Application/Stub/EmptyRun/.placeholder.php diff --git a/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php b/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php index 8e056c27..501c7d3d 100644 --- a/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php +++ b/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php @@ -64,17 +64,8 @@ public function runTest(TestInfo $info, callable $next): TestResult } /** - * Runs the test with {@see $handler} installed via {@see \set_error_handler()}, keeping it - * bound to this test across fiber suspensions. - * - * set_error_handler()/restore_error_handler() operate on one process-global stack, so under - * concurrent (fiber-based) execution — where sibling tests interleave with this one — a plain - * install-before/restore-after around $next() would leak errors into the wrong test's - * CapturedErrors, and an interleaved resume could pop a sibling's handler instead of ours. On - * every suspension we restore whichever handler was active before this test installed its own - * (the native stack does that for free); on resumption we re-install this test's handler. - * Mirrors {@see \Testo\Bridge\Mockery\Internal\MockeryInterceptor::run()} and - * {@see \Testo\Application\Internal\MessengerHub::scope()}. + * The handler stack is process-global. Inside a fiber the handler is removed on every suspension + * and re-installed on resumption, so errors fired by an interleaved sibling test never land here. * * @param callable(TestInfo): TestResult $next */ diff --git a/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php b/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php index c3e1072b..dd011184 100644 --- a/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php +++ b/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php @@ -165,7 +165,6 @@ public function handlerIsRestoredAfterTestCompletes(): void $info = self::createTestInfo(); $next = static fn(TestInfo $info): TestResult => new TestResult(info: $info, status: Status::Passed); - // Zero-param closure: PHP discards extra arguments silently, avoiding S1172. $count = 0; \set_error_handler(static function () use (&$count): bool { $count++; @@ -186,7 +185,6 @@ public function handlerIsRestoredEvenWhenTestThrows(): void { $interceptor = new ErrorHandlerInterceptor(); $info = self::createTestInfo(); - // Arrow function with no params: throw is a valid expression in PHP 8+. $next = static fn(): TestResult => throw new \RuntimeException('unexpected throw'); $count = 0; @@ -199,7 +197,6 @@ public function handlerIsRestoredEvenWhenTestThrows(): void try { $interceptor->runTest($info, $next); } catch (\RuntimeException) { - // expected } \trigger_error('after throw', \E_USER_NOTICE); } finally { @@ -209,13 +206,6 @@ public function handlerIsRestoredEvenWhenTestThrows(): void Assert::same($count, 1); } - /** - * The error-handler stack is process-global, and Testo can run tests inside fibers with - * sibling tests interleaving on suspend/resume. A plain install-before/restore-after around - * $next() would leave this test's handler installed for the entire suspension window, so a - * sibling's error fired while this test is suspended would wrongly be captured here instead - * of reaching whatever was active before this test started. - */ public function restoresTheOuterHandlerWhileSuspendedAndReinstallsItsOwnOnResume(): void { $interceptor = new ErrorHandlerInterceptor(); @@ -235,14 +225,10 @@ public function restoresTheOuterHandlerWhileSuspendedAndReinstallsItsOwnOnResume return new TestResult(info: $info, status: Status::Passed); }; - // runTest() only takes the fiber-aware branch when a fiber is already active, so - // drive it inside our own fiber here — exactly how Testo's scheduler runs a test. $fiber = new \Fiber(static fn(): TestResult => $interceptor->runTest($info, $next)); $fiber->start(); - // While this test is suspended, an error fired by anything else running in the - // process (a sibling test interleaving via the scheduler) must fall through to - // whatever was active before this test installed its own handler. + // Fired while the test is suspended: must reach the outer handler, not the test. \trigger_error('fired while suspended', \E_USER_NOTICE); Assert::same($outerCount, 1); diff --git a/tests/Application/Stub/EmptyRun/.placeholder.php b/tests/Application/Stub/EmptyRun/.placeholder.php deleted file mode 100644 index 72680edf..00000000 --- a/tests/Application/Stub/EmptyRun/.placeholder.php +++ /dev/null @@ -1,10 +0,0 @@ - Date: Sun, 13 Sep 2026 20:07:55 +0400 Subject: [PATCH 08/16] test(error-handler): pin down silenced errors and a handler left installed by the test Four tests fail against the current interceptor. Errors under @ or outside error_reporting() are captured and fail the test, because the handler never consults the reporting mask. A test that installs its own handler without restoring it makes restore_error_handler() pop that handler instead of ours, so ours stays on the stack: it shadows the outer handler after the test and, inside a fiber, keeps capturing sibling errors while the test is suspended. Assisted-By: Claude Fable 5.1 --- .../src/Internal/ErrorHandlerInterceptor.php | 3 +- .../Unit/ErrorHandlerInterceptorTest.php | 90 +++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php b/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php index 501c7d3d..730b4477 100644 --- a/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php +++ b/plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php @@ -27,8 +27,7 @@ { /** * @param bool $failOnError When true, any captured error upgrades a passing test to - * {@see Status::Failed} with the first error wrapped in an - * {@see \ErrorException} as the failure. + * {@see Status::Failed} with the first error wrapped in an {@see \ErrorException} as the failure. */ public function __construct( private bool $failOnError = false, diff --git a/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php b/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php index dd011184..c0a3c732 100644 --- a/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php +++ b/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php @@ -246,6 +246,96 @@ public function restoresTheOuterHandlerWhileSuspendedAndReinstallsItsOwnOnResume } } + public function silencedErrorIsNotCaptured(): void + { + $interceptor = new ErrorHandlerInterceptor(failOnError: true); + $info = self::createTestInfo(); + $next = static function (TestInfo $info): TestResult { + @\trigger_error('silenced', \E_USER_WARNING); + return new TestResult(info: $info, status: Status::Passed); + }; + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Passed); + Assert::null($result->getAttribute(CapturedErrors::class)); + } + + public function errorOutsideErrorReportingIsNotCaptured(): void + { + $interceptor = new ErrorHandlerInterceptor(failOnError: true); + $info = self::createTestInfo(); + $next = static function (TestInfo $info): TestResult { + \trigger_error('deprecated', \E_USER_DEPRECATED); + return new TestResult(info: $info, status: Status::Passed); + }; + + $level = \error_reporting(\E_ALL & ~\E_USER_DEPRECATED); + try { + $result = $interceptor->runTest($info, $next); + } finally { + \error_reporting($level); + } + + Assert::same($result->status, Status::Passed); + Assert::null($result->getAttribute(CapturedErrors::class)); + } + + public function handlerLeftByTestDoesNotShadowOuterHandler(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(); + $next = static function (TestInfo $info): TestResult { + \set_error_handler(static fn(): bool => true); + return new TestResult(info: $info, status: Status::Passed); + }; + + $count = 0; + \set_error_handler(static function () use (&$count): bool { + $count++; + return true; + }); + + try { + $interceptor->runTest($info, $next); + \trigger_error('after test', \E_USER_NOTICE); + } finally { + \restore_error_handler(); + } + + Assert::same($count, 1); + } + + public function handlerLeftByTestDoesNotCaptureSiblingErrorsWhileSuspended(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(); + $next = static function (TestInfo $info): TestResult { + \set_error_handler(static fn(): bool => true); + \Fiber::suspend(); + return new TestResult(info: $info, status: Status::Passed); + }; + + $outerCount = 0; + \set_error_handler(static function () use (&$outerCount): bool { + $outerCount++; + return true; + }); + + try { + $fiber = new \Fiber(static fn(): TestResult => $interceptor->runTest($info, $next)); + $fiber->start(); + + \trigger_error('fired while suspended', \E_USER_NOTICE); + Assert::same($outerCount, 1); + + $fiber->resume(); + Assert::null($fiber->getReturn()->getAttribute(CapturedErrors::class)); + } finally { + \restore_error_handler(); + } + } + private static function createTestInfo(): TestInfo { $reflection = new \ReflectionMethod(self::class, 'createTestInfo'); From de281a9e37b1517975ffe6d88c14f55f2e6ead6f Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Sun, 13 Sep 2026 22:26:17 +0400 Subject: [PATCH 09/16] test(error-handler): specify forwarding, risky handler changes, and the ExpectErrorHandlerChange contract Errors must still reach the handler that was installed before the test. A test that leaves its own handler behind or removes ours is Risky unless it declares the change with the attribute; a declared change that never happens is Risky too. Inside a fiber the handlers a test installed above ours must come back with it on resume. Assisted-By: Claude Fable 5.1 --- .../tests/Stub/HandlerChange.php | 18 ++ .../tests/Stub/HandlerChangeCase.php | 16 ++ .../Unit/ErrorHandlerInterceptorTest.php | 206 +++++++++++++++++- 3 files changed, 237 insertions(+), 3 deletions(-) create mode 100644 plugin/error-handler/tests/Stub/HandlerChange.php create mode 100644 plugin/error-handler/tests/Stub/HandlerChangeCase.php diff --git a/plugin/error-handler/tests/Stub/HandlerChange.php b/plugin/error-handler/tests/Stub/HandlerChange.php new file mode 100644 index 00000000..c960991a --- /dev/null +++ b/plugin/error-handler/tests/Stub/HandlerChange.php @@ -0,0 +1,18 @@ +runTest($info, $next); + } finally { + \restore_error_handler(); + } + + Assert::same($seen, ['forwarded']); + $errors = $result->getAttribute(CapturedErrors::class); + Assert::instanceOf($errors, CapturedErrors::class); + Assert::same($errors->errors[0]->message, 'forwarded'); + } + + public function handlerLeftByTestMarksPassingTestRisky(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(); + $next = static function (TestInfo $info): TestResult { + \set_error_handler(static fn(): bool => true); + return new TestResult(info: $info, status: Status::Passed); + }; + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Risky); + Assert::false($result->messages->isEmpty()); + } + + public function handlerLeftByTestDoesNotOverrideFailedStatus(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(); + $failure = new \RuntimeException('assertion failure'); + $next = static function (TestInfo $info) use ($failure): TestResult { + \set_error_handler(static fn(): bool => true); + return new TestResult(info: $info, status: Status::Failed, failure: $failure); + }; + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Failed); + Assert::same($result->failure, $failure); + } + + public function handlerRemovedByTestMarksPassingTestRiskyAndKeepsOuterHandler(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(); + $next = static function (TestInfo $info): TestResult { + \restore_error_handler(); + return new TestResult(info: $info, status: Status::Passed); + }; + + $count = 0; + \set_error_handler(static function () use (&$count): bool { + $count++; + return true; + }); + + try { + $result = $interceptor->runTest($info, $next); + \trigger_error('after test', \E_USER_NOTICE); + } finally { + \restore_error_handler(); + } + + Assert::same($result->status, Status::Risky); + Assert::same($count, 1); + } + + public function declaredHandlerChangeKeepsPassedStatus(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(new \ReflectionMethod(HandlerChange::class, 'declared')); + $next = static function (TestInfo $info): TestResult { + \set_error_handler(static fn(): bool => true); + return new TestResult(info: $info, status: Status::Passed); + }; + + $count = 0; + \set_error_handler(static function () use (&$count): bool { + $count++; + return true; + }); + + try { + $result = $interceptor->runTest($info, $next); + \trigger_error('after test', \E_USER_NOTICE); + } finally { + \restore_error_handler(); + } + + Assert::same($result->status, Status::Passed); + Assert::same($count, 1); + } + + public function declaredHandlerChangeOnClassAppliesToItsTests(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(new \ReflectionMethod(HandlerChangeCase::class, 'inherited')); + $next = static function (TestInfo $info): TestResult { + \set_error_handler(static fn(): bool => true); + return new TestResult(info: $info, status: Status::Passed); + }; + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Passed); + } + + public function declaredHandlerChangeThatDoesNotHappenIsRisky(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(new \ReflectionMethod(HandlerChange::class, 'declared')); + $next = static fn(TestInfo $info): TestResult => new TestResult(info: $info, status: Status::Passed); + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Risky); + } + + public function undeclaredStubMethodIsHeldToThePlainContract(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(new \ReflectionMethod(HandlerChange::class, 'undeclared')); + $next = static function (TestInfo $info): TestResult { + \set_error_handler(static fn(): bool => true); + return new TestResult(info: $info, status: Status::Passed); + }; + + $result = $interceptor->runTest($info, $next); + + Assert::same($result->status, Status::Risky); + } + + public function handlerInstalledByTestIsBackAfterResume(): void + { + $interceptor = new ErrorHandlerInterceptor(); + $info = self::createTestInfo(new \ReflectionMethod(HandlerChange::class, 'declared')); + + $ownCount = 0; + $next = static function (TestInfo $info) use (&$ownCount): TestResult { + \set_error_handler(static function () use (&$ownCount): bool { + $ownCount++; + return true; + }); + \Fiber::suspend(); + \trigger_error('after resume', \E_USER_NOTICE); + return new TestResult(info: $info, status: Status::Passed); + }; + + $outerCount = 0; + \set_error_handler(static function () use (&$outerCount): bool { + $outerCount++; + return true; + }); + + try { + $fiber = new \Fiber(static fn(): TestResult => $interceptor->runTest($info, $next)); + $fiber->start(); + + \trigger_error('fired while suspended', \E_USER_NOTICE); + Assert::same($outerCount, 1); + Assert::same($ownCount, 0); + + $fiber->resume(); + Assert::same($ownCount, 1); + Assert::same($fiber->getReturn()->status, Status::Passed); + Assert::null($fiber->getReturn()->getAttribute(CapturedErrors::class)); + + \trigger_error('after test', \E_USER_NOTICE); + Assert::same($outerCount, 2); + } finally { + \restore_error_handler(); + } + } + + private static function createTestInfo(?\ReflectionMethod $reflection = null): TestInfo + { + $reflection ??= new \ReflectionMethod(self::class, 'createTestInfo'); + $caseDefinition = new CaseDefinition( + name: 'TestCase', + type: 'test', + file: Path::create(__FILE__), + reflection: $reflection->getDeclaringClass(), + ); $caseInfo = new CaseInfo(definition: $caseDefinition, suiteIdentity: new SuiteIdentity('ErrorHandler/Unit')); $testDefinition = new TestDefinition(reflection: $reflection); From 56fb2e8e9fdef10cd14e5a194ca14c0ff2cf8d24 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Sun, 13 Sep 2026 22:32:52 +0400 Subject: [PATCH 10/16] feat(error-handler): forward errors, honour error_reporting, and police the handler stack per test feat(error-handler): add #[ExpectErrorHandlerChange] Errors under @ or outside error_reporting() are no longer captured, and every error is forwarded to the handler that was installed before the test, so the plugin observes rather than replaces the application's handler. The test's slice of the handler stack (our handler plus anything the test installs above it) leaves with the test on a fiber suspension and comes back on resumption, so a handler the test installed is in place when it continues and absent while a sibling runs. After the test the stack is put back as found; a passing test that changed it is Risky, unless it declares the change with the attribute, in which case an unchanged stack fails the test instead. Assisted-By: Claude Fable 5.1 --- .../src/Exception/ErrorHandlerUnchanged.php | 20 +++ .../src/ExpectErrorHandlerChange.php | 26 +++ .../src/Internal/ErrorHandlerInterceptor.php | 84 ++++++--- .../src/Internal/HandlerScope.php | 168 ++++++++++++++++++ .../Unit/ErrorHandlerInterceptorTest.php | 17 +- 5 files changed, 285 insertions(+), 30 deletions(-) create mode 100644 plugin/error-handler/src/Exception/ErrorHandlerUnchanged.php create mode 100644 plugin/error-handler/src/ExpectErrorHandlerChange.php create mode 100644 plugin/error-handler/src/Internal/HandlerScope.php diff --git a/plugin/error-handler/src/Exception/ErrorHandlerUnchanged.php b/plugin/error-handler/src/Exception/ErrorHandlerUnchanged.php new file mode 100644 index 00000000..da56f6ad --- /dev/null +++ b/plugin/error-handler/src/Exception/ErrorHandlerUnchanged.php @@ -0,0 +1,20 @@ + $errors */ - $errors = []; - - $handler = static function (int $severity, string $message, string $file, int $line) use (&$errors): bool { - $errors[] = new CapturedError($severity, $message, $file, $line); - return true; - }; + $scope = new HandlerScope(); + $result = $this->run($info, $next, $scope); - $result = $this->run($info, $next, $handler); + if ($result->status === Status::Passed) { + $declared = self::declaresHandlerChange($info); + if ($declared && !$scope->changed()) { + $result = $result->with(status: Status::Failed)->withFailure(new ErrorHandlerUnchanged()); + } elseif (!$declared && $scope->changed()) { + $result = $result + ->with(status: Status::Risky) + ->withMessages(new MessageLog([ + ...$result->messages->all(), + new Message(\microtime(true), self::CHANNEL, Level::Warning, $scope->removed() + ? 'Test code or tested code removed error handlers other than its own.' + : 'Test code or tested code did not remove its own error handlers.'), + ])); + } + } - if ($errors === []) { + if ($scope->errors === []) { return $result; } - $result = $result->withAttribute(CapturedErrors::class, new CapturedErrors($errors)); + $result = $result->withAttribute(CapturedErrors::class, new CapturedErrors($scope->errors)); if ($this->failOnError && $result->status === Status::Passed) { - $first = $errors[0]; + $first = $scope->errors[0]; $result = $result ->with(status: Status::Failed) ->withFailure(new \ErrorException($first->message, 0, $first->severity, $first->file, $first->line)); @@ -62,15 +83,32 @@ public function runTest(TestInfo $info, callable $next): TestResult return $result; } + private static function declaresHandlerChange(TestInfo $info): bool + { + if (Reflection::fetchFunctionAttributes( + $info->testDefinition->reflection, + attributeClass: ExpectErrorHandlerChange::class, + ) !== []) { + return true; + } + + $class = $info->caseInfo->definition->reflection; + + return $class !== null && Reflection::fetchClassAttributes( + $class, + attributeClass: ExpectErrorHandlerChange::class, + ) !== []; + } + /** - * The handler stack is process-global. Inside a fiber the handler is removed on every suspension - * and re-installed on resumption, so errors fired by an interleaved sibling test never land here. + * Inside a fiber the scope leaves the stack on every suspension and comes back on resumption, + * so errors fired by an interleaved sibling test never land here. * * @param callable(TestInfo): TestResult $next */ - private function run(TestInfo $info, callable $next, \Closure $handler): TestResult + private function run(TestInfo $info, callable $next, HandlerScope $scope): TestResult { - \set_error_handler($handler); + $scope->install(); try { if (\Fiber::getCurrent() === null) { return $next($info); @@ -79,16 +117,16 @@ private function run(TestInfo $info, callable $next, \Closure $handler): TestRes $fiber = new \Fiber(static fn(): TestResult => $next($info)); $value = $fiber->start(); while (!$fiber->isTerminated()) { - \restore_error_handler(); + $scope->suspend(); try { $resume = \Fiber::suspend($value); } catch (\Throwable $e) { - \set_error_handler($handler); + $scope->resume(); $value = $fiber->throw($e); continue; } - \set_error_handler($handler); + $scope->resume(); $value = $fiber->resume($resume); } @@ -96,7 +134,7 @@ private function run(TestInfo $info, callable $next, \Closure $handler): TestRes $result = $fiber->getReturn(); return $result; } finally { - \restore_error_handler(); + $scope->release(); } } } diff --git a/plugin/error-handler/src/Internal/HandlerScope.php b/plugin/error-handler/src/Internal/HandlerScope.php new file mode 100644 index 00000000..62e9b510 --- /dev/null +++ b/plugin/error-handler/src/Internal/HandlerScope.php @@ -0,0 +1,168 @@ + */ + public array $errors = []; + + private readonly \Closure $handler; + + /** @var list Stack as it was before {@see install()}, bottom first. */ + private array $before = []; + + /** @var list Handlers the test installed above ours, bottom first. */ + private array $above = []; + + private ?\Closure $previous = null; + private bool $removed = false; + private bool $left = false; + + public function __construct() + { + $this->handler = function (int $severity, string $message, string $file, int $line): bool { + (\error_reporting() & $severity) === 0 or $this->errors[] = new CapturedError($severity, $message, $file, $line); + + return $this->previous === null || (bool) ($this->previous)($severity, $message, $file, $line); + }; + } + + public function install(): void + { + $this->before = self::snapshot(); + $previous = \set_error_handler($this->handler); + $this->previous = $previous === null ? null : $previous(...); + } + + /** + * Takes our handler and everything above it off the stack for the time the test is suspended. + */ + public function suspend(): void + { + $stack = self::snapshot(); + $position = self::position($stack, $this->handler); + if ($position === null) { + $this->removed = true; + $this->above = []; + return; + } + + $this->above = \array_slice($stack, $position + 1); + self::pop(\count($stack) - $position); + } + + public function resume(): void + { + if ($this->removed) { + return; + } + + \set_error_handler($this->handler); + self::push($this->above); + } + + /** + * Removes the test's slice and puts the stack back as it was before {@see install()}. + */ + public function release(): void + { + $stack = self::snapshot(); + $position = self::position($stack, $this->handler); + + if ($position === null) { + $this->removed = true; + self::pop(\count($stack)); + self::push($this->before); + return; + } + + $this->left = \count($stack) - $position > 1; + self::pop(\count($stack) - $position); + } + + /** + * Whether the test left the stack different from how it found it: a handler of its own still + * installed, or ours gone. + */ + public function changed(): bool + { + return $this->removed || $this->left; + } + + public function removed(): bool + { + return $this->removed; + } + + /** + * The whole stack, bottom first, put back untouched. + * + * @return list + */ + private static function snapshot(): array + { + $stack = []; + while (true) { + $top = \set_error_handler(static fn(): bool => false); + \restore_error_handler(); + if ($top === null) { + break; + } + + $stack[] = $top; + \restore_error_handler(); + } + + $stack = \array_reverse($stack); + self::push($stack); + + return $stack; + } + + /** + * @param list $stack + */ + private static function position(array $stack, \Closure $handler): ?int + { + foreach ($stack as $i => $entry) { + if ($entry === $handler) { + return $i; + } + } + + return null; + } + + private static function pop(int $count): void + { + for ($i = 0; $i < $count; $i++) { + \restore_error_handler(); + } + } + + /** + * @param list $handlers Bottom first. + */ + private static function push(array $handlers): void + { + foreach ($handlers as $handler) { + \set_error_handler($handler); + } + } +} diff --git a/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php b/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php index 44111f75..75312b4c 100644 --- a/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php +++ b/plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php @@ -16,6 +16,7 @@ use Testo\Core\Value\Status; use Testo\ErrorHandler\CapturedError; use Testo\ErrorHandler\CapturedErrors; +use Testo\ErrorHandler\Exception\ErrorHandlerUnchanged; use Testo\ErrorHandler\ExpectErrorHandlerChange; use Testo\ErrorHandler\Internal\ErrorHandlerInterceptor; use Testo\Test; @@ -215,9 +216,9 @@ public function restoresTheOuterHandlerWhileSuspendedAndReinstallsItsOwnOnResume $interceptor = new ErrorHandlerInterceptor(); $info = self::createTestInfo(); - $outerCount = 0; - \set_error_handler(static function () use (&$outerCount): bool { - $outerCount++; + $outer = []; + \set_error_handler(static function (int $severity, string $message) use (&$outer): bool { + $outer[] = $message; return true; }); @@ -232,9 +233,9 @@ public function restoresTheOuterHandlerWhileSuspendedAndReinstallsItsOwnOnResume $fiber = new \Fiber(static fn(): TestResult => $interceptor->runTest($info, $next)); $fiber->start(); - // Fired while the test is suspended: must reach the outer handler, not the test. + // Fired while the test is suspended: reaches the outer handler directly, not via the test. \trigger_error('fired while suspended', \E_USER_NOTICE); - Assert::same($outerCount, 1); + Assert::same($outer, ['before suspend', 'fired while suspended']); $fiber->resume(); Assert::true($fiber->isTerminated()); @@ -245,6 +246,7 @@ public function restoresTheOuterHandlerWhileSuspendedAndReinstallsItsOwnOnResume Assert::same(\count($errors->errors), 2); Assert::same($errors->errors[0]->message, 'before suspend'); Assert::same($errors->errors[1]->message, 'after resume'); + Assert::same($outer, ['before suspend', 'fired while suspended', 'after resume']); } finally { \restore_error_handler(); } @@ -464,7 +466,7 @@ public function declaredHandlerChangeOnClassAppliesToItsTests(): void Assert::same($result->status, Status::Passed); } - public function declaredHandlerChangeThatDoesNotHappenIsRisky(): void + public function declaredHandlerChangeThatDoesNotHappenFails(): void { $interceptor = new ErrorHandlerInterceptor(); $info = self::createTestInfo(new \ReflectionMethod(HandlerChange::class, 'declared')); @@ -472,7 +474,8 @@ public function declaredHandlerChangeThatDoesNotHappenIsRisky(): void $result = $interceptor->runTest($info, $next); - Assert::same($result->status, Status::Risky); + Assert::same($result->status, Status::Failed); + Assert::instanceOf($result->failure, ErrorHandlerUnchanged::class); } public function undeclaredStubMethodIsHeldToThePlainContract(): void From 28dbbcff8c09895bc8e0f7238602c362a0d16b07 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Sun, 13 Sep 2026 22:43:15 +0400 Subject: [PATCH 11/16] chore(phpunit-mirror): leave the error-handler plugin tests out of the PHPUnit run PHPUnit masks error_reporting() down to fatal levels for the duration of a test, so the interceptor under test treats every triggered error as silenced and the mirrored tests cannot pass by construction. Assisted-By: Claude Fable 5.1 --- tools/phpunit/phpunit.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/phpunit/phpunit.xml b/tools/phpunit/phpunit.xml index 4b56a7fd..904c0ef4 100644 --- a/tools/phpunit/phpunit.xml +++ b/tools/phpunit/phpunit.xml @@ -42,6 +42,11 @@ ../../tests/PhpUnit/*/Stub ../../tests/PhpUnit/*/*/Stub ../../tests/PhpUnit/*/*/*/Stub + + ../../tests/PhpUnit/ErrorHandler