Skip to content

Commit

Permalink
Merge pull request #912 from ckeditor/ck/14769
Browse files Browse the repository at this point in the history 
Other (release-tools): The `reassignNpmTags()` task processes all packages asynchronously to improve the performance results. Closes ckeditor/ckeditor5#14769.
  • Loading branch information
@pomek
pomek authored Aug 18, 2023
2 parents b0fe2a5 + 2798d87 commit baf3340
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 44 deletions.
61 changes: 39 additions & 22 deletions packages/ckeditor5-dev-release-tools/lib/tasks/reassignnpmtags.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
const chalk = require( 'chalk' );
const columns = require( 'cli-columns' );
const { tools } = require( '@ckeditor/ckeditor5-dev-utils' );
const util = require( 'util' );
const exec = util.promisify( require( 'child_process' ).exec );
const assertNpmAuthorization = require( '../utils/assertnpmauthorization' );

/**
* Used to switch the tags from `staging` to `latest` for specified array of packages.
* Each operation will be retried up to 3 times in case of failure.
*
* @param {Object} options
* @param {String} options.npmOwner User that is authorized to release packages.
Expand All @@ -33,30 +36,29 @@ module.exports = async function reassignNpmTags( { npmOwner, version, packages }
const counter = tools.createSpinner( 'Reassigning npm tags...', { total: packages.length } );
counter.start();

for ( const packageName of packages ) {
const latestVersion = await exec( `npm show ${ packageName }@latest version` )
.then( version => version.trim() )
.catch( error => {
errors.push( trimErrorMessage( error.message ) );
} );

if ( latestVersion === version ) {
packagesSkipped.push( packageName );

continue;
}

await exec( `npm dist-tag add ${ packageName }@${ version } latest` )
.then( () => {
packagesUpdated.push( packageName );
const updateTagPromises = packages.map( async packageName => {
const updateLatestTagRetryable = retry( () => exec( `npm dist-tag add ${ packageName }@${ version } latest` ) );
await updateLatestTagRetryable()
.then( response => {
if ( response.stdout ) {
packagesUpdated.push( packageName );
} else if ( response.stderr ) {
throw new Error( response.stderr );
}
} )
.catch( error => {
errors.push( trimErrorMessage( error.message ) );
if ( error.message.includes( 'is already set to version' ) ) {
packagesSkipped.push( packageName );
} else {
errors.push( trimErrorMessage( error.message ) );
}
} )
.finally( () => {
counter.increase();
} );
}
} );

await Promise.allSettled( updateTagPromises );

counter.finish();

Expand Down Expand Up @@ -85,9 +87,24 @@ function trimErrorMessage( message ) {
}

/**
* @param {String} command
* @returns {Promise.<String>}
* @param {Function} callback
* @param {Number} times
* @returns {RetryCallback}
*/
async function exec( command ) {
return tools.shExec( command, { verbosity: 'silent', async: true } );
function retry( callback, times = 3 ) {
return ( ...args ) => Promise.resolve()
.then( () => callback( ...args ) )
.catch( err => {
if ( times > 0 ) {
return retry( callback, times - 1 )( ...args );
}

throw err;
} );
}

/**
* @callback RetryCallback
* @param {...*} args
* @returns {Promise}
*/
41 changes: 19 additions & 22 deletions packages/ckeditor5-dev-release-tools/tests/tasks/reassignnpmtags.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ describe( 'reassignNpmTags()', () => {

stubs = {
tools: {
shExec: sinon.stub(),
createSpinner: sinon.stub().callsFake( () => {
return stubs.spinner;
} )
Expand All @@ -43,15 +42,18 @@ describe( 'reassignNpmTags()', () => {
columns: sinon.stub(),
console: {
log: sinon.stub( console, 'log' )
}
},
util: {
promisify: sinon.stub().callsFake( () => stubs.exec )
},
exec: sinon.stub()
};

stubs.tools.shExec.withArgs( sinon.match( 'npm show' ) ).resolves( '1.0.0' );

mockery.registerMock( '@ckeditor/ckeditor5-dev-utils', { tools: stubs.tools } );
mockery.registerMock( '../utils/assertnpmauthorization', stubs.assertNpmAuthorization );
mockery.registerMock( 'cli-columns', stubs.columns );
mockery.registerMock( 'chalk', stubs.chalk );
mockery.registerMock( 'util', stubs.util );

reassignNpmTags = require( '../../lib/tasks/reassignnpmtags' );
} );
Expand All @@ -64,7 +66,7 @@ describe( 'reassignNpmTags()', () => {

it( 'should throw an error when assertNpmAuthorization throws error', async () => {
stubs.assertNpmAuthorization.throws( new Error( 'User not logged in error' ) );
const npmDistTagAdd = stubs.tools.shExec.withArgs( sinon.match( 'npm dist-tag add' ) );
const npmDistTagAdd = stubs.exec.withArgs( sinon.match( 'npm dist-tag add' ) );

try {
await reassignNpmTags( { npmOwner: 'correct-npm-user', version: '1.0.1', packages: [ 'package1' ] } );
Expand All @@ -76,25 +78,18 @@ describe( 'reassignNpmTags()', () => {
expect( npmDistTagAdd.callCount ).to.equal( 0 );
} );

it( 'should still try to update tags when can not obtain package version from npm', async () => {
stubs.tools.shExec.withArgs( sinon.match( 'npm show' ) ).throws( new Error( 'Can not obtain package version.' ) );
const npmDistTagAdd = stubs.tools.shExec.withArgs( sinon.match( 'npm dist-tag add' ) );

await reassignNpmTags( { npmOwner: 'authorized-user', version: '1.0.1', packages: [ 'package1', 'package2' ] } );

expect( npmDistTagAdd.callCount ).to.equal( 2 );
} );

it( 'should skip updating tags when provided version matches existing version for tag latest', async () => {
const npmDistTagAdd = stubs.tools.shExec.withArgs( sinon.match( 'npm dist-tag add' ) );
stubs.columns.returns( 'package1 | package2' );
stubs.exec.withArgs( sinon.match( 'npm dist-tag add' ) ).throws( new Error( 'is already set to version' ) );

await reassignNpmTags( { npmOwner: 'authorized-user', version: '1.0.0', packages: [ 'package1', 'package2' ] } );

expect( npmDistTagAdd.callCount ).to.equal( 0 );
expect( stubs.console.log.firstCall.args[ 0 ] ).to.equal( '⬇️ Packages skipped:' );
expect( stubs.console.log.secondCall.args[ 0 ] ).to.deep.equal( 'package1 | package2' );
} );

it( 'should update tags when tag latest for provided version does not yet exist', async () => {
const npmDistTagAdd = stubs.tools.shExec.withArgs( sinon.match( 'npm dist-tag add' ) );
const npmDistTagAdd = stubs.exec.withArgs( sinon.match( 'npm dist-tag add' ) ).resolves( { stdout: '+latest' } );

await reassignNpmTags( { npmOwner: 'authorized-user', version: '1.0.1', packages: [ 'package1', 'package2' ] } );

Expand All @@ -103,7 +98,7 @@ describe( 'reassignNpmTags()', () => {
} );

it( 'should continue updating packages even if first package update fails', async () => {
const npmDistTagAdd = stubs.tools.shExec.withArgs( sinon.match( 'npm dist-tag add' ) );
const npmDistTagAdd = stubs.exec.withArgs( sinon.match( 'npm dist-tag add' ) );
npmDistTagAdd.onFirstCall().throws( new Error( 'Npm error while updating tag.' ) );

await reassignNpmTags( { npmOwner: 'authorized-user', version: '1.0.1', packages: [ 'package1', 'package2' ] } );
Expand Down Expand Up @@ -131,7 +126,7 @@ describe( 'reassignNpmTags()', () => {
} );

it( 'should increase the spinner counter after failure processing a package', async () => {
const npmDistTagAdd = stubs.tools.shExec.withArgs( sinon.match( 'npm dist-tag add' ) );
const npmDistTagAdd = stubs.exec.withArgs( sinon.match( 'npm dist-tag add' ) );
npmDistTagAdd.onFirstCall().throws( new Error( 'Npm error while updating tag.' ) );

await reassignNpmTags( { npmOwner: 'authorized-user', version: '1.0.1', packages: [ 'package1' ] } );
Expand All @@ -140,7 +135,7 @@ describe( 'reassignNpmTags()', () => {
} );

it( 'should finish the spinner once all packages have been processed', async () => {
const npmDistTagAdd = stubs.tools.shExec.withArgs( sinon.match( 'npm dist-tag add' ) );
const npmDistTagAdd = stubs.exec.withArgs( sinon.match( 'npm dist-tag add' ) );
npmDistTagAdd.onFirstCall().throws( new Error( 'Npm error while updating tag.' ) );

await reassignNpmTags( { npmOwner: 'authorized-user', version: '1.0.1', packages: [ 'package1', 'package2' ] } );
Expand All @@ -154,6 +149,7 @@ describe( 'reassignNpmTags()', () => {
} );

it( 'should display skipped packages in a column', async () => {
stubs.exec.withArgs( sinon.match( 'npm dist-tag add' ) ).throws( new Error( 'is already set to version' ) );
stubs.columns.returns( '1 | 2 | 3' );

await reassignNpmTags( { npmOwner: 'authorized-user', version: '1.0.0', packages: [ 'package1', 'package2' ] } );
Expand All @@ -168,6 +164,7 @@ describe( 'reassignNpmTags()', () => {
} );

it( 'should display processed packages in a column', async () => {
stubs.exec.withArgs( sinon.match( 'npm dist-tag add' ) ).resolves( { stdout: '+latest' } );
stubs.columns.returns( '1 | 2 | 3' );

await reassignNpmTags( { npmOwner: 'authorized-user', version: '1.0.1', packages: [ 'package1', 'package2' ] } );
Expand All @@ -182,8 +179,8 @@ describe( 'reassignNpmTags()', () => {
} );

it( 'should display errors found during processing a package', async () => {
const npmDistTagAdd = stubs.tools.shExec.withArgs( sinon.match( 'npm dist-tag add' ) );
npmDistTagAdd.onFirstCall().throws( new Error( 'Npm error while updating tag.' ) );
const npmDistTagAdd = stubs.exec.withArgs( sinon.match( 'npm dist-tag add' ) );
npmDistTagAdd.throws( new Error( 'Npm error while updating tag.' ) );

await reassignNpmTags( { npmOwner: 'authorized-user', version: '1.0.1', packages: [ 'package1' ] } );

Expand Down

0 comments on commit baf3340

Please sign in to comment.