test(http): Extract error handler from ApplicationTest to ApplicationErrorHandlerTest class.
#139
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ionErrorHandlerTest` class.
|
Warning Rate limit exceeded@terabytesoftw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 42 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 (2)
WalkthroughAdds a new final PHPUnit test class validating stateless PSR-bridge HTTP error handling, removes overlapping tests from ApplicationTest, and adds two data providers to StatelessApplicationProvider for error-view logic and exception rendering formats. No production code changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant App as StatelessApplication
participant Ctrl as Controller
participant EH as ErrorHandler
participant Resp as Response
participant Logger as Logger
Client->>App: HTTP request (e.g. site/trigger-exception)
App->>Ctrl: dispatch action
Ctrl-->>App: throws Exception/Throwable
App->>EH: handleException(exception)
alt valid errorAction configured
EH->>App: invoke errorAction -> returns view or Response
App->>Resp: produce formatted output (HTML/JSON/RAW)
else fallback (invalid/missing action)
EH->>EH: build fallback output
Note right of EH #f9f9c5: redact sensitive $_SERVER entries in fallback
EH->>Resp: produce fallback formatted output
end
EH->>Logger: log exception (may not flush)
Resp-->>Client: HTTP response (status, Content-Type, body)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 #139 +/- ##
===========================================
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tests/provider/StatelessApplicationProvider.php (2)
115-115: Nit: stray blank lineThe extra blank line after
$cookieWithObjectcreation is non-blocking. If you want to keep diffs tight, consider removing it; otherwise OK.- $cookieWithObject = new stdClass(); - + $cookieWithObject = new stdClass();
193-229: Make HTML expectations resilient to YII_DEBUG stateRight now the HTML case expects 'Stack trace:' unconditionally. That’s only guaranteed when
YII_DEBUGis true; if a test runner toggles it, this data set will cause false negatives. Generate the expected fragments conditionally.Consider this change:
public static function exceptionRenderingFormats(): array { - return [ - 'HTML format with exception' => [ - Response::FORMAT_HTML, - 'text/html; charset=UTF-8', - 500, - 'site/trigger-exception', - [ - Exception::class, - 'Exception error message.', - 'Stack trace:', - ], - ], + $expectedHtmlContent = [ + Exception::class, + 'Exception error message.', + ]; + if (\defined('YII_DEBUG') && YII_DEBUG) { + $expectedHtmlContent[] = 'Stack trace:'; + } + + return [ + 'HTML format with exception' => [ + Response::FORMAT_HTML, + 'text/html; charset=UTF-8', + 500, + 'site/trigger-exception', + $expectedHtmlContent, + ], 'JSON format with exception' => [ Response::FORMAT_JSON, 'application/json; charset=UTF-8', 500, 'site/trigger-exception', ['"message"'], ], 'RAW format with exception' => [ Response::FORMAT_RAW, '', 500, 'site/trigger-exception', [ Exception::class, 'Exception error message.', ], ], ]; }If you’d rather keep the provider static, alternatively assert the 'Stack trace:' fragment in the test only when
YII_DEBUGis true.tests/http/stateless/ApplicationErrorHandlerTest.php (2)
21-103: Relax brittle assertion on $_SERVER labelThe check for the debug dump label includes a leading newline. Templates or formatters can change surrounding whitespace. Match the label without relying on a preceding newline to reduce flakiness.
- self::assertStringContainsString( - "\n\$_SERVER = [", - $body, - "Response body should contain '\$_SERVER = [' in correct order (label before array) for fallback " . - 'exception debug output.', - ); + self::assertStringContainsString( + '$_SERVER = [', + $body, + "Response body should contain '\$_SERVER = [' label in fallback exception debug output.", + );Optionally, to validate redaction rather than omission, you could assert keys are present while values are not, e.g.,
assertStringContainsString('API_KEY', $body)plusassertStringNotContainsString('not-a-secret-api-key', $body). Only if your handler masks values instead of removing keys.
155-166: RAW format checks are appropriateAsserting the absence of
<pre>tags is a good heuristic for raw output. If you want to be more explicit, you could also assert that the Content-Type header is absent via$response->hasHeader('Content-Type') === falseinstead of checking for an empty header line. Optional.
📜 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 (3)
tests/http/stateless/ApplicationErrorHandlerTest.php(1 hunks)tests/http/stateless/ApplicationTest.php(1 hunks)tests/provider/StatelessApplicationProvider.php(3 hunks)
🧰 Additional context used
🧠 Learnings (11)
📚 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.phptests/provider/StatelessApplicationProvider.phptests/http/stateless/ApplicationTest.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.phptests/provider/StatelessApplicationProvider.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/provider/StatelessApplicationProvider.phptests/http/stateless/ApplicationTest.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/ApplicationTest.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/ApplicationTest.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/ApplicationTest.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class configures the runtime path as `dirname(__DIR__) . '/runtime'` in the application configuration. Tests that need to save files to the runtime directory can rely on this pre-configured path.
Applied to files:
tests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-08T15:28:00.166Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/adapter/ServerRequestAdapterTest.php:2215-2215
Timestamp: 2025-08-08T15:28:00.166Z
Learning: In yii2-extensions/psr-bridge, tests extend tests/TestCase which defines a protected const COOKIE_VALIDATION_KEY. Test code should use self::COOKIE_VALIDATION_KEY instead of hardcoded cookieValidationKey literals.
Applied to files:
tests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-08T15:28:00.166Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/adapter/ServerRequestAdapterTest.php:2215-2215
Timestamp: 2025-08-08T15:28:00.166Z
Learning: In yii2-extensions/psr-bridge tests, prefer using self::COOKIE_VALIDATION_KEY from tests/TestCase over hardcoded 'cookieValidationKey' strings to avoid secret scanners FP and improve maintainability.
Applied to files:
tests/http/stateless/ApplicationTest.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/ApplicationTest.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/http/stateless/ApplicationTest.php
🧬 Code graph analysis (3)
tests/http/stateless/ApplicationErrorHandlerTest.php (3)
tests/provider/StatelessApplicationProvider.php (1)
StatelessApplicationProvider(15-322)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createServerRequestCreator(154-161)tests/TestCase.php (2)
TestCase(23-227)statelessApplication(134-197)
tests/provider/StatelessApplicationProvider.php (1)
src/http/Response.php (1)
Response(38-150)
tests/http/stateless/ApplicationTest.php (1)
src/http/StatelessApplication.php (1)
StatelessApplication(49-553)
⏰ 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). (2)
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (6)
tests/http/stateless/ApplicationTest.php (1)
12-12: Import narrowing looks goodDropping the unused
Responseimport and keeping onlyStatelessApplicationreduces noise and reflects the moved coverage. No further changes needed in this file for the extraction.tests/provider/StatelessApplicationProvider.php (2)
8-10: New imports are appropriate and used
yii\base\Exceptionandhttp\Responseare correctly imported for the new exception-format provider. Matches usage below.
13-13: Importing PHP_INT_SIZE is fineUsing
use const PHP_INT_SIZE;is correct and keeps references concise in memory-related providers.tests/http/stateless/ApplicationErrorHandlerTest.php (3)
15-17: Good split: dedicated error handler testsCreating
ApplicationErrorHandlerTestkeepsApplicationTestfocused and makes the error-path matrix easier to grow via a provider. Nice move.
168-180: JSON parsing + shape assertion: nice safety netDecoding and asserting
messageexists ensures the body is valid error JSON beyond simple substring checks. LGTM.
110-147: Data-driven format test: solid coverage with clear expectationsThe provider-driven matrix and explicit assertions for status and Content-Type read well. I’ve confirmed that tests/bootstrap.php defines
YII_DEBUGastrueby default (line 7), so CI will run in debug mode and you shouldn’t encounter HTML-case mismatches. If you’d like to simplify this further, you can adopt the conditional “Stack trace:” change in the provider to remove any debug-mode guards altogether. Otherwise, this test is good to go.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tests/provider/StatelessApplicationProvider.php (2)
193-257: Data provider is good; consider reducing brittleness of HTML equality checksThe dataset shape and messages are clear. One concern: consumers currently do a full-string equality check against the rendered HTML. Minor template whitespace changes could cause fragile failures.
- Optional: Switch the test to assert key fragments (headline text, exception class, and message) rather than full HTML equality.
As a minimal change, you can relax the assertion in
ApplicationErrorHandlerTest::testErrorViewLogic():- self::assertSame( - self::normalizeLineEndings($expectedErrorViewContent), - self::normalizeLineEndings($response->getBody()->getContents()), - $expectedAssertMessage, - ); + $body = self::normalizeLineEndings($response->getBody()->getContents()); + self::assertStringContainsString('Custom error page from errorAction.', $body, $expectedAssertMessage); + self::assertStringContainsString('exception-type', $body, $expectedAssertMessage); + self::assertStringContainsString('exception-message', $body, $expectedAssertMessage);If you prefer to keep the provider driving assertions, we can also reshape the provider to return an array of substrings to check rather than a single HTML blob. Happy to draft that if you want.
259-295: Format datasets look solid; add an Exception+debug case?The format coverage (HTML/JSON/RAW) is clear and pragmatic. For completeness, you might also include a dataset for “HTML format with UserException” and/or “HTML format with Exception under YII_DEBUG=false” to ensure both branches of ErrorHandler logic remain covered across formats.
If helpful, I can add those two rows mirroring the existing structure.
tests/http/stateless/ApplicationErrorHandlerTest.php (2)
70-152: Sensitive-variable redaction checks are good; consider asserting presence of redaction markerThe negative checks ensure secrets don’t leak. If your ErrorHandler replaces values with a well-known token (e.g., '[redacted]' or '***'), asserting for that marker would strengthen the guarantee. If the marker isn’t stable by design, keeping the current absence checks is fine.
I can add conditional assertions for a known marker if you confirm the placeholder string.
159-230: Format matrix is effective; small robustness tweak for RAW Content-TypeThe assertions cover status, Content-Type, and body fragments nicely. For RAW format, some stacks may emit a default 'text/plain' header; the test currently expects an empty header string. If this proves flaky across environments, consider allowing either empty or 'text/plain; charset=UTF-8'.
For the RAW branch:
- self::assertSame( - $expectedContentType, - $response->getHeaderLine('Content-Type'), - "Expected Content-Type '{$expectedContentType}' for route '{$route}'.", - ); + if ($format === Response::FORMAT_RAW) { + self::assertTrue( + $response->getHeaderLine('Content-Type') === '' || + $response->getHeaderLine('Content-Type') === 'text/plain; charset=UTF-8', + "Expected empty or 'text/plain; charset=UTF-8' Content-Type for RAW format.", + ); + } else { + self::assertSame( + $expectedContentType, + $response->getHeaderLine('Content-Type'), + "Expected Content-Type '{$expectedContentType}' for route '{$route}'.", + ); + }
📜 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 (3)
tests/http/stateless/ApplicationErrorHandlerTest.php(1 hunks)tests/http/stateless/ApplicationTest.php(1 hunks)tests/provider/StatelessApplicationProvider.php(3 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
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.
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.
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: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.
📚 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.phptests/http/stateless/ApplicationTest.phptests/provider/StatelessApplicationProvider.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.phptests/provider/StatelessApplicationProvider.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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.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/ApplicationTest.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/ApplicationTest.phptests/provider/StatelessApplicationProvider.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class configures the runtime path as `dirname(__DIR__) . '/runtime'` in the application configuration. Tests that need to save files to the runtime directory can rely on this pre-configured path.
Applied to files:
tests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-08T15:28:00.166Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/adapter/ServerRequestAdapterTest.php:2215-2215
Timestamp: 2025-08-08T15:28:00.166Z
Learning: In yii2-extensions/psr-bridge, tests extend tests/TestCase which defines a protected const COOKIE_VALIDATION_KEY. Test code should use self::COOKIE_VALIDATION_KEY instead of hardcoded cookieValidationKey literals.
Applied to files:
tests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-08T15:28:00.166Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/adapter/ServerRequestAdapterTest.php:2215-2215
Timestamp: 2025-08-08T15:28:00.166Z
Learning: In yii2-extensions/psr-bridge tests, prefer using self::COOKIE_VALIDATION_KEY from tests/TestCase over hardcoded 'cookieValidationKey' strings to avoid secret scanners FP and improve maintainability.
Applied to files:
tests/http/stateless/ApplicationTest.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/ApplicationTest.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/http/stateless/ApplicationTest.php
🧬 Code graph analysis (3)
tests/http/stateless/ApplicationErrorHandlerTest.php (3)
tests/provider/StatelessApplicationProvider.php (1)
StatelessApplicationProvider(15-388)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createServerRequestCreator(154-161)tests/TestCase.php (2)
TestCase(23-227)statelessApplication(134-197)
tests/http/stateless/ApplicationTest.php (1)
src/http/StatelessApplication.php (1)
StatelessApplication(49-553)
tests/provider/StatelessApplicationProvider.php (1)
src/http/Response.php (1)
Response(38-150)
⏰ 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). (2)
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (5)
tests/http/stateless/ApplicationTest.php (1)
12-12: Import cleanup looks correctDropping the unused
Responseimport and keeping onlyStatelessApplicationreduces coupling and avoids IDE/static-analysis warnings. No further action.tests/provider/StatelessApplicationProvider.php (3)
8-10: New imports are appropriate and used
yii\base\Exceptionandhttp\Responseare correctly introduced for the new data providers.
13-14: Constant import aligns with usage
use const PHP_INT_SIZE;matches the later conditional dataset inmemoryLimitPositive().
115-115: Whitespace-only changeNo functional impact.
tests/http/stateless/ApplicationErrorHandlerTest.php (1)
7-17: Test scaffolding and imports are tidyAttributes and dependencies are minimal and focused; test class is properly grouped under 'http'.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/http/stateless/ApplicationTest.php (1)
126-131: Fix inverted output-buffer restoration to avoid leaking buffers across testsThe loops add buffers when fewer are present; they should close extra buffers when more are present. Current logic can skew global OB state and cause subtle follow-up test issues.
Apply:
- while (ob_get_level() < $initialBufferLevel) { - ob_start(); - } + while (ob_get_level() > $initialBufferLevel) { + @ob_end_clean(); + }and later:
- while (ob_get_level() < $initialBufferLevel) { - ob_start(); - } + while (ob_get_level() > $initialBufferLevel) { + @ob_end_clean(); + }Also applies to: 179-181
♻️ Duplicate comments (1)
tests/http/stateless/ApplicationErrorHandlerTest.php (1)
35-35: Restore YII_DEBUG via try/finally for safetyCapture the previous value and restore it in a finally block to avoid cross-test state leakage if an assertion fails. You noted YII_DEBUG is always defined; this is purely defensive.
Apply:
- @\runkit_constant_redefine('YII_DEBUG', $debug); + $prevDebug = \YII_DEBUG; + @\runkit_constant_redefine('YII_DEBUG', $debug); + try { @@ - @\runkit_constant_redefine('YII_DEBUG', true); + } finally { + @\runkit_constant_redefine('YII_DEBUG', $prevDebug); + }Also applies to: 68-69
🧹 Nitpick comments (5)
tests/http/stateless/ApplicationTest.php (1)
140-184: Wrap YII_ENV_TEST redefinition in try/finally to prevent cross-test leakage on failureIf an assertion fails mid-test, YII_ENV_TEST might stay false. Guard with try/finally and restore the previous value.
Apply:
- @\runkit_constant_redefine('YII_ENV_TEST', false); + $prevEnvTest = \YII_ENV_TEST; + @\runkit_constant_redefine('YII_ENV_TEST', false); + try { @@ - ini_set('display_errors', $originalDisplayErrors); - - while (ob_get_level() < $initialBufferLevel) { - ob_start(); - } - - @\runkit_constant_redefine('YII_ENV_TEST', true); + ini_set('display_errors', $originalDisplayErrors); + while (ob_get_level() > $initialBufferLevel) { + @ob_end_clean(); + } + } finally { + @\runkit_constant_redefine('YII_ENV_TEST', $prevEnvTest); + }tests/http/stateless/ApplicationErrorHandlerTest.php (4)
62-66: Avoid brittle full-body equality for HTML error viewAsserting the entire HTML body is fragile to harmless whitespace/formatting changes. Use substring containment for stability.
Apply:
- self::assertSame( - self::normalizeLineEndings($expectedErrorViewContent), - self::normalizeLineEndings($response->getBody()->getContents()), - $expectedAssertMessage, - ); + self::assertStringContainsString( + self::normalizeLineEndings($expectedErrorViewContent), + self::normalizeLineEndings($response->getBody()->getContents()), + $expectedAssertMessage, + );
187-191: Remove duplicate status-code assertionThe 500 status is asserted twice. Keep one to reduce noise.
Apply:
- self::assertSame( - 500, - $response->getStatusCode(), - "Response 'status code' should be '500' when an exception occurs.", - );Also applies to: 200-204
273-278: Prefer asserting header absence for RAW formatFor RAW, asserting an empty Content-Type string is indirect. Checking header absence is clearer and avoids coupling to header formatting.
Apply:
- self::assertSame( - $expectedContentType, - $response->getHeaderLine('Content-Type'), - "Expected Content-Type '{$expectedContentType}' for route '{$route}'.", - ); + if ($expectedContentType === '') { + self::assertFalse( + $response->hasHeader('Content-Type'), + 'Content-Type header should be absent for RAW format.', + ); + } else { + self::assertSame( + $expectedContentType, + $response->getHeaderLine('Content-Type'), + "Expected Content-Type '{$expectedContentType}' for route '{$route}'.", + ); + }
282-288: Minor duplication in JSON expectationsYou both substring-check for '"message"' and then decode and assert the 'message' key exists. The decode-based assertion is sufficient; consider removing the substring check for JSON or drop '"message"' from the data provider.
Also applies to: 303-315
📜 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 (2)
tests/http/stateless/ApplicationErrorHandlerTest.php(1 hunks)tests/http/stateless/ApplicationTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
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.
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.
📚 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/ApplicationTest.phptests/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/ApplicationTest.phptests/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/ApplicationTest.phptests/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/ApplicationTest.phptests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class configures the runtime path as `dirname(__DIR__) . '/runtime'` in the application configuration. Tests that need to save files to the runtime directory can rely on this pre-configured path.
Applied to files:
tests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-08T15:28:00.166Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/adapter/ServerRequestAdapterTest.php:2215-2215
Timestamp: 2025-08-08T15:28:00.166Z
Learning: In yii2-extensions/psr-bridge, tests extend tests/TestCase which defines a protected const COOKIE_VALIDATION_KEY. Test code should use self::COOKIE_VALIDATION_KEY instead of hardcoded cookieValidationKey literals.
Applied to files:
tests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-08T15:28:00.166Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/adapter/ServerRequestAdapterTest.php:2215-2215
Timestamp: 2025-08-08T15:28:00.166Z
Learning: In yii2-extensions/psr-bridge tests, prefer using self::COOKIE_VALIDATION_KEY from tests/TestCase over hardcoded 'cookieValidationKey' strings to avoid secret scanners FP and improve maintainability.
Applied to files:
tests/http/stateless/ApplicationTest.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/ApplicationTest.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/http/stateless/ApplicationTest.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/ApplicationTest.phptests/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-08-23T23:29:18.577Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#139
File: tests/http/stateless/ApplicationErrorHandlerTest.php:21-65
Timestamp: 2025-08-23T23:29:18.577Z
Learning: In Yii2, the YII_DEBUG constant is always defined, so checking if it's defined before accessing it is unnecessary. The constant can be safely redefined using runkit7 without defensive checks.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
🧬 Code graph analysis (2)
tests/http/stateless/ApplicationTest.php (1)
src/http/StatelessApplication.php (1)
StatelessApplication(49-553)
tests/http/stateless/ApplicationErrorHandlerTest.php (5)
src/http/Response.php (1)
Response(38-150)tests/provider/StatelessApplicationProvider.php (1)
StatelessApplicationProvider(15-388)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createServerRequestCreator(154-161)tests/TestCase.php (2)
TestCase(23-227)statelessApplication(134-197)src/http/StatelessApplication.php (1)
handle(269-295)
⏰ 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.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (1)
tests/http/stateless/ApplicationTest.php (1)
11-11: Import narrowing looks correct.Limiting the import to StatelessApplication aligns with the refactor that moved error-rendering assertions out of this class. No issues here.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/http/stateless/ApplicationErrorHandlerTest.php (1)
30-74: Guard YII_DEBUG with try/finally; relax brittle full-body equality
- Restore YII_DEBUG via try/finally to avoid cross-test leakage on early assertion failures.
- Optional: Use contains instead of full-body equality; HTML error views are brittle to incidental whitespace/format changes.
- @\runkit_constant_redefine('YII_DEBUG', $debug); + $prevDebug = \YII_DEBUG; + @\runkit_constant_redefine('YII_DEBUG', $debug); + try { @@ - self::assertSame( - self::normalizeLineEndings($expectedErrorViewContent), - self::normalizeLineEndings($response->getBody()->getContents()), - $expectedAssertMessage, - ); + self::assertStringContainsString( + self::normalizeLineEndings($expectedErrorViewContent), + self::normalizeLineEndings($response->getBody()->getContents()), + $expectedAssertMessage, + ); @@ - @\runkit_constant_redefine('YII_DEBUG', true); + } finally { + @\runkit_constant_redefine('YII_DEBUG', $prevDebug); + }
🧹 Nitpick comments (1)
tests/http/stateless/ApplicationErrorHandlerTest.php (1)
345-416: Format-specific error rendering assertions — minor nitOptional explicitness: pass the $asArray flag to Json::decode to future-proof against signature defaults.
- $decodedResponse = Json::decode($body); + $decodedResponse = Json::decode($body, true);
📜 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 (2)
tests/http/stateless/ApplicationErrorHandlerTest.php(1 hunks)tests/http/stateless/ApplicationTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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.
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.
📚 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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-23T23:29:18.577Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#139
File: tests/http/stateless/ApplicationErrorHandlerTest.php:21-65
Timestamp: 2025-08-23T23:29:18.577Z
Learning: In Yii2, the YII_DEBUG constant is always defined, so checking if it's defined before accessing it is unnecessary. The constant can be safely redefined using runkit7 without defensive checks.
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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.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/ApplicationTest.php
🧬 Code graph analysis (2)
tests/http/stateless/ApplicationErrorHandlerTest.php (5)
src/http/Response.php (1)
Response(38-150)tests/provider/StatelessApplicationProvider.php (1)
StatelessApplicationProvider(15-388)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createServerRequestCreator(154-161)tests/TestCase.php (2)
TestCase(23-227)statelessApplication(134-197)src/http/StatelessApplication.php (1)
handle(269-295)
tests/http/stateless/ApplicationTest.php (1)
src/http/StatelessApplication.php (1)
StatelessApplication(49-553)
⏰ 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)
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: phpunit / PHP 8.2-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (3)
tests/http/stateless/ApplicationTest.php (1)
8-8: Imports streamlined — LGTMNarrowing imports and explicitly importing StatelessApplication aligns with the test scope after extracting error-handler tests. No concerns.
Also applies to: 11-11
tests/http/stateless/ApplicationErrorHandlerTest.php (2)
79-160: Sensitive $_SERVER redaction test covers key cases — LGTMGood coverage of redaction vs. safe values in fallback view. Conditional assertions under YII_DEBUG are appropriate.
165-239: Exception logging assertions — LGTMAsserting in-memory Logger messages with flushLogger=false is effective and avoids FS coupling. Category/message checks are precise.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
tests/http/stateless/ApplicationErrorHandlerTest.php (4)
42-42: Restore YII_DEBUG to its previous value (avoid cross-test state leakage)Instead of hardcoding YII_DEBUG back to true, restore whatever the previous value was. This keeps the suite resilient if the bootstrap changes the default or if other tests intentionally toggle it.
- @\runkit_constant_redefine('YII_DEBUG', $debug); + $prevDebug = \YII_DEBUG; + @\runkit_constant_redefine('YII_DEBUG', $debug); @@ - @\runkit_constant_redefine('YII_DEBUG', true); + @\runkit_constant_redefine('YII_DEBUG', $prevDebug);Also applies to: 75-76
485-486: Also restore YII_DEBUG to previous value in the “Response object from errorAction” testSame rationale as earlier: avoid relying on a hardcoded true postcondition.
- @\runkit_constant_redefine('YII_DEBUG', false); + $prevDebug = \YII_DEBUG; + @\runkit_constant_redefine('YII_DEBUG', false); @@ - @\runkit_constant_redefine('YII_DEBUG', true); + @\runkit_constant_redefine('YII_DEBUG', $prevDebug);Also applies to: 526-527
246-256: Fix output buffer restoration and restore YII_ENV_TEST to previous value
- The cleanup currently increases the buffer level when it’s lower; it should close extra buffers when it’s higher.
- Don’t hardcode YII_ENV_TEST to true; restore the previous value captured before redefining it.
- Add the missing import for ob_end_clean for clarity.
This prevents buffer leaks across tests and ensures environment flags are faithfully restored.
@@ -use function array_filter; +use function array_filter; +use function ob_end_clean; @@ - @\runkit_constant_redefine('YII_ENV_TEST', false); + $prevEnvTest = \YII_ENV_TEST; + @\runkit_constant_redefine('YII_ENV_TEST', false); @@ - restore_error_handler(); - - while (ob_get_level() < $initialBufferLevel) { - ob_start(); - } - - @\runkit_constant_redefine('YII_ENV_TEST', true); + restore_error_handler(); + + while (ob_get_level() > $initialBufferLevel) { + ob_end_clean(); + } + + @\runkit_constant_redefine('YII_ENV_TEST', $prevEnvTest);Also applies to: 331-339, 16-16
348-352: Mirror the same buffer/env restoration fix in display_errors testApply the same corrections here: capture and restore YII_ENV_TEST, and close extra buffers instead of opening new ones.
- @\runkit_constant_redefine('YII_ENV_TEST', false); + $prevEnvTest = \YII_ENV_TEST; + @\runkit_constant_redefine('YII_ENV_TEST', false); @@ - ini_set('display_errors', $originalDisplayErrors); - - while (ob_get_level() < $initialBufferLevel) { - ob_start(); - } - - @\runkit_constant_redefine('YII_ENV_TEST', true); + ini_set('display_errors', $originalDisplayErrors); + + while (ob_get_level() > $initialBufferLevel) { + ob_end_clean(); + } + + @\runkit_constant_redefine('YII_ENV_TEST', $prevEnvTest);Also applies to: 391-398
🧹 Nitpick comments (2)
tests/http/stateless/ApplicationErrorHandlerTest.php (2)
64-73: Reduce brittleness: prefer substring/assertContains over full-body equalityFull-body HTML equality tends to be fragile (whitespace, minor formatting). Consider asserting key fragments instead. If you keep strict equality, no change needed; this is a test-hardening suggestion.
- self::assertSame( - self::normalizeLineEndings($expectedErrorViewContent), - self::normalizeLineEndings($response->getBody()->getContents()), - $expectedAssertMessage, - ); + self::assertStringContainsString( + self::normalizeLineEndings(trim($expectedErrorViewContent)), + self::normalizeLineEndings($response->getBody()->getContents()), + $expectedAssertMessage, + );
205-239: Nice verification of in-memory logging; minor robustness tweak optionalGood call keeping flushLogger=false and inspecting Logger::messages. If you want to harden the assertion further, also assert the message level and category counts to protect against unrelated errors populating the buffer. No change required.
📜 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 (2)
tests/http/stateless/ApplicationErrorHandlerTest.php(1 hunks)tests/http/stateless/ApplicationTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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.
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.
📚 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.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-23T23:29:18.577Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#139
File: tests/http/stateless/ApplicationErrorHandlerTest.php:21-65
Timestamp: 2025-08-23T23:29:18.577Z
Learning: In Yii2, the YII_DEBUG constant is always defined, so checking if it's defined before accessing it is unnecessary. The constant can be safely redefined using runkit7 without defensive checks.
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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-13T23:36:57.972Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#90
File: tests/http/StatelessApplicationTest.php:1375-1378
Timestamp: 2025-08-13T23:36:57.972Z
Learning: In yii2-extensions/psr-bridge, unserialize() calls should follow the same pattern as Yii2 core framework (using unserialize($data) without allowed_classes restriction) to maintain consistency with the upstream framework's security model and avoid compatibility issues.
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/ApplicationTest.php
🧬 Code graph analysis (2)
tests/http/stateless/ApplicationErrorHandlerTest.php (4)
tests/provider/StatelessApplicationProvider.php (1)
StatelessApplicationProvider(15-388)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createServerRequestCreator(154-161)tests/TestCase.php (2)
TestCase(23-227)statelessApplication(134-197)src/http/StatelessApplication.php (1)
handle(269-295)
tests/http/stateless/ApplicationTest.php (2)
tests/http/stateless/ApplicationErrorHandlerTest.php (1)
Group(26-528)src/http/StatelessApplication.php (1)
StatelessApplication(49-553)
⏰ 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.2-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (1)
tests/http/stateless/ApplicationTest.php (1)
7-12: LGTM: imports tightened and class finalization align with the test splitThe narrowed imports and keeping only StatelessApplication here are consistent with moving error-handling assertions to ApplicationErrorHandlerTest. No issues spotted.
…ErrorHandlerTest` for better organization.
…dRouteIsMissing` to `ApplicationErrorHandlerTest` for better organization.
…RouteIsMissing` from `ApplicationTest` to `ApplicationErrorHandlerTest` for better organization.
…se application properly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
tests/http/stateless/ApplicationErrorHandlerTest.php (3)
44-78: Restore YII_DEBUG via try/finally to avoid cross-test leakage (optional)Wrap YII_DEBUG redefinition in try/finally and restore the prior value. Prevents leakage if an assertion throws mid-test.
Apply:
- @\runkit_constant_redefine('YII_DEBUG', $debug); + $prevDebug = \YII_DEBUG; + @\runkit_constant_redefine('YII_DEBUG', $debug); + try { @@ - @\runkit_constant_redefine('YII_DEBUG', true); + } finally { + @\runkit_constant_redefine('YII_DEBUG', $prevDebug); + }
250-252: Fix output-buffer restoration and restore YII_ENV_TEST to previous value
- Don’t open new buffers to “reach” the initial level; close extras if the level grew.
- Preserve and restore the prior YII_ENV_TEST value instead of hardcoding true.
This avoids buffer leaks and keeps ErrorHandler’s min-level logic consistent across the suite.
Apply:
- @\runkit_constant_redefine('YII_ENV_TEST', false); + $prevEnvTest = \YII_ENV_TEST; + @\runkit_constant_redefine('YII_ENV_TEST', false); @@ - } finally { + } finally { restore_error_handler(); - - while (ob_get_level() < $initialBufferLevel) { - ob_start(); - } - - @\runkit_constant_redefine('YII_ENV_TEST', true); + while (ob_get_level() > $initialBufferLevel) { + ob_end_clean(); + } + @\runkit_constant_redefine('YII_ENV_TEST', $prevEnvTest); }Also applies to: 333-341
18-27: Add missing import for ob_end_clean used in buffer restoration fixYou’re restoring buffer levels in finally blocks. To close surplus buffers correctly (see next comments), you’ll need ob_end_clean imported alongside ob_get_level/ob_start.
Apply:
use function array_filter; use function ini_get; use function ini_set; use function is_array; +use function ob_end_clean; use function ob_get_level; use function ob_start; use function restore_error_handler; use function set_error_handler; use function str_contains;
🧹 Nitpick comments (2)
tests/http/stateless/ApplicationErrorHandlerTest.php (2)
517-526: Reduce brittleness: use contains instead of full-body equalityFull-body equality can be fragile to harmless template changes. Asserting a stable substring keeps intent while reducing maintenance churn.
Apply:
- self::assertSame( - self::normalizeLineEndings( - <<<HTML - <div id="custom-response-error"> - Custom Response object from error action: Exception error message. - </div> - HTML, - ), - self::normalizeLineEndings($response->getBody()->getContents()), - "Response body should contain content from Response object returned by 'errorAction'.", - ); + self::assertStringContainsString( + 'Custom Response object from error action: Exception error message.', + $response->getBody()->getContents(), + "Response body should contain content from Response object returned by 'errorAction'.", + );
487-529: Optionally guard YII_DEBUG redefinition with try/finally here tooSame rationale as earlier: avoid leaking YII_DEBUG=false if an assertion fails midway.
Apply:
- @\runkit_constant_redefine('YII_DEBUG', false); + $prevDebug = \YII_DEBUG; + @\runkit_constant_redefine('YII_DEBUG', false); @@ - @\runkit_constant_redefine('YII_DEBUG', true); + @\runkit_constant_redefine('YII_DEBUG', $prevDebug);
📜 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 (2)
tests/http/stateless/ApplicationErrorHandlerTest.php(1 hunks)tests/http/stateless/ApplicationTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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.
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.
📚 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.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-23T23:29:18.577Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#139
File: tests/http/stateless/ApplicationErrorHandlerTest.php:21-65
Timestamp: 2025-08-23T23:29:18.577Z
Learning: In Yii2, the YII_DEBUG constant is always defined, so checking if it's defined before accessing it is unnecessary. The constant can be safely redefined using runkit7 without defensive checks.
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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.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.phptests/http/stateless/ApplicationTest.php
📚 Learning: 2025-08-13T23:36:57.972Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#90
File: tests/http/StatelessApplicationTest.php:1375-1378
Timestamp: 2025-08-13T23:36:57.972Z
Learning: In yii2-extensions/psr-bridge, unserialize() calls should follow the same pattern as Yii2 core framework (using unserialize($data) without allowed_classes restriction) to maintain consistency with the upstream framework's security model and avoid compatibility issues.
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/ApplicationTest.php
🧬 Code graph analysis (2)
tests/http/stateless/ApplicationErrorHandlerTest.php (7)
src/http/Response.php (1)
Response(38-150)tests/provider/StatelessApplicationProvider.php (1)
StatelessApplicationProvider(15-388)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createServerRequestCreator(154-161)tests/TestCase.php (2)
TestCase(23-227)statelessApplication(134-197)src/http/StatelessApplication.php (1)
handle(269-295)src/exception/Message.php (1)
getMessage(223-226)src/http/Request.php (1)
resolve(767-787)
tests/http/stateless/ApplicationTest.php (2)
tests/http/stateless/ApplicationErrorHandlerTest.php (1)
Group(28-663)src/http/StatelessApplication.php (1)
StatelessApplication(49-553)
⏰ 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)
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (3)
tests/http/stateless/ApplicationTest.php (1)
7-9: Import cleanup aligns with extracted responsibilitiesNice trim. Only keeping Group, TestWith, InvalidConfigException, and StatelessApplication matches the file’s new scope (non-error-handling). No unused imports left behind.
tests/http/stateless/ApplicationErrorHandlerTest.php (2)
176-242: Logging assertions are robust and isolatedSetting flushLogger => false and asserting in-memory messages prevents premature flush and cross-test coupling. Category/message matching is precise. Good.
28-30: Good separation of concerns: extracted suite is focused and data-drivenThe new final test class consolidates error-handling scenarios and leverages providers. Clearer intent, smaller ApplicationTest, and better maintainability.
…plicationErrorHandlerTest` to `ApplicationTest` for better organization..
This was referenced Aug 25, 2025
This was referenced Aug 25, 2025
Closed
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit