WP_Env: tests-cli phpunit executable file not found in $PATH: unknown

[jrtashjian]

What?

When running commands with wp-env run container, use /bin/bash instead of /bin/sh (which docker exec defaults to).

Why?

Fixes #51686. The phpunit command exists in ~/.composer/vendor/bin directory, installed as a dependency of yoast/phpunit-polyfills, and the global composer bin directory is added to $PATH. However, when running npx wp-env run tests-cli phpunit the following error was displayed:

OCI runtime exec failed: exec failed: unable to start container process: exec: "phpunit": executable file not found in $PATH: unknown

The command was failing when executed in a /bin/sh environment but was successful in a /bin/bash environment.

Testing Instructions

To see the failure:

1. start an environment with the current public release of wp-env
2. Attempt to run phpunit with npx wp-env run tests-cli phpunit
3. Observe error.

To see success

1. start an environment with wp-env on this branch
2. Attempt to run phpunit with npx wp-env run tests-cli phpunit
3. Observe phpunit default output.

⭐ Kudos to @dawidurbanski for their help in this investigation!

[jrtashjian]

There is still some work to be done here. Although adding bash -c fixes the problem with phpunit not being found in $PATH it does alter how commands are run, ultimately only using the first argument and none of the flags. For example:

Executes only phpunit

$ [...]tests-cli /bin/bash -c phpunit --configuration phpunit.xml.dist --verbose --coverage-html phpunit/coverage

Same result with the double-dash

$ [...]tests-cli /bin/bash -c phpunit -- --configuration phpunit.xml.dist --verbose --coverage-html phpunit/coverage

Adding quotes executes the entire command with flags

$ [...]tests-cli /bin/bash -c "phpunit --configuration phpunit.xml.dist --verbose --coverage-html phpunit/coverage"

Without /bin/bash -c fails

$ [...]tests-cli phpunit --configuration phpunit.xml.dist --verbose --coverage-html phpunit/coverage

Related: #42286

[ObliviousHarmony]

Hey @jrtashjian,

Honestly, I'd really like to avoid ever doing anything like this. In #50559 I reworked the way that commands are passed to docker-compose run in order to have it behave more consistently. Having your commands pass through two different shells before being executed requires jumping through a bunch of different hoops in order to properly escape the input. Even when you manage to do that there are still commands that you can't ever write because of shell expansion. We should really keep wp-env run completely consistent with docker-compose run's command behavior.

---

If wonder if this failure is a problem with Docker rather than wp-env. I'm 99.999999% positive that this exact syntax worked when it was added. If we look at the PR that removed the phpunit service (#50408), there are pull requests referencing it using the syntax described in the error message. It's possible that it never did though 😄

As far as I can tell the problem is that Docker isn't using the $PATH variable from the correct user when running docker-compose exec. I verified this by adding ENV PATH="\${PATH}:/home/$HOST_USERNAME/.composer/vendor/bin" below USER www-data in cliDockerFileContents(). This adds phpunit to the path and it works as expected. What's interesting though is that once the executable has been started it uses the correct user with all of the correct environment variables. Check out npx wp-env run tests-cli printenv and you can see this behavior in action.

Editing the path of www-data is a possible solution but I don't think it's a very good one. Any other $PATH modifications to the container user still won't work correctly with wp-env run for whatever reason. It's probably worth diving into the Docker CLI and Composer issue trackers to see if this is something that is known, and if not, report the issue and see what happens.

[alexstine]

@ObliviousHarmony Try hardcoding the $PATH in entrypoint? If you were able to do this for every user, it should work.

echo "/some-path/modification:$PATH" >> ~/.bashrc

Maybe something like this?

for user in $(ls -1 /home); do
echo "/some-path/modification:$PATH" >> "/home/${user}/.bashrc"
done

I just wonder, if the www-data user does not have its own home directory, maybe you need to pass "--" in the command to inherit environment variables from the current context which I would guess is root by default?

[jrtashjian]

Thanks, @ObliviousHarmony! This is the feedback I was hoping for. I was sure I was missing some important context about the rework in #50559. During my investigation, I encountered similar issues to those you've outlined.

I can undo the modification I made and propose removing the global installation of yoast/phpunit-polyfills, which also brings phpunit as a dependency, allowing projects to handle it on a per-project basis. There doesn't appear to be a strong justification for a global installation of this dependency since its necessity varies significantly among different projects. Notably, even in this repository, it has been included as a dependency and is not run globally. PHPUnit documentation discourages global installations and instead recommends managing it as a project-local dependency.

This approach would completely avoid our current issue. I'll also update the wp-env readme, as I suspect it might have contributed to some confusion, given that phpunit used to work globally with docker run in the past.

[ObliviousHarmony]

Thanks for the suggestion @alexstine, but since we're using custom Docker images with wp-env we don't need to do that. I do want to thank you though; your suggestion made something click and I realized what is going wrong and what next steps we should take to get this working.

The problem is that the ~ token we're setting in the $PATH environment variable has no meaning until after the user has been switched to. docker-composer exec doesn't do this until after the process has been started, so, the ~ can't be expanded into anything. The fix is to use an absolute path to the composer bin directory.

[ObliviousHarmony]

There doesn't appear to be a strong justification for a global installation of this dependency since its necessity varies significantly among different projects.

Before I started spending a lot of time writing GitHub Workflows I would have 100% agreed with you here. Since initially removing the container, however, I have come across some important use-cases for keeping this feature alive.

Assuming that a plugin has a minimum version of PHP they should be setting that as an config.platform.php option in their composer.json file. This makes sure you consistently install dependency versions that fit your minimum PHP version. You would then set the phpVersion in your .wp-env.json file to the minimum as well and you're good to go. This is also why the unit tests in Gutenberg's package.json don't use the global PHPUnit.

The caveat here is that by setting the config.platform.php option you are going to lock the version of PHPUnit to whichever one is compatible with the version of PHP you've defined. This becomes a problem when you try to change the phpVersion to an incompatible one, such as in a CI testing matrix that runs on multiple PHP versions to test for compatibility. It doesn't matter what version of PHP is installed, when you run composer install, it will give you the PHPUnit for the defined version of PHP.

By globally installing the Yoast Polyfills, however, we are guaranteed to get the version of PHPUnit that is associated with the version of PHP the container is currently running. In hindsight, we probably shouldn't be installing the polyfills and should instead just be installing PHPUnit with an appropriate constraint and let Composer resolve it automatically based on the version of PHP running. There's nothing to be gained from installing the Yoast package. In any case, this means you can safely run PHPUnit in the container regardless of PHP version.

---

What do you think of this @jrtashjian:

• Change ENV PATH="\${PATH}:~/.composer/vendor/bin" to ENV PATH="\${PATH}:/home/$HOST_USERNAME/.composer/vendor/bin" in init-config.js. This makes it so that it can find the executable in wp-env run without any other changes.
• Replace RUN composer global require --dev yoast/phpunit-polyfills:"^1.0" with RUN composer global require --dev phpunit/phpunit:"^5.7.21 || ^6.0 || ^7.0 || ^8.0 || ^9.0 || ^10.0". This removes the unnecessary dependency but achieves the same effect in installing the correct PHPUnit.

This would solve the linked bug while better supporting the intended usage.

[dawidurbanski]

Replace RUN composer global require --dev yoast/phpunit-polyfills:"^1.0" with RUN composer global require --dev phpunit/phpunit:"^5.7.21 || ^6.0 || ^7.0 || ^8.0 || ^9.0 || ^10.0".

Are you guys sure about that? This would probably defeat the purpose of having phpunit in the context of testing in the WordPress environment.

If you create a test, that uses test case extending WP_UnitTestCase (which is probably most of the PHPunit tests written and ran using wp-env), and you only have plain phpunit available, you'll get:

Error: The PHPUnit Polyfills library is a requirement for running the WP test suite.
If you are trying to run plugin/theme integration tests, make sure the PHPUnit Polyfills library (https://github.com/Yoast/PHPUnit-Polyfills) is available and either load the autoload file of this library in your own test bootstrap before calling the WP Core test bootstrap file; or set the absolute path to the PHPUnit Polyfills library in a "WP_TESTS_PHPUNIT_POLYFILLS_PATH" constant to allow the WP Core bootstrap to load the Polyfills.

So in 99% of cases I will still be required to require polyfills myself in the project (which has phpunit as dependency).

Not to even mention that yoast/phpunit-polyfills now released version 2.0 that supports PHPUnit 10 and testing in PHP version > 8.1, which would be even better to include.

EDIT: to know why polyfills are now requirement for WP_UnitTestCase see this.

Also from the "What has changed?" section:

7. The WordPress Core test bootstrap file will throw an error when the PHPUnit Polyfills are not available or do not comply with the minimum version requirements.

So having this minimal (required to run your WordPress tests) bootstrap file, is going to throw the above error:

<?php

// bootstrap.php

$wp_tests_dir = getenv( 'WP_TESTS_DIR' ) ? getenv( 'WP_TESTS_DIR' ) : sys_get_temp_dir();

require_once $wp_tests_dir . '/includes/bootstrap.php';

This comes from: https://github.com/WordPress/wordpress-develop/blob/8161a8c62a2bfdee02fdbeb4aa7ea773f42e7afa/tests/phpunit/includes/bootstrap.php#L80

[ObliviousHarmony]

So in 99% of cases I will still be required to require polyfills myself in the project (which has phpunit as dependency).

I'm only talking about the global install. Your project will still need to install the polyfills. The polyfills provide a consistent API that can replace methods in the PHPUnit base class that have breaking changes between versions. The only thing we are talking about here is the global PHPUnit installation.

[jrtashjian]

Thanks, @ObliviousHarmony! That worked perfectly. I've pushed up the new changes for review.

[ObliviousHarmony]

Thanks @jrtashjian,

This tests well and looks good.

[ObliviousHarmony]

Oh, one thing! It needs a changelog @jrtashjian 😄

[BrianHenryIE]

On MacOS, I have a bug that looks like it will be fixed with this. Running:

npx wp-env start --xdebug

I get:

...

#28 [tests-wordpress 23/24] RUN rm /tmp/composer-setup.php
#28 CACHED

#29 [tests-wordpress 24/24] RUN composer global require --dev yoast/phpunit-polyfills:"^1.0"
#29 0.137 /bin/sh: 1: composer: not found
#29 ERROR: process "/bin/sh -c composer global require --dev yoast/phpunit-polyfills:\"^1.0\"" did not complete successfully: exit code: 127
------
 > [tests-wordpress 24/24] RUN composer global require --dev yoast/phpunit-polyfills:"^1.0":
0.137 /bin/sh: 1: composer: not found
------
failed to solve: process "/bin/sh -c composer global require --dev yoast/phpunit-polyfills:\"^1.0\"" did not complete successfully: exit code: 127