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

Replace npm shell commands with pacote #1018

Merged
merged 4 commits into from
Oct 7, 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 @@ -32,16 +32,12 @@ export default async function verifyPackagesPublishedCorrectly( options ) {
for ( const packageToVerify of packagesToVerify ) {
const packageJson = await fs.readJson( upath.join( packageToVerify, 'package.json' ) );

try {
const packageWasUploadedCorrectly = !await checkVersionAvailability( version, packageJson.name );

if ( packageWasUploadedCorrectly ) {
await fs.remove( packageToVerify );
} else {
errors.push( packageJson.name );
}
} catch {
const isPackageVersionAvailable = await checkVersionAvailability( version, packageJson.name );

if ( isPackageVersionAvailable ) {
errors.push( packageJson.name );
} else {
await fs.remove( packageToVerify );
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,41 +3,25 @@
* For licensing, see LICENSE.md.
*/

import { tools } from '@ckeditor/ckeditor5-dev-utils';
import shellEscape from 'shell-escape';
import pacote from 'pacote';

/**
* Checks if the provided version for the package exists in the npm registry.
*
* Returns a promise that resolves to `true` if the provided version does not exist or resolves the promise to `false` otherwise.
* If the `npm show` command exits with an error, it is re-thrown.
*
* @param {string} version
* @param {string} packageName
* @returns {Promise}
*/
export default async function checkVersionAvailability( version, packageName ) {
const command = `npm show ${ shellEscape( [ packageName ] ) }@${ shellEscape( [ version ] ) } version`;

return tools.shExec( command, { verbosity: 'silent', async: true } )
.then( result => {
// Explicit check for npm < 8.13.0, which does not return anything (an empty result) and it exits with a zero status code when
// the version for the provided package does not exist in the npm registry.
if ( !result ) {
return true;
}

// Provided version exists in the npm registry.
return pacote.manifest( `${ packageName }@${ version }` )
.then( () => {
// If `pacote.manifest` resolves, a package with the given version exists.
return false;
} )
.catch( error => {
// All errors except the "E404" are re-thrown.
if ( !error.toString().includes( 'code E404' ) ) {
throw error;
}

// Npm >= 8.13.0 throws an "E404" error if a version does not exist.
// Npm < 8.13.0 should never reach this line.
.catch( () => {
// When throws, the package does not exist.
return true;
} );
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
* For licensing, see LICENSE.md.
*/

import { tools } from '@ckeditor/ckeditor5-dev-utils';
import semver from 'semver';
import shellEscape from 'shell-escape';
import pacote from 'pacote';

/**
* This util aims to verify if the given `packageName` can be published with the given `version` on the `npmTag`.
Expand All @@ -16,10 +15,9 @@ import shellEscape from 'shell-escape';
* @returns {Promise.<boolean>}
*/
export default async function isVersionPublishableForTag( packageName, version, npmTag ) {
const command = `npm view ${ shellEscape( [ packageName ] ) }@${ shellEscape( [ npmTag ] ) } version --silent`;
const npmVersion = await tools.shExec( command, { async: true, verbosity: 'silent' } )
.then( value => value.trim() )
// An `npmTag` does not exist.
const npmVersion = await pacote.manifest( `${ packageName }@${ npmTag }` )
.then( ( { version } ) => version )
// An `npmTag` does not exist, or it's a first release of a package.
.catch( () => null );

if ( npmVersion && semver.lte( version, npmVersion ) ) {
Expand Down
5 changes: 3 additions & 2 deletions packages/ckeditor5-dev-release-tools/lib/utils/versions.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import { tools } from '@ckeditor/ckeditor5-dev-utils';
import pacote from 'pacote';
import getChangelog from './getchangelog.js';
import getPackageJson from './getpackagejson.js';

Expand Down Expand Up @@ -37,9 +38,9 @@ export function getLastFromChangelog( cwd = process.cwd() ) {
export function getLastPreRelease( releaseIdentifier, cwd = process.cwd() ) {
const packageName = getPackageJson( cwd ).name;

return tools.shExec( `npm view ${ packageName } versions --json`, { verbosity: 'silent', async: true } )
return pacote.packument( packageName )
.then( result => {
const lastVersion = JSON.parse( result )
const lastVersion = Object.keys( result.versions )
.filter( version => version.startsWith( releaseIdentifier ) )
.sort( ( a, b ) => a.localeCompare( b, undefined, { numeric: true } ) )
.pop();
Expand Down
1 change: 1 addition & 0 deletions packages/ckeditor5-dev-release-tools/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"inquirer": "^11.0.0",
"lodash-es": "^4.17.21",
"minimatch": "^9.0.0",
"pacote": "^19.0.0",
"semver": "^7.6.3",
"shell-escape": "^0.2.0",
"upath": "^2.0.1"
Expand Down
2 changes: 0 additions & 2 deletions packages/ckeditor5-dev-release-tools/tests/tasks/push.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
* For licensing, see LICENSE.md.
*/

'use strict';

import { beforeEach, describe, expect, it, vi } from 'vitest';
import { tools } from '@ckeditor/ckeditor5-dev-utils';
import shellEscape from 'shell-escape';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
* For licensing, see LICENSE.md.
*/

'use strict';

import { beforeEach, describe, expect, it, vi } from 'vitest';

import columns from 'cli-columns';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
* For licensing, see LICENSE.md.
*/

'use strict';

import { beforeEach, describe, expect, it, vi } from 'vitest';
import { glob } from 'glob';
import upath from 'upath';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
* For licensing, see LICENSE.md.
*/

'use strict';

import { beforeEach, describe, expect, it, vi } from 'vitest';
import { glob } from 'glob';
import fs from 'fs-extra';
Expand Down Expand Up @@ -34,7 +32,7 @@ describe( 'verifyPackagesPublishedCorrectly()', () => {
expect( vi.mocked( checkVersionAvailability ) ).not.toHaveBeenCalled();
} );

it( 'should verify packages and remove them from the release directory on "npm show" command success', async () => {
it( 'should verify packages and remove them from the release directory on if their version are already taken', async () => {
vi.mocked( glob ).mockResolvedValue( [ 'package1', 'package2' ] );
vi.mocked( fs ).readJson
.mockResolvedValueOnce( { name: '@namespace/package1' } )
Expand Down Expand Up @@ -72,24 +70,4 @@ describe( 'verifyPackagesPublishedCorrectly()', () => {

expect( vi.mocked( fs ).remove ).toHaveBeenCalledExactlyOnceWith( 'package2' );
} );

it( 'should not remove package from release directory when checking version on npm throws error', async () => {
vi.mocked( glob ).mockResolvedValue( [ 'package1', 'package2' ] );
vi.mocked( fs ).readJson
.mockResolvedValueOnce( { name: '@namespace/package1' } )
.mockResolvedValueOnce( { name: '@namespace/package2' } );
vi.mocked( checkVersionAvailability )
.mockRejectedValueOnce( )
.mockResolvedValueOnce( false );

const packagesDirectory = '/workspace/ckeditor5/release/npm';
const version = 'latest';
const onSuccess = vi.fn();

await expect( verifyPackagesPublishedCorrectly( { packagesDirectory, version, onSuccess } ) )
.rejects
.toThrow( 'Packages that were uploaded incorrectly, and need manual verification:\n@namespace/package1' );

expect( vi.mocked( fs ).remove ).toHaveBeenCalledExactlyOnceWith( 'package2' );
} );
} );
Original file line number Diff line number Diff line change
Expand Up @@ -3,61 +3,23 @@
* For licensing, see LICENSE.md.
*/

import { beforeEach, describe, expect, it, vi } from 'vitest';
import { tools } from '@ckeditor/ckeditor5-dev-utils';
import shellEscape from 'shell-escape';
import { describe, expect, it, vi } from 'vitest';
import pacote from 'pacote';
import checkVersionAvailability from '../../lib/utils/checkversionavailability.js';

vi.mock( 'shell-escape' );
vi.mock( '@ckeditor/ckeditor5-dev-utils' );
vi.mock( 'pacote' );

describe( 'checkVersionAvailability()', () => {
beforeEach( () => {
vi.mocked( shellEscape ).mockImplementation( v => v[ 0 ] );
} );

it( 'should resolve to true if version does not exist (npm >= 8.13.0 && npm < 10.0.0)', async () => {
vi.mocked( tools ).shExec.mockRejectedValue( new Error( 'npm ERR! code E404' ) );

const result = await checkVersionAvailability( '1.0.1', 'stub-package' );

expect( vi.mocked( tools ).shExec ).toHaveBeenCalledExactlyOnceWith( 'npm show [email protected] version', expect.any( Object ) );
expect( result ).toBe( true );
} );

it( 'should resolve to true if version does not exist (npm >= 10.0.0)', async () => {
vi.mocked( tools ).shExec.mockRejectedValue( new Error( 'npm error code E404' ) );

const result = await checkVersionAvailability( '1.0.1', 'stub-package' );
expect( vi.mocked( tools ).shExec ).toHaveBeenCalledExactlyOnceWith( 'npm show [email protected] version', expect.any( Object ) );
expect( result ).toBe( true );
} );

it( 'should resolve to true if version does not exist (npm < 8.13.0)', async () => {
vi.mocked( tools ).shExec.mockResolvedValue( '' );
it( 'should resolve to true if version does not exist', async () => {
vi.mocked( pacote.manifest ).mockRejectedValue( 'E404' );

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

expect( pacote.manifest ).toHaveBeenCalledExactlyOnceWith( '[email protected]' );
} );
it( 'should resolve to false if version exists', async () => {
vi.mocked( tools ).shExec.mockResolvedValue( '1.0.1' );
pacote.manifest.mockResolvedValue( '1.0.1' );

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

it( 'should re-throw an error if unknown error occurred', async () => {
vi.mocked( tools ).shExec.mockRejectedValue( new Error( 'Unknown error.' ) );

await expect( checkVersionAvailability( '1.0.1', 'stub-package' ) )
.rejects.toThrow( 'Unknown error.' );
} );

it( 'should escape arguments passed to a shell command', async () => {
vi.mocked( tools ).shExec.mockRejectedValue( new Error( 'npm ERR! code E404' ) );

await checkVersionAvailability( '1.0.1', 'stub-package' );
expect( vi.mocked( shellEscape ) ).toHaveBeenCalledTimes( 2 );
expect( vi.mocked( shellEscape ) ).toHaveBeenCalledWith( [ 'stub-package' ] );
expect( vi.mocked( shellEscape ) ).toHaveBeenCalledWith( [ '1.0.1' ] );
} );
} );
Original file line number Diff line number Diff line change
Expand Up @@ -3,67 +3,49 @@
* For licensing, see LICENSE.md.
*/

import { describe, it, expect, vi, beforeEach } from 'vitest';
import { tools } from '@ckeditor/ckeditor5-dev-utils';
import { describe, expect, it, vi } from 'vitest';
import pacote from 'pacote';
import semver from 'semver';
import shellEscape from 'shell-escape';

import isVersionPublishableForTag from '../../lib/utils/isversionpublishablefortag.js';

vi.mock( '@ckeditor/ckeditor5-dev-utils' );
vi.mock( 'pacote' );
vi.mock( 'semver' );
vi.mock( 'shell-escape' );

describe( 'isVersionPublishableForTag()', () => {
beforeEach( () => {
vi.mocked( shellEscape ).mockImplementation( v => v[ 0 ] );
} );

it( 'should return true if given version is available', async () => {
vi.mocked( semver.lte ).mockReturnValue( false );
vi.mocked( tools.shExec ).mockResolvedValue( '1.0.0\n' );
it( 'should return false if given version is not available', async () => {
vi.mocked( semver.lte ).mockReturnValue( true );
vi.mocked( pacote.manifest ).mockResolvedValue( ( {
version: '1.0.0'
} ) );

const result = await isVersionPublishableForTag( 'package-name', '1.0.1', 'latest' );
const result = await isVersionPublishableForTag( 'package-name', '1.0.0', 'latest' );

expect( result ).to.equal( true );
expect( semver.lte ).toHaveBeenCalledTimes( 1 );
expect( semver.lte ).toHaveBeenCalledWith( '1.0.1', '1.0.0' );
expect( tools.shExec ).toHaveBeenCalledTimes( 1 );
expect( tools.shExec ).toHaveBeenCalledWith( 'npm view package-name@latest version --silent', expect.anything() );
expect( result ).to.equal( false );
expect( semver.lte ).toHaveBeenCalledExactlyOnceWith( '1.0.0', '1.0.0' );
expect( pacote.manifest ).toHaveBeenCalledExactlyOnceWith( 'package-name@latest' );
} );

it( 'should return false if given version is not available', async () => {
it( 'should return false if given version is not higher than the latest published', async () => {
vi.mocked( semver.lte ).mockReturnValue( true );
vi.mocked( tools.shExec ).mockResolvedValue( '1.0.0\n' );

vi.mocked( pacote.manifest ).mockResolvedValue( ( {
version: '1.0.1'
} ) );

const result = await isVersionPublishableForTag( 'package-name', '1.0.0', 'latest' );

expect( result ).to.equal( false );
expect( semver.lte ).toHaveBeenCalledTimes( 1 );
expect( semver.lte ).toHaveBeenCalledWith( '1.0.0', '1.0.0' );
expect( tools.shExec ).toHaveBeenCalledTimes( 1 );
expect( tools.shExec ).toHaveBeenCalledWith( 'npm view package-name@latest version --silent', expect.anything() );
expect( semver.lte ).toHaveBeenCalledExactlyOnceWith( '1.0.0', '1.0.1' );
} );

it( 'should return true if given npm tag is not published yet', async () => {
vi.mocked( tools.shExec ).mockRejectedValue( 'E404' );
vi.mocked( pacote.manifest ).mockRejectedValue( 'E404' );

const result = await isVersionPublishableForTag( 'package-name', '1.0.0', 'alpha' );

expect( result ).to.equal( true );
expect( semver.lte ).not.toHaveBeenCalled();
expect( tools.shExec ).toHaveBeenCalledTimes( 1 );
expect( tools.shExec ).toHaveBeenCalledWith( 'npm view package-name@alpha version --silent', expect.anything() );
} );

it( 'should escape arguments passed to a shell command', async () => {
vi.mocked( semver.lte ).mockReturnValue( false );
vi.mocked( tools.shExec ).mockResolvedValue( '1.0.0\n' );

await isVersionPublishableForTag( 'package-name', '1.0.0', 'alpha' );

expect( shellEscape ).toHaveBeenCalledTimes( 2 );
expect( shellEscape ).toHaveBeenNthCalledWith( 1, [ 'package-name' ] );
expect( shellEscape ).toHaveBeenNthCalledWith( 2, [ 'alpha' ] );
expect( pacote.manifest ).toHaveBeenCalledExactlyOnceWith( 'package-name@alpha' );
} );
} );
Loading