Skip to content

Build/Test Tools: Improve dev environment's CLI in speed, non-interactive usage, and argument handling #8969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ jobs:
run: npm run env:restart

- name: Test a CLI command
run: npm run env:cli wp option get siteurl
run: npm run env:cli option get siteurl
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wp is removed here because previously the entrypoint would inject it if it was missing: https://github.com/WordPress/wpdev-docker-images/blob/becc6740684d6ff6414d6539df040633b5ac5794/entrypoint/entrypoint-cli.sh#L16-L17

With this PR, the wp is always assumed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is technically a breaking change, I searched github for npm run env:cli wp and there is only one reference which seems to be from a fork of a demo rather than actual production code, but it still might be worth a PR there after this lands and a quick note in the core room so folks are aware. I don't think it needs a dev note though since this isn't a public API.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


- name: Test logs command
run: npm run env:logs
Expand Down
3 changes: 3 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ services:
volumes:
- ./:/var/www

# Keeps the service alive.
command: 'sleep infinity'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a true idling container I would rather use
command: 'tail -f /dev/null'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose this for consistency with wp-env.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there were no big explanations
WordPress/gutenberg@c11312a

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Gutenberg switches back to tail we can switch it back here too.


# The init directive ensures the command runs with a PID > 1, so Ctrl+C works correctly.
init: true

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@
"env:clean": "node ./tools/local-env/scripts/docker.js down -v --remove-orphans",
"env:reset": "node ./tools/local-env/scripts/docker.js down --rmi all -v --remove-orphans",
"env:install": "node ./tools/local-env/scripts/install.js",
"env:cli": "node ./tools/local-env/scripts/docker.js run --rm cli",
"env:cli": "node ./tools/local-env/scripts/docker.js exec cli wp --allow-root",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"env:logs": "node ./tools/local-env/scripts/docker.js logs",
"env:pull": "node ./tools/local-env/scripts/docker.js pull",
"test:performance": "wp-scripts test-playwright --config tests/performance/playwright.config.js",
Expand Down
33 changes: 24 additions & 9 deletions tools/local-env/scripts/docker.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,36 @@
const dotenv = require( 'dotenv' );
/* jshint node:true */

const dotenv = require( 'dotenv' );
const dotenvExpand = require( 'dotenv-expand' );
const { execSync } = require( 'child_process' );
const { spawnSync } = require( 'child_process' );
const local_env_utils = require( './utils' );

dotenvExpand.expand( dotenv.config() );

const composeFiles = local_env_utils.get_compose_files();

if (process.argv.includes('--coverage-html')) {
if ( process.argv.includes( '--coverage-html' ) ) {
process.env.LOCAL_PHP_XDEBUG = 'true';
process.env.LOCAL_PHP_XDEBUG_MODE = 'coverage';
}

// This try-catch prevents the superfluous Node.js debugging information from being shown if the command fails.
try {
// Execute any Docker compose command passed to this script.
execSync( 'docker compose ' + composeFiles + ' ' + process.argv.slice( 2 ).join( ' ' ), { stdio: 'inherit' } );
} catch ( error ) {
process.exit( 1 );
// Add --no-TTY (-T) arg after exec and run commands when STDIN is not a TTY.
const dockerCommand = process.argv.slice( 2 );
if ( [ 'exec', 'run' ].includes( dockerCommand[0] ) && ! process.stdin.isTTY ) {
dockerCommand.splice( 1, 0, '--no-TTY' );
}

// Execute any Docker compose command passed to this script.
const returns = spawnSync(
'docker',
[
'compose',
...composeFiles
.map( ( composeFile ) => [ '-f', composeFile ] )
.flat(),
...dockerCommand,
],
{ stdio: 'inherit' }
);

process.exit( returns.status );
9 changes: 5 additions & 4 deletions tools/local-env/scripts/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ dotenvExpand.expand( dotenv.config() );
local_env_utils.determine_auth_option();

// Create wp-config.php.
wp_cli( `config create --dbname=wordpress_develop --dbuser=root --dbpass=password --dbhost=mysql --force --config-file=${process.env.LOCAL_DIR}/../wp-config.php` );
wp_cli( `config create --dbname=wordpress_develop --dbuser=root --dbpass=password --dbhost=mysql --force --config-file="wp-config.php"` );

// Since WP-CLI runs as root, the wp-config.php created above will be read-only. This needs to be writable for the sake of E2E tests.
execSync( 'node ./tools/local-env/scripts/docker.js exec cli chmod 666 wp-config.php' );
Comment on lines +18 to +19
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me why this is needed now. Without it, E2E tests fail due to wp-config.php not being writable: https://github.com/WordPress/wordpress-develop/actions/runs/15627027244/job/44023152411

In 0e4ddbb I tried using the node API to set the file permissions, but this failed because it's not being run as root: https://github.com/WordPress/wordpress-develop/actions/runs/15624512371/job/44016093197

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in 0e4ddbb it was running on the local filesystem whereas in this implementation it is running in the container.


// Add the debug settings to wp-config.php.
// Windows requires this to be done as an additional step, rather than using the --extra-php option in the previous step.
Expand Down Expand Up @@ -61,7 +64,5 @@ wait_on( {
* @param {string} cmd The WP-CLI command to run.
*/
function wp_cli( cmd ) {
const composeFiles = local_env_utils.get_compose_files();

execSync( `docker compose ${composeFiles} run --quiet-pull --rm cli ${cmd} --path=/var/www/${process.env.LOCAL_DIR}`, { stdio: 'inherit' } );
execSync( `npm --silent run env:cli -- ${cmd} --path=/var/www/${process.env.LOCAL_DIR}`, { stdio: 'inherit' } );
}
61 changes: 51 additions & 10 deletions tools/local-env/scripts/start.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
/* jshint node:true */

const dotenv = require( 'dotenv' );
const dotenvExpand = require( 'dotenv-expand' );
const { execSync } = require( 'child_process' );
const { execSync, spawnSync } = require( 'child_process' );
const local_env_utils = require( './utils' );
const { constants, copyFile } = require( 'node:fs' );

// Copy the default .env file when one is not present.
copyFile( '.env.example', '.env', constants.COPYFILE_EXCL, (e) => {
copyFile( '.env.example', '.env', constants.COPYFILE_EXCL, () => {
console.log( '.env file already exists. .env.example was not copied.' );
});

Expand All @@ -28,18 +30,38 @@ try {
}

// Start the local-env containers.
const containers = ( process.env.LOCAL_PHP_MEMCACHED === 'true' )
? 'wordpress-develop memcached'
: 'wordpress-develop';
execSync( `docker compose ${composeFiles} up --quiet-pull -d ${containers}`, { stdio: 'inherit' } );
const containers = [ 'wordpress-develop', 'cli' ];
if ( process.env.LOCAL_PHP_MEMCACHED === 'true' ) {
containers.push( 'memcached' );
}

spawnSync(
'docker',
[
'compose',
...composeFiles.map( ( composeFile ) => [ '-f', composeFile ] ).flat(),
'up',
'--quiet-pull',
'-d',
...containers,
],
{ stdio: 'inherit' }
);

// If Docker Toolbox is being used, we need to manually forward LOCAL_PORT to the Docker VM.
if ( process.env.DOCKER_TOOLBOX_INSTALL_PATH ) {
// VBoxManage is added to the PATH on every platform except Windows.
const vboxmanage = process.env.VBOX_MSI_INSTALL_PATH ? `${ process.env.VBOX_MSI_INSTALL_PATH }/VBoxManage` : 'VBoxManage'
const vboxmanage = process.env.VBOX_MSI_INSTALL_PATH ? `${ process.env.VBOX_MSI_INSTALL_PATH }/VBoxManage` : 'VBoxManage';

// Check if the port forwarding is already configured for this port.
const vminfoBuffer = execSync( `"${ vboxmanage }" showvminfo "${ process.env.DOCKER_MACHINE_NAME }" --machinereadable` );
const vminfoBuffer = spawnSync(
vboxmanage,
[
'showvminfo',
process.env.DOCKER_MACHINE_NAME,
'--machinereadable'
]
).stdout;
const vminfo = vminfoBuffer.toString().split( /[\r\n]+/ );

vminfo.forEach( ( info ) => {
Expand All @@ -53,10 +75,29 @@ if ( process.env.DOCKER_TOOLBOX_INSTALL_PATH ) {

// Delete rules that are using the port we need.
if ( rule[ 3 ] === process.env.LOCAL_PORT || rule[ 5 ] === process.env.LOCAL_PORT ) {
execSync( `"${ vboxmanage }" controlvm "${ process.env.DOCKER_MACHINE_NAME }" natpf1 delete ${ rule[ 0 ] }`, { stdio: 'inherit' } );
spawnSync(
vboxmanage,
[
'controlvm',
process.env.DOCKER_MACHINE_NAME,
'natpf1',
'delete',
rule[ 0 ]
],
{ stdio: 'inherit' }
);
}
} );

// Add our port forwarding rule.
execSync( `"${ vboxmanage }" controlvm "${ process.env.DOCKER_MACHINE_NAME }" natpf1 "tcp-port${ process.env.LOCAL_PORT },tcp,127.0.0.1,${ process.env.LOCAL_PORT },,${ process.env.LOCAL_PORT }"`, { stdio: 'inherit' } );
spawnSync(
vboxmanage,
[
'controlvm',
process.env.DOCKER_MACHINE_NAME,
'natpf1',
`tcp-port${ process.env.LOCAL_PORT },tcp,127.0.0.1,${ process.env.LOCAL_PORT },,${ process.env.LOCAL_PORT }`
],
{ stdio: 'inherit' }
);
}
10 changes: 7 additions & 3 deletions tools/local-env/scripts/utils.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* jshint node:true */

const { existsSync } = require( 'node:fs' );

const local_env_utils = {
Expand All @@ -10,12 +12,14 @@ const local_env_utils = {
*
* When PHP 7.2 or 7.3 is used in combination with MySQL 8.4, an override file will also be returned to ensure
* that the mysql_native_password plugin authentication plugin is on and available for use.
*
* @return {string[]} Compose files.
*/
get_compose_files: function() {
var composeFiles = '-f docker-compose.yml';
const composeFiles = [ 'docker-compose.yml' ];

if ( existsSync( 'docker-compose.override.yml' ) ) {
composeFiles = composeFiles + ' -f docker-compose.override.yml';
composeFiles.push( 'docker-compose.override.yml' );
}

if ( process.env.LOCAL_DB_TYPE !== 'mysql' ) {
Expand All @@ -28,7 +32,7 @@ const local_env_utils = {

// PHP 7.2/7.3 in combination with MySQL 8.4 requires additional configuration to function properly.
if ( process.env.LOCAL_DB_VERSION === '8.4' ) {
composeFiles = composeFiles + ' -f tools/local-env/old-php-mysql-84.override.yml';
composeFiles.push( 'tools/local-env/old-php-mysql-84.override.yml' );
}

return composeFiles;
Expand Down
Loading