-
-
Notifications
You must be signed in to change notification settings - Fork 0
test(http): Add test for EVENT_AFTER_REQUEST trigger during exception handling in ApplicationErrorHandlerTest.
#163
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
Conversation
… handling in `ApplicationErrorHandlerTest`.
WalkthroughAdds imports and a new test to ApplicationErrorHandlerTest to assert that StatelessApplication emits EVENT_AFTER_REQUEST after handling an exception, and enhances an existing test to assert the HTTP response body contains the rendered exception content. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant A as StatelessApplication
participant EH as ErrorHandler
participant R as Response
C->>A: send HTTP request
A->>A: processing -> exception thrown
A->>EH: handle exception
EH->>R: render exception into response body
EH-->>A: return control
note right of A: emit EVENT_AFTER_REQUEST (post-exception)
A-->>C: send HTTP response (contains rendered exception)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #163 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 362 362
===========================================
Files 13 13
Lines 909 909
===========================================
Hits 909 909 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/http/stateless/ApplicationErrorHandlerTest.php (1)
78-142: Assert single trigger of EVENT_AFTER_REQUESTOptional: count invocations to ensure the event fires exactly once:
@@ - $eventTriggered = false; - $eventName = null; - $eventSender = null; + $eventTriggered = false; + $eventName = null; + $eventSender = null; + $eventCount = 0; @@ - $app->on( + $app->on( StatelessApplication::EVENT_AFTER_REQUEST, - static function (Event $event) use (&$eventTriggered, &$eventName, &$eventSender): void { + static function (Event $event) use (&$eventTriggered, &$eventName, &$eventSender, &$eventCount): void { if ($event->name === StatelessApplication::EVENT_AFTER_REQUEST) { $eventTriggered = true; $eventName = $event->name; $eventSender = $event->sender; + $eventCount++; } }, ); @@ self::assertSame( $app, $eventSender, 'Event sender should be the StatelessApplication instance.', ); + self::assertSame( + 1, + $eventCount, + 'EVENT_AFTER_REQUEST should be triggered exactly once when handling an exception.', + );
YII_DEBUGdefaults totrueintests/bootstrap.php, so debug-mode variance is already controlled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/http/stateless/ApplicationErrorHandlerTest.php(3 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-24T11:52:50.563Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.563Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method (line 408: `$this->response = new Response($this->components['response'] ?? []);`), then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods, so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods (lines 28 and 32), so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-07-20T16:33:57.495Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.495Z
Learning: The TestCase class in yii2-extensions/psr-bridge automatically handles $_SERVER superglobal cleanup by saving its original state before each test and restoring it afterward in setUp() and tearDown() methods. Manual $_SERVER cleanup in individual test methods is unnecessary when extending this TestCase.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-08T15:24:06.085Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/TestCase.php:23-27
Timestamp: 2025-08-08T15:24:06.085Z
Learning: In yii2-extensions/psr-bridge (tests/TestCase.php), maintainer preference: it’s acceptable to use random-looking strings for test-only constants like COOKIE_VALIDATION_KEY; no need to replace with an obviously non-secret value unless CI/secret scanners become problematic.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-06T22:52:05.608Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#64
File: tests/http/StatelessApplicationTest.php:1939-1967
Timestamp: 2025-08-06T22:52:05.608Z
Learning: In yii2-extensions/psr-bridge tests, when testing specific component methods like Request::resolve(), it's necessary to call $app->handle($request) first to initialize all application components before testing the method in isolation. This ensures proper component lifecycle initialization.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-25T16:08:54.379Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#149
File: tests/http/stateless/ApplicationCoreTest.php:24-27
Timestamp: 2025-08-25T16:08:54.379Z
Learning: In the yii2-extensions/psr-bridge project, avoid importing functions that are not actually used in the code, even if they might seem logically related to the functionality being tested.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
🧬 Code graph analysis (1)
tests/http/stateless/ApplicationErrorHandlerTest.php (4)
src/http/Response.php (1)
Response(38-142)src/http/StatelessApplication.php (2)
StatelessApplication(49-555)handle(269-295)tests/TestCase.php (1)
statelessApplication(134-197)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createRequest(68-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (3)
tests/http/stateless/ApplicationErrorHandlerTest.php (3)
8-8: Import of Event is correct and necessary.Used for the listener type-hint; avoids FQCN clutter.
11-11: Importing StatelessApplication is appropriate.Direct constant reference improves clarity over string event names.
265-273: Good addition validating rendered exception content.The contains-check is resilient and consistent with existing normalization usage.
Summary by CodeRabbit