Refactor(tests): Update PHPDocs in tests for consistency and clarity. #188
Conversation
|
Warning Rate limit exceeded@terabytesoftw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 8 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 (1)
WalkthroughAdds/updates PHPDocs across many tests and stubs, introduces multiple new data providers in StatelessApplicationProvider, moves a PSR‑7 request binding earlier in CookiesPsr7Test, simplifies UploadedFileCreatorTest helpers and call sites, renames one stateless uploaded test class, and updates CHANGELOG.md. (No production API changes.) Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test case
participant PSR7 as PSR-7 request fixture
participant Wrapper as Request wrapper / Adapter
participant Validator as Cookie validation
Note over Test,PSR7: Setup sequence moved earlier
Test->>PSR7: create PSR-7 request
Test->>Wrapper: setPsr7Request(psr7Request) %% changed: bind before validation
Wrapper->>Wrapper: bind PSR-7 request
Test->>Wrapper: invoke getCookies() / access cookies
Wrapper->>Validator: trigger cookie validation (may throw)
Validator-->>Wrapper: throws InvalidConfigException (propagates)
Wrapper-->>Test: exception observed by test
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #188 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 362 362
===========================================
Files 13 13
Lines 909 909
===========================================
Hits 909 909 ☔ 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 (3)
tests/creator/UploadedFileCreatorTest.php (2)
150-150: Consider explicit empty array for test clarity.The test name
testCreateFromGlobalsWithEmptyArraysuggests the intent is to explicitly test with an empty array. While callingcreateFromGlobals()is functionally equivalent (using the default parameter), passing[]explicitly would more clearly match the test name and intent.- $creator->createFromGlobals(), + $creator->createFromGlobals([]),
1139-1141: Consider documenting the 'deep' key expectation.The simplified implementation now directly accesses
$result['deep']. While correct for current usage (all call sites pass results fromcreateDeeplyNestedFileStructure), the PHPDoc could clarify this expectation for future maintainability./** * Navigate to the deepest file in a nested structure. * - * @param array $result Processed file structure. + * @param array $result Processed file structure with a 'deep' key. * @param int $expectedDepth Expected depth to navigate. * * @return mixed Deepest file found. * * @phpstan-param array<array<mixed>|UploadedFileInterface> $result */tests/provider/StatelessApplicationProvider.php (1)
607-608: Fix phpstan return annotation for remote IP provider.The dataset uses named string keys, but the phpstan annotation describes a list keyed by ints. Update the annotation so static analysis keeps the correct key type.
- * @phpstan-return array<array{int|string, string|null, string}> + * @phpstan-return array<string, array{int|string, string|null, string}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
CHANGELOG.md(1 hunks)tests/adapter/CookiesPsr7Test.php(2 hunks)tests/adapter/HeadersPsr7Test.php(1 hunks)tests/adapter/ResponseAdapterTest.php(1 hunks)tests/adapter/ServerParamsPsr7Test.php(1 hunks)tests/adapter/ServerRequestAdapterTest.php(1 hunks)tests/adapter/UploadedFilesPsr7Test.php(1 hunks)tests/creator/ServerRequestCreatorTest.php(1 hunks)tests/creator/UploadedFileCreatorTest.php(3 hunks)tests/emitter/SapiEmitterTest.php(3 hunks)tests/http/ErrorHandlerTest.php(3 hunks)tests/http/RequestTest.php(6 hunks)tests/http/ResponseTest.php(1 hunks)tests/http/ServerExitCodeTest.php(1 hunks)tests/http/UploadedFileTest.php(1 hunks)tests/http/stateless/ApplicationAuthTest.php(1 hunks)tests/http/stateless/ApplicationContentTypesTest.php(1 hunks)tests/http/stateless/ApplicationCookieTest.php(1 hunks)tests/http/stateless/ApplicationCoreTest.php(1 hunks)tests/http/stateless/ApplicationErrorHandlerTest.php(1 hunks)tests/http/stateless/ApplicationEventTest.php(2 hunks)tests/http/stateless/ApplicationMemoryTest.php(2 hunks)tests/http/stateless/ApplicationRoutingTest.php(1 hunks)tests/http/stateless/ApplicationServerParamsTest.php(1 hunks)tests/http/stateless/ApplicationSessionTest.php(1 hunks)tests/http/stateless/ApplicationUploadedTest.php(1 hunks)tests/provider/EmitterProvider.php(3 hunks)tests/provider/RequestProvider.php(16 hunks)tests/provider/ServerParamsPsr7Provider.php(4 hunks)tests/provider/StatelessApplicationProvider.php(10 hunks)tests/support/stub/ComplexUploadedFileModel.php(1 hunks)tests/support/stub/EventComponent.php(1 hunks)tests/support/stub/Identity.php(1 hunks)tests/support/stub/SiteController.php(1 hunks)tests/support/stub/UploadedFileModel.php(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-08-24T11:52:50.563Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.563Z
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/ApplicationSessionTest.phptests/adapter/ServerRequestAdapterTest.phptests/http/stateless/ApplicationServerParamsTest.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/ApplicationSessionTest.phptests/http/stateless/ApplicationCookieTest.phptests/adapter/CookiesPsr7Test.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/adapter/ServerRequestAdapterTest.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/adapter/ServerRequestAdapterTest.phptests/http/stateless/ApplicationServerParamsTest.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/adapter/ServerRequestAdapterTest.phptests/http/stateless/ApplicationServerParamsTest.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/adapter/ServerRequestAdapterTest.phptests/http/stateless/ApplicationServerParamsTest.phptests/adapter/HeadersPsr7Test.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/ApplicationCookieTest.phptests/adapter/CookiesPsr7Test.php
📚 Learning: 2025-08-28T22:08:51.812Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.812Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Applied to files:
tests/adapter/UploadedFilesPsr7Test.phptests/support/stub/UploadedFileModel.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/adapter/CookiesPsr7Test.php
📚 Learning: 2025-08-26T15:37:52.160Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#155
File: tests/http/stateless/ApplicationMemoryTest.php:198-211
Timestamp: 2025-08-26T15:37:52.160Z
Learning: In yii2-extensions/psr-bridge, tests can use the TestSupport trait from php-forge/support package which provides an invokeMethod() helper for calling protected/private methods during testing, eliminating the need for custom reflection code.
Applied to files:
tests/http/ErrorHandlerTest.php
📚 Learning: 2025-08-25T16:08:54.379Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#149
File: tests/http/stateless/ApplicationCoreTest.php:24-27
Timestamp: 2025-08-25T16:08:54.379Z
Learning: In the yii2-extensions/psr-bridge project, avoid importing functions that are not actually used in the code, even if they might seem logically related to the functionality being tested.
Applied to files:
tests/http/ErrorHandlerTest.php
🧬 Code graph analysis (4)
tests/emitter/SapiEmitterTest.php (1)
tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createResponse(109-127)
tests/creator/UploadedFileCreatorTest.php (2)
src/creator/ServerRequestCreator.php (1)
createFromGlobals(89-123)src/creator/UploadedFileCreator.php (1)
createFromGlobals(157-165)
tests/http/stateless/ApplicationUploadedTest.php (1)
tests/TestCase.php (1)
TestCase(25-235)
tests/adapter/CookiesPsr7Test.php (1)
src/http/Request.php (1)
setPsr7Request(790-797)
🪛 GitHub Actions: ecs
tests/provider/RequestProvider.php
[error] 1-1: PhpCsFixer (PhpdocIndentFixer) reported 1 fixable error during ECS check. Run 'vendor/bin/ecs check --fix' or apply fixes to the file to resolve formatting issues.
⏰ 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). (20)
- GitHub Check: linter / Super Linter
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: phpstan / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit-dev / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit-dev / PHP 8.6-ubuntu-latest
- GitHub Check: easy-coding-standard / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (8)
tests/emitter/SapiEmitterTest.php (1)
479-481: Clearer$filenaming reads cleaner.Renaming the resource variable from
$fopento$filebetter reflects its role and avoids confusion with the function name, while keeping behavior intact.Also applies to: 686-688
tests/creator/UploadedFileCreatorTest.php (1)
17-31: LGTM! Comprehensive test suite documentation.The added PHPDoc provides excellent coverage of the test class's purpose, behavior verification, and test coverage scope. The documentation structure is clear and informative.
tests/adapter/CookiesPsr7Test.php (1)
81-82: LGTM! Improved test setup order.Moving
setPsr7Request()before the exception expectations improves the test's logical flow by ensuring the PSR-7 request is bound before triggering the cookie validation path. This makes the test setup clearer and more maintainable.tests/http/RequestTest.php (1)
273-273: Cannot locatevalidateCsrfTokendefinition—please confirm it defaults its parameter tonull.
Without evidence of a signature likevalidateCsrfToken($token = null), dropping the explicitnullmay alter behavior.tests/http/stateless/ApplicationMemoryTest.php (2)
28-41: LGTM! Excellent documentation.The class-level PHPDoc provides a comprehensive overview of the test suite's purpose, test coverage areas, and scope. This greatly improves code maintainability and helps developers understand the test objectives at a glance.
213-213: Verify@throws InvalidConfigExceptionon tests callingstatelessApplication()
Confirm whetherstatelessApplication()can throwInvalidConfigException; if it does, re-add@throws InvalidConfigExceptionto any test docblocks that invoke it (e.g.testParseMemoryLimitaround line 213).tests/provider/RequestProvider.php (2)
7-21: LGTM! Comprehensive class documentation.The class-level PHPDoc provides excellent context for the data provider's purpose, key features, and the scenarios it covers. This documentation significantly improves code discoverability and maintainability.
24-1051: Excellent PHPDoc improvements.The updated method-level PHPDocs across all data provider methods are comprehensive and well-structured. They provide clear descriptions of:
- The test scenarios being supplied
- The structure and purpose of test data
- Expected inputs and outputs
These documentation improvements significantly enhance code maintainability and make the test data providers much easier to understand and use.
…ridge into fix-mini-166
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/provider/RequestProvider.php (1)
7-21: Great addition of class-level documentation!The comprehensive class-level PHPDoc significantly improves code clarity by providing context for the entire provider class.
Minor suggestion: Consider changing "Key features." on Line 13 to "Key features:" (with a colon) for consistency with standard list formatting.
Apply this diff for the minor formatting improvement:
-* Key features. +* Key features:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/provider/RequestProvider.php(16 hunks)
⏰ 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). (20)
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit-dev / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit-dev / PHP 8.6-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: easy-coding-standard / PHP 8.4-ubuntu-latest
- GitHub Check: composer-require-checker / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit-dev / PHP 8.5-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: phpstan / PHP 8.4-ubuntu-latest
🔇 Additional comments (1)
tests/provider/RequestProvider.php (1)
24-36: Excellent documentation improvements across all data provider methods!The expanded PHPDoc blocks for all data provider methods significantly enhance code documentation by:
- Providing clear descriptions of test scenarios covered
- Explaining the structure and purpose of test data
- Maintaining consistent formatting and style across all methods
- Preserving precise PHPStan type annotations
The indentation issue flagged in the previous review for the
getIsPjax()method (lines 258-270) has been properly resolved.Also applies to: 67-78, 109-121, 229-240, 258-270, 289-300, 318-330, 361-372, 391-403, 540-551, 566-577, 640-653, 810-822, 837-849, 898-915, 1038-1051
terabytesoftw
changed the title
1 participant
Pull Request
Summary by CodeRabbit
Documentation
Tests
Chores