-
-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(ErrorHandler): replace PHP_SAPI with php_sapi_name for consistency in SAPI tests.
#59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…onsistency in `SAPI` tests.
|
Warning Rate limit exceeded@terabytesoftw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 13 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 (4)
WalkthroughThe changes introduce a mechanism for simulating and controlling the PHP SAPI (Server API) environment within tests. The HTTP function stubs are enhanced to allow setting and retrieving the simulated SAPI name, and the error handler test is updated for data-driven testing across different SAPI values. Mocking infrastructure is also extended to support the new SAPI simulation. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as ErrorHandlerTest
participant HTTPFns as HTTPFunctions
participant Mocker as MockerExtension
Test->>HTTPFns: set_sapi('apache' or 'cli')
Test->>Mocker: Register mock for php_sapi_name
Test->>HTTPFns: php_sapi_name()
HTTPFns-->>Test: Return current SAPI
Test->>Test: Run error handler test logic
Test->>HTTPFns: reset() (in tearDown)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
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 #59 +/- ##
============================================
+ Coverage 98.79% 99.32% +0.53%
Complexity 292 292
============================================
Files 12 12
Lines 746 746
============================================
+ Hits 737 741 +4
+ Misses 9 5 -4 ☔ View full report in Codecov by Sentry. |
…onsistency in `ErrorHandler` class checks.
…onsistency in CLI checks.
…onsistency in CLI checks.
…simulation and retrieval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request refactors the ErrorHandler to use php_sapi_name() function instead of the PHP_SAPI constant for SAPI environment detection, enabling better testability and consistency. The change is accompanied by comprehensive test infrastructure improvements to support SAPI simulation.
- Replaced
PHP_SAPIconstant withphp_sapi_name()function call in ErrorHandler - Added SAPI simulation capability to test infrastructure with HTTPFunctions stub
- Enhanced test coverage with data-driven testing for multiple SAPI environments
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/http/ErrorHandler.php | Changed SAPI detection from constant to function call |
| tests/support/stub/HTTPFunctions.php | Added SAPI simulation methods and state management |
| tests/support/MockerExtension.php | Registered php_sapi_name function mock for testing |
| tests/http/ErrorHandlerTest.php | Added tearDown method and parameterized test for SAPI environments |
| .styleci.yml | Disabled function_to_constant rule to allow the refactoring |
Comments suppressed due to low confidence (1)
tests/http/ErrorHandlerTest.php:111
- The test method only sets the SAPI but doesn't assert any behavior differences between 'apache' and 'cli' environments. Consider adding assertions to verify that the ErrorHandler behaves differently for each SAPI type.
public function testHandleExceptionWithHttpException(string $sapi): void
… exception handling.
Summary by CodeRabbit