Remove PHPUnit and Composer packages #50408
Conversation
| }, | ||
| volumes: { | ||
| ...( ! config.env.development.coreSource && { wordpress: {} } ), | ||
| ...( ! config.env.tests.coreSource && { 'tests-wordpress': {} } ), | ||
| mysql: {}, | ||
| 'mysql-test': {}, | ||
| 'phpunit-uploads': {}, |
There was a problem hiding this comment.
I'm curious why I had to add this in the first place, but I can't remember 🙃
| @@ -318,9 +318,9 @@ | |||
| "test:unit:debug": "wp-scripts --inspect-brk test-unit-js --runInBand --no-cache --verbose --config test/unit/jest.config.js ", | |||
| "test:unit:profile": "wp-scripts --cpu-prof test-unit-js --runInBand --no-cache --verbose --config test/unit/jest.config.js ", | |||
| "pretest:unit:php": "wp-env start", | |||
| "test:unit:php": "wp-env run tests-wordpress /var/www/html/wp-content/plugins/gutenberg/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/gutenberg/phpunit.xml.dist --verbose", | |||
| "test:unit:php": "wp-env run --env-cwd=\"wp-content/plugins/gutenberg\" tests-wordpress vendor/bin/phpunit -c phpunit.xml.dist --verbose", | |||
This commit removes these containers because they don't work the way you'd expect them to anymore. It also adds Composer and PHPUnit globally to all of the containers so that people can support the workflows these containers used to.
Co-authored-by: Noah Allen <noahtallen@gmail.com>
There was a problem hiding this comment.
To make the migration easier, I'd like to add a clearer messaging when the containers don't exist. Since running npx wp-env run phpunit ... will fail anyways, I think we should add a helpful message to recommend using npx wp-env run cli phpunit instead, since not everyone will read the changelog
|
That's a good idea @noahtallen. I've also gone ahead and added better error messaging for invalid commands since Docker's message is a bit obtuse. |
There was a problem hiding this comment.
This is looking good to me!
Given that:
- We don't really document the phpunit & composer options already.
- Both containers have been broken recently.
- At least one of the containers has been broken for a while on M* macs.
- It's confusing to have multiple containers that aren't much different from each other.
This makes a lot of sense :)
I also like how this makes the composer and phpunit globally available in different containers without any extra work, which hopefully makes at least PHPUnit a lot easier to set up.
What?
I have removed the
composerandphpunitDocker services in favor of globally installingcomposerandphpunitin thewordpress,cli,tests-wordpress, andtests-clicontainers.Why?
The current approach comes with a few compatibility issues:
phpunitcomposerHow?
This pull request removes the
composerandphpunitservices and installs the respective commands globally. In place ofnpx wp-env run composeryou can now runnpx wp-env run cli composer. One caveat is that thecomposerservice used to default to the directory containing the config file. Instead, since it defaults to/var/www/html, they will need to run the command from the correct directory manually.As an aside, I also got
sudoinstalled in the CLI container so you have root access available in all of the containers. This is potentially useful for people who might install additional services or packages or what have you in a setup script.Testing Instructions
npx wp-env destroy.npx wp-env start --update.npx wp-env run cli bash.composerandphpunitand confirm they run.wordpress,tests-wordpress, andtests-clicontainers.npm run format:phpand confirm that it works as expected.npm run lint:phpand confirm that it works as expected.