Local test workflow changes for installing Composer dependencies

[hellofromTonya]

Related to: #47381, #46149

With the adoption of PHPUnit Polyfills (see ticket #46149 and changeset [51559]), its dependencies are installed via and tests run using Composer (see ticket #46149 and changeset [51545]).

For the CI, this makes sense. What about the ​instead build the Composer extra steps `npm` / Docker workflow when testing locally?

The Problem

The npm workflow for running PHPUnit tests locally:

npm install
npm run build:dev
npm run env:start
npm run env:install
npm run test:php

• This works on the WP 5.8 branch ✅
• Does not work/run on trunk/master branch ❌

  Error: The PHPUnit Polyfills library is a requirement for running the WP test suite.
  You need to run `composer update` before running the tests.
  Once the dependencies are installed, you can run the tests using the Composer-installed version of PHPUnit or using a PHPUnit phar file, but the dependencies do need to be installed whichever way the tests are run.
  

Extra steps currently needed are:

• Step 1: Install and setup Composer locally
• Step 2: Install dependencies:

  composer install
  npm install
  npm run build:dev
  npm run env:start
  npm run env:install
  npm run test:php
  

These extra steps increase the complexity, barrier, and effort for contributors.

Proposal

This ticket proposes to move the extra steps into the Docker images as part of the existing npm workflow. In other words, build it into the tooling.

Why?

To avoid passing these steps on to contributors. With this change, contributors can continue using the same npm commands to locally run tests.

What about alternative workflows?

Composer is and has been an alternative workflow. More advanced testing scenarios may need it to enable/disable different PHP extensions. For contributors who wish to use Composer locally, this alternative workflow is available for them.

For those contributors who do not wish to use Composer or npm/Docker, they can manually install PHPUnit, PHPUnit Polyfills, etc. and use the WP_TESTS_PHPUNIT_POLYFILLS_PATH constant to specify where the polyfills exist on their machine.

[hellofromTonya]

Whoopsie copy/paste issue with the link to running tests in the README.md.

[azaozz]

Hehe, not sure "Proposal" is the right word here, it should be "accepted" by default. This is a part of making the WP tests depend on third-party software. If the Polyfills are installed globally in the Docker image (hopefully with only the needed version of PHPUnit as the PHP version is "fixed" there), testing will work again as it used to.

This is by far the easiest/most user friendly way to handle the changes in PHP testing.

[netweb]

I'm not able to get this working on either the /trunk or /branches/5.8 branches

It seems to hang when running the npm script npm run env:install

I've added the wp_cli_info() script here only for debugging so that an additional script is run after install_composer_dependancies() as this was where the scripts appeared to be hung

tools/local-env/scripts/install.js

@@ -22 +22 @@
 
 install_wp_importer();
 
+install_composer_dependancies();
+
+wp_cli_info();
+
 // Read in wp-tests-config-sample.php, edit it to work with our config, then write it to wp-tests-config.php.
 const testConfig = readFileSync( 'wp-tests-config-sample.php', 'utf8' )
        .replace( 'youremptytestdbnamehere', 'wordpress_develop_tests' )
@@ -57 +61 @@
        execSync( `docker-compose exec -T php rm -rf ${testPluginDirectory}`, { stdio: 'inherit' } );
        execSync( `docker-compose exec -T php git clone https://github.com/WordPress/wordpress-importer.git ${testPluginDirectory} --depth=1`, { stdio: 'inherit' } );
 }
+
+/**
+ * Installs the Composer package dependancies.
+ */
+function install_composer_dependancies() {
+       execSync( `docker-compose exec -T php composer install`, { stdio: 'inherit' } );
+}
+
+/**
+ * Get the environment information.
+ */
+function wp_cli_info() {
+       wp_cli( `cli info` );
+
+}

With the above patch, composer install is run and the yoast/phpunit-polyfills package is installed in the PHP Docker container:

No composer.lock file present. Updating dependencies to latest instead of installing from lock file. See https://getcomposer.org/install for more information.
Loading composer repositories with package information
Updating dependencies
Lock file operations: 41 installs, 0 updates, 0 removals
  - Locking dealerdirect/phpcodesniffer-composer-installer (v0.7.1)
  - Locking doctrine/instantiator (1.4.0)
  - Locking myclabs/deep-copy (1.10.2)
  - Locking nikic/php-parser (v4.12.0)
  - Locking phar-io/manifest (2.0.3)
  - Locking phar-io/version (3.1.0)
  - Locking phpcompatibility/php-compatibility (9.3.5)
  - Locking phpcompatibility/phpcompatibility-paragonie (1.3.1)
  - Locking phpcompatibility/phpcompatibility-wp (2.1.2)
  - Locking phpdocumentor/reflection-common (2.2.0)
  - Locking phpdocumentor/reflection-docblock (5.2.2)
  - Locking phpdocumentor/type-resolver (1.4.0)
  - Locking phpspec/prophecy (1.13.0)
  - Locking phpunit/php-code-coverage (9.2.6)
  - Locking phpunit/php-file-iterator (3.0.5)
  - Locking phpunit/php-invoker (3.1.1)
  - Locking phpunit/php-text-template (2.0.4)
  - Locking phpunit/php-timer (5.0.3)
  - Locking phpunit/phpunit (9.5.8)
  - Locking sebastian/cli-parser (1.0.1)
  - Locking sebastian/code-unit (1.0.8)
  - Locking sebastian/code-unit-reverse-lookup (2.0.3)
  - Locking sebastian/comparator (4.0.6)
  - Locking sebastian/complexity (2.0.2)
  - Locking sebastian/diff (4.0.4)
  - Locking sebastian/environment (5.1.3)
  - Locking sebastian/exporter (4.0.3)
  - Locking sebastian/global-state (5.0.3)
  - Locking sebastian/lines-of-code (1.0.3)
  - Locking sebastian/object-enumerator (4.0.4)
  - Locking sebastian/object-reflector (2.0.4)
  - Locking sebastian/recursion-context (4.0.4)
  - Locking sebastian/resource-operations (3.0.3)
  - Locking sebastian/type (2.3.4)
  - Locking sebastian/version (3.0.2)
  - Locking squizlabs/php_codesniffer (3.6.0)
  - Locking symfony/polyfill-ctype (v1.23.0)
  - Locking theseer/tokenizer (1.2.1)
  - Locking webmozart/assert (1.10.0)
  - Locking wp-coding-standards/wpcs (2.3.0)
  - Locking yoast/phpunit-polyfills (1.0.1)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 41 installs, 0 updates, 0 removals
  - Downloading squizlabs/php_codesniffer (3.6.0)
  - Downloading dealerdirect/phpcodesniffer-composer-installer (v0.7.1)
  - Downloading myclabs/deep-copy (1.10.2)
  - Downloading phar-io/version (3.1.0)
  - Downloading phar-io/manifest (2.0.3)
  - Downloading phpcompatibility/php-compatibility (9.3.5)
  - Downloading phpcompatibility/phpcompatibility-paragonie (1.3.1)
  - Downloading phpcompatibility/phpcompatibility-wp (2.1.2)
  - Downloading phpdocumentor/reflection-common (2.2.0)
  - Downloading phpdocumentor/type-resolver (1.4.0)
  - Downloading sebastian/recursion-context (4.0.4)
  - Downloading sebastian/exporter (4.0.3)
  - Downloading sebastian/diff (4.0.4)
  - Downloading sebastian/comparator (4.0.6)
  - Downloading symfony/polyfill-ctype (v1.23.0)
  - Downloading webmozart/assert (1.10.0)
  - Downloading phpdocumentor/reflection-docblock (5.2.2)
  - Downloading doctrine/instantiator (1.4.0)
  - Downloading phpspec/prophecy (1.13.0)
  - Downloading theseer/tokenizer (1.2.1)
  - Downloading sebastian/version (3.0.2)
  - Downloading nikic/php-parser (v4.12.0)
  - Downloading sebastian/lines-of-code (1.0.3)
  - Downloading sebastian/environment (5.1.3)
  - Downloading sebastian/complexity (2.0.2)
  - Downloading sebastian/code-unit-reverse-lookup (2.0.3)
  - Downloading phpunit/php-text-template (2.0.4)
  - Downloading phpunit/php-file-iterator (3.0.5)
  - Downloading phpunit/php-code-coverage (9.2.6)
  - Downloading phpunit/php-invoker (3.1.1)
  - Downloading phpunit/php-timer (5.0.3)
  - Downloading sebastian/cli-parser (1.0.1)
  - Downloading sebastian/code-unit (1.0.8)
  - Downloading sebastian/object-reflector (2.0.4)
  - Downloading sebastian/global-state (5.0.3)
  - Downloading sebastian/object-enumerator (4.0.4)
  - Downloading sebastian/resource-operations (3.0.3)
  - Downloading sebastian/type (2.3.4)
  - Downloading wp-coding-standards/wpcs (2.3.0)
  - Downloading phpunit/phpunit (9.5.8)
  - Downloading yoast/phpunit-polyfills (1.0.1)
  0/41 [>---------------------------]   0%
  8/41 [=====>----------------------]  19%
  9/41 [======>---------------------]  21%
 10/41 [======>---------------------]  24%
 15/41 [==========>-----------------]  36%
 18/41 [============>---------------]  43%
 19/41 [============>---------------]  46%
 20/41 [=============>--------------]  48%
 25/41 [=================>----------]  60%
 27/41 [==================>---------]  65%
 29/41 [===================>--------]  70%
 34/41 [=======================>----]  82%
 38/41 [=========================>--]  92%
 39/41 [==========================>-]  95%
 40/41 [===========================>]  97%
 41/41 [============================] 100%
  - Installing squizlabs/php_codesniffer (3.6.0): Extracting archive
  - Installing dealerdirect/phpcodesniffer-composer-installer (v0.7.1): Extracting archive
  - Installing myclabs/deep-copy (1.10.2): Extracting archive
  - Installing phar-io/version (3.1.0): Extracting archive
  - Installing phar-io/manifest (2.0.3): Extracting archive
  - Installing phpcompatibility/php-compatibility (9.3.5): Extracting archive
  - Installing phpcompatibility/phpcompatibility-paragonie (1.3.1): Extracting archive
  - Installing phpcompatibility/phpcompatibility-wp (2.1.2): Extracting archive
  - Installing phpdocumentor/reflection-common (2.2.0): Extracting archive
  - Installing phpdocumentor/type-resolver (1.4.0): Extracting archive
  - Installing sebastian/recursion-context (4.0.4): Extracting archive
  - Installing sebastian/exporter (4.0.3): Extracting archive
  - Installing sebastian/diff (4.0.4): Extracting archive
  - Installing sebastian/comparator (4.0.6): Extracting archive
  - Installing symfony/polyfill-ctype (v1.23.0): Extracting archive
  - Installing webmozart/assert (1.10.0): Extracting archive
  - Installing phpdocumentor/reflection-docblock (5.2.2): Extracting archive
  - Installing doctrine/instantiator (1.4.0): Extracting archive
  - Installing phpspec/prophecy (1.13.0): Extracting archive
  - Installing theseer/tokenizer (1.2.1): Extracting archive
  - Installing sebastian/version (3.0.2): Extracting archive
  - Installing nikic/php-parser (v4.12.0): Extracting archive
  - Installing sebastian/lines-of-code (1.0.3): Extracting archive
  - Installing sebastian/environment (5.1.3): Extracting archive
  - Installing sebastian/complexity (2.0.2): Extracting archive
  - Installing sebastian/code-unit-reverse-lookup (2.0.3): Extracting archive
  - Installing phpunit/php-text-template (2.0.4): Extracting archive
  - Installing phpunit/php-file-iterator (3.0.5): Extracting archive
  - Installing phpunit/php-code-coverage (9.2.6): Extracting archive
  - Installing phpunit/php-invoker (3.1.1): Extracting archive
  - Installing phpunit/php-timer (5.0.3): Extracting archive
  - Installing sebastian/cli-parser (1.0.1): Extracting archive
  - Installing sebastian/code-unit (1.0.8): Extracting archive
  - Installing sebastian/object-reflector (2.0.4): Extracting archive
  - Installing sebastian/global-state (5.0.3): Extracting archive
  - Installing sebastian/object-enumerator (4.0.4): Extracting archive
  - Installing sebastian/resource-operations (3.0.3): Extracting archive
  - Installing sebastian/type (2.3.4): Extracting archive
  - Installing wp-coding-standards/wpcs (2.3.0): Extracting archive
  - Installing phpunit/phpunit (9.5.8): Extracting archive
  - Installing yoast/phpunit-polyfills (1.0.1): Extracting archive
  0/39 [>---------------------------]   0%
  9/39 [======>---------------------]  23%
 10/39 [=======>--------------------]  25%
 17/39 [============>---------------]  43%
 19/39 [=============>--------------]  48%
 25/39 [=================>----------]  64%
 29/39 [====================>-------]  74%
 35/39 [=========================>--]  89%
 37/39 [==========================>-]  94%
 38/39 [===========================>]  97%
 39/39 [============================] 100%
9 package suggestions were added by new dependencies, use `composer suggest` to see details.
Generating autoload files
26 packages you are using are looking for funding.
Use the `composer fund` command to find out more!

This is as far as I can get, but I'd be interested to here any feedback from others who are able to run this in either the /trunk or /branches/5.8 branches

[hellofromTonya]

During a ​live mob working session with @netweb @johnbillion @desrosj @jrf and me, achieved consensus on how to make the Composer install invisible to contributors when doing local npm/Docker testing.

• In the install.js file, adding the following function and command:

  /**
   * Installs the Composer package dependencies within the Docker environment.
   */
  function install_composer_dependencies() {
  	execSync( `docker-compose run -T php composer update -W`, { stdio: 'inherit' } );
  }
  

• What about if someone deletes the vendor folder and/or composer.lock file? Add a cascade command with the test:php and test:php-composer commands to first run composer update. Johnathan proposed to package.json "scripts":

  "test:php-composer": "node ./tools/local-env/scripts/docker.js run -rm composer update -W && node ./tools/local-env/scripts/docker.js run --rm phpunit php ./vendor/bin/phpunit",
  

As agreed to during a live mob working session, this PR achieves the following:

• Installs the Composer package dependencies within the Docker environment as part of the installation process.
• Ensures these deps are installed when running npm run test:php or npm run test:php-composer, i.e. runs composer update -W to guard for scenario where contributor removes the vendor and/or composer.lock file.

## Notes

For cascading composer update command within the npm run test:php-composer and npm run test:php script commands, the proposed run -rm composer update -W worked but outputs the following messages in the command-line:

Run a one-off command on a service.

For example:

    $ docker-compose run web python manage.py shell

By default, linked services will be started, unless they are already
running. If you do not want to start linked services, use
`docker-compose run --no-deps SERVICE COMMAND [ARGS...]`.

Usage:
    run [options] [-v VOLUME...] [-p PORT...] [-e KEY=VAL...] [-l KEY=VALUE...] [--]
        SERVICE [COMMAND] [ARGS...]

As this message could confuse contributors, this PR switches to using the same command as in the install.js script:

run -T php composer update -W

Using this command also works but without the above Run a one-off command on a service. notification.

## Testing Scenarios

Are available in the Trac ticket.

Trac ticket: https://core.trac.wordpress.org/ticket/53945

[hellofromTonya]

Testing Scenarios

In these examples:

• the @group formatting is used to run a smaller sample of tests (i.e. to speed up testing):

npm run test:php -- --group formatting

or

npm run test:php-composer -- --group formatting

• the ... (in the command-line output example)s means more packages are Locking or Downloading. Using this notation for brevity.

Scenario 1: Docker not running (or container destroyed) and npm and composer packages not installed.

Assumes Docker is installed locally.

Expectations:

• Composer package dependencies are installed
• composer.lock file is created
• Tests run

Instructions:

Contributor types the following commands into their command-line tool:

npm install
npm run build:dev
npm run env:start
npm run env:install

When running npm run env:install, the expected command-line output (or similar):

Creating wordpress-develop_cli_run ... done
Success: Generated 'wp-config.php' file.
Creating wordpress-develop_cli_run ... done
Success: Added the constant 'WP_DEBUG' to the 'wp-config.php' file with the raw value 'true'.
Creating wordpress-develop_cli_run ... done
Success: Added the constant 'WP_DEBUG_LOG' to the 'wp-config.php' file with the raw value 'true'.
Creating wordpress-develop_cli_run ... done
Success: Added the constant 'WP_DEBUG_DISPLAY' to the 'wp-config.php' file with the raw value 'true'.
Creating wordpress-develop_cli_run ... done
Success: Added the constant 'SCRIPT_DEBUG' to the 'wp-config.php' file with the raw value 'true'.
Creating wordpress-develop_cli_run ... done
Success: Added the constant 'WP_ENVIRONMENT_TYPE' to the 'wp-config.php' file with the value 'local'.
Cloning into 'tests/phpunit/data/plugins/wordpress-importer'...
Cloning into 'tests/phpunit/data/plugins/wordpress-importer'...
Creating wordpress-develop_php_run ... done
Loading composer repositories with package information
Updating dependencies
Lock file operations: 41 installs, 0 updates, 0 removals
  - Locking dealerdirect/phpcodesniffer-composer-installer (v0.7.1)
 ...
  - Locking yoast/phpunit-polyfills (1.0.1)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 41 installs, 0 updates, 0 removals
  - Downloading squizlabs/php_codesniffer (3.6.0)
  ...
  - Downloading phpunit/phpunit (9.5.8)
  - Downloading yoast/phpunit-polyfills (1.0.1)
  0/41 [>---------------------------]   0%
  3/41 [==>-------------------------]   7%
  8/41 [=====>----------------------]  19%
 11/41 [=======>--------------------]  26%
 16/41 [==========>-----------------]  39%
 19/41 [============>---------------]  46%
 22/41 [===============>------------]  53%
 27/41 [==================>---------]  65%
 31/41 [=====================>------]  75%
 35/41 [=======================>----]  85%
 39/41 [==========================>-]  95%
 40/41 [===========================>]  97%
 41/41 [============================] 100%
  - Installing squizlabs/php_codesniffer (3.6.0): Extracting archive
  ...
  - Installing phpunit/phpunit (9.5.8): Extracting archive
  - Installing yoast/phpunit-polyfills (1.0.1): Extracting archive
  0/39 [>---------------------------]   0%
  6/39 [====>-----------------------]  15%
  9/39 [======>---------------------]  23%
 11/39 [=======>--------------------]  28%
 14/39 [==========>-----------------]  35%
 17/39 [============>---------------]  43%
 20/39 [==============>-------------]  51%
 23/39 [================>-----------]  58%
 26/39 [==================>---------]  66%
 29/39 [====================>-------]  74%
 31/39 [======================>-----]  79%
 34/39 [========================>---]  87%
 36/39 [=========================>--]  92%
 37/39 [==========================>-]  94%
 38/39 [===========================>]  97%
 39/39 [============================] 100%
9 package suggestions were added by new dependencies, use `composer suggest` to see details.
Generating autoload files
26 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
PHP CodeSniffer Config installed_paths set to ../../phpcompatibility/php-compatibility,../../phpcompatibility/phpcompatibility-paragonie,../../phpcompatibility/phpcompatibility-wp,../../wp-coding-standards/wpcs
Creating wordpress-develop_cli_run ... done
Success: Database reset.
Creating wordpress-develop_cli_run ... done
Success: WordPress installed successfully.

Once completed, the contributor runs the tests:

npm run test:php -- --group formatting

or

npm run test:php-composer -- --group formatting

Scenario 2: composer.lock file is deleted

The contributor has already run the above commands to setup the local testing environment but then deletes the composer.lock file.

Expectations:

• composer.lock file is created
• Tests run

Instructions:

Notes for testers: Run scenario 1 first. Then delete the composer.lock file.

Contributor types the following in their command-line tool:

npm run test:php -- --group formatting

or

npm run test:php-composer -- --group formatting

Expected command-line output (or similar):

Creating wordpress-develop_php_run ... done
Loading composer repositories with package information
Updating dependencies
Lock file operations: 41 installs, 0 updates, 0 removals
  - Locking dealerdirect/phpcodesniffer-composer-installer (v0.7.1)
  ...
  - Locking yoast/phpunit-polyfills (1.0.1)
Writing lock file
Installing dependencies from lock file (including require-dev)
Nothing to install, update or remove
9 package suggestions were added by new dependencies, use `composer suggest` to see details.
Generating autoload files
26 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
Creating wordpress-develop_phpunit_run ... done
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

.............................................................   61 / 1473 (  4%)
...
.........                                                     1473 / 1473 (100%)

Time: 7.07 seconds, Memory: 128.00 MB
OK (1473 tests, 2741 assertions)

Notes for testers: The PHPUnit version will depend upon your PHP version. The Time and Memory values may be different on your machine.

Scenario 3: vendor folder is deleted

The contributor has already run the above commands to setup the local testing environment but then deletes the vendor folder.

Expectations:

• Composer package dependencies are installed
• No changes to the composer.lock file
• Tests run

Instructions

Notes for testers: Same as Scenario 2 but this time delete the vendor folder.

Contributor types the following in their command-line tool:

npm run test:php -- --group formatting

or

npm run test:php-composer -- --group formatting

Expected command-line output (or similar):

Creating wordpress-develop_php_run ... done
Loading composer repositories with package information
Updating dependencies
Nothing to modify in lock file
Installing dependencies from lock file (including require-dev)
Package operations: 41 installs, 0 updates, 0 removals
  - Downloading squizlabs/php_codesniffer (3.6.0)
  ...
  - Downloading yoast/phpunit-polyfills (1.0.1)
  0/41 [>---------------------------]   0%
  7/41 [====>-----------------------]  17%
  8/41 [=====>----------------------]  19%
 12/41 [========>-------------------]  29%
 16/41 [==========>-----------------]  39%
 23/41 [===============>------------]  56%
 27/41 [==================>---------]  65%
 32/41 [=====================>------]  78%
 37/41 [=========================>--]  90%
 40/41 [===========================>]  97%
 41/41 [============================] 100%
  - Installing squizlabs/php_codesniffer (3.6.0): Extracting archive
  ...
  - Installing yoast/phpunit-polyfills (1.0.1): Extracting archive
  0/39 [>---------------------------]   0%
  6/39 [====>-----------------------]  15%
  9/39 [======>---------------------]  23%
 10/39 [=======>--------------------]  25%
 11/39 [=======>--------------------]  28%
 14/39 [==========>-----------------]  35%
 16/39 [===========>----------------]  41%
 19/39 [=============>--------------]  48%
 22/39 [===============>------------]  56%
 24/39 [=================>----------]  61%
 28/39 [====================>-------]  71%
 32/39 [======================>-----]  82%
 33/39 [=======================>----]  84%
 34/39 [========================>---]  87%
 35/39 [=========================>--]  89%
 36/39 [=========================>--]  92%
 37/39 [==========================>-]  94%
 38/39 [===========================>]  97%
 39/39 [============================] 100%
Generating autoload files
26 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
PHP CodeSniffer Config installed_paths set to ../../phpcompatibility/php-compatibility,../../phpcompatibility/phpcompatibility-paragonie,../../phpcompatibility/phpcompatibility-wp,../../wp-coding-standards/wpcs
Creating wordpress-develop_phpunit_run ... done
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

.............................................................   61 / 1473 (  4%)
...
.........                                                     1473 / 1473 (100%)

Time: 6.64 seconds, Memory: 128.00 MB

OK (1473 tests, 2741 assertions)

Notes for testers: The PHPUnit version will depend upon your PHP version. The Time and Memory values may be different on your machine.

Scenario 4: vendor folder and composer.lock are deleted

The contributor has already run the above commands to setup the local testing environment but then deletes the vendor folder and composer.lock file.

Notes for Testers: Delete both the vendor folder and composer.lock file.

Expectations:

• Composer package dependencies are installed
• composer.lock file is created
• Tests run

Instructions:

Notes for Testers: Delete both the vendor folder and composer.lock file.

Contributor types the following in their command-line tool:

npm run test:php -- --group formatting

or

npm run test:php-composer -- --group formatting

Expected command-line output is similar to Scenario 3.

[hellofromTonya]

Testing Report

Env

I tested on 3 different Macs:

Mac 1:

• OS: macOS Big Sur 11.4
• Docker Desktop: 3.3.3 (64133)
• npm: 6.14.13
• Command-line tool: iterm

Mac 2:

• OS: macOS Big Sur 11.5.1
• Docker Desktop: 3.5.2 (3.5.2.18)
• npm: 7.6.0
• Command-line tool: iterm

Mac 3:

• OS: macOS Big Sur 11.5.2
• Docker Desktop: 3.5.2 (3.5.2.18)
• npm: 6.14.13
• Command-line tool: iterm

Results

For all 3 of my machines.

Before applying ​PR 1627:

• All 4 scenarios failed as expected ✅

The following message displays in the command-line tool:

Error: The PHPUnit Polyfills library is a requirement for running the WP test suite.
You need to run `composer update` before running the tests.
Once the dependencies are installed, you can run the tests using the Composer-installed version of PHPUnit or using PHPUnit phar file, but the dependencies do need to be installed whichever way the tests are run.

After applying ​PR 1627:

• Scenario 1: worked as expected ✅
• Scenario 2: worked as expected ✅
• Scenario 3: worked as expected ✅
• Scenario 4: worked as expected ✅

[johnbillion]

This also covers the scenario where an out of date composer.lock file is in place, which can be the case for existing contributors who have previously installed Composer dependencies but try to re-run the tests without running composer update first.

[johnbillion]

In 51685:

Build/Test Tools: Install the Composer package dependencies within the Docker environment as part of the local development environment installation and testing processes.

This makes the use of Composer on the host machine optional when using the Docker environment, which means there is no change to the process for installing, updating, and running the tests since [51559].

Props hellofromTonya, azaozz, netweb, desrosj, jrf, johnbillion

Fixes #53945

[prbot]

​johnbillion commented on ​PR #1627:

https://core.trac.wordpress.org/changeset/51685

[jrf]

Uh oh... commit [51685] has broken the tests from running against PHP 8.1....

The reason for this is quite simple: not all PHPUnit dependencies have released a version yet which has declared compatibility with PHP 8.1, so on PHP 8.1 the composer update needs to run with --ignore-platform-requirements as otherwise PHPUnit 4.8.x will be installed (the last PHPUnit version which didn't have version constraints declared).

Previously in GH Actions, this was (and still is) handled by special casing PHP 8.1 in the "Composer install" step, but as the "Install WordPress" step is run after that step and now runs composer update as well, the "Install WordPress" step removes the previously installed PHPUnit 9.x version and replaces it with PHPUnit 4.x.

Also see:
​https://github.com/WordPress/wordpress-develop/blob/29531de7685a8d166707a0f2409ab8517b1b2dff/.github/workflows/phpunit-tests.yml#L143-L152

And:
​https://github.com/WordPress/wordpress-develop/runs/3452011214?check_suite_focus=true#step:16:83

I can currently think of two potential solution directions, though am not sure which one is best/feasible:

1. Presuming the install.js script has access to the PHP version on which the script is being run (does it ?), special case the command for PHP 8.1 to use --ignore-platform-requirements.
2. Presuming the install.js script has access to an environment variable indicating that it is being run in the context of a GitHub actions run, skip the call to install_composer_dependencies().

We may even need a combination of the two as with solution (1), it would be best to remove the "Composer install" step from the GitHub Actions unit test and code coverage workflows as that step is now handled via the Docker "Install WordPress" step, but on the other hand, if we do that, this may break the caching of the Composer downloads in GitHub Actions, slowing down the workflows.

@johnbillion @desrosj @netweb What do think would be the best way to solve this ?

[netweb]

Replying to jrf:

Uh oh... commit [51685] has broken the tests from running against PHP 8.1....

Re-opening this issue...

1. Presuming the install.js script has access to the PHP version on which the script is being run (does it ?), special case the command for PHP 8.1 to use --ignore-platform-requirements.

We should be able to use wp-cli and the wp-cli() helper function to check for that PHP version:

wp_cli( `eval 'echo PHP_VERSION;'` );

2. Presuming the install.js script has access to an environment variable indicating that it is being run in the context of a GitHub actions run, skip the call to install_composer_dependencies().

Unure of what GitHub Actions environment variables are available to us here

[jrf]

Just noting that as a quick-fix for the time being, while we figure out the Docker side of things, we could move the "Install Composer" step to after the "Install WordPress" step (or duplicate it) to make sure that PHP 8.1 will get the correct PHPUnit version again.

[hellofromTonya]

What if the installer is removed from install.js? Instead of installing the dependencies during the npm installation process, install them when they are needed, i.e. when running the automated tests npm run test and/or running phpcs.

Why?

Yesterday when setting up an environment for manually testing a patch, I noticed that the Composer dependencies were installed. For testers doing manual testing, this extra step adds more set up time and is unnecessary.

In [51685] added install_composer_dependencies() to tools/local-env/scripts/install.js to handle installing the
Composer dependencies during the npm installation workflow.

However, testers doing manual testing do not need these dependencies installed. This addition added extra and
unnecessary step up time for contributors.

By removing the installation of the Composer dependencies from the npm installation process, the dependencies will be installed when needed, i.e. when running npm run test:php.

This also fixes the introduced problem that broke running tests on PHP 8.1.

Trac ticket: https://core.trac.wordpress.org/ticket/53945

[prbot]

​hellofromtonya commented on ​PR #1647:

Bummer, didn't resolve the PHP 8.1 issue as it still installs4.8.36 when running the tests:

 - Downloading phpunit/phpunit (4.8.36)

​https://github.com/WordPress/wordpress-develop/pull/1647/checks?check_run_id=3486341755

[prbot]

​hellofromtonya commented on ​PR #1647:

​https://github.com/hellofromtonya/wordpress-develop/commit/8a7d2f720be37e5e6c493144cd7fd2c1466b0970 removes the composer update from the test:php-composer script as this is the script the CI/GHA uses whereas contributors use npm run test:php.

Results:

The right version of PHPUnit is installed for the PHP 8.1 tests in the ​PHP 8.1 job:

PHPUnit 9.5.9 by Sebastian Bergmann and contributors.

[hellofromTonya]

​PR 1647 proposes a viable solution

• Removes the installation logic from tools/local-env/scripts/install.js
• Removes the composer update cascading command from the test:php-composer script in the package.json file

Both of these changes fixes the broken tests on PHP 8.1 in the CI GHA. Why? The phpunit-tests.yml workflow includes logic to:

• run the right command for installing the Composer dependencies (​see it here)
• uses npm run test:php-composer to run the tests (​starts here)

What about contributors testing locally?

• Documentation in ​the handbook and ​the repo's README.md state to use npm run test:php.
• Removing the composer install from the npm run env:install process solves the problem I noted above where folks doing local testing don't need these dependencies.

IMO the npm run test:php-composer can exist for the CI's automated test runs while the local testing recommendation remains npm run test:php.

cc @johnbillion @netweb @desrosj What do you all think?

Note: I'm removing commit keyword until there's consensus.

[pbearne]

Tested on win 10 and win 11
all tests ran it looks good

[jrf]

@hellofromTonya Just looked at your PR, but correct me if I'm wrong, that looks like it doesn't solve the original problem ? nor the PHP 8.1 issue (not really).

It solves the current GH build problem only, but brings the rest of this ticket back to square 1.

As far as I can see, the changes currently proposed:

• Would work only if someone uses the PHPUnit PHAR / test:php command to run the tests. In that case, composer update is run as part of the test:php command and would install/update the PHPUnit Polyfills (if necessary). The PHPUnit version installed is not (well, barely) relevant in that case as the Docker image will use the appropriate PHAR for the chosen PHP image.
• Running test:php-composer would not work unless someone has already run a composer update/install prior to choosing that command (then again: that was also the case prior to this change).
• Running test:php-composer may still show the "you need to update the PHPUnit Polyfills" message.
• Running the test:php-composer command may be broken depending in the PHP version used when running the original composer update/install command, if that version does not match the PHP version of the Docker image used to run the tests. Think: local default PHP is PHP 5.6 and the Docker 8.0 image is used.
• Would still necessitate instructing people to manually run composer update -W --ignore-platform-reqs for running the tests on PHP 8.1 with test:php-composer.

There also seems to be a misconception: "Running the composer update/install command would slow down the general install".
While this is true in a situation when an install/update is actually needed (mismatch between the versions installed and the current PHP version), if there is no version mismatch, the command will just say "nothing to update" and move on, so for most users, the slowdown would barely be noticeable.

I have a feeling I may be missing something, so what am I missing ?

[hellofromTonya]

Replying to jrf:

• Would work only if someone uses the PHPUnit PHAR / test:php command to run the tests. In that case, composer update is run as part of the test:php command and would install/update the PHPUnit Polyfills (if necessary). The PHPUnit version installed is not (well, barely) relevant in that case as the Docker image will use the appropriate PHAR for the chosen PHP image.

I think the disconnect is this: the recommended command for running tests locally is npm run test:php, not npm run test:php-composer. This works for those using npm/Docker workflow too.

Here is where it's recommended:

• ​In the testing section of the `README.md`:

  To run the tests
  These commands run the PHP and end-to-end test suites, respectively:
  

  npm run test:php
npm run test:e2e
  

• ​in the PHPUnit section of the handbook:

Running the tests

Once the docker container is set up and provisioned, you can run the tests from the command-line: npm run test:php.

There is a ​note in this post about the intended differences between these 2 commands:

// Run within the local Docker container using version of
// PHPUnit installed within the container.
npm run test:php
 
// Run within the local Docker container using version of
// PHPUnit installed via Composer.
npm run test:php-composer

However, we've changed this as the PHPUnit phar is no longer loaded in the container and instead is installed via Composer.

So I'm thinking test:php-composer command is for the CI or for those contributors who don't want to install Composer or have it installed globally somewhere.

Replying to jrf:

There also seems to be a misconception: "Running the composer update/install command would slow down the general install".
While this is true in a situation when an install/update is actually needed (mismatch between the versions installed and the current PHP version), if there is no version mismatch, the command will just say "nothing to update" and move on, so for most users, the slowdown would barely be noticeable.

The time it takes depends upon a contributor's internet download speed as well as if there are any blips in the connectivity chain. While for many, it may be super duper fast, there is a potential for some contributors that it slows them down.

[hellofromTonya]

Looking at the differences between the scripts:

npm run test:php:

node ./tools/local-env/scripts/docker.js run -T php composer update -W && node ./tools/local-env/scripts/docker.js run --rm phpunit phpunit

whereas npm run test:php-composer:

node ./tools/local-env/scripts/docker.js run -T php composer update -W && node ./tools/local-env/scripts/docker.js run --rm phpunit php ./vendor/bin/phpunit

the subtle difference of run phpunit phpunit vs run phpunit php ./vendor/bin/phpunit.

Running npm run test:php on one of my machines that doesn't have PHPUnit phar installed nor PHPUnit installed via composer globally, it works. The only instance of PHPUnit is in the vendor bin.

Why does it work? Hmm, I'll do more investigation and testing to discover why.

Update:

Okay I see. The phar is installed in the Docker container and that's used when running npm run test:php. I mistakingly thought this phar file wasn't installed in the container but instead only Composer version was used. My mistake.

So yes, ​PR 1647 is invalid.

Also, we'll need to update the handbook and README.md for when to use test:php vs. test:php-composer.

[prbot]

​hellofromtonya commented on ​PR #1647:

See this comment in the Trac ticket.

I mistakenly thought the phar was no longer installed in the container, i.e. thinking it also switched to using the Composer installed version. This means running npm run test:php uses the phar file in the container and not the Composer installed version.

This PR then is invalid as contributors need to use npm run test:php-composer when doing local testing.

[azaozz]

Replying to hellofromTonya:

I think the disconnect is this: the recommended command for running tests locally is npm run test:php, not npm run test:php-composer. This works for those using npm/Docker workflow too.
...
However, we've changed this as the PHPUnit phar is no longer loaded in the container and instead is installed via Composer.

As far as I see PHPUnit is always installed in /vendor when installing the (other) dependencies, either with local Composer or by npm run env:install. If that's correct, for simplicity's sake perhaps better to always run it from there? Then the test:php-composer command won't be needed.

Replying to jrf:

There also seems to be a misconception: "Running the composer update/install command would slow down the general install".
While this is true in a situation when an install/update is actually needed (mismatch between the versions installed and the current PHP version), if there is no version mismatch, the command will just say "nothing to update" and move on, so for most users, the slowdown would barely be noticeable.

The time it takes depends upon a contributor's internet download speed as well as if there are any blips in the connectivity chain. While for many, it may be super duper fast, there is a potential for some contributors that it slows them down.

Right. I'm also noticing a delay of about 2.5-3 min every time the tests are run with npm run test:php or npm run test:php-composer. It happens after Use the 'composer fund' command to find out more! so presuming after composer has checked for updates. However skipping composer update -W and running only % node ./tools/local-env/scripts/docker.js run --rm phpunit phpunit "--group" "formatting" doesn't trigger that delay.

That ~3 min delay is pretty annoying especially when writing or fixing tests and running the same test 10-20 times in a row. Even if it's fixed, perhaps Composer updates should not be running every time PHPUnit is run, still takes 5-10 sec to check :)

> [email protected] test:php /Users/andrew/dev/wp/trunk
> node ./tools/local-env/scripts/docker.js run -T php composer update -W && node ./tools/local-env/scripts/docker.js run --rm phpunit phpunit "--group" "formatting"

Starting trunk-wpenv_mysql_1 ... done
Loading composer repositories with package information
Updating dependencies
Nothing to modify in lock file
Installing dependencies from lock file (including require-dev)
Nothing to install, update or remove
Generating autoload files
26 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
----------> delay happens here <---------
Starting trunk-wpenv_mysql_1 ... done
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

.............................................................   61 / 1473 (  4%)
...

[jrf]

Replying to azaozz:

Right. I'm also noticing a delay of about 2.5-3 min every time the tests are run with npm run test:php or npm run test:php-composer. It happens after Use the 'composer fund' command to find out more! so presuming after composer has checked for updates. However skipping composer update -W and running only % node ./tools/local-env/scripts/docker.js run --rm phpunit phpunit "--group" "formatting" doesn't trigger that delay.

That ~3 min delay is pretty annoying especially when writing or fixing tests and running the same test 10-20 times in a row. Even if it's fixed, perhaps Composer updates should not be running every time PHPUnit is run, still takes 5-10 sec to check :)

The delay happening after the "composer fund" to me indicates that this has nothing to do with Composer as Composer is already finished by then.

If I remember correctly, I believe @johnbillion also mentioned something about running the tests via Docker being significantly slower (independently of this change and before this change was even made). He also mentioned a time difference of ~3 minutes.

So, yes, that must be really annoying, but I somehow have a feeling that that delay is unrelated to what we're doing in this ticket.

The ~3 minute delay sounds like a 180 second process timeout, so I wonder whether there is something going on in Docker where either a process is not passing an exit code and the script is waiting for one or where the exit code is not being checked correctly. Just guessing here, but might be a starting point for debugging this.

Either way, the delay sounds like something which most likely has nothing to do with Composer and is outside of the scope of this ticket.

I think a separate ticket needs to be opened to look into debugging that.

Changeset [51685] broke the PHP 8.1 tests as it used composer update without --ignore-platform-reqs which caused the wrong version of PHPUnit to be installed/updated on PHP 8.1 runs. This happened due to 2 changes:

• composer update is run during env:install process
• composer update is run again each time test:php-composer run

Both of the changes were for local testing and not the CI.

This PR fixes the CI while retaining the needed improvements for local testing when running npm run test:php-composer by:

• Switches the CI to use the native docker-compose run instead of the npm script test:php-composer
• Removes the composer update command from the env:install process

Trac ticket: https://core.trac.wordpress.org/ticket/53945

[azaozz]

Replying to jrf:

The delay happening after the "composer fund" to me indicates that this has nothing to do with Composer as Composer is already finished by then.

Right, but it is caused by running composer update -W that was added in [51685]. If that is not run, there is no delay.

If I remember correctly, I believe @johnbillion also mentioned something about running the tests via Docker being significantly slower (independently of this change and before this change was even made). He also mentioned a time difference of ~3 minutes.

Right, it is slower, by about 1.5 min here. As far as I understand this is caused by running from a Docker container vs. directly in the OS. All of WP runs a bit slower. That's not the same as the delay after composer update finishes and before phpunit --group 12345 starts.

The ~3 minute delay sounds like a 180 second process timeout, so I wonder whether there is something going on in Docker

Yeah, could be. I'm not that good at debugging Docker, but will try looking more there.

My other point was that it doesn't make sense to run composer update -W every time when doing npm run test:php. When working on tests the same test case is usually run repeatedly, so checking for Composer updates 20-30 times per hour is not good.

For that reason I think that part of [51685] should be reverted. Perhaps another method can be found to ensure Composer updates are run from time to time, or that can be a documented step like with npm updates.

[hellofromTonya]

​PR 1656 solves the CI problem introduced in [51685] by:

• Removing the composer update logic from the install.js script (as not needed for the CI or for those who are not running the PHPUnit or phpcs tasks)
• Switching to use the original (pre-51685) node full command instead of the npm run test:php-composer

The PR solves the CI problem while retaining the objectives of this ticket to hide the composer update commands when working locally.

[johnbillion]

In 51736:

Build/Test Tools: Fix CI for running tests on PHP 8.1.

This fixes the CI for PHP 8.1 while retaining the needed improvements for local testing when running npm run test:php-composer by:

• Replacing the npm script with the full node command
• Removing the composer update command from the env:install process

Props BinaryKitten, hellofromTonya

See #53945

[prbot]

​johnbillion commented on ​PR #1656:

https://core.trac.wordpress.org/changeset/51736

[hellofromTonya]

Test Report

Env

• node: 14.17.0
• npm: 6.14.13
• Docker Desktop: 3.5.2
• WordPress: trunk
• OS: macOS Big Sur 11.5.2
• Command-Line tool: iTerm2 3.4.8

First run steps

Set up a clean first run:

• Delete the container in the Docker Desktop app
• Clean/Purge data in the Docker Desktop app
• Delete the node_modules and vendor folders
• Delete the composer.lock file
• Clear the local cache:
  • npm cache clean --force
  • composer clearcache
• Fetch and merge the latest from trunk:
  • git fetch upstream
  • git merge upstream/master
• Start up the npm Docker environment:
  • npm install
  • npm run build:dev
  • npm run env:start
  • npm run env:install

• Test composer update and clock the time delay once the message appears

• Repeat all the above steps to test npm run test:php -- --group cron

• Repeat all the above steps to test npm run test:php-composer -- --group cron

Repeat run steps

After the first run (see above), clock the time delay once the message appears but don't do the clean up step.

Repeatedly run:

• composer update and then note the time delay once the message appears. Repeat.
• Repeat for npm run test:php -- --group cron
• Repeat for npm run test:php-composer -- --group cron

Results

The noted time delay after the Use the 'composer fund' command to find out more! message appears:

• composer update
  • First run: 2.36 seconds
  • Rerunning: 1.02 sec, 0.96 sec, 1.13 sec, 1.15 sec

• npm run test:php -- --group cron
  • First run: 18.85 secs
  • Rerunning: 2.52 sec, 1.35 sec, 1.60 sec, 1.59 sec, 1.73 sec

• npm run test:php-composer -- --group cron
  • First run: 19.22 secs
  • Rerunning: 1.14 sec, 1.93 sec, 1.55 sec, 2.06 sec, 1.83 sec

[hellofromTonya]

Replying to azaozz:

I'm also noticing a delay of about 2.5-3 min every time the tests are run with npm run test:php or npm run test:php-composer. It happens after Use the 'composer fund' command to find out more!

With the latest trunk update (that include both changesets in this ticket), I ran a series of tests to clock the delay that happens after that composer fund message appears. See Test Report.

I wasn't able to reproduce the 2-3 minute delay after the composer fund message appeared. Rather, on the first run it was about 18-20 seconds and on average about 1.5 seconds for rerunning tests over and over again.

@azaozz can you check again on your machine with the latest trunk? Please try it before and after the Composer deps are installed.

[azaozz]

Replying to hellofromTonya:

I wasn't able to reproduce the 2-3 minute delay after the composer fund message appeared.

Right, after updating and resetting Docker I only see a delay on the first run. Seems the problem was there. In consequent runs the delay is only about a second.

on the first run it was about 18-20 seconds and on average about 1.5 seconds for rerunning tests over and over again.

Yeah, the numbers here are similar: 20-25 sec. on the first run, then the longer delay after Composer and before PHPUnit (more like 50-60 seconds now). Running the tests again, the delays are a lot smaller.

Tried to figure out what's going on and it looks like npm is running some update checks (perhaps) as it is making few hundreds requests during that first-run delay. Seems that's from the Creating trunk-wpenv_phpunit_run ... done and Installing...:

...
26 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
Creating trunk-wpenv_phpunit_run ... done
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
...

Another thing I've noticed is that npm run test:php will fail if the network connection is shaky/faulty and Composer is not able to check for updates. It also fails on subsequent runs when there will be no updates available. The PHP tests (mostly) run with no internet connection (although some fail instead of being skipped). Not a big deal but may be good to fix.