Please add relevant [Type] label and milestone to this PR.

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: AhmarZaidi <[email protected]>
Co-authored-by: westonruter <[email protected]>
Co-authored-by: thelovekesh <[email protected]>
Co-authored-by: mukeshpanchal27 <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

I suggest we handle the exit code from PHPUnit to detect if it exits due to the absence of the WP tests setup. This way, we can wrap the test command in a bash or PHP script, avoiding the need to check global variables that might be initialized at multiple points during the WP test suite lifecycle.

I suggest we handle the exit code from PHPUnit to detect if it exits due to the absence of the WP tests setup. This way, we can wrap the test command in a bash or PHP script, avoiding the need to check global variables that might be initialized at multiple points during the WP test suite lifecycle.

Makes sense but I just have one doubt.

Currently this is the error we are getting on running the composer test commands directly:

➜  composer test:plugins
> phpunit '--verbose' '--testsuite' 'auto-sizes'
Error: The following required constants are not defined: WP_TESTS_DOMAIN, WP_TESTS_EMAIL, WP_TESTS_TITLE, WP_PHP_BINARY.
Please check out `wp-tests-config-sample.php` for an example.
Script phpunit handling the test event returned with error code 1
Script @test --verbose --testsuite auto-sizes was called via test:auto-sizes
Script @test:auto-sizes was called via test:plugins

This error code 1 is from vendor/wp-phpunit/wp-phpunit/includes/bootstrap.php. A lot of other errors produce error code 1 so how would we determine that this is from incorrect WP test setup without checking for missing environment variables?

Humm yes, this is what I was concerned about. A better exit code between 0 to 255 can be used apart from the reserved ones to communicate the error type. But I am not sure if it can be added to the core tests bootstrap file.

Also, I think adding an exit with the proper message above this line should work in most of cases.

It can check if $_test_root exists or not and then show the message.

@westonruter Any thoughts on if we can propose to add a meaningful error code if the test process is exiting prematurely in case dependencies not met?

I guess any error code other than what phpunit returns in the case of a failure. I think it returns 1, so if you want a different exit code to be able to determine it failed for some other reason, then 2 seems fine.

Also, I think adding an exit with the proper message above this line should work in most of cases.

performance/tests/bootstrap.php

Line 35 in d300b2c

require_once $_test_root . '/includes/functions.php';

It can check if $_test_root exists or not and then show the message.

The $_test_root is also present on local without proper test env setup because it is being set by vendor/wp-phpunit/wp-phpunit/__loaded.php on composer install.

putenv( sprintf( 'WP_PHPUNIT__DIR=%s', dirname( __FILE__ ) ) );

elseif ( false !== getenv( 'WP_PHPUNIT__DIR' ) ) {
	$_test_root = getenv( 'WP_PHPUNIT__DIR' );
}

So, I don't think we can use that to determine if we should show error.

Also, the original issue was that if the user runs composer test:plugins command then they get a non descriptive error but if I'm not wrong, we would need to run composer test:plugins command from external script (bash or php) in order to get the exit code so that wouldn't solve the issue.

Please feel free correct me if I have misunderstood something here.

The $_test_root is also present on local without proper test env setup because it is being set by vendor/wp-phpunit/wp-phpunit/__loaded.php on composer install.

Well, that's another case, I am not aware of. Also, I am not sure if we use wp-phpunit apart from symbol identification for IDEs. Can we remove it? I am not fully sure why it was added.

Also, the #1256 was that if the user runs composer test:plugins command then they get a non descriptive error but if I'm not wrong, we would need to run composer test:plugins command from external script (bash or php) in order to get the exit code so that wouldn't solve the issue.Also, the #1256 was that if the user runs composer test:plugins command then they get a non descriptive error but if I'm not wrong, we would need to run composer test:plugins command from external script (bash or php) in order to get the exit code so that wouldn't solve the issue.

That doesn't mean we can't add a bash script to run the tests inside this command... right? But it's not doable right now because of the existing return code.

The issue with modifying the bootstrap file is that it can be loaded by PHPUnit in various environments, whereas this change only considers wp-env adjustments. Currently, if I run composer test:plugin in Lando, I encounter the following error:

➜  performance git:(trunk) ✗ lando composer test:plugins
> phpunit '--verbose' '--testsuite' 'auto-sizes'
Please use 'npm run test-php' instead of running the composer test command directly.
If you are intending to run PHPUnit directly, the following environment variables are missing: WORDPRESS_DB_PASSWORD, WORDPRESS_DB_HOST, WORDPRESS_DB_USER, WORDPRESS_DB_NAME
Script phpunit handling the test event returned with error code 1
Script @test --verbose --testsuite auto-sizes was called via test:auto-sizes
Script @test:auto-sizes was called via test:plugins

Without this change, my tests run as expected in the Lando environment.

The correct behavior should be: We need to detect when tests are failing due to a missing WP test suite setup and suggest using wp-env to run their tests.

Also, I think adding an exit with the proper message above this line should work in most of cases.

performance/tests/bootstrap.php

Line 35 in d300b2c

require_once $_test_root . '/includes/functions.php';

It can check if $_test_root exists or not and then show the message.

We can still achieve this. If $_test_root contains wp-phpunit/wp-phpunit, then check if the WP_PHPUNIT__TESTS_CONFIG environment variable or WP_TESTS_CONFIG_FILE_PATH exists. If it doesn't, we can exit with an error message like this:

echo 'Error: wp-tests-config.php is missing! Please use wp-tests-config-sample.php to create a config file.' . PHP_EOL;
echo 'Info: Alternatively, you can run `npm run wp-env start && npm run test-php` to execute tests in the wp-env environment.' . PHP_EOL;
exit(1);

In the rest of the cases, WP tests bootstrap will handle on its own:

https://github.com/WordPress/wordpress-develop/blob/df256e414c183ff001cdf26ebf37b19b960180de/tests/phpunit/includes/bootstrap.php#L25-L28

It will only jump to other cases if the user is using wp-env or some custom setup, so they know what they are doing :)

Given what @westonruter noted in #1314 (comment), I doubt if these lines are relevant anymore:

because wp-phpunit always defines the WP_PHPUNIT__DIR env variable.