test(http): Simplify assertion messages in ApplicationCoreTest for improved clarity and consistency.
#134
Conversation
…improved clarity and consistency.
|
Warning Rate limit exceeded@terabytesoftw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis PR updates tests in tests/http/stateless/ApplicationCoreTest.php: simplifies assertion messages, adjusts expected status codes for a route, and adds/updates Content-Type header assertions for specific routes. No public API signatures are changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ 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 #134 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 318 318
===========================================
Files 12 12
Lines 808 808
===========================================
Hits 808 808 ☔ View full report in Codecov by Sentry. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tests/http/stateless/ApplicationCoreTest.php (4)
59-86: Assertion message simplification looks good; consider trimming redundancy.The shorter messages read cleaner and align with the PR goal. Minor nit: you can drop the explanatory “ensuring PSR-7 … available” suffixes to keep them even tighter, since the asserted condition already communicates intent.
110-116: Content-Type assertions added — consider making them robust to case and parameter order.Header parameter case and ordering can vary by emitter (e.g., “utf-8” vs “UTF-8”, or swapped param order). To avoid brittle tests, prefer a case-insensitive regex match (or parse and compare media type + charset). Example for site/index:
- self::assertSame( - 'application/json; charset=UTF-8', - $response1->getHeaderLine('Content-Type'), - "Expected Content-Type 'application/json; charset=UTF-8' for route 'site/index'.", - ); + self::assertMatchesRegularExpression( + '/^application\/json(?:\s*;\s*charset\s*=\s*utf-8)?$/i', + $response1->getHeaderLine('Content-Type'), + "Expected Content-Type application/json with UTF-8 charset for route 'site/index'.", + );Apply similarly for the text/html assertions.
Also applies to: 154-157, 208-212, 327-331
135-136: Tone/style consistency in messages.For a crisper tone, consider removing quotes around method names and phrases like “confirming … behavior.” The assertion name and condition already convey that. Example: “Adapter instance should be cached across multiple getPsr7Response() calls.”
Also applies to: 189-190, 246-247, 299-301
364-365: Trailing comma in assertion message.Minor punctuation nit: end the message with a period instead of a comma.
- "'@webroot' alias should be set to the parent directory of the test directory after handling a request,", + "'@webroot' alias should be set to the parent directory of the test directory after handling a request.",
📜 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/ApplicationCoreTest.php(12 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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/ApplicationCoreTest.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, 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/ApplicationCoreTest.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/ApplicationCoreTest.php
📚 Learning: 2025-07-22T00:50:26.546Z
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.
Applied to files:
tests/http/stateless/ApplicationCoreTest.php
🔇 Additional comments (1)
tests/http/stateless/ApplicationCoreTest.php (1)
162-166: Statelessness and adapter isolation checks are on-point.Asserting different Response instances per request and different adapters per request aligns with the project’s stateless application model we’ve seen previously. Good coverage here.
Also applies to: 221-225
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary by CodeRabbit