Introduce plugins folder for standalone plugins

[felixarntz]

This issue is part 1/2 for setting up the new infrastructure for developing standalone plugins.

Requirements

This work should be implemented against a new feature/modules-to-plugins feature branch.

• Create new plugins folder in the repository root.
• Add a .gitkeep to it.
• Add the plugins folder to be ignored in .gitattributes so that it won't be part of the Performance Lab plugin build.
• Restructure the plugins.json file:
  • Going forward, there should be always two top level properties: modules and plugins.
  • modules should have the map of "module => plugin data (slug and version)", i.e. the same thing that the file contains today.
  • plugins should have an array of plugin slugs.
• Update the existing workflows that use the plugins.json file to look for modules within the new modules property in the file.
• Add references to the new plugins folder in any dev tooling that currently lints/covers the modules folder (e.g. in phpcs.xml.dist etc.).

Note: Additional changes to build/publish the new plugins should be implemented in a separate follow-up issue.

[felixarntz]

@mukeshpanchal27 @joemcgill @swissspidy See #946 (comment): This brought up another thing that needs to be addressed which is the actual test coverage.

I wonder how we can address this in the most efficient way. phpunit.xml.dist seems to support multiple testsuites, so maybe we can somehow have individual test suites per plugin in there? One for Performance Lab, and one for each standalone plugin in the plugins folder? The tests for any plugins in plugins folder could live in a separate folder then too, e.g. tests/plugins, so then the PL test suite would ignore that folder, while the individual plugins' test suites would only consider tests from their relevant directory.

Maybe the test suites could then also set a constant or environment variable which the bootstrap.php file can use to determine which plugin to force-activate.

Not sure whether this makes sense, I'm curious on your feedback or any alternative ideas you may have to make this possible but also keep it as simple as possible.

[swissspidy]

Right, with this becoming more like a monorepo of individual plugins we need to run tests and static analysis separately for each plugin.

Right now the PHPUnit bootstrap.php just loads everything, which is certainly not ideal. Separate test suites and then some way to only load what's needed sounds good. I'd say let's just give it a try in a PR and see what works and what doesn't. These are just tests, we can quickly iterate and tweak the setup as we see fit.

[felixarntz]

#948 has been merged, still keeping this open for the follow up effort of test setup (see above).

[mukeshpanchal27]

In #639, we implemented a command to test building the plugin. Could we implement a similar setup, or do you have any other proposed way to handle this?

[felixarntz]

@mukeshpanchal27 I don't think we need a new JS script to enable unit testing the standalone plugins, I think it should be possible to accomplish this via using multiple test suites in the phpunit.xml.dist config file. I assume we can then trigger a specific test suite via CLI argument, e.g. we could use the respective plugin slug?

For convenience, it would probably help to have a script for this in package.json. Though maybe it could be possible by reusing the existing npm run test-php? It would be neat to have this command run the Performance Lab tests by default, but if we call it something like npm run test-php --{someFlag}=webp-uploads, it would run the PHPUnit tests for the WebP Uploads standalone plugin. I wonder whether that's possible.

cc @swissspidy

[swissspidy]

Yeah something like that is possible. PHPUnit supports --testsuite=<name>. But for passing this to npm run test-php we'd need to update wp-env, which ideally we do via #544. Then we could do npm run test-php -- -- --testsuite=webp-uploads

[mukeshpanchal27]

@swissspidy When i ran npm run test-php -- -- --testsuite=webp-uploads and update config for PHPUnit it did not work for specific suite. Do you know why?

[swissspidy]

Have you tried that in the #544 branch? It requires a change like that to the package.json script to work so the argument is correctly passed to the container. Alternatively just use vendor/bin/phpunit locally for testing.

[mukeshpanchal27]

Yes.

[swissspidy]

If you have a PR that implements the individual testsuites I'm happy to take a look.

At first glance it looks like npm run test-php -- -- --testsuite=webp-uploads would work, as that correctly forwards --testsuite to Composer and then phpunit:

$ npm run test-php -- -- --testsuite=webp-uploads
> test-php
> wp-env run tests-cli --env-cwd=/var/www/html/wp-content/plugins/$(basename $(pwd)) composer test -- --testsuite=webp-uploads

$ composer test -- --testsuite=webp-uploads
> phpunit -c phpunit.xml.dist --verbose '--testsuite=webp-uploads'

So the command itself works. But requires #544 and of course the testsuites to be set up.

[mukeshpanchal27]

POC added in PR #956

[mukeshpanchal27]

Close in #956

[thelovekesh]

I think testsuite based approach has coupled the unit testing of the main plugin vs the standalone plugin. Now if in any instance a standalone plugin requires its test suite(which can be required in case of external HTTP test cases) then we will again need to touch the main config file.

I think a cleaner approach can be to include its own PHPUnit config in the plugins and let the main PHPUnit config handle its single responsibility which is unit testing for the main plugin.

[westonruter]

I seem to recall suggesting having a separate phpunit.xml.dist for each plugin but that this was ruled out. I can't find that conversation, however.

Since we're going to have a separate phpcs.xml.dist for each standalone plugin (#1002), it makes sense to me that we'd also have a phpunit.xml.dist for each standalone plugin.

If we do that, it would necessarily mean the bootstrap logic would have to change: instead of looking at the --testsuite arg, it would instead have to look at either the current working directory or else if there is config supplied on the $argv.

But assuming that the tests all remain in /tests and don't also move to the /plugins directory, there would still need to be a plugin-specific testsuite in the plugin's dedicated phpunit.xml.dist:

performance/phpunit.xml.dist

Lines 15 to 17 in d20057d

	<testsuite name="auto-sizes">
	<directory suffix=".php">./tests/plugins/auto-sizes</directory>
	</testsuite>

But since the config is in plugins/auto-sizes the directory would need to be modified:

 <testsuite name="auto-sizes"> 
 	<directory suffix=".php">../../tests/plugins/auto-sizes</directory> 
 </testsuite> 

So instead of it being in the root phpunit.xml.dist it would be located in /plugins/auto-sizes/phpunit.xml.dist.

I think this approach is a bit cleaner, but it seems maintenance is about the same. Perhaps something to file in a new issue?

[thelovekesh]

Since we are moving toward more of a "monorepo" approach for plugins, I think we should also unify the development for a standalone plugin in its place which will add some more benefits for DX. Currently, if someone working on a plugin, they need to go to the top-level tests directory and find the right place to put the tests in that directory, Rather what we can do is bring the test config and tests directory to the plugin directory so all things related to the single plugin can be handled within it.

ping @swissspidy @mukeshpanchal27 @joemcgill for visibility.

[swissspidy]

Yeah there are definitely multiple ways to improve DX in this new monorepo structure. Let‘s open a new issue to discuss this properly.

[thelovekesh]

@swissspidy here we go - #1012