Add unit tests for AssetManager#20789
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #20789 +/- ##
============================================
+ Coverage 80.44% 80.65% +0.20%
Complexity 11540 11540
============================================
Files 374 374
Lines 30235 30235
============================================
+ Hits 24323 24385 +62
+ Misses 5912 5850 -62 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ 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). (9)
🧰 Additional context used🧠 Learnings (4)📚 Learning: 2026-04-11T18:58:31.942ZApplied to files:
📚 Learning: 2026-04-14T10:43:29.381ZApplied to files:
📚 Learning: 2026-04-21T21:24:32.138ZApplied to files:
📚 Learning: 2026-05-01T02:09:33.591ZApplied to files:
🪛 PHPMD (2.15.0)tests/data/web/AssetManagerTestSourceBundle.php[warning] 16-18: Avoid variables with short names like $js. Configured minimum length is 3. (undefined) (ShortVariable) tests/data/web/AssetManagerTestSimpleBundle.php[warning] 17-19: Avoid variables with short names like $js. Configured minimum length is 3. (undefined) (ShortVariable) tests/framework/web/AssetManagerTest.php[warning] 28-969: The class AssetManagerTest has 69 public methods and attributes. Consider reducing the number of public items to less than 45. (undefined) (ExcessivePublicCount) [warning] 28-969: The class AssetManagerTest has 72 non-getter- and setter-methods. Consider refactoring AssetManagerTest to keep number of methods under 25. (undefined) (TooManyMethods) [warning] 28-969: The class AssetManagerTest has 69 public methods. Consider refactoring AssetManagerTest to keep number of public methods under 10. (undefined) (TooManyPublicMethods) [warning] 28-969: The class AssetManagerTest has an overall complexity of 84 which is very high. The configured complexity threshold is 50. (undefined) (ExcessiveClassComplexity) [error] 37-37: Avoid using static access to class '\Yii' in method 'setUp'. (undefined) (StaticAccess) [error] 38-38: Avoid using static access to class '\Yii' in method 'setUp'. (undefined) (StaticAccess) [error] 39-39: Avoid using static access to class '\Yii' in method 'setUp'. (undefined) (StaticAccess) [error] 40-40: Avoid using static access to class '\Yii' in method 'setUp'. (undefined) (StaticAccess) [error] 41-41: Avoid using static access to class '\Yii' in method 'setUp'. (undefined) (StaticAccess) [error] 43-43: Avoid using static access to class '\Yii' in method 'setUp'. (undefined) (StaticAccess) [error] 65-65: Avoid using static access to class '\yii\helpers\FileHelper' in method 'cleanAssetsDir'. (undefined) (StaticAccess) [error] 66-68: The method cleanAssetsDir uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined) (ElseExpression) [error] 67-67: Avoid using static access to class '\yii\helpers\FileHelper' in method 'cleanAssetsDir'. (undefined) (StaticAccess) [warning] 82-82: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 84-84: Avoid using static access to class '\Yii' in method 'testInitResolvesBasePathAlias'. (undefined) (StaticAccess) [warning] 89-89: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 95-95: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 106-106: Avoid using static access to class '\Yii' in method 'testCheckBasePathPermissionThrowsOnNonWritableDir'. (undefined) (StaticAccess) [error] 107-107: Avoid using static access to class '\yii\helpers\FileHelper' in method 'testCheckBasePathPermissionThrowsOnNonWritableDir'. (undefined) (StaticAccess) [error] 111-111: Avoid using static access to class '\yii\helpers\FileHelper' in method 'testCheckBasePathPermissionThrowsOnNonWritableDir'. (undefined) (StaticAccess) [warning] 115-115: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 124-124: Avoid using static access to class '\yii\helpers\FileHelper' in method 'testCheckBasePathPermissionThrowsOnNonWritableDir'. (undefined) (StaticAccess) [warning] 130-130: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 139-139: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 152-152: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 166-166: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 179-179: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 193-193: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 206-206: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 215-215: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 225-225: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 235-235: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 245-245: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 256-256: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 266-266: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 275-275: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 284-284: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 285-285: Avoid using static access to class '\Yii' in method 'testPublishFileByPath'. (undefined) (StaticAccess) [warning] 298-298: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 299-299: Avoid using static access to class '\Yii' in method 'testPublishDirectoryByPath'. (undefined) (StaticAccess) [warning] 310-310: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 311-311: Avoid using static access to class '\Yii' in method 'testPublishReturnsCachedResult'. (undefined) (StaticAccess) [warning] 321-321: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 341-341: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 353-353: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 354-354: Avoid using static access to class '\Yii' in method 'testPublishFileWithLinkAssets'. (undefined) (StaticAccess) [warning] 364-364: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 365-365: Avoid using static access to class '\Yii' in method 'testPublishDirectoryWithLinkAssets'. (undefined) (StaticAccess) [error] 372-372: Avoid using static access to class '\yii\helpers\FileHelper' in method 'testPublishDirectoryWithLinkAssets'. (undefined) (StaticAccess) [error] 375-398: Remove error control operator '@' on line 388. (undefined) (ErrorControlOperator) [warning] 380-380: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 381-381: Avoid using static access to class '\Yii' in method 'testPublishFileWithLinkAssetsRethrowsOnSymlinkFailure'. (undefined) (StaticAccess) [error] 387-387: Avoid using static access to class '\yii\helpers\FileHelper' in method 'testPublishFileWithLinkAssetsRethrowsOnSymlinkFailure'. (undefined) (StaticAccess) [error] 400-421: Remove error control operator '@' on line 411. (undefined) (ErrorControlOperator) [warning] 405-405: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 406-406: Avoid using static access to class '\Yii' in method 'testPublishDirectoryWithLinkAssetsRethrowsOnSymlinkFailure'. (undefined) (StaticAccess) [warning] 429-429: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 430-430: Avoid using static access to class '\Yii' in method 'testPublishFileWithFileMode'. (undefined) (StaticAccess) [warning] 440-440: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 441-441: Avoid using static access to class '\Yii' in method 'testPublishFileWithAppendTimestamp'. (undefined) (StaticAccess) [warning] 450-450: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 451-451: Avoid using static access to class '\Yii' in method 'testPublishFileWithoutAppendTimestamp'. (undefined) (StaticAccess) [warning] 461-461: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 462-462: Avoid unused parameters such as '$from'. (undefined) (UnusedFormalParameter) [warning] 462-462: Avoid unused parameters such as '$to'. (undefined) (UnusedFormalParameter) [error] 467-467: Avoid using static access to class '\Yii' in method 'testPublishDirectoryWithAfterCopy'. (undefined) (StaticAccess) [warning] 476-476: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 482-482: Avoid using static access to class '\Yii' in method 'testPublishDirectoryOptionsAfterCopyOverridesClassAfterCopy'. (undefined) (StaticAccess) [warning] 483-483: Avoid unused parameters such as '$from'. (undefined) (UnusedFormalParameter) [warning] 483-483: Avoid unused parameters such as '$to'. (undefined) (UnusedFormalParameter) [warning] 495-495: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 496-496: Avoid unused parameters such as '$to'. (undefined) (UnusedFormalParameter) [error] 501-501: Avoid using static access to class '\Yii' in method 'testPublishDirectoryWithForceCopyOption'. (undefined) (StaticAccess) [warning] 515-515: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 517-517: Avoid unused parameters such as '$to'. (undefined) (UnusedFormalParameter) [error] 522-522: Avoid using static access to class '\Yii' in method 'testPublishDirectoryWithForceCopyProperty'. (undefined) (StaticAccess) [warning] 536-536: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 538-538: Avoid unused parameters such as '$to'. (undefined) (UnusedFormalParameter) [error] 543-543: Avoid using static access to class '\Yii' in method 'testPublishDirectoryForceCopyOptionFalseOverridesProperty'. (undefined) (StaticAccess) [warning] 558-558: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 559-559: Avoid using static access to class '\Yii' in method 'testGetPublishedPathForPublishedFile'. (undefined) (StaticAccess) [warning] 570-570: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 571-571: Avoid using static access to class '\Yii' in method 'testGetPublishedPathForUnpublishedFile'. (undefined) (StaticAccess) [warning] 582-582: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 583-583: Avoid using static access to class '\Yii' in method 'testGetPublishedPathForDirectory'. (undefined) (StaticAccess) [warning] 595-595: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 602-602: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 603-603: Avoid using static access to class '\Yii' in method 'testGetPublishedPathUsesCache'. (undefined) (StaticAccess) [warning] 610-610: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 611-611: Avoid using static access to class '\Yii' in method 'testGetPublishedUrlForPublishedFile'. (undefined) (StaticAccess) [warning] 622-622: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 623-623: Avoid using static access to class '\Yii' in method 'testGetPublishedUrlForUnpublishedFile'. (undefined) (StaticAccess) [warning] 635-635: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 636-636: Avoid using static access to class '\Yii' in method 'testGetPublishedUrlForDirectory'. (undefined) (StaticAccess) [warning] 648-648: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 655-655: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 656-656: Avoid using static access to class '\Yii' in method 'testGetPublishedUrlUsesCache'. (undefined) (StaticAccess) [warning] 663-663: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 664-664: Avoid unused parameters such as '$path'. (undefined) (UnusedFormalParameter) [error] 668-668: Avoid using static access to class '\Yii' in method 'testHashWithCustomCallback'. (undefined) (StaticAccess) [warning] 677-677: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 679-679: Avoid using static access to class '\Yii' in method 'testGetAssetUrlWithRelativeAsset'. (undefined) (StaticAccess) [warning] 689-689: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 700-700: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 711-711: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 713-713: Avoid using static access to class '\Yii' in method 'testGetAssetUrlWithAppendTimestampEnabled'. (undefined) (StaticAccess) [warning] 723-723: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 725-725: Avoid using static access to class '\Yii' in method 'testGetAssetUrlWithAppendTimestampDisabled'. (undefined) (StaticAccess) [warning] 735-735: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 737-737: Avoid using static access to class '\Yii' in method 'testGetAssetUrlWithPerAssetTimestampOverride'. (undefined) (StaticAccess) [warning] 747-747: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 749-749: Avoid using static access to class '\Yii' in method 'testGetAssetUrlWithPerAssetTimestampDisableOverride'. (undefined) (StaticAccess) [warning] 759-759: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 774-774: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 780-780: Avoid using static access to class '\Yii' in method 'testGetAssetUrlWithAssetMapSuffixMatch'. (undefined) (StaticAccess) [warning] 790-790: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 804-804: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 806-806: Avoid using static access to class '\Yii' in method 'testGetAssetPathForRelativeAsset'. (undefined) (StaticAccess) [error] 810-810: Avoid using static access to class '\Yii' in method 'testGetAssetPathForRelativeAsset'. (undefined) (StaticAccess) [warning] 815-815: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 817-817: Avoid using static access to class '\Yii' in method 'testGetAssetPathReturnsFalseForAbsoluteUrl'. (undefined) (StaticAccess) [warning] 824-824: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [error] 830-830: Avoid using static access to class '\Yii' in method 'testGetAssetPathWithAssetMapReturnsBasePath'. (undefined) (StaticAccess) [warning] 839-839: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 850-850: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 864-864: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 879-879: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 892-892: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 907-907: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 922-922: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 933-933: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 947-947: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) [warning] 961-961: Avoid variables with short names like $am. Configured minimum length is 3. (undefined) (ShortVariable) 🔇 Additional comments (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a comprehensive PHPUnit test suite for yii\web\AssetManager plus two test AssetBundle fixtures, exercising initialization, bundles/converter, publishing (copy/link), caching, asset mapping, and URL/path resolution. ChangesAsset Manager Test Suite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/framework/web/AssetManagerTest.php (2)
51-69: Close directory handle viafinallyfor safer cleanup paths.If an exception is thrown during deletion,
closedir()may be skipped. Wrapping the loop intry/finallymakes cleanup deterministic.♻️ Proposed refactor
private function cleanAssetsDir(): void { $handle = opendir($this->assetsPath); if ($handle === false) { return; } - while (($file = readdir($handle)) !== false) { - if ($file === '.' || $file === '..' || $file === '.gitignore') { - continue; - } - $path = $this->assetsPath . DIRECTORY_SEPARATOR . $file; - if (is_dir($path)) { - FileHelper::removeDirectory($path); - } else { - FileHelper::unlink($path); - } + try { + while (($file = readdir($handle)) !== false) { + if ($file === '.' || $file === '..' || $file === '.gitignore') { + continue; + } + $path = $this->assetsPath . DIRECTORY_SEPARATOR . $file; + if (is_dir($path)) { + FileHelper::removeDirectory($path); + } else { + FileHelper::unlink($path); + } + } + } finally { + closedir($handle); } - closedir($handle); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/framework/web/AssetManagerTest.php` around lines 51 - 69, The cleanAssetsDir function currently opens a directory with opendir($this->assetsPath) and calls closedir($handle) at the end, but an exception during the readdir/unlink/removeDirectory loop can skip closedir; wrap the while-loop and deletion logic in a try { ... } finally { closedir($handle); } block after checking $handle !== false so the handle is always closed, while preserving the early return when opendir fails and keeping calls to FileHelper::removeDirectory and FileHelper::unlink unchanged.
673-817: Consider extracting bundle setup helpers to reduce test boilerplate.
AssetManagerTestSimpleBundlecreation andbasePath/baseUrlassignments are repeated across many tests; a small helper would improve readability and reduce drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/framework/web/AssetManagerTest.php` around lines 673 - 817, Many tests repeat creating AssetManagerTestSimpleBundle and setting basePath/baseUrl; extract a small helper (e.g., a private method in the test class like createBundle or setupBundle) that accepts optional basePath and baseUrl and returns a configured AssetManagerTestSimpleBundle, then update existing tests (e.g., testGetAssetUrlWithRelativeAsset, testGetAssetUrlWithAbsoluteUrl, testGetAssetUrlWithAppendTimestampEnabled, testGetAssetPathForRelativeAsset, testGetAssetPathReturnsFalseForAbsoluteUrl, etc.) to call createBundle(...) instead of repeating the instantiation and property assignments; keep createManager(...) calls as-is and ensure the helper uses Yii::getAlias when resolving default paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/framework/web/AssetManagerTest.php`:
- Around line 51-69: The cleanAssetsDir function currently opens a directory
with opendir($this->assetsPath) and calls closedir($handle) at the end, but an
exception during the readdir/unlink/removeDirectory loop can skip closedir; wrap
the while-loop and deletion logic in a try { ... } finally { closedir($handle);
} block after checking $handle !== false so the handle is always closed, while
preserving the early return when opendir fails and keeping calls to
FileHelper::removeDirectory and FileHelper::unlink unchanged.
- Around line 673-817: Many tests repeat creating AssetManagerTestSimpleBundle
and setting basePath/baseUrl; extract a small helper (e.g., a private method in
the test class like createBundle or setupBundle) that accepts optional basePath
and baseUrl and returns a configured AssetManagerTestSimpleBundle, then update
existing tests (e.g., testGetAssetUrlWithRelativeAsset,
testGetAssetUrlWithAbsoluteUrl, testGetAssetUrlWithAppendTimestampEnabled,
testGetAssetPathForRelativeAsset, testGetAssetPathReturnsFalseForAbsoluteUrl,
etc.) to call createBundle(...) instead of repeating the instantiation and
property assignments; keep createManager(...) calls as-is and ensure the helper
uses Yii::getAlias when resolving default paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: be82af6b-7415-4f9c-bfa9-89e5d4dafdb0
📒 Files selected for processing (1)
tests/framework/web/AssetManagerTest.php
3 participants
What does this PR do?
Adds
AssetManagerTest- unit tests forAssetManager.Coverage
69 tests. Line coverage: 149/149 (100%). MSI: 94% (265/281).
Changelog not required, because no source files changed.