Make WordPress Core

Opened 13 months ago

Last modified 7 weeks ago

#57189 new defect (bug)

The PHP tests initialize very slowly in wp-env

Reported by: azaozz's profile azaozz Owned by:
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description

When run in wp-env (Docker), the PHP tests take very long time (over 30 seconds) to initialize. This is especially annoying/time wasting when running them multiple times.

Not sure if Docker or Composer is the cause, but I see this:

> WordPress@6.2.0 test:php /dev/wp/trunk
> node ./tools/local-env/scripts/docker.js run -T php composer update -W && node ./tools/local-env/scripts/docker.js run php ./vendor/bin/phpunit

Creating trunk_php_run ... done
.................................... (waits here for about 30-35 seconds)
Loading composer repositories with package information
Info from https://repo.packagist.org: #StandWithUkraine
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
25 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
Creating trunk_php_run ... done
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
..... (rest of the PHPUnit output)

The problem seems to be that Composer is run every time to re-check the dependencies, i.e. the script there does the equivalent of npm i every time before it starts PHPUnit. I understand the desire to keep things updated, but this is not productive.

Attachments (1)

57189.diff (1.3 KB) - added by desrosj 2 months ago.

Download all attachments as: .zip

Change History (9)

#1 @azaozz
13 months ago

Seems an easy fix here would be to split the "Composer install" part from the actual "run PHPUnit" part in npm run test:php. Something like:

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

A better way would be to have an equivalent of a lock file and run Composer only once per week.

Last edited 13 months ago by azaozz (previous) (diff)

#2 @azaozz
13 months ago

Another thing is that if a PHPUnit test fails, the (npm/Docker) script fails too. Seems not a big problem but needs fixing nevertheless. Error is:

ERROR: 1
child_process.js:836
    throw err;
    ^

Error: Command failed: docker-compose run php ./vendor/bin/phpunit

This ticket was mentioned in Slack in #core by costdev. View the logs.


9 months ago

#4 @hellofromTonya
9 months ago

  • Milestone changed from 6.2 to 6.3

Given that RC1 is fast approaching (next week) and this needs more discussion, moving it to 6.3. But if can be committed at any time when ready.

#5 @swissspidy
5 months ago

  • Keywords needs-patch added
  • Milestone changed from 6.3 to 6.4

+1 to this. I always manually remove this part during development because it's so wasteful.

#6 @johnbillion
5 months ago

This was intentionally added in #53945 but I'd love to get rid of it

@desrosj
2 months ago

#7 @desrosj
2 months ago

  • Keywords has-patch added; needs-patch removed

I think this is still important to have because of the edge cases detailed in #53945.

What about moving this to the env:start command? That would handle the scenarios that ticket aimed to solve and would not run composer update every time tests run. Arguably that's the more appropriate time to run that command anyway since npm run env:restart is required if the desired PHP version is changed. It also seems to run much faster.

If the consensus is to remove it I'm not opposed. But we'll need to provide better documentation around when to run the command and what the indicators are when it needs to be run.

#8 @hellofromTonya
7 weeks ago

  • Milestone changed from 6.4 to 6.5

6.4 RC1 is happening in a couple of hours and trunk will be branched. Moving this to 6.5.

Note: See TracTickets for help on using tickets.