feat(error-handler): add ErrorHandlerInterceptor plugin#262
feat(error-handler): add ErrorHandlerInterceptor plugin#262rossaddison wants to merge 4 commits into
Conversation
…hp-testo#159) When Infection mutates values inside a #[TestInline(arguments: [...], result: ...)] attribute it is mutating test data, not production logic. Every such mutation is "killed" (Assert::same catches the wrong expected value), but the kills are semantically meaningless — they verify that the assertion works, not that the source code handles a mutation correctly. This inflates the mutation score with noise and wastes CI runner time. Add `global-ignoreSourceCodeByRegex` to infection.json so Infection skips any mutation whose source line contains `#[TestInline`. This covers all single-line attribute declarations in both the Self-test fixtures under plugin/inline/tests/Self/ and any production code that uses the attribute. Method bodies on adjacent lines are unaffected and continue to be mutated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…is mirrored 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 <noreply@anthropic.com>
…urceCodeByRegex
Specifying "mutators": {} without "@default" treats the block as an
allowlist, so all default mutators were silently disabled — producing
0 mutations and an MSI failure. Adding "@default": true restores the
full default mutator set while the global regex filter still skips
lines containing #[TestInline.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements the error handler interceptor described in issue php-testo#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 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Implementation summary —
|
| File | Purpose |
|---|---|
src/CapturedError.php |
Value object — severity, message, file, line for one PHP error |
src/Internal/CapturedErrors.php |
Collection stored as a TestResult attribute under CapturedErrors::class |
src/Internal/ErrorHandlerInterceptor.php |
TestRunInterceptor at ORDER_CLOSE_TO_TEST priority — installs/restores handler, attaches attribute, optionally fails the test |
src/ErrorHandlerPlugin.php |
PluginConfigurator users add to their ApplicationConfig |
tests/Unit/ErrorHandlerInterceptorTest.php |
10 unit tests (all passing) |
Behaviour
| Mode | Result |
|---|---|
new ErrorHandlerPlugin() (default) |
Errors collected; test status unchanged; renderers may inspect CapturedErrors attribute |
new ErrorHandlerPlugin(failOnError: true) |
First captured error upgrades a passing test to Status::Failed with an ErrorException; pre-existing Failed/Error results are not overridden |
restore_error_handler() is always called in a finally block — the previous handler is restored even when the test pipeline throws.
Test coverage
| Test | What it verifies |
|---|---|
noErrorsPassesResultThrough |
No errors → result unmodified, no attribute |
capturedErrorIsStoredAsAttribute |
Single error → correct message and severity |
multipleErrorsAreAllCaptured |
All triggered errors appear in order |
collectModePreservesPassingStatus |
Default mode keeps Status::Passed |
failModeUpgradesPassingTestToFailed |
failOnError: true → Status::Failed + ErrorException |
failModeUsesFirstErrorAsFailure |
First error wins as the failure, not the last |
failModeDoesNotOverrideAlreadyFailedTest |
Pre-existing Status::Failed + original throwable preserved |
failModeDoesNotOverrideErrorStatus |
Pre-existing Status::Error preserved |
handlerIsRestoredAfterTestCompletes |
Outer handler is active again after a normal run |
handlerIsRestoredEvenWhenTestThrows |
Outer handler is active again even when the pipeline throws |
Note on zero-param closures in tests
The outer-handler closures in the restoration tests use static function () use (&$count) with no declared parameters. PHP silently discards extra arguments when a callable declares fewer params than the caller passes — standard PHP behaviour — and the zero-param form avoids SonarQube S1172 (unused parameter) without needing suppress annotations or $_-prefix workarounds.
Monorepo wiring
composer.json—require,autoload-dev, path-repository version entrytesto.php— src exclusion +suites.phpinclude.github/workflows/split-publish.yml—error-handler-[0-9]*tag for subtree split on release
There was a problem hiding this comment.
Pull request overview
Adds a new testo/error-handler plugin package that intercepts PHP errors during test execution, records them on TestResult, and optionally turns captured errors into test failures—integrating it into the monorepo (Composer wiring, suite registration, and split-publish tagging).
Changes:
- Introduces
ErrorHandlerInterceptor+ value objects (CapturedError,CapturedErrors) to capture and persist PHP errors raised during a test run. - Adds an
ErrorHandlerPluginconfigurator to register the interceptor (with afailOnErroroption). - Wires the new plugin into the monorepo: root Composer requirement + path version mapping, Testo suites inclusion, Infection config tweak, and split-publish tag pattern.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Application/Stub/EmptyRun/.placeholder.php | Adds an empty fixture file to ensure the stub directory exists in mirrored test layouts. |
| testo.php | Registers the new plugin test suite location and includes its suites.php. |
| plugin/error-handler/tests/Unit/ErrorHandlerInterceptorTest.php | Adds unit tests for capturing/accumulating errors and handler restoration behavior. |
| plugin/error-handler/tests/suites.php | Declares the ErrorHandler unit test suite for Testo’s suite discovery. |
| plugin/error-handler/src/Internal/ErrorHandlerInterceptor.php | Implements the interceptor that installs/restores an error handler and records errors on the result. |
| plugin/error-handler/src/Internal/CapturedErrors.php | Adds the collection type used to store captured errors on TestResult. |
| plugin/error-handler/src/ErrorHandlerPlugin.php | Adds the public plugin configurator that registers the interceptor. |
| plugin/error-handler/src/CapturedError.php | Adds the value object representing a single captured PHP error. |
| plugin/error-handler/composer.json | Introduces the standalone package metadata and autoloading for the new plugin. |
| infection.json | Updates Infection mutator configuration to ignore inline-test source patterns. |
| composer.json | Adds the new plugin to root dependencies and monorepo package version mapping/autoload-dev. |
| .github/workflows/split-publish.yml | Extends subtree split publishing to recognize error-handler-* tags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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)); | ||
| } |
| /** | ||
| * Plugin that captures PHP errors raised during test execution. | ||
| * | ||
| * By default errors are collected and stored as a {@see Internal\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. | ||
| * | ||
| * @api | ||
| */ |
roxblnfk
left a comment
There was a problem hiding this comment.
Thanks for the time you're putting in.
I see you're leaning on AI heavily. So am I — but opening raw AI output as a PR without a human review pass is dismissive of the reviewer's time.
This PR has the same defect as #254: process-global state that isn't fiber-safe. The error-handler stack is process-global and tests can run inside fibers — when a test suspends, its handler stays on the stack while another runs, so errors leak into the wrong test's CapturedErrors, and interleaved resume makes restore_error_handler() pop the wrong frame. The fix is the scope-swap already in MockeryInterceptor::run() / MessengerHub::scope(): inside a fiber, restore the previous handler on suspend and re-install the test's on resume.
A second, open question: a test may change the error handler itself during the run. The interceptor should notice that and react — but the right behavior isn't obvious, so it's worth researching and discussing before implementing.
If you want to contribute to the project, I ask you to:
- carry the fixes from earlier reviews of your PRs into future ones;
- understand the issue first, and ask questions before implementing if you have any;
- validate the AI agent's output yourself. That part is the contributor's, not the reviewer's.
Closes #73
Summary
plugin/error-handler/package (testo/error-handler)ErrorHandlerPlugin— thePluginConfiguratorusers add to theirApplicationConfigErrorHandlerInterceptor— wraps each test inset_error_handler()/restore_error_handler()atORDER_CLOSE_TO_TESTpriorityCapturedError— value object for a single PHP error (severity,message,file,line)CapturedErrors— collection stored as aTestResultattribute underCapturedErrors::classBehaviour
failOnError: false(default)CapturedErrorsattributefailOnError: trueStatus::Failed, wrapping the first error in anErrorException; already-failed/errored tests are not overriddenIn both modes:
restore_error_handler()is called in afinallyblock, so the previous handler is always restored even when the test throwsTests (10, all passing)
noErrorsPassesResultThroughcapturedErrorIsStoredAsAttributemultipleErrorsAreAllCapturedcollectModePreservesPassingStatusStatus::PassedfailModeUpgradesPassingTestToFailedfailOnError: truesetsStatus::Failed+ErrorExceptionfailurefailModeUsesFirstErrorAsFailurefailModeDoesNotOverrideAlreadyFailedTestStatus::Failed+ original throwable are preservedfailModeDoesNotOverrideErrorStatusStatus::Erroris preservedhandlerIsRestoredAfterTestCompleteshandlerIsRestoredEvenWhenTestThrowsMonorepo wiring
composer.json—require,autoload-dev, path-repository version entrytesto.php— src exclusion +suites.phpinclude.github/workflows/split-publish.yml—error-handler-[0-9]*tag for subtree splitNote on
set_error_handlercallback signaturesThe two outer-handler closures in the test use zero parameters (
static function () use (&$count)) rather than(int $errno, string $errstr). PHP silently discards extra arguments when a callable declares fewer params than the caller passes — this is standard PHP behaviour. The zero-param form avoids SonarQube S1172 (unused parameter) without needing@SuppressWarningsor the$_-prefix convention.