test(http): Enhance ErrorHandlerTest class with output buffer handling and mocking support.
#144
Conversation
…ing and mocking support.
|
Warning Rate limit exceeded@terabytesoftw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 54 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)
WalkthroughRemoves coverage-ignore pragmas around the ob_clean() fallback in Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test harness / MockerFunctions
participant EH as ErrorHandler::clearOutput()
participant PHP as PHP output buffers
Note over Test,EH: Tests may register a mock for ob_end_clean()
EH->>PHP: call ob_end_clean()
alt ob_end_clean() succeeds
PHP-->>EH: returns true (buffer closed)
else ob_end_clean() fails
PHP-->>EH: returns false
loop while buffers > target
EH->>PHP: call ob_clean()
PHP-->>EH: cleaned one buffer
end
end
Note right of EH: tests assert buffer level and restore env/INI
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 #144 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 318 318
===========================================
Files 12 12
Lines 808 809 +1
===========================================
+ Hits 808 809 +1 ☔ 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: 2
🧹 Nitpick comments (2)
tests/http/ErrorHandlerTest.php (2)
23-49: Either remove this duplicate non-test clearOutput() check or harden it with try/finallyThis method overlaps with testClearOutputCleansAllBuffersInNonTestEnvironment(). If you keep it, wrap the YII_ENV_TEST toggling and buffer restoration in try/finally to avoid leaving the suite in a bad state on failure.
Option A — remove the duplicate:
- #[RequiresPhpExtension('runkit7')] - public function test(): void - { - @\runkit_constant_redefine('YII_ENV_TEST', false); - - $initialBufferLevel = ob_get_level(); - - ob_start(); - ob_start(); - - $errorHandler = new ErrorHandler(['discardExistingOutput' => true]); - - $errorHandler->clearOutput(); - - self::assertSame( - 0, - ob_get_level(), - "All output buffers should be cleared to level '0' in non-test environment.", - ); - - while (ob_get_level() < $initialBufferLevel) { - ob_start(); - } - - @\runkit_constant_redefine('YII_ENV_TEST', true); - }Option B — keep it but add safety:
#[RequiresPhpExtension('runkit7')] - public function test(): void + public function testClearOutputCleansAllBuffersInNonTestEnvironment_ShortCase(): void { - @\runkit_constant_redefine('YII_ENV_TEST', false); - - $initialBufferLevel = ob_get_level(); + $initialBufferLevel = ob_get_level(); + try { + @\runkit_constant_redefine('YII_ENV_TEST', false); ob_start(); ob_start(); $errorHandler = new ErrorHandler(['discardExistingOutput' => true]); $errorHandler->clearOutput(); self::assertSame( 0, ob_get_level(), "All output buffers should be cleared to level '0' in non-test environment.", ); - - while (ob_get_level() < $initialBufferLevel) { - ob_start(); - } - - @\runkit_constant_redefine('YII_ENV_TEST', true); + } finally { + while (ob_get_level() < $initialBufferLevel) { + ob_start(); + } + @\runkit_constant_redefine('YII_ENV_TEST', true); + } }
50-72: Ensure stub state is restored after simulating ob_end_clean failureThe test resets output buffers but leaves the failure flag enabled. If a global reset isn’t guaranteed per-test, this can bleed into other tests. Explicitly restore here for safety.
Apply this diff:
} finally { while (ob_get_level() > 1) { @ob_end_clean(); } + // Restore default behavior to avoid test-order dependencies. + MockerFunctions::setObEndCleanShouldFail(false); }
📜 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 (6)
src/http/ErrorHandler.php(0 hunks)tests/http/ErrorHandlerTest.php(3 hunks)tests/http/stateless/ApplicationErrorHandlerTest.php(0 hunks)tests/http/stateless/ApplicationTest.php(1 hunks)tests/support/MockerExtension.php(1 hunks)tests/support/stub/MockerFunctions.php(4 hunks)
💤 Files with no reviewable changes (2)
- src/http/ErrorHandler.php
- tests/http/stateless/ApplicationErrorHandlerTest.php
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-08-24T11:52:50.524Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.524Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/stateless/ApplicationTest.phptests/http/ErrorHandlerTest.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.phptests/http/ErrorHandlerTest.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/ErrorHandlerTest.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/ErrorHandlerTest.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/ErrorHandlerTest.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-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.phptests/http/ErrorHandlerTest.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 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-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.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/ErrorHandlerTest.php
🧬 Code graph analysis (2)
tests/support/MockerExtension.php (1)
tests/support/stub/MockerFunctions.php (2)
MockerFunctions(44-277)ob_end_clean(202-211)
tests/http/ErrorHandlerTest.php (2)
tests/support/stub/MockerFunctions.php (3)
MockerFunctions(44-277)setObEndCleanShouldFail(251-254)ob_end_clean(202-211)src/http/ErrorHandler.php (2)
ErrorHandler(39-262)clearOutput(81-94)
⏰ 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/support/stub/MockerFunctions.php (2)
87-95: State fields for ob_end_clean mocking: good additionThe new call counter and failure flag are clear and scoped properly for test control. No issues with visibility or typing.
221-223: Reset logic includes ob_end_clean stateResetting both the counter and the failure flag here is correct and necessary for test isolation. LGTM.
tests/support/MockerExtension.php (2)
88-92: New ob_end_clean mock: correct namespace and signatureMapping yii2\extensions\psrbridge\http\ob_end_clean() to the stub is correct and matches ErrorHandler’s calling namespace. Return type and arity are accurate.
28-33: Remove redundant stub reset in the extensionUpon inspection, the static stub‐state is already being cleared in your test harness:
• In tests/TestCase.php, the base setUp() invokes
MockerFunctions::reset()on line 48, ensuring that any flags (e.g. setObEndCleanShouldFail) are reset before every test.Because of this central reset, adding another call to
MockerFunctions::reset()inside thePreparationStartedSubscriberis unnecessary. You can safely ignore the suggested diff to modify MockerExtension.php.Likely an incorrect or invalid review comment.
tests/http/stateless/ApplicationTest.php (1)
7-7: Import cleanup aligns with removed RunKit-dependent testDropping RequiresPhpExtension import is consistent with the removal of the RunKit-based test. No issues.
tests/http/ErrorHandlerTest.php (1)
80-112: Non-test environment clearOutput(): robust cleanup and restorationUsing try/finally to restore YII_ENV_TEST and PHPUnit’s buffer level is correct. The assertions align with ErrorHandler::clearOutput() semantics. LGTM.
…ce MockerFunctions for `ob_end_clean` control.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/http/stateless/ApplicationErrorHandlerTest.php (2)
404-408: Consider a more precise assertion for buffer level validation.The current assertion checks that buffers are cleared but the message "'clearOutput()' should properly clean output buffers" doesn't specify the expected behavior. Based on the test setup and the error handler's
discardExistingOutputconfiguration, it would be clearer to assert the exact expected buffer level.- self::assertLessThanOrEqual( - $bufferBeforeLevel, - $bufferAfterLevel, - "'clearOutput()' should properly clean output buffers", - ); + self::assertSame( + 0, + $bufferAfterLevel, + "Output buffers should be cleared to level 0 when discardExistingOutput is true in non-test environment", + );
410-418: Buffer restoration logic appears incomplete.The while loop restores buffers when the current level is less than the initial level, but there's no handling for the case where the buffer level might be greater than expected. This could leave extra buffers that weren't cleaned up properly.
Consider adding cleanup for excess buffers:
+ // Clean up any excess buffers first + while (ob_get_level() > $bufferBeforeLevel) { + ob_end_clean(); + } + while (ob_get_level() < $bufferBeforeLevel) { ob_start(); }
📜 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(2 hunks)tests/support/stub/MockerFunctions.php(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/support/stub/MockerFunctions.php
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods (lines 28 and 32), so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-07-20T16:33:57.495Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.495Z
Learning: The TestCase class in yii2-extensions/psr-bridge automatically handles $_SERVER superglobal cleanup by saving its original state before each test and restoring it afterward in setUp() and tearDown() methods. Manual $_SERVER cleanup in individual test methods is unnecessary when extending this TestCase.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods, so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-24T11:52:50.524Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.524Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-08T15:24:06.085Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/TestCase.php:23-27
Timestamp: 2025-08-08T15:24:06.085Z
Learning: In yii2-extensions/psr-bridge (tests/TestCase.php), maintainer preference: it’s acceptable to use random-looking strings for test-only constants like COOKIE_VALIDATION_KEY; no need to replace with an obviously non-secret value unless CI/secret scanners become problematic.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-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/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/ApplicationErrorHandlerTest.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/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 (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/ApplicationErrorHandlerTest.php
🧬 Code graph analysis (1)
tests/http/stateless/ApplicationErrorHandlerTest.php (4)
tests/http/ErrorHandlerTest.php (2)
RequiresPhpExtension(23-48)RequiresPhpExtension(74-113)tests/TestCase.php (1)
statelessApplication(134-197)src/http/StatelessApplication.php (1)
handle(269-295)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createServerRequestCreator(154-161)
⏰ 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.4-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (2)
tests/http/stateless/ApplicationErrorHandlerTest.php (2)
18-26: New imports look good and align with the test requirements.The added imports
ini_get,ini_set,restore_error_handler, andset_error_handlerare correctly imported and necessary for the new test method that handles error configurations and display settings.
351-423: Test implementation is comprehensive and well-structured.The new test method
testRenderExceptionSetsDisplayErrorsInDebugModeis well-designed and covers the critical aspects of exception rendering in debug mode. The test properly:
- Toggles YII_ENV_TEST to simulate non-test environment
- Sets up nested output buffers to validate clearing behavior
- Verifies display_errors is set to '1' during debug mode
- Ensures proper cleanup of output buffers
- Restores all modified states in the finally/cleanup sections
This addresses the PR objective of enhancing output buffer handling in the error handler tests.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/http/stateless/ApplicationErrorHandlerTest.php (1)
19-25: Consider using Yii2's built-in error handler functions.The test imports low-level PHP error handling functions. Since this is testing Yii2's error handler functionality, consider whether Yii2 provides higher-level abstractions that could be used instead.
If Yii2 provides error handler abstractions that could replace direct usage of
set_error_handlerandrestore_error_handler, consider using those for better integration with the framework's error handling system.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/http/stateless/ApplicationErrorHandlerTest.php(2 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods (lines 28 and 32), so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-07-20T16:33:57.495Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.495Z
Learning: The TestCase class in yii2-extensions/psr-bridge automatically handles $_SERVER superglobal cleanup by saving its original state before each test and restoring it afterward in setUp() and tearDown() methods. Manual $_SERVER cleanup in individual test methods is unnecessary when extending this TestCase.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods, so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-24T11:52:50.524Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.524Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-08T15:24:06.085Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/TestCase.php:23-27
Timestamp: 2025-08-08T15:24:06.085Z
Learning: In yii2-extensions/psr-bridge (tests/TestCase.php), maintainer preference: it’s acceptable to use random-looking strings for test-only constants like COOKIE_VALIDATION_KEY; no need to replace with an obviously non-secret value unless CI/secret scanners become problematic.
Applied to files:
tests/http/stateless/ApplicationErrorHandlerTest.php
📚 Learning: 2025-08-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/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/ApplicationErrorHandlerTest.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/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 (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/ApplicationErrorHandlerTest.php
🧬 Code graph analysis (1)
tests/http/stateless/ApplicationErrorHandlerTest.php (4)
tests/http/ErrorHandlerTest.php (2)
RequiresPhpExtension(23-48)RequiresPhpExtension(74-113)tests/TestCase.php (1)
statelessApplication(134-197)src/http/StatelessApplication.php (1)
handle(269-295)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createServerRequestCreator(154-161)
⏰ 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 (1)
tests/http/stateless/ApplicationErrorHandlerTest.php (1)
351-423: LGTM! Well-structured test for debug mode behavior.The test properly verifies that
display_errorsis set to '1' during exception rendering in debug mode and that output buffers are correctly cleared whendiscardExistingOutputis true. The cleanup logic ensures the environment is properly restored.
| public function testRenderExceptionSetsDisplayErrorsInDebugMode(): void | ||
| { | ||
| @\runkit_constant_redefine('YII_ENV_TEST', false); | ||
|
|
||
| $bufferBeforeLevel = ob_get_level(); | ||
|
|
||
| $_SERVER = [ | ||
| 'REQUEST_METHOD' => 'GET', | ||
| 'REQUEST_URI' => 'site/trigger-exception', | ||
| ]; | ||
|
|
||
| ob_start(); | ||
| echo 'buffer content that should be cleared'; | ||
| ob_start(); | ||
| echo 'nested buffer content'; | ||
|
|
||
| $originalDisplayErrors = ini_get('display_errors'); | ||
|
|
||
| $app = $this->statelessApplication( | ||
| [ | ||
| 'components' => [ | ||
| 'errorHandler' => [ | ||
| 'discardExistingOutput' => true, | ||
| 'errorAction' => null, | ||
| ], | ||
| ], | ||
| ], | ||
| ); | ||
|
|
||
| $response = $app->handle(FactoryHelper::createServerRequestCreator()->createFromGlobals()); | ||
|
|
||
| self::assertSame( | ||
| 500, | ||
| $response->getStatusCode(), | ||
| "Expected HTTP '500' for route 'site/trigger-exception'.", | ||
| ); | ||
| self::assertSame( | ||
| 'text/html; charset=UTF-8', | ||
| $response->getHeaderLine('Content-Type'), | ||
| "Expected Content-Type 'text/html; charset=UTF-8' for route 'site/trigger-exception'.", | ||
| ); | ||
| self::assertSame( | ||
| '1', | ||
| ini_get('display_errors'), | ||
| "'display_errors' should be set to '1' in debug mode when rendering exception.", | ||
| ); | ||
|
|
||
| $bufferAfterLevel = ob_get_level(); | ||
|
|
||
| self::assertSame( | ||
| 0, | ||
| $bufferAfterLevel, | ||
| "Output buffers should be cleared to level '0' when 'discardExistingOutput' is 'true'.", | ||
| ); | ||
|
|
||
| while (ob_get_level() < $bufferBeforeLevel) { | ||
| ob_start(); | ||
| } | ||
|
|
||
| self::assertSame( | ||
| $bufferBeforeLevel, | ||
| ob_get_level(), | ||
| 'Output buffers should be restored to initial level.', | ||
| ); | ||
|
|
||
| ini_set('display_errors', $originalDisplayErrors); | ||
|
|
||
| @\runkit_constant_redefine('YII_ENV_TEST', true); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the interaction with other buffer-manipulating tests.
While the test properly restores buffer levels and ini settings, it's worth verifying that the combination of RunKit7 constant redefinition and buffer manipulation doesn't interfere with other tests, especially since this test clears all buffers to level 0.
🏁 Script executed:
#!/bin/bash
# Check for other tests that manipulate ob_get_level() or YII_ENV_TEST to ensure no conflicts
echo "=== Searching for other tests that manipulate output buffers ==="
rg -n "ob_get_level\(\)" --type php tests/ -A 3 -B 3
echo -e "\n=== Searching for other tests that redefine YII_ENV_TEST ==="
rg -n "runkit_constant_redefine.*YII_ENV_TEST" --type php tests/ -A 2 -B 2Length of output: 12898
Wrap RunKit constant redefinition and buffer cleanup in a try/finally
To prevent leaking YII_ENV_TEST or buffer levels into subsequent tests if an assertion fails midway, mirror the pattern used in testRenderExceptionPassesExceptionParameterToTemplateView (lines 340–349) by enclosing the setup, execution, and cleanup in a try { … } finally { … } block.
• In tests/http/stateless/ApplicationErrorHandlerTest.php (testRenderExceptionSetsDisplayErrorsInDebugMode):
- Move the
@\runkit_constant_redefine('YII_ENV_TEST', false);and initialob_get_level()calls inside thetry. - In the
finallyblock, restore bothini_set('display_errors', $originalDisplayErrors);and@\runkit_constant_redefine('YII_ENV_TEST', true);, and re-open buffers up to the original level.
This guarantees that even if an assertion throws, the global constant and output buffers are reliably reset before any other test runs.
…dling in `ErrorHandlerTest`.
Summary by CodeRabbit