DX improvements in new monorepo

As per #654, we are moving published modules as standalone plugins, making the current repo a mono repo for multiple plugins.

This brings a new challenge to streamline the current tooling and build setup used to maintain coding standards, testing, and publishing the plugin. With this, we are exploring integrating existing workflows in such a way that it doesn't hurt the DX and keeps the maintenance cost low.

Tasklist

[Done] PHPCS setup for mono repo Separate phpcs.xml.dist Files for Each Plugin to Isolate Text Domains #997 (Separate phpcs.xml.dist Files for Each Plugin to Isolate Text Domains #1002)
Setup PHPUnit on plugin level instead of testsuite in main PHPUnit config(See: Introduce plugins folder for standalone plugins #934 (comment) and Introduce plugins folder for standalone plugins #934 (comment)). Testsuite-based setup was introduced in Add test suite for Plugins folder #956.
[Done] Reconsider build setup for both main and standalone plugins. Currently, we use a custom CLI tool which often requires tuning on a use case basis which can be replaced with a tool like Webpack with build steps for each use case. (Update plugin(s) build setup for distribution #1033)

@westonruter

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: from (#934 (comment))

@westonruter I think it would make more sense to include the plugin test in the plugin directory itself. It will help us in multiple ways:

With this new approach, we won't need to switch between directories or hunt down related files scattered throughout multiple directories.
When a plugin becomes part of the core, removing it is much easier. Simply delete the dedicated directory, and there's no need for a tedious search and delete across the entire plugin.
No need to modify configurations based on changes in the plugins/ directory.

This new approach ensures a smooth testing experience with just one command for any plugin.

./vendor/bin/phpunit -c plugins/foo/phpunit.xml.dist # Run tests for plugin: Foo 
./vendor/bin/phpunit -c plugins/bar/phpunit.xml.dist # Run tests for plugin: Bar

Please take a look at the POC - #1013

I do like the idea of organizing the tests into the plugin's directory. It's more helpful for DX to keep the plugin-related code together than it is to keep all test-related code together.

+1 on the idea to keep the plugin's source and test code in a similar location. For example, a plugin plugins/auto-sizes could have its tests in a plugins/auto-sizes/tests directory. This should be relatively straightforward and we would just need to ensure the tests folder is omitted from the standalone plugin builds.

What I'm not a fan of is the separate phpunit.xml.dist files. What's the benefit of that? A partial reason for not going with individual files is to ease maintenance, I don't see a benefit of having the almost identical PHPUnit config file duplicated for every plugin in the monorepo. IMO it's easier from a maintenance perspective to have a single file, and add the test suite for a new plugin once it's added. It's a one-and-done situation, and there are already other places in the overall codebase anyway that need to be touched for administrative purposes in this scenario (e.g. the .github/CODEOWNERS file).

So from my end:

+1 to putting the actual tests (the PHP files) together with the standalone plugins
-1 to having an individual PHPUnit config file for each plugin

@felixarntz

@felixarntz A benefit to having a separate phpunit.xml.dist file for each plugin, which I @thelovekesh first shared in #934 (comment), is that the test suite would no longer be bound 1:1 to the plugin. A plugin could benefit from having multiple test suites. For example, there could be an external-http test suite for a given plugin which is slower. Nevertheless, the bootstrap logic is currently bound up with the test suite name being exactly the same as the slug for the standalone plugin. So if a new test suite is needed for a plugin, this bootstrapping logic wouldn't work anymore. By locating the phpunit.xml.dist in each plugin's respective directory, there wouldn't be any more need for hacking the --testsuite arg.

performance/tests/bootstrap.php

Lines 37 to 90 in 082a106

    
           // Check if we use the plugin's test suite. If so, disable the PL plugin and only load the requested plugin. 
        
           $plugin_name = ''; 
        
           foreach ( $_SERVER['argv'] as $index => $arg ) { 
        
           	if ( 
        
           		'--testsuite' === $arg && 
        
           		isset( $_SERVER['argv'][ $index + 1 ] ) && 
        
           		'performance-lab' !== $_SERVER['argv'][ $index + 1 ] && 
        
           		file_exists( TESTS_PLUGIN_DIR . '/plugins/' . $_SERVER['argv'][ $index + 1 ] ) 
        
           	) { 
        
           		$plugin_name = $_SERVER['argv'][ $index + 1 ]; 
        
           	} 
        
           } 
        
           if ( $plugin_name ) { 
        
           	$plugin_test_path = TESTS_PLUGIN_DIR . '/plugins/' . $plugin_name; 
        
           	tests_add_filter( 
        
           		'plugins_loaded', 
        
           		static function () use ( $plugin_test_path, $plugin_name ) { 
        
           			// Check if plugin has a "plugin/plugin.php" file. 
        
           			if ( file_exists( $plugin_test_path . '/' . $plugin_name . '.php' ) ) { 
        
           				require_once $plugin_test_path . '/' . $plugin_name . '.php'; 
        
           			} elseif ( file_exists( $plugin_test_path . '/load.php' ) ) { 
        
           				// Check if plugin has a "plugin/load.php" file. 
        
           				require_once $plugin_test_path . '/load.php'; 
        
           			} else { 
        
           				echo "Unable to locate standalone plugin bootstrap file in $plugin_test_path."; 
        
           				exit( 1 ); 
        
           			} 
        
           		}, 
        
           		1 
        
           	); 
        
           } else { 
        
           	// Force plugin to be active. 
        
           	$GLOBALS['wp_tests_options'] = array( 
        
           		'active_plugins' => array( basename( TESTS_PLUGIN_DIR ) . '/load.php' ), 
        
           	); 
        
           	// Add filter to ensure the plugin's admin integration and all modules are loaded for tests. 
        
           	tests_add_filter( 
        
           		'plugins_loaded', 
        
           		static function () { 
        
           			require_once TESTS_PLUGIN_DIR . '/admin/load.php'; 
        
           			require_once TESTS_PLUGIN_DIR . '/admin/server-timing.php'; 
        
           			require_once TESTS_PLUGIN_DIR . '/admin/plugins.php'; 
        
           			$module_files = glob( TESTS_PLUGIN_DIR . '/modules/*/*/load.php' ); 
        
           			if ( $module_files ) { 
        
           				foreach ( $module_files as $module_file ) { 
        
           					require_once $module_file; 
        
           				} 
        
           			} 
        
           		}, 
        
           		1 
        
           	); 
        
           }

The standalone plugins can still share the bootstrapping logic as I mentioned in #1013 (comment) while they each have their own phpunit.xml.dist.

@westonruter

@westonruter I'm not sure the value is worth the tradeoff. I can't envision a single plugin would use multiple test suites. Certainly there are a few complex plugins that may make use of that, but we haven't used it so far, and I'm not sure when or why we would.

While for PHPCS I see a benefit of individual files because of minor configuration changes, for PHPUnit the config files typically look the same for every plugin. I'd say we should reevaluate this if we encounter a real need for it, but otherwise it creates overhead of duplicate files without a real benefit.

@felixarntz

@felixarntz The Embed Optimizer plugin could, for example, have an external-http test suite that pulls down oEmbed data from each of the providers rather than using fixtures. I suppose that could also be achieved with an environment variable.

Another benefit is the DX improvement. If you're working on Speculation Rules, you could just run phpunit right from the /plugins/speculation-rules plugin and it would just run the tests for that plugin. Otherwise, you'd have to do phpunit --testsuite speculation-rules, or if using our NPM wrappers, in reality you'd have to do the very unergonomic:

npm run test-php -- -- -- --testsuite=speculation-rules

@westonruter

@westonruter

The Embed Optimizer plugin could, for example, have an external-http test suite that pulls down oEmbed data from each of the providers rather than using fixtures. I suppose that could also be achieved with an environment variable.

Again, IMO this is a theoretical benefit as we have no plans to make use of it at this point. If there is a need for it, we could re-evaluate. But as you're saying there may be more efficient ways to achieve it than duplicating the config files.

Another benefit is the DX improvement. If you're working on Speculation Rules, you could just run phpunit right from the /plugins/speculation-rules plugin and it would just run the tests for that plugin.

Would that use the correct PHPUnit version (the one locally installed via PL composer.json)? Given you wouldn't be in the directory that has composer.json and vendor, I'm not 100% sure about it.

Otherwise, you'd have to do phpunit --testsuite speculation-rules, or if using our NPM wrappers, in reality you'd have to do the very unergonomic:

npm run test-php -- -- -- --testsuite=speculation-rules

Wouldn't we be able to simplify the call using our NPM wrappers? #1002 already adds a wrapper to simplify this for e.g. auto-sizes, so we could do that here too.

Would that use the correct PHPUnit version (the one locally installed via PL composer.json)? Given you wouldn't be in the directory that has composer.json and vendor, I'm not 100% sure about it.

That's true. Although that can be easily resolved with a shell script that sets up the environment to work on the project. Someone could just add this to their .bashrc:

function workhere() {
    export PATH="$PATH:$(pwd)/vendor/bin"
}

And then when navigating to the performance repo do:

$ workhere

And then the phpunit command would be scoped to the project.

Interestingly, PhpStorm (er, IntelliJ) does this automatically for Node scripts. (VSCode does not currently implement this.) When I'm in the built-in IDE terminal, I see my $PATH is includes ~/repos/performance/node_modules/.bin. So I can actually skip doing npm run wp-env or npx wp-env and just do wp-env. (Good to know!) Interestingly, PhpStorm does not implement this also for Composer. (But I just filed a feature request.)

Wouldn't we be able to simplify the call using our NPM wrappers? #1002 already adds a wrapper to simplify this for e.g. auto-sizes, so we could do that here too.

True. Although the need to pass -- -- -- in order to send args to PHPUnit remains, for example to add --filter or --group. Also, the npm wrappers add additional latency and noise to running commands, so it would be nice if we didn't have to use them.

A partial reason for not going with individual files is to ease maintenance, I don't see a benefit of having the almost identical PHPUnit config file duplicated for every plugin in the monorepo.

TBH the testsuite approach looks like having more maintenance cost. Why?

Modifying the main config for individual plugin tests becomes cumbersome as the number of plugins grows. Also adding/removing entries in the testsuites section becomes repetitive for every plugin addition/removal.
Adjusting the main bootstrap file for specific plugin needs is less flexible and potentially error-prone. I think in a monorepo, it makes sense to share the configs but editing them for each other is not ideal.

Plugin name resolution logic in bootstrap file

performance/tests/bootstrap.php

Lines 38 to 67 in 082a106

    
           $plugin_name = ''; 
        
           foreach ( $_SERVER['argv'] as $index => $arg ) { 
        
           	if ( 
        
           		'--testsuite' === $arg && 
        
           		isset( $_SERVER['argv'][ $index + 1 ] ) && 
        
           		'performance-lab' !== $_SERVER['argv'][ $index + 1 ] && 
        
           		file_exists( TESTS_PLUGIN_DIR . '/plugins/' . $_SERVER['argv'][ $index + 1 ] ) 
        
           	) { 
        
           		$plugin_name = $_SERVER['argv'][ $index + 1 ]; 
        
           	} 
        
           } 
        
           if ( $plugin_name ) { 
        
           	$plugin_test_path = TESTS_PLUGIN_DIR . '/plugins/' . $plugin_name; 
        
           	tests_add_filter( 
        
           		'plugins_loaded', 
        
           		static function () use ( $plugin_test_path, $plugin_name ) { 
        
           			// Check if plugin has a "plugin/plugin.php" file. 
        
           			if ( file_exists( $plugin_test_path . '/' . $plugin_name . '.php' ) ) { 
        
           				require_once $plugin_test_path . '/' . $plugin_name . '.php'; 
        
           			} elseif ( file_exists( $plugin_test_path . '/load.php' ) ) { 
        
           				// Check if plugin has a "plugin/load.php" file. 
        
           				require_once $plugin_test_path . '/load.php'; 
        
           			} else { 
        
           				echo "Unable to locate standalone plugin bootstrap file in $plugin_test_path."; 
        
           				exit( 1 ); 
        
           			} 
        
           		}, 
        
           		1 
        
           	);

Running tests from within a specific plugin directory becomes non-trivial due to automatic config resolution and can hinder the development/debugging of the individual plugin(s).
While development using --testsuite requires users to know the specific test suite name within the global phpunit config to run tests for a particular plugin. Now we can include test command as npm/composer script for each plugin but that would also require taking a look at npm/composer scripts to determine for which plugin which test command to run.
Also adding tests commands as npm/composer script approaches require managing and maintaining separate entries for each plugin. As the number of plugins grows, this leads to an increasingly cumbersome script management experience.
If you want to work on the plugin from its directory, the current setup has limitations. With the current approach, it will resolve the config automatically.

So, the tradeoff between both is making changes to the main PHPUnit config + updating tests bootstrap file + maintaining it on use cases vs keeping separate PHPUnit config which is just a one-time copy from any other plugin.

@thelovekesh

@thelovekesh I'm not sure I agree with that assessment in terms of maintenance cost.

I think the only difference from a maintenance perspective is the following:

With individual files, you copy the PHPUnit config for every new plugin.
With a single config, you copy the testsuite bit within the one PHPUnit config file.

Both of that isn't really different in the amount of effort. However with option 1 you end up with a lot of duplicated config, while with option 2 you don't. When something in PHPUnit changes that leads to a change in the configuration, we need to change it everywhere, instead of in a single place.

That's why I think maintenance cost of individual PHPUnit config files is higher.

The only way that this could be resolved without all the duplication would be if we could embed a PHPUnit config file from within the individual other PHPUnit config files (similar to how #1002 does for PHPCS). Though I'm not sure that is possible.

Just so we're on the same page, what is the duplicate code in question?

phpunit.xml.dist

performance/phpunit.xml.dist

Lines 1 to 20 in 28fced3

    
           <phpunit 
        
           	bootstrap="tests/bootstrap.php" 
        
           	backupGlobals="false" 
        
           	colors="true" 
        
           	convertErrorsToExceptions="true" 
        
           	convertNoticesToExceptions="true" 
        
           	convertWarningsToExceptions="true" 
        
           	> 
        
           	<testsuites> 
        
           		<testsuite name="default"> 
        
           			<directory suffix=".php">./tests</directory> 
        
               		<exclude>./tests/utils</exclude> 
        
           		</testsuite> 
        
           	</testsuites> 
        
           	<groups> 
        
           		<exclude> 
        
           			<group>ms-required</group> 
        
           		</exclude> 
        
           	</groups> 
        
           </phpunit>

tests/multisite.xml

performance/tests/multisite.xml

Lines 1 to 23 in 28fced3

    
           <phpunit 
        
           	bootstrap="./bootstrap.php" 
        
           	backupGlobals="false" 
        
           	colors="true" 
        
           	convertErrorsToExceptions="true" 
        
           	convertNoticesToExceptions="true" 
        
           	convertWarningsToExceptions="true" 
        
           	> 
        
           	<php> 
        
           		<const name="WP_TESTS_MULTISITE" value="1" /> 
        
           	</php> 
        
           	<testsuites> 
        
           		<testsuite name="default"> 
        
           			<directory suffix=".php">./</directory> 
        
               		<exclude>./utils</exclude> 
        
           		</testsuite> 
        
           	</testsuites> 
        
           	<groups> 
        
           		<exclude> 
        
           			<group>ms-excluded</group> 
        
           		</exclude> 
        
           	</groups> 
        
           </phpunit>

Aside: Both configs include this:

	<groups>
		<exclude>
			<group>ms-excluded</group>
		</exclude>
	</groups>

But this group doesn't exist in the tests. So perhaps it is dead code?

If the duplicated code consists of:

<phpunit
	bootstrap="./tests/bootstrap.php"
	backupGlobals="false"
	colors="true"
	convertErrorsToExceptions="true"
	convertNoticesToExceptions="true"
	convertWarningsToExceptions="true"
	>
	<testsuites>
		<testsuite name="default">
			<directory suffix=".php">./tests</directory>
		</testsuite>
	</testsuites>
</phpunit>

This doesn't seem like a problem as it's not a lot of config. The bootstrap PHP logic can be reused across the plugins. If the concern is that the configs diverge, a script could be added which syncs them all.

When something in PHPUnit changes that leads to a change in the configuration, we need to change it everywhere, instead of in a single place.

Also, has this ever happened before? If not, perhaps it is a theoretical problem?

As such, I think having a shared set of Unit Tests for the entire codebase gives us the opportunity to take advantage of shared test functionality and test data across the test classes that cover different individual plugins developed within this repo rather than needing to duplicate data and test helpers across different plugins. For example, we have several plugins that implement different features related to image performance. Being able to share test data and develop shared test helpers, traits, etc. makes creating tests for these types of plugins easier.

We can still do this even when the unit tests are located with each individual plugin, even when there are separate configs. The test helpers can be located in a root /tests directory which are included by individual plugins' tests as needed. For example, in /plugins/auto-sizes/tests/bootstrap.php

<?php
require_once __DIR__ . '/../../../tests/helpers.php';
// ...

@joemcgill

Thanks @joemcgill.

Reading back through this conversation, I guess I have a bit of a different perspective on how we intend to use this repo and how we prioritize tradeoffs between treating each plugin as an individual unit vs thinking of these as a collection of features that are intended to be developed and used together, but happen to be published in a manner that allows individual use.

Viewed at a high level, these plugins represent performance-focused features, yet each holds its unique functionality at a granular level. For instance, WebP Uploads handles image generation, while Optimization Detective specializes in optimization tasks. Given their standalone nature, they deserve development flexibility. Integration, if required, should be managed through integration tests, maintaining both individual integrity and collaborative cohesion.

At a detailed level, the flexibility introduced by #1065 encompasses all possible combinations. For instance, if plugin X relies on Y, simply include Y while testing X in its bootstrap file. This approach also facilitates testing Unit Tests for X independently and integration tests separately with the Y plugin enabled, offering comprehensive flexibility.

My perspective is that the Performance Lab program exists primarily to allow us to support developing and testing performance enhancements for WordPress in a collective manner, rather than as individual efforts.

In any scenario, I believe that none of the Developer Experience (DX) initiatives affect this; instead, they streamline processes for improved efficiency.

The benefits that we get from this strategy are many—including being able to share tooling, processes, configuration, and visibility of the projects being supported by the WordPress Performance Team.

There seems to be a misunderstanding regarding how we approached extended configuration for PHPCS, and this misunderstanding is being generalized to other tools, which may not apply uniformly due to their differing natures and functionalities. For instance, PHPCS is primarily designed to import rulesets, allowing CS packages to expose their sniffs to the PHPCS runner. Conversely, in PHPUnit, the configuration intended for importation, or sharing does not revolve around its XML configuration but rather involves extensions and test runner wrappers.

Even though PHPUnit can share configurations when needed, we must recognize the unique features of each tool. Right now, we're using shared tooling, and we can adapt as necessary.

Ultimately, if the preference is that we prioritize the DX for working on standalone plugins in isolation, then I think we may want to consider whether and to what extent each plugin should define its own dependencies, configuration, tooling, etc. rather than trying to manage some at the repo level and some at the plugin level.

Our approach to single-plugin functionality within modules was effective, but the transition to a mono repo presents an opportunity to refine our performance features with a more concentrated focus. Similar to the streamlined management in a monorepo, we can now optimize our development process for greater efficiency and effectiveness.

@thelovekesh

@thelovekesh @joemcgill @westonruter I have spent some time looking around at other projects in regards to the points that are being discussed here, which has reshaped my perspective a little.

Sharing a few thoughts here:

Our approach to single-plugin functionality within modules was effective, but the transition to a mono repo presents an opportunity to refine our performance features with a more concentrated focus. Similar to the streamlined management in a monorepo, we can now optimize our development process for greater efficiency and effectiveness.

It's worth noting that modules were just as much standalone as the plugins are now, this was a mandate from the beginning of the repository. So we were using shared tooling from the beginning, while the production codebases of the modules (now plugins) were not allowed to rely on anything external.

we must recognize the unique features of each tool. Right now, we're using shared tooling, and we can adapt as necessary.

+1 to that. Some tools are more limited than others in catering for the workflows we would want to have. I think when we run into such limitations, we have three options:

Find ways to work within the limitations of the tool.
Implement our own tooling (e.g. a custom script) that works around the limitations of the third-party tool.
Use another third-party tool that doesn't have the limitation.

With the latter, we also have to consider many other factors, such as how well another tool is maintained, familiarity of the WordPress developer community with it, how appropriate it is for the job etc.

Most importantly though, I think we are getting stuck at this point in tooling discussions, but we have not even determined as a team what our developer experience should holistically look like.

An example is that some of us suggest developing from a specific plugin's directory, while others suggest developing from the repository root. Those are entirely different approaches we need to discuss more. Potentially we can later find a tooling approach that works for both. What are the benefits of one or the other? Do we want to optimize for one or the other?

I do agree with @joemcgill's assessment that this project and monorepo is a collective effort. So we should not just have individual plugin directories where the maintainers of said plugin can then implement anything in any way they like. A shared approach and, to some degree, shared tooling is crucial here. That said, this does not necessarily mean we can't have individual plugin specific tooling. But I think we need to think about tooling collectively.

We're already doing that in some instances. Having a central PHPCS config while also having a bare minimum PHPCS config per plugin which mostly imports the central one is a great solution that caters for both the collective and individual needs.

A more complicated example is PHPUnit (see #1065), where the PHPCS approach is not really feasible as the config files cannot be composed. Seeing that several other popular monorepo projects (e.g. https://github.com/symfony/symfony or https://github.com/Automattic/jetpack) are using phpunit.xml.dist files per subdirectory makes me more comfortable with that approach, potentially. Though, if we go with that approach, I would also argue that the approach of using NPM for PHP tooling just because of composer.json limitations (see #1090) does not make sense - we might as well have composer.json files per plugin in that case, which is just as common as phpunit.xml.dist files per plugin.

I realize I now mentioned specific examples here, though that was mostly to clarify my overarching perspective. Let's take a step back and define which kind of developer workflows we want to encourage and optimize for, holistically, rather than discussing individual tooling concerns without having defined the bigger picture.

An example is that some of us suggest developing from a specific plugin's directory, while others suggest developing from the repository root. Those are entirely different approaches we need to discuss more. Potentially we can later find a tooling approach that works for both. What are the benefits of one or the other? Do we want to optimize for one or the other?

If we do facilitate developing with the working directory being the plugin directory, this doesn't mean that the ability to develop from the root would have to be sacrificed. I think both should be facilitated. But IMO, it makes more sense to optimize for developing with the standalone plugin as the working directory (e.g. running phpcs from there automatically locates the correct phpcs.xml.dist).

An example is that some of us suggest developing from a specific plugin's directory, while others suggest developing from the repository root. Those are entirely different approaches we need to discuss more. Potentially we can later find a tooling approach that works for both. What are the benefits of one or the other? Do we want to optimize for one or the other?

I'm in favor of making it work both ways, as we can't mandate developers to work from a specific root or plugin directory; it's entirely their choice. Adopting the NPM script runner can address this issue promptly, ensuring equal functionality in both scenarios. By standardizing our scripts in NPM instead of Composer, we can achieve this goal. While I acknowledge that multiple scripts might result in a bloated package.json file, we can explore Joe's suggestion of consolidating commands and passing arguments to target specific plugin(s), thereby maintaining simplicity with a single command.

Example:

# A single command can be used to handle all use cases IMO.
npm run lint:php # Run linting for PL plugin.
npm run lint:php -- --phpcs-args # Pass arguments to tool executable.
npm run lint:php --plugin=foo -- --phpcs-args # Pass both NPM and tool args.
npm run lint:php --plugin=foo # Run linting for specific plugin.
npm run lint:php --plugin=foo,bar # Run linting for two plugins at a time.
npm run lint:php --all # Run linting on all files.

Though, if we go with that approach, I would also argue that the approach of using NPM for PHP tooling just because of composer.json limitations (see #1090) does not make sense - we might as well have composer.json files per plugin in that case, which is just as common as phpunit.xml.dist files per plugin.

While we can do so, is it truly necessary? Considering that we already have an executable binary for each tool in ./vendor/bin by default, and NPM can execute them from any directory out of the box. The exception lies with Jetpack, where multiple use cases exist, and composer autoloading per plugin is one of them, which cannot be accomplished with a single composer config. Moreover, given the considerable weight of the Jetpack repo, configuring per plugin aids in use cases like sparse git checkout, where users may prefer to clone only what is necessary.

EDIT: Seems like I just deleted the previous comment history which was an edit in above commands example.

@westonruter

But IMO, it makes more sense to optimize for developing with the standalone plugin as the working directory (e.g. running phpcs from there automatically locates the correct phpcs.xml.dist).

@westonruter We can accomplish this in both scenarios by utilizing NPM as a script runner. You can refer to the examples provided above. Alternatively, those who prefer not to use an NPM run the command can execute binaries directly, such as ../../vendor/bin/phpcs. Another option is to use phpcs directly if ./vendor/bin is added to the PATH by their IDE/bashrc or themselves 😎.

@felixarntz

@felixarntz Just checking if we have any updates here to unblock #1065.

@thelovekesh

@thelovekesh I think we need to further discuss what we actually want here. Kind of following up on #1012 (comment), I think we should take a step back from further developing towards a solution we haven't even defined what it should look like.

The major overarching question to answer here is: Is our encouraged development workflow from the repository's root directory, or from the individual plugin directories? Making a decision on that will guide our current and future engineering decisions.

While ideally we would be able to cater equally for both, we're already seeing in several instances that that is either not possible or at least requiring complicated workarounds that lead to further discussions because they may seem hacky. Additionally, catering for both, in part because of those complexities, will require more time and resource investment and thus distract us from what we actually should be doing - work on performance features for the end users.

Let's discuss the tradeoffs between the two development approaches. After considering these, we should make a decision on what is our encouraged and formally supported workflow. From there, we can implement accordingly.

@joemcgill

Catching up after my long leave, any updates in regards to this issue? I saw #1262 got merged, which is fair, though it seems that #1262 (comment) still needs to be addressed.

Anything else that happened related to this topic but outside this issue?

Curious for any updates @joemcgill @westonruter @thelovekesh

@felixarntz

@felixarntz Welcome back 🎉

A couple other PRs that are related to this issue:

These make it much easier to run the tests for a single standalone plugin.

Also related is:

Move Optimization Detective test cases into separate files and add helper trait to reduce code duplication #1405

This helper trait is in the shared /tests directory and is reused by multiple plugins. This would work regardless of whether there is a single phpunit.xml for the entire plugin suite or individual phpunit.xml configs for each standalone plugin. Nevertheless, there doesn't seem to be a strong need to have separate phpunit.xml configs at the moment. So I think we can close this issue and reconsider down the road.

though it seems that #1262 (comment) still needs to be addressed

The Enhanced Responsive Images (auto-sizes) plugin now bootstraps Image Prioritizer and Optimization Detective when it runs its tests since when that plugin is active it has additional integrations (#1322). The same is true for Embed Optimizer which bootstraps Optimization Detective (#1302). And of course Image Prioritizer has to bootstrap Optimization Detective since there is a plugin dependency. Other than these cases, we don't have other situations where different combinations of plugins are tested. But it seems these could be added in /tests as the need arises. We'd have to re-think the --testsuite arg we use in bootstrapping.

I think we can close this issue as completed.

The Enhanced Responsive Images (auto-sizes) plugin now bootstraps Image Prioritizer and Optimization Detective when it runs its tests since when that plugin is active it has additional integrations (#1322). The same is true for Embed Optimizer which bootstraps Optimization Detective (#1302). And of course Image Prioritizer has to bootstrap Optimization Detective since there is a plugin dependency. Other than these cases, we don't have other situations where different combinations of plugins are tested. But it seems these could be added in /tests as the need arises. We'd have to re-think the --testsuite arg we use in bootstrapping.

And yet, see #1433 (comment).

Left a reply in #1433 (comment). I feel like generic tests for a "random" combination of our plugins active wouldn't make to much sense from a PHPUnit perspective. IMO this would be mostly about ensuring there's no fatal error from the mere presence of those plugins together.

For more specific scenarios of testing a plugin with its dependency, I think those are still first and foremost testing of that plugin and therefore make more sense in the plugin's own tests directory, maybe as a second test suite with the dependency plugin active.

On another note, +1 to closing this issue as completed.

For more specific scenarios of testing a plugin with its dependency, I think those are still first and foremost testing of that plugin and therefore make more sense in the plugin's own tests directory, maybe as a second test suite with the dependency plugin active.

Absolutely! Ideally, if plugin Foo is designed to work with plugin Bar, it should have two test suites:

Its own unit test suite.
An integration test suite for plugin Foo with Bar.

With our current setup, it would be challenging to test things this way because we only have a single bootstrap and PHPUnit configuration file. Having an independent PHPUnit config and bootstrap file for each plugin, located in its own test directory, is something to consider in such cases.

Navigation Menu

Repository navigation

DX improvements in new monorepo #1012

Tasklist

18 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	// Check if we use the plugin's test suite. If so, disable the PL plugin and only load the requested plugin.
	$plugin_name = '';
	foreach ( $_SERVER['argv'] as $index => $arg ) {
	if (
	'--testsuite' === $arg &&
	isset( $_SERVER['argv'][ $index + 1 ] ) &&
	'performance-lab' !== $_SERVER['argv'][ $index + 1 ] &&
	file_exists( TESTS_PLUGIN_DIR . '/plugins/' . $_SERVER['argv'][ $index + 1 ] )
	) {
	$plugin_name = $_SERVER['argv'][ $index + 1 ];
	}
	}

	if ( $plugin_name ) {
	$plugin_test_path = TESTS_PLUGIN_DIR . '/plugins/' . $plugin_name;
	tests_add_filter(
	'plugins_loaded',
	static function () use ( $plugin_test_path, $plugin_name ) {
	// Check if plugin has a "plugin/plugin.php" file.
	if ( file_exists( $plugin_test_path . '/' . $plugin_name . '.php' ) ) {
	require_once $plugin_test_path . '/' . $plugin_name . '.php';
	} elseif ( file_exists( $plugin_test_path . '/load.php' ) ) {
	// Check if plugin has a "plugin/load.php" file.
	require_once $plugin_test_path . '/load.php';
	} else {
	echo "Unable to locate standalone plugin bootstrap file in $plugin_test_path.";
	exit( 1 );
	}
	},
	1
	);
	} else {
	// Force plugin to be active.
	$GLOBALS['wp_tests_options'] = array(
	'active_plugins' => array( basename( TESTS_PLUGIN_DIR ) . '/load.php' ),
	);

	// Add filter to ensure the plugin's admin integration and all modules are loaded for tests.
	tests_add_filter(
	'plugins_loaded',
	static function () {
	require_once TESTS_PLUGIN_DIR . '/admin/load.php';
	require_once TESTS_PLUGIN_DIR . '/admin/server-timing.php';
	require_once TESTS_PLUGIN_DIR . '/admin/plugins.php';
	$module_files = glob( TESTS_PLUGIN_DIR . '/modules///load.php' );
	if ( $module_files ) {
	foreach ( $module_files as $module_file ) {
	require_once $module_file;
	}
	}
	},
	1
	);
	}

	<phpunit
	bootstrap="tests/bootstrap.php"
	backupGlobals="false"
	colors="true"
	convertErrorsToExceptions="true"
	convertNoticesToExceptions="true"
	convertWarningsToExceptions="true"
	>
	<testsuites>
	<testsuite name="default">
	<directory suffix=".php">./tests</directory>
	<exclude>./tests/utils</exclude>
	</testsuite>
	</testsuites>
	<groups>
	<exclude>
	<group>ms-required</group>
	</exclude>
	</groups>
	</phpunit>

	<phpunit
	bootstrap="./bootstrap.php"
	backupGlobals="false"
	colors="true"
	convertErrorsToExceptions="true"
	convertNoticesToExceptions="true"
	convertWarningsToExceptions="true"
	>
	<php>
	<const name="WP_TESTS_MULTISITE" value="1" />
	</php>
	<testsuites>
	<testsuite name="default">
	<directory suffix=".php">./</directory>
	<exclude>./utils</exclude>
	</testsuite>
	</testsuites>
	<groups>
	<exclude>
	<group>ms-excluded</group>
	</exclude>
	</groups>
	</phpunit>

DX improvements in new monorepo #1012

Description

Tasklist

Activity

thelovekesh commented on Feb 26, 2024

westonruter commented on Feb 26, 2024

felixarntz commented on Feb 26, 2024

westonruter commented on Feb 27, 2024

felixarntz commented on Feb 27, 2024

westonruter commented on Feb 27, 2024

felixarntz commented on Feb 27, 2024

westonruter commented on Feb 27, 2024

thelovekesh commented on Feb 27, 2024

felixarntz commented on Feb 27, 2024

westonruter commented on Feb 27, 2024

westonruter commented on Feb 27, 2024

18 remaining items

westonruter commented on Apr 4, 2024

thelovekesh commented on Apr 5, 2024

felixarntz commented on Apr 5, 2024

westonruter commented on Apr 5, 2024

thelovekesh commented on Apr 5, 2024

thelovekesh commented on Apr 5, 2024

thelovekesh commented on Apr 10, 2024

felixarntz commented on Apr 10, 2024

felixarntz commented on Aug 5, 2024

westonruter commented on Aug 5, 2024

westonruter commented on Aug 6, 2024

felixarntz commented on Aug 6, 2024

thelovekesh commented on Aug 6, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions