Conversation
WalkthroughAdds a protected Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Areas to review:
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
| private array $retryStatusCodes = [500, 503]; | ||
| private mixed $jsonEncodeFlags; | ||
|
|
||
| protected string $baseUrl = ''; |
There was a problem hiding this comment.
Let's update tests to try and set base URL
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/ClientTest.php (1)
292-309: Consider adding a comment to clarify the empty string URL.The test correctly validates that an empty string relative URL resolves to the base URL. However, a brief comment explaining this intent would improve readability.
Apply this diff:
$client = new Client(); $client->setBaseUrl('http://localhost:8000'); + // Empty string should resolve to base URL $resp = $client->fetch( url: '', method: Client::METHOD_GET );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Client.php(3 hunks)tests/ClientTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Client.php
🧰 Additional context used
🧬 Code graph analysis (1)
tests/ClientTest.php (2)
src/Client.php (4)
Client(9-441)setBaseUrl(42-46)getBaseUrl(48-51)fetch(255-360)src/Response.php (2)
getStatusCode(70-73)json(88-95)
🔇 Additional comments (5)
tests/ClientTest.php (5)
274-286: LGTM! Clear and straightforward test.The setter/getter test is well-structured and follows the established pattern for similar tests in this file.
315-331: LGTM! Effective test for path appending.The test properly validates that relative paths are correctly appended to the base URL by using the existing redirect endpoint.
337-354: LGTM! Important test case for absolute URL handling.This test correctly validates that absolute URLs bypass the base URL, which is essential behavior to prevent incorrect URL construction.
360-384: LGTM! Good coverage of query parameter handling.The test properly validates that query parameters work correctly with base URLs, ensuring the URL construction logic doesn't interfere with query string handling.
390-407: LGTM! Essential test for slash normalization.This test correctly validates the trailing/leading slash normalization logic, preventing double slashes in the constructed URL. The comment on line 394 is helpful.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.