Rename Plugin => ViteHelperPlugin #24
Conversation
josbeir
commented
Dec 14, 2025
josbeir
commented
- Renames Plugin.php => ViteHelperPlugin.php confirming with Cake 5.3 plugin name deprecation + adds test for the plugin file.
- Fixes phpcs.xml errors
- Adds more php version to ci version matrix
There was a problem hiding this comment.
Pull request overview
This PR renames the plugin class from Plugin to ViteHelperPlugin to comply with CakePHP 5.3's deprecation of generic plugin names, while also fixing code style issues and expanding CI test coverage.
- Renames the main plugin class to follow CakePHP 5.3 naming conventions
- Adds trailing commas to multi-line arrays for consistent code style
- Expands PHP version testing in CI and fixes phpcs.xml configuration
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/ViteHelperPlugin.php |
Renames class from Plugin to ViteHelperPlugin |
tests/TestCase/ViteHelperPluginTest.php |
Adds new test file for the renamed plugin class |
src/View/Helper/ViteScriptsHelper.php |
Adds trailing commas to multi-line arrays |
src/Utilities/ViteManifest.php |
Adds trailing comma to function arguments |
phpcs.xml |
Removes outdated config path, adds file paths for linting |
.github/workflows/ci.yml |
Expands PHP version matrix for CI testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public function testBootstrapWithConfigFile(): void | ||
| { | ||
| // We can't easily test with a real config file without manipulating ROOT, | ||
| // but we can verify the plugin instantiates and bootstraps correctly | ||
| $plugin = new ViteHelperPlugin(); | ||
| $app = $this->getMockBuilder(PluginApplicationInterface::class)->getMock(); | ||
|
|
||
| // This should not throw any exceptions | ||
| $plugin->bootstrap($app); | ||
|
|
||
| $this->assertInstanceOf(ViteHelperPlugin::class, $plugin); | ||
| } |
There was a problem hiding this comment.
The testBootstrapWithConfigFile test does not actually test the scenario where a config file is present. According to the comment on line 55, the test acknowledges it "can't easily test with a real config file" but then doesn't actually verify any config-file-related behavior. The test only verifies that bootstrap doesn't throw an exception and that the plugin is an instance of ViteHelperPlugin, which is redundant with the testPluginInstantiation test. This test should either be removed or properly test the config file loading scenario by creating a temporary config file or mocking the file_exists/Configure::load calls.
| public function testPluginInstantiation(): void | ||
| { | ||
| $plugin = new ViteHelperPlugin(); | ||
| $this->assertInstanceOf(ViteHelperPlugin::class, $plugin); | ||
| } |
There was a problem hiding this comment.
The testPluginInstantiation test is redundant. It only verifies that a ViteHelperPlugin object can be created and is an instance of ViteHelperPlugin, which is already covered by the other tests that instantiate the plugin. Consider removing this test or enhancing it to test meaningful plugin properties or behavior.
| protected ?string $originalRoot = null; | ||
|
|
||
| protected function setUp(): void | ||
| { | ||
| parent::setUp(); | ||
| $this->originalRoot = defined('ROOT') ? ROOT : null; |
There was a problem hiding this comment.
The originalRoot property is declared in setUp but never used in any of the tests. Either use this property to test scenarios that depend on the ROOT constant, or remove it to keep the test code clean.
| protected ?string $originalRoot = null; | |
| protected function setUp(): void | |
| { | |
| parent::setUp(); | |
| $this->originalRoot = defined('ROOT') ? ROOT : null; | |
| protected function setUp(): void | |
| { | |
| parent::setUp(); |
| <file>src/</file> | ||
| <file>tests/</file> |
There was a problem hiding this comment.
Inconsistent indentation: lines 4-5 use spaces for indentation while the rest of the file uses tabs. For consistency with the existing file style (lines 3, 6-9), these lines should use tabs instead of spaces.
| <file>src/</file> | |
| <file>tests/</file> | |
| <file>src/</file> | |
| <file>tests/</file> |
|
Closing, duplicate of #23 |
1 participant
