-
-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(tests): Enhance response assertions in StatelessApplicationTest for clarity and consistency.
#43
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
…Test` for clarity and consistency.
WalkthroughA new test was added to verify that Changes
Sequence Diagram(s)sequenceDiagram
participant Test as TestCase
participant EH as ErrorHandler
Test->>EH: Start multiple output buffers
Test->>EH: Call clearOutput()
EH-->>Test: All output buffers cleared
sequenceDiagram
participant Test as TestCase
participant App as StatelessApplication
participant EH as ErrorHandler
Test->>App: Request with invalid error handler action
App->>EH: Handle error
EH-->>App: Generate HTML error response with error details
App-->>Test: Return 200 response with HTML error
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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 #43 +/- ##
============================================
+ Coverage 94.37% 98.07% +3.70%
Complexity 284 284
============================================
Files 12 12
Lines 729 728 -1
============================================
+ Hits 688 714 +26
+ Misses 41 14 -27 ☔ View full report in Codecov by Sentry. |
…`clearOutput()` test.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/http/ErrorHandlerTest.php(1 hunks)tests/http/StatelessApplicationTest.php(35 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
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.
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.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#21
File: tests/http/PSR7ResponseTest.php:0-0
Timestamp: 2025-07-22T00:50:26.546Z
Learning: In yii2-extensions/psr-bridge, the ResponseAdapter::formatCookieHeader() method uses `$expire !== 1` to skip validation for Yii2's special deletion cookies, but this should be extended to handle all expired cookies, not just the special case where expire=1.
tests/http/ErrorHandlerTest.php (3)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
tests/http/StatelessApplicationTest.php (1)
Learnt from: terabytesoftw
PR: #21
File: tests/http/PSR7ResponseTest.php:0-0
Timestamp: 2025-07-22T00:50:26.546Z
Learning: In yii2-extensions/psr-bridge, the ResponseAdapter::formatCookieHeader() method uses $expire !== 1 to skip validation for Yii2's special deletion cookies, but this should be extended to handle all expired cookies, not just the special case where expire=1.
🪛 GitHub Actions: build
tests/http/ErrorHandlerTest.php
[warning] 25-25: Risky test: Test code or tested code closed output buffers other than its own.
tests/http/StatelessApplicationTest.php
[error] 1389-1389: PHPUnit failure: Response 'body' does not contain expected error message '
Not Found: Page not found.' during request handling in StatelessApplication.
[warning] 597-597: Risky test: Test code or tested code closed output buffers other than its own.
[warning] 1364-1364: Risky test: Test code or tested code closed output buffers other than its own.
⏰ 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: phpunit / PHP 8.3-windows-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (3)
tests/http/ErrorHandlerTest.php (1)
15-21: Good addition of required function imports.The new
use functionimports are appropriate for the new test method functionality, including buffer operations and the uopz extension check.tests/http/StatelessApplicationTest.php (2)
65-66: LGTM! Assertion messages standardized for better consistency.The addition of quotes around response attribute names ('status code', 'body', 'content-type') improves message consistency and readability across the test suite.
Also applies to: 120-121, 238-239, 250-251, 276-277, 352-353, 387-388, 420-421, 488-489, 661-662, 691-692, 721-722, 757-758, 764-765, 792-793, 799-800, 852-853, 861-862, 890-891, 900-901, 922-923, 949-950, 971-972, 982-984, 1004-1005, 1032-1033, 1062-1063, 1081-1082, 1121-1122, 1150-1151, 1169-1170, 1380-1381, 1391-1393, 1448-1449, 1467-1468, 1486-1487, 1505-1506
594-639: Ignore the suggested change—keep<pre>wrapper and HTML-escaped quotes
ErrorHandler::handleFallbackExceptionMessage()wraps the message in<pre>and useshtmlspecialchars(..., ENT_QUOTES), so quotes appear as". The existing assertion already matches that output and should remain unchanged.Likely an incorrect or invalid review comment.
…tput buffer handling in `StatelessApplicationTest` class.
Summary by CodeRabbit