refactor(tests): Enhance HTTPFunctions to track http_response_code() calls and add test for exception handling in CLI SAPI.
#87
Conversation
…()` calls and add test for exception handling in CLI SAPI.
WalkthroughAdds two unit tests for ErrorHandler SAPI behavior, registers a mock for http_response_code in test mocks, extends the HTTPFunctions test stub to count http_response_code() calls, and simplifies status-code assignment in ErrorHandler::handleException without changing control flow. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #87 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 321 321
===========================================
Files 12 12
Lines 805 803 -2
===========================================
- Hits 805 803 -2 ☔ View full report in Codecov by Sentry. |
…tions` for accurate testing in `ErrorHandlerTest`.
![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 (1)
tests/support/stub/HTTPFunctions.php (1)
66-70: Good addition; minor doc nitpickCounter field looks good. For consistency with the rest of the file, consider fully-qualifying the reference in the docblock as {@see \http_response_code()}.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/http/ErrorHandlerTest.php(1 hunks)tests/support/MockerExtension.php(1 hunks)tests/support/stub/HTTPFunctions.php(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/http/ErrorHandlerTest.php (2)
tests/support/stub/HTTPFunctions.php (4)
HTTPFunctions(37-221)reset(180-190)php_sapi_name(175-178)getHttpResponseCodeCalls(104-107)src/http/ErrorHandler.php (2)
ErrorHandler(42-273)handleException(116-147)
tests/support/MockerExtension.php (1)
tests/support/stub/HTTPFunctions.php (2)
HTTPFunctions(37-221)http_response_code(164-173)
🪛 GitHub Actions: build
tests/http/ErrorHandlerTest.php
[error] 90-90: PHPUnit test failed: ErrorHandlerTest::testHandleExceptionDoesNotCallHttpResponseCodeInCliSapi; Should not call 'http_response_code()' in CLI SAPI; call count must remain '0'. Actual: 36.
🪛 GitHub Actions: mutation test
tests/http/ErrorHandlerTest.php
[error] 87-87: PHPUnit test failed: ErrorHandlerTest::testHandleExceptionDoesNotCallHttpResponseCodeInCliSapi - expected 0 http_response_code() calls, but got 36.
⏰ 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). (1)
- GitHub Check: phpunit / PHP 8.4-windows-latest
🔇 Additional comments (2)
tests/support/stub/HTTPFunctions.php (1)
166-167: LGTM: instrumentation pointIncrementing the counter on each call is appropriate for tracking invocations.
tests/support/MockerExtension.php (1)
76-82: LGTM: mock correctly wired to the http namespaceThe mock’s namespace matches ErrorHandler, and the signature/return type align with expectations. This enables accurate call tracking in tests.
…HttpResponseCodeInCliSapi` for improved test accuracy.
…update `HTTPFunctions` to track `http_response_code()` calls.
…eption()` method.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/http/ErrorHandler.php (1)
121-125: Optional hardening: treat phpdbg as CLI and avoid setting status when headers already sentIf desired, make the guard slightly more robust without changing the overall flow:
- Count phpdbg as CLI to prevent accidental status emissions under phpdbg.
- Skip calling http_response_code() when headers have already been sent.
Note: If tests assert exact call counts, they may need adjusting depending on the default of headers_sent() in the mock.
- if (php_sapi_name() !== 'cli') { - $statusCode = $exception instanceof HttpException ? $exception->statusCode : 500; - http_response_code($statusCode); - } + $sapi = php_sapi_name(); + if ($sapi !== 'cli' && $sapi !== 'phpdbg' && !headers_sent()) { + $statusCode = $exception instanceof HttpException ? $exception->statusCode : 500; + http_response_code($statusCode); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/http/ErrorHandler.php(1 hunks)tests/http/ErrorHandlerTest.php(1 hunks)tests/support/stub/HTTPFunctions.php(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/http/ErrorHandlerTest.php
- tests/support/stub/HTTPFunctions.php
⏰ 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.3-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (1)
src/http/ErrorHandler.php (1)
121-125: Ternary consolidation and unqualified http_response_code() look good
- Compact status-code resolution is clear and correct.
- Leaving http_response_code() unqualified in the namespace enables the test stub to intercept it as intended. No behavior change in production.
Summary by CodeRabbit