Skip to content

Commit

Permalink
Merge pull request #1038 from ckeditor/i/17331
Browse files Browse the repository at this point in the history
Other (release-tools): Slow down the `npm publish` command due to weird errors on CI. Closes ckeditor/ckeditor5#17331.

Other (utils): Support for the `silent` logger mode.
  • Loading branch information
pomek authored Oct 25, 2024
2 parents f207575 + 013bf89 commit 4cfcf41
Show file tree
Hide file tree
Showing 13 changed files with 106 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export default async function publishPackages( options ) {
requireEntryPoint = false,
optionalEntryPointPackages = [],
cwd = process.cwd(),
concurrency = 4,
concurrency = 2,
attempts = 3
} = options;

Expand Down Expand Up @@ -107,8 +107,12 @@ export default async function publishPackages( options ) {
throw new Error( 'Some packages could not be published.' );
}

listrTask.output = 'Let\'s give an npm a moment for taking a breath (~10 sec)...';

// Let's give an npm a moment for taking a breath...
await wait( 1000 * 15 );
await wait( 1000 * 10 );

listrTask.output = 'Done. Let\'s continue.';

// ...and try again.
return publishPackages( {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import pacote from 'pacote';
* @returns {Promise}
*/
export default async function checkVersionAvailability( version, packageName ) {
return pacote.manifest( `${ packageName }@${ version }`, { cache: null } )
return pacote.manifest( `${ packageName }@${ version }`, { cache: null, preferOnline: true } )
.then( () => {
// If `pacote.manifest` resolves, a package with the given version exists.
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ function getPackagesGroupedByThreads( packages, concurrency ) {
/**
* @typedef {object} ListrTaskObject
*
* @see https://listr2.kilic.dev/api/classes/ListrTaskObject.html
* @see https://listr2.kilic.dev/task/title.html
* @see https://listr2.kilic.dev/task/output.html
*
* @property {string} title Title of the task.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import pacote from 'pacote';
* @returns {Promise.<boolean>}
*/
export default async function isVersionPublishableForTag( packageName, version, npmTag ) {
const npmVersion = await pacote.manifest( `${ packageName }@${ npmTag }`, { cache: null } )
const npmVersion = await pacote.manifest( `${ packageName }@${ npmTag }`, { cache: null, preferOnline: true } )
.then( ( { version } ) => version )
// An `npmTag` does not exist, or it's a first release of a package.
.catch( () => null );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export default async function publishPackageOnNpmCallback( packagePath, taskOpti
await tools.shExec( `npm publish --access=public --tag ${ taskOptions.npmTag }`, {
cwd: packagePath,
async: true,
verbosity: 'error'
verbosity: 'silent'
} );

// Do nothing if `npm publish` says "OK". We cannot trust anything npm says.
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-dev-release-tools/lib/utils/versions.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function getLastFromChangelog( cwd = process.cwd() ) {
export function getLastPreRelease( releaseIdentifier, cwd = process.cwd() ) {
const packageName = getPackageJson( cwd ).name;

return pacote.packument( packageName, { cache: null } )
return pacote.packument( packageName, { cache: null, preferOnline: true } )
.then( result => {
const lastVersion = Object.keys( result.versions )
.filter( version => version.startsWith( releaseIdentifier ) )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,14 @@ describe( 'publishPackages()', () => {
} ) ).rejects.toThrow( 'The version tag "rc" from "ckeditor5-foo" package does not match the npm tag "staging".' );
} );

it( 'should use two threads by default when publishing packages', async () => {
await publishPackages( {} );

expect( vi.mocked( executeInParallel ) ).toHaveBeenCalledExactlyOnceWith( expect.objectContaining( {
concurrency: 2
} ) );
} );

it( 'should pass parameters for publishing packages', async () => {
const listrTask = {};
const abortController = new AbortController();
Expand Down Expand Up @@ -459,7 +467,8 @@ describe( 'publishPackages()', () => {
const promise = publishPackages( {
packagesDirectory: 'packages',
npmOwner: 'pepe',
confirmationCallback
confirmationCallback,
listrTask: {}
} );

await vi.advanceTimersToNextTimerAsync();
Expand Down Expand Up @@ -491,15 +500,56 @@ describe( 'publishPackages()', () => {

const promise = publishPackages( {
packagesDirectory: 'packages',
npmOwner: 'pepe'
npmOwner: 'pepe',
listrTask: {}
} );

await vi.advanceTimersToNextTimerAsync();

await promise;
const dateAfter = new Date();

expect( differenceInMilliseconds( dateAfter, dateBefore ) ).toEqual( 15000 );
expect( differenceInMilliseconds( dateAfter, dateBefore ) ).toEqual( 10000 );
} );

it( 'should inform a user about a timeout that hangs the process', async () => {
vi.mocked( findPathsToPackages )
// First execution.
.mockResolvedValueOnce( [
'/work/project/packages/ckeditor5-bar'
] )
// Check for failed packages.
.mockResolvedValueOnce( [
'/work/project/packages/ckeditor5-bar'
] )
// Repeat execution: look for packages to release.
.mockResolvedValueOnce( [
'/work/project/packages/ckeditor5-bar'
] )
// Check for failed packages.
.mockResolvedValue( [] );

vi.mocked( fs ).readJson.mockResolvedValue( {} );

const listrTask = {
output: ''
};

const confirmationCallback = vi.fn().mockReturnValue( true );
const promise = publishPackages( {
packagesDirectory: 'packages',
npmOwner: 'pepe',
confirmationCallback,
listrTask
} );

await vi.advanceTimersByTimeAsync( 0 );
expect( listrTask.output ).not.toEqual( '' );

await vi.advanceTimersToNextTimerAsync();
expect( listrTask.output ).toEqual( 'Done. Let\'s continue.' );

await promise;
} );

it( 'should try to publish packages thrice before rejecting a promise', async () => {
Expand All @@ -515,7 +565,8 @@ describe( 'publishPackages()', () => {

const promise = publishPackages( {
packagesDirectory: 'packages',
npmOwner: 'pepe'
npmOwner: 'pepe',
listrTask: {}
} );

// Needed twice because the third attempt does not setup a timeout.
Expand Down Expand Up @@ -566,7 +617,8 @@ describe( 'publishPackages()', () => {

const promise = publishPackages( {
packagesDirectory: 'packages',
npmOwner: 'pepe'
npmOwner: 'pepe',
listrTask: {}
} );

await vi.advanceTimersToNextTimerAsync();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe( 'checkVersionAvailability()', () => {

await expect( checkVersionAvailability( '1.0.1', 'stub-package' ) ).resolves.toBe( true );

expect( pacote.manifest ).toHaveBeenCalledExactlyOnceWith( 'stub-package@1.0.1', { cache: null } );
expect( pacote.manifest ).toHaveBeenCalledExactlyOnceWith( 'stub-package@1.0.1', expect.any( Object ) );
} );
it( 'should resolve to false if version exists', async () => {
pacote.manifest.mockResolvedValue( '1.0.1' );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe( 'isVersionPublishableForTag()', () => {

expect( result ).to.equal( false );
expect( semver.lte ).toHaveBeenCalledExactlyOnceWith( '1.0.0', '1.0.0' );
expect( pacote.manifest ).toHaveBeenCalledExactlyOnceWith( 'package-name@latest', { cache: null } );
expect( pacote.manifest ).toHaveBeenCalledExactlyOnceWith( 'package-name@latest', expect.any( Object ) );
} );

it( 'should return false if given version is not higher than the latest published', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe( 'publishPackageOnNpmCallback()', () => {
} );
} );

it( 'should set the verbosity level to "error" during publishing packages', () => {
it( 'should set the verbosity level to "silent" during publishing packages', () => {
const packagePath = '/workspace/ckeditor5/packages/ckeditor5-foo';

return publishPackageOnNpmCallback( packagePath, { npmTag: 'nightly' } )
Expand All @@ -56,7 +56,7 @@ describe( 'publishPackageOnNpmCallback()', () => {
expect( tools.shExec ).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining( {
verbosity: 'error'
verbosity: 'silent'
} )
);
} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ describe( 'versions', () => {
return getLastPreRelease( '42.0.0-alpha' )
.then( () => {
expect( vi.mocked( pacote ).packument ).toHaveBeenCalledTimes( 1 );
expect( vi.mocked( pacote ).packument ).toHaveBeenCalledWith( 'ckeditor5', { cache: null } );
expect( vi.mocked( pacote ).packument ).toHaveBeenCalledWith( 'ckeditor5', expect.any( Object ) );
} );
} );

Expand Down
8 changes: 2 additions & 6 deletions packages/ckeditor5-dev-utils/lib/logger/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,9 @@ import chalk from 'chalk';

const levels = new Map();

// Displays everything.
levels.set( 'silent', new Set( [] ) );
levels.set( 'info', new Set( [ 'info' ] ) );

// Displays warning and error logs.
levels.set( 'warning', new Set( [ 'info', 'warning' ] ) );

// Displays error logs only.
levels.set( 'error', new Set( [ 'info', 'warning', 'error' ] ) );

/**
Expand Down Expand Up @@ -45,7 +41,7 @@ levels.set( 'error', new Set( [ 'info', 'warning', 'error' ] ) );
*
* Additionally, the `logger#error()` method prints the error instance if provided as the second argument.
*
* @param {string} moduleVerbosity='info' Level of the verbosity for all log methods.
* @param {'info'|'warning'|'error'|'silent'} [moduleVerbosity='info'] Level of the verbosity for all log methods.
* @returns {object} logger
* @returns {Function} logger.info
* @returns {Function} logger.warning
Expand Down
30 changes: 30 additions & 0 deletions packages/ckeditor5-dev-utils/tests/logger/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,36 @@ describe( 'logger()', () => {
} );
} );

describe( 'verbosity = silent', () => {
beforeEach( () => {
log = logger( 'silent' );
} );

describe( 'logger.info()', () => {
it( 'should not log any message', () => {
log.info( logMessage );

expect( vi.mocked( console ).log ).not.toHaveBeenCalled();
} );
} );

describe( 'logger.warning()', () => {
it( 'should not log any message', () => {
log.warning( logMessage );

expect( vi.mocked( console ).log ).not.toHaveBeenCalled();
} );
} );

describe( 'logger.error()', () => {
it( 'should not log any message', () => {
log.error( logMessage );

expect( vi.mocked( console ).log ).not.toHaveBeenCalled();
} );
} );
} );

describe( 'uses default verbosity', () => {
beforeEach( () => {
log = logger();
Expand Down

0 comments on commit 4cfcf41

Please sign in to comment.