test: add unit tests - #9
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis change set introduces comprehensive testing and development tooling for a TYPO3 CMS extension. It adds a PHPUnit configuration ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (13)
✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (8)
Tests/Unit/Domain/Repository/DateRepositoryTest.php (2)
32-38: Redundant test methods detected.The
repositoryCanBeInstantiatedandrepositoryExtendsBaseRepositorymethods test essentially the same functionality - both verify inheritance from the base Repository class.Consider consolidating these into a single test method:
- #[Test] - public function repositoryCanBeInstantiated(): void - { - $reflection = new \ReflectionClass(DateRepository::class); - - self::assertTrue($reflection->isSubclassOf(\TYPO3\CMS\Extbase\Persistence\Repository::class)); - } - - #[Test] - public function repositoryExtendsBaseRepository(): void - { - $reflection = new \ReflectionClass(DateRepository::class); - $parentClass = $reflection->getParentClass(); - - self::assertNotNull($parentClass); - self::assertEquals(\TYPO3\CMS\Extbase\Persistence\Repository::class, $parentClass->getName()); - } + #[Test] + public function repositoryExtendsBaseRepository(): void + { + $reflection = new \ReflectionClass(DateRepository::class); + + self::assertTrue($reflection->isSubclassOf(\TYPO3\CMS\Extbase\Persistence\Repository::class)); + + $parentClass = $reflection->getParentClass(); + self::assertNotNull($parentClass); + self::assertEquals(\TYPO3\CMS\Extbase\Persistence\Repository::class, $parentClass->getName()); + }
40-48: Address PHPStan static analysis hint.PHPStan correctly identifies that
getParentClass()will never returnfalsefor this class since we already verified inheritance. TheassertNotNullis redundant.Apply this diff to address the static analysis hint:
#[Test] public function repositoryExtendsBaseRepository(): void { $reflection = new \ReflectionClass(DateRepository::class); $parentClass = $reflection->getParentClass(); - self::assertNotNull($parentClass); self::assertEquals(\TYPO3\CMS\Extbase\Persistence\Repository::class, $parentClass->getName()); }Tests/Unit/Backend/ToolbarItems/NewsItemTest.php (1)
103-109: Remove redundant type assertion.PHPStan correctly identifies that
assertIsArrayis redundant sincegetAdditionalAttributes()always returns an array type.Apply this diff to remove the redundant assertion:
#[Test] public function getAdditionalAttributesReturnsEmptyArray(): void { $result = $this->subject->getAdditionalAttributes(); - self::assertIsArray($result); self::assertEmpty($result); }Tests/Unit/Service/DateServiceTest.php (3)
86-87: Remove redundant assertIsArray() calls.PHPStan correctly identifies that these
assertIsArray()calls are redundant since the methods always return arrays.- self::assertIsArray($result); self::assertEmpty($result);Apply this pattern to all similar occurrences in the test methods.
Also applies to: 99-100, 108-109, 133-134, 146-147, 159-160
40-45: Fix unused parameter in mock language service.The
$keyparameter in the mocksLmethod is unused, which PHPMD flags correctly.- public function sL(string $key): string + public function sL(string $key): string { + // Unused parameter is acceptable in test mocks return 'Notification message'; }Alternatively, prefix with underscore:
public function sL(string $_key): string
100-101: Consider enhancing test coverage beyond basic existence checks.While method existence tests are useful, consider adding more behavioral verification to increase test value.
The comments suggest that detailed testing would require more complex mocking, which is understandable. These placeholder tests are acceptable for now but could be enhanced in the future with proper TYPO3 test framework setup.
Also applies to: 147-148, 160-161
Tests/Unit/Widgets/InternalNewsWidgetTest.php (2)
126-127: Remove redundant assertIsArray() calls.PHPStan correctly identifies these redundant assertions since the methods always return arrays.
- self::assertIsArray($options); self::assertEquals(['option1' => 'value1'], $options);Apply similar fixes to
getCssFiles()andgetJavaScriptModuleInstructions()test methods.Also applies to: 135-136, 145-146
213-217: Fix unused parameters in mock backend user.The mock method parameters are unused, which PHPMD flags correctly.
- public function check(string $table, string $action): bool + public function check(string $_table, string $_action): bool { return true; // Always allow for testing }Prefixing with underscore indicates intentionally unused parameters in test mocks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/tests.yml(1 hunks).gitignore(1 hunks)README.md(2 hunks)Tests/Unit/Backend/ToolbarItems/NewsItemTest.php(1 hunks)Tests/Unit/ConfigurationTest.php(1 hunks)Tests/Unit/Controller/DateControllerTest.php(1 hunks)Tests/Unit/Domain/Model/DateTest.php(1 hunks)Tests/Unit/Domain/Model/NewsTest.php(1 hunks)Tests/Unit/Domain/Repository/DateRepositoryTest.php(1 hunks)Tests/Unit/Domain/Repository/NewsRepositoryTest.php(1 hunks)Tests/Unit/Service/CacheServiceTest.php(1 hunks)Tests/Unit/Service/DateServiceTest.php(1 hunks)Tests/Unit/Utilities/BackendUserHelperTest.php(1 hunks)Tests/Unit/Utilities/ViewFactoryHelperTest.php(1 hunks)Tests/Unit/Widgets/InternalNewsWidgetTest.php(1 hunks)composer.json(2 hunks)phpstan.neon(1 hunks)phpunit.xml(1 hunks)rector.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
Tests/Unit/ConfigurationTest.php (1)
Classes/Configuration.php (2)
Configuration(26-30)Configuration(7-11)
Tests/Unit/Utilities/BackendUserHelperTest.php (4)
Classes/Utilities/BackendUserHelper.php (3)
BackendUserHelper(26-40)checkAndSetModuleDate(28-39)BackendUserHelper(7-21)Tests/Unit/Service/CacheServiceTest.php (3)
setUp(37-44)createMockBackendUser(93-101)tearDown(46-49)Tests/Unit/Domain/Repository/NewsRepositoryTest.php (3)
setUp(37-48)createMockBackendUser(156-166)tearDown(50-53)Tests/Unit/Widgets/InternalNewsWidgetTest.php (3)
setUp(43-59)createMockBackendUser(210-218)tearDown(61-64)
Tests/Unit/Backend/ToolbarItems/NewsItemTest.php (6)
Tests/Unit/Domain/Model/NewsTest.php (14)
Test(42-49)Test(51-55)Test(57-64)Test(66-70)Test(72-79)Test(81-85)Test(87-95)Test(97-101)Test(103-114)Test(116-123)Test(125-132)Test(134-141)Test(143-149)setUp(37-40)Classes/Backend/ToolbarItems/NewsItem.php (11)
NewsItem(31-122)checkAccess(43-46)hasDropDown(71-74)getIndex(118-121)getAdditionalAttributes(104-107)getItem(53-64)getDropDown(81-89)NewsItem(13-108)getItem(36-47)__construct(16-19)checkAccess(26-29)Classes/Domain/Repository/NewsRepository.php (2)
NewsRepository(30-77)NewsRepository(11-58)Tests/Unit/Domain/Repository/NewsRepositoryTest.php (1)
setUp(37-48)Tests/Unit/Utilities/BackendUserHelperTest.php (1)
setUp(34-38)Classes/Widgets/Provider/InternalNewsDataProvider.php (1)
InternalNewsDataProvider(10-20)
Tests/Unit/Domain/Repository/DateRepositoryTest.php (1)
Classes/Domain/Repository/DateRepository.php (2)
DateRepository(28-28)DateRepository(9-11)
🪛 PHPMD (2.15.0)
Tests/Unit/Domain/Repository/NewsRepositoryTest.php
34-34: Avoid unused private fields such as '$subject'. (Unused Code Rules)
(UnusedPrivateField)
Tests/Unit/Service/DateServiceTest.php
41-41: Avoid unused parameters such as '$key'. (Unused Code Rules)
(UnusedFormalParameter)
Tests/Unit/Widgets/InternalNewsWidgetTest.php
213-213: Avoid unused parameters such as '$table'. (Unused Code Rules)
(UnusedFormalParameter)
213-213: Avoid unused parameters such as '$action'. (Unused Code Rules)
(UnusedFormalParameter)
🪛 PHPStan (2.1.17)
Tests/Unit/Domain/Repository/NewsRepositoryTest.php
34-34: Property Xima\XimaTypo3InternalNews\Tests\Unit\Domain\Repository\NewsRepositoryTest::$subject is unused.
See: https://phpstan.org/developing-extensions/always-read-written-properties
(property.unused)
35-35: Property Xima\XimaTypo3InternalNews\Tests\Unit\Domain\Repository\NewsRepositoryTest::$cacheServiceMock is never read, only written.
See: https://phpstan.org/developing-extensions/always-read-written-properties
(property.onlyWritten)
134-134: Call to static method PHPUnit\Framework\Assert::assertNotNull() with ReflectionClass|false will always evaluate to true.
Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.
(staticMethod.alreadyNarrowedType)
Tests/Unit/Utilities/BackendUserHelperTest.php
32-32: Property Xima\XimaTypo3InternalNews\Tests\Unit\Utilities\BackendUserHelperTest::$originalModuleData is never read, only written.
See: https://phpstan.org/developing-extensions/always-read-written-properties
(property.onlyWritten)
Tests/Unit/Service/DateServiceTest.php
86-86: Call to static method PHPUnit\Framework\Assert::assertIsArray() with array will always evaluate to true.
(staticMethod.alreadyNarrowedType)
99-99: Call to static method PHPUnit\Framework\Assert::assertIsArray() with array will always evaluate to true.
(staticMethod.alreadyNarrowedType)
108-108: Call to static method PHPUnit\Framework\Assert::assertIsArray() with array will always evaluate to true.
(staticMethod.alreadyNarrowedType)
133-133: Call to static method PHPUnit\Framework\Assert::assertIsArray() with array will always evaluate to true.
(staticMethod.alreadyNarrowedType)
146-146: Call to static method PHPUnit\Framework\Assert::assertIsArray() with array will always evaluate to true.
(staticMethod.alreadyNarrowedType)
159-159: Call to static method PHPUnit\Framework\Assert::assertIsArray() with array will always evaluate to true.
(staticMethod.alreadyNarrowedType)
Tests/Unit/Backend/ToolbarItems/NewsItemTest.php
107-107: Call to static method PHPUnit\Framework\Assert::assertIsArray() with array will always evaluate to true.
(staticMethod.alreadyNarrowedType)
Tests/Unit/Widgets/InternalNewsWidgetTest.php
126-126: Call to static method PHPUnit\Framework\Assert::assertIsArray() with array will always evaluate to true.
(staticMethod.alreadyNarrowedType)
135-135: Call to static method PHPUnit\Framework\Assert::assertIsArray() with array will always evaluate to true.
(staticMethod.alreadyNarrowedType)
145-145: Call to static method PHPUnit\Framework\Assert::assertIsArray() with list<TYPO3\CMS\Core\Page\JavaScriptModuleInstruction> will always evaluate to true.
(staticMethod.alreadyNarrowedType)
Tests/Unit/Domain/Repository/DateRepositoryTest.php
46-46: Call to static method PHPUnit\Framework\Assert::assertNotNull() with ReflectionClass|false will always evaluate to true.
Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.
(staticMethod.alreadyNarrowedType)
🪛 GitHub Actions: CGL
Tests/Unit/Domain/Repository/NewsRepositoryTest.php
[error] 18-163: EditorConfig: File has no final new line and trailing whitespaces found on lines 42, 45, 60, 69, 77, 79, 82, 95, 97, 102, 116, 118, 122, 133, 142, 151, 163.
Tests/Unit/ConfigurationTest.php
[error] 3-51: EditorConfig: File has no final new line and trailing whitespaces found on lines 48, 51.
Tests/Unit/Utilities/ViewFactoryHelperTest.php
[error] 10-121: EditorConfig: File has no final new line and trailing whitespaces found on lines 52, 55, 70, 80, 83, 86, 97, 109, 121.
Tests/Unit/Utilities/BackendUserHelperTest.php
[error] 21-176: EditorConfig: File has no final new line and trailing whitespaces found on lines 51, 53, 62, 66, 77, 79, 87, 90, 100, 104, 106, 115, 118, 126, 136, 140, 145, 162, 171, 176.
Tests/Unit/Domain/Model/DateTest.php
[error] 8-135: EditorConfig: File has no final new line and trailing whitespaces found on lines 44, 59, 74, 89, 105, 120, 135.
Tests/Unit/Controller/DateControllerTest.php
[error] 27-204: EditorConfig: File has no final new line and trailing whitespaces found on lines 44, 48, 55, 72, 80, 89, 91, 100, 109, 111, 114, 120, 132, 134, 137, 148, 150, 153, 164, 166, 169, 180, 189, 192, 201, 204.
Tests/Unit/Service/DateServiceTest.php
[error] 23-184: EditorConfig: File has no final new line and trailing whitespaces found on lines 46, 61, 63, 71, 73, 83, 85, 96, 98, 107, 116, 118, 130, 132, 143, 145, 156, 158, 167, 171, 173, 184.
Tests/Unit/Backend/ToolbarItems/NewsItemTest.php
[error] 27-211: EditorConfig: File has no final new line and multiple trailing whitespaces found on lines 42, 56, 65, 67, 70, 72, 82, 90, 98, 106, 115, 121, 127, 133, 139, 145, 157, 160, 166, 178, 180, 183, 193, 202, 206, 211.
Tests/Unit/Widgets/InternalNewsWidgetTest.php
[error] 23-203: EditorConfig: File has no final new line and trailing whitespaces found on lines 48, 56, 76, 87, 89, 92, 99, 110, 112, 115, 125, 134, 144, 147, 156, 165, 167, 176, 183, 189, 201, 203.
Tests/Unit/Domain/Repository/DateRepositoryTest.php
[error] 6-73: EditorConfig: File has no final new line and trailing whitespaces found on lines 36, 45, 54, 63, 73.
Tests/Unit/Service/CacheServiceTest.php
[error] 8-89: EditorConfig: File has no final new line and trailing whitespaces found on lines 41, 66, 68, 77, 79, 87, 89.
Tests/Unit/Domain/Model/NewsTest.php
[error] 10-138: EditorConfig: File has no final new line and trailing whitespaces found on lines 47, 62, 77, 92, 109, 111, 120, 130, 138.
🔇 Additional comments (26)
.gitignore (1)
15-15: LGTM! Correct PHPUnit cache exclusion.Adding
.phpunit.result.cacheto.gitignoreis the standard practice for PHPUnit projects to exclude the test result cache file from version control.rector.php (1)
40-40: LGTM! Proper inclusion of test directory in Rector analysis.Including
Tests/Unitin Rector's paths ensures that automated refactoring and code quality improvements also apply to the test code, maintaining consistency across the codebase.phpstan.neon (1)
26-26: LGTM! Appropriate extension of static analysis to tests.Adding
Tests/Unitto PHPStan paths ensures static analysis coverage extends to the test code, helping maintain code quality and catch potential issues in test files.README.md (2)
12-12: LGTM! Corrected license badge URL.The license badge URL has been updated to reflect the correct package ownership on Packagist.
49-49: LGTM! Added TER downloads badge.The TER downloads badge provides useful information about extension adoption and complements the existing version badge.
.github/workflows/tests.yml (1)
1-14: Ensure CI workflow stability and compatibility
- The reusable workflow
tests.ymlexists injackd248/reusable-github-actionsand was last updated on 2025-07-25, so it’s actively maintained. To avoid unintended breakages, consider pinning to a fixed release tag (e.g.@v1) rather than@main.- Please confirm via the official TYPO3 release notes that PHP 8.4 is supported by TYPO3 12.4 and 13.4 before including it in the test matrix.
composer.json (2)
33-35: LGTM! Well-configured testing dependencies.The addition of PHPUnit and PHPStan PHPUnit extension with appropriate version constraints supports modern PHP and TYPO3 versions effectively.
98-99: LGTM! Well-structured test scripts.The test scripts are properly configured with separate commands for running tests with and without coverage, using XDEBUG_MODE for coverage collection.
Tests/Unit/ConfigurationTest.php (2)
32-36: LGTM! Solid constant value verification.The test correctly verifies the extension key constant value matches the expected string.
38-42: LGTM! Proper extension name verification.The test correctly verifies the extension name constant value.
phpunit.xml (1)
1-33: LGTM! Comprehensive PHPUnit configuration.The configuration properly sets up test suites, coverage reporting (Clover XML, HTML, text), JUnit logging, and source inclusion. The bootstrap and directory structure are correctly configured for a TYPO3 extension.
Tests/Unit/Domain/Repository/NewsRepositoryTest.php (1)
55-62: LGTM! Appropriate testing approach for framework constraints.Using reflection to test repository structure is a reasonable approach when full TYPO3 framework integration isn't feasible in unit tests.
Tests/Unit/Utilities/BackendUserHelperTest.php (4)
46-55: LGTM! Excellent test coverage for new values.The test properly verifies that the method returns
truefor new values.
57-70: LGTM! Great test for idempotency.The test excellently verifies that the method returns
falsewhen the same value is set again, ensuring proper idempotency behavior.
96-108: LGTM! Comprehensive edge case testing.The test properly covers multiple values within the same module, including duplicate detection.
122-155: LGTM! Thorough method signature verification.The reflection-based tests properly verify the method is static, public, and has correct parameter types. The handling of mixed type parameters is well-implemented.
Tests/Unit/Backend/ToolbarItems/NewsItemTest.php (1)
32-217: Comprehensive test coverage achieved.The test suite provides excellent structural validation of the
NewsItemclass, covering interface compliance, method signatures, constructor dependencies, and property types. This thorough approach ensures the toolbar item maintains its contract with the TYPO3 framework.Tests/Unit/Utilities/ViewFactoryHelperTest.php (2)
46-63: Excellent approach for testing static utility methods.The use of reflection to validate method signatures without executing complex rendering logic is appropriate and well-implemented. The comment explaining the testing limitations is helpful for future maintainers.
35-44: Good test isolation practices.The setUp and tearDown methods properly reset the GeneralUtility container, ensuring tests run in isolation. This is crucial for TYPO3 testing.
Tests/Unit/Domain/Model/NewsTest.php (2)
143-149: Good documentation of testing limitations.The comment explaining why
tstampcan only be tested for its default value is helpful and demonstrates understanding of TYPO3's persistence layer responsibilities.
103-114: Comprehensive ObjectStorage testing.The test properly validates both the setting of ObjectStorage collections and their contents, ensuring the domain model correctly manages related entities.
Tests/Unit/Controller/DateControllerTest.php (3)
84-102: Excellent validation of TYPO3 attributes.The test for the
AsControllerattribute demonstrates deep understanding of TYPO3's controller registration mechanism. This ensures the controller will be properly recognized by the framework.
49-55: Realistic mock configuration.The extension configuration mock includes realistic values that reflect actual usage patterns, making the test more meaningful and closer to real-world scenarios.
104-126: Thorough dependency injection testing.The constructor test comprehensively validates both parameter names and types, ensuring proper dependency injection configuration. This is crucial for TYPO3's DI container.
Tests/Unit/Service/CacheServiceTest.php (1)
32-101: Excellent test coverage for cache identifier generation logic.The tests comprehensively cover all scenarios:
- Admin users getting the universal "all" identifier
- Non-admin users getting sorted group-based identifiers
- Empty groups handling
The mock setup is clean and appropriate for unit testing.
Tests/Unit/Widgets/InternalNewsWidgetTest.php (1)
66-208: Exceptional test coverage and structure.This test suite is remarkably comprehensive, covering all aspects of the widget:
- Interface implementations
- Constructor parameter validation
- Method existence, visibility, and return types
- Property accessibility
- Namespace verification
The thoroughness demonstrates excellent testing practices.
1 participant
Summary by CodeRabbit
New Features
Chores
.gitignoreto exclude PHPUnit result cache files.Documentation