Skip to content

Commit

Permalink
Made executeLifecycleScript Asynchronous
Browse files Browse the repository at this point in the history
This keeps the spinner from freezing and also has the
benefit of making the script output more consumable.
  • Loading branch information
ObliviousHarmony committed May 18, 2023
1 parent 08b0b0e commit 9c3a0f7
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 64 deletions.
2 changes: 1 addition & 1 deletion packages/env/lib/commands/clean.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ module.exports = async function clean( {
await Promise.all( tasks );

if ( scripts ) {
executeLifecycleScript( 'afterClean', config, spinner );
await executeLifecycleScript( 'afterClean', config, spinner );
}

spinner.text = `Cleaned ${ description }.`;
Expand Down
2 changes: 1 addition & 1 deletion packages/env/lib/commands/destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ module.exports = async function destroy( { spinner, scripts, debug } ) {
}

if ( scripts ) {
executeLifecycleScript( 'beforeDestroy', config, spinner );
await executeLifecycleScript( 'beforeDestroy', config, spinner );
}

spinner.text = 'Removing docker images, volumes, and networks.';
Expand Down
2 changes: 1 addition & 1 deletion packages/env/lib/commands/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ module.exports = async function start( {
}

if ( scripts ) {
executeLifecycleScript( 'afterStart', config, spinner );
await executeLifecycleScript( 'afterStart', config, spinner );
}

const siteUrl = config.env.development.config.WP_SITEURL;
Expand Down
56 changes: 42 additions & 14 deletions packages/env/lib/execute-lifecycle-script.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
/**
* External dependencies
*/
const { execSync } = require( 'child_process' );
const { exec } = require( 'child_process' );

/**
* @typedef {import('./config').WPConfig} WPConfig
Expand All @@ -25,31 +25,59 @@ class LifecycleScriptError extends Error {
* @param {string} event The lifecycle event to run the script for.
* @param {WPConfig} config The config object to use.
* @param {Object} spinner A CLI spinner which indciates progress.
*
* @return {Promise} Resolves when the script has completed and rejects when there is an error.
*/
function executeLifecycleScript( event, config, spinner ) {
if ( ! config.lifecycleScripts[ event ] ) {
return;
return Promise.resolve();
}

spinner.text = `Executing ${ event } Script`;
return new Promise( ( resolve, reject ) => {
// We're going to append the script output to the spinner while it's executing.
const spinnerMessage = `Executing ${ event } Script`;
spinner.text = spinnerMessage;

try {
let output = execSync( config.lifecycleScripts[ event ], {
// Execute the script asynchronously so that it won't block the spinner.
const childProc = exec( config.lifecycleScripts[ event ], {
encoding: 'utf-8',
stdio: 'pipe',
env: process.env,
} );

// Remove any trailing whitespace for nicer output.
output = output.trimRight();
// Collect all of the output so that we can make use of it.
let output = '';
childProc.stdout.on( 'data', ( data ) => {
output += data;

// We don't need to bother with any output if there isn't any.
if ( output ) {
spinner.info( `${ event }:\n${ output }` );
}
} catch ( error ) {
throw new LifecycleScriptError( event, error.stderr );
}
// Keep the spinner updated with the command output.
spinner.text = `${ spinnerMessage }\n${ output }`;
} );

// Listen for any error output so we can display it if the command fails.
let error = '';
childProc.stderr.on( 'data', ( data ) => {
error += data;
} );

// Pass any process creation errors directly up.
childProc.on( 'error', reject );

// Handle the completion of the command based on whether it was successful or not.
childProc.on( 'exit', ( code ) => {
if ( code === 0 ) {
// Keep the output of the command in debug mode.
if ( config.debug ) {
spinner.info( `${ event } Script:\n${ output.trimEnd() }` );
}

resolve();
return;
}

reject( new LifecycleScriptError( event, error.trimEnd() ) );
} );
} );
}

module.exports = {
Expand Down
70 changes: 23 additions & 47 deletions packages/env/lib/test/execute-lifecycle-script.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
'use strict';
/**
* External dependencies
*/
const { execSync } = require( 'child_process' );

/* eslint-disable jest/no-conditional-expect */
/**
* Internal dependencies
*/
Expand All @@ -12,10 +8,6 @@ const {
executeLifecycleScript,
} = require( '../execute-lifecycle-script' );

jest.mock( 'child_process', () => ( {
execSync: jest.fn(),
} ) );

describe( 'executeLifecycleScript', () => {
const spinner = {
info: jest.fn(),
Expand All @@ -25,58 +17,42 @@ describe( 'executeLifecycleScript', () => {
jest.clearAllMocks();
} );

it( 'should do nothing without event option', () => {
executeLifecycleScript(
it( 'should do nothing without event option when debugging', async () => {
await executeLifecycleScript(
'test',
{ lifecycleScripts: { test: null } },
{ lifecycleScripts: { test: null }, debug: true },
spinner
);

expect( spinner.info ).not.toHaveBeenCalled();
} );

it( 'should run event option and print output without extra whitespace', () => {
execSync.mockReturnValue( 'Test \n' );

executeLifecycleScript(
it( 'should run event option and print output when debugging', async () => {
await executeLifecycleScript(
'test',
{ lifecycleScripts: { test: 'Test' } },
{ lifecycleScripts: { test: 'echo "Test \n"' }, debug: true },
spinner
);

expect( execSync ).toHaveBeenCalled();
expect( execSync.mock.calls[ 0 ][ 0 ] ).toEqual( 'Test' );
expect( spinner.info ).toHaveBeenCalledWith( 'test:\nTest' );
expect( spinner.info ).toHaveBeenCalledWith( 'test Script:\nTest' );
} );

it( 'should print nothing if event returns no output', () => {
execSync.mockReturnValue( '' );

executeLifecycleScript(
'test',
{ lifecycleScripts: { test: 'Test' } },
spinner
);

expect( execSync ).toHaveBeenCalled();
expect( execSync.mock.calls[ 0 ][ 0 ] ).toEqual( 'Test' );
expect( spinner.info ).not.toHaveBeenCalled();
} );

it( 'should throw LifecycleScriptError when process errors', () => {
execSync.mockImplementation( ( command ) => {
expect( command ).toEqual( 'Test' );
throw { stderr: 'Something bad happened.' };
} );

expect( () =>
executeLifecycleScript(
it( 'should throw LifecycleScriptError when process errors', async () => {
try {
await executeLifecycleScript(
'test',
{ lifecycleScripts: { test: 'Test' } },
{
lifecycleScripts: {
test: 'echo "test error" 1>&2 && false',
},
},
spinner
)
).toThrow(
new LifecycleScriptError( 'test', 'Something bad happened.' )
);
);
} catch ( error ) {
expect( error ).toEqual(
new LifecycleScriptError( 'test', 'test error' )
);
}
} );
} );
/* eslint-enable jest/no-conditional-expect */

0 comments on commit 9c3a0f7

Please sign in to comment.