Skip to content
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

Try to slow down the "npm publish" command #1038

Merged
merged 2 commits into from
Oct 25, 2024
Merged
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 @@ -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( '[email protected]', { cache: null } );
expect( pacote.manifest ).toHaveBeenCalledExactlyOnceWith( '[email protected]', 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