Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
16 changes: 13 additions & 3 deletions tools/release/commands/packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ async function runNpmPublishPreflight(
// TODO: Consider bounded concurrency here if this preflight becomes too slow.
// Keep registry checks sequential so errors stay easy to read.
for ( const { name, version } of releasePackages ) {
let registryPackage;
let registryOutput;
try {
const { stdout } = await commandFn(
`npm view ${ name }@${ version } version gitHead dist-tags --json`,
Expand All @@ -783,7 +783,7 @@ async function runNpmPublishPreflight(
stdio: 'pipe',
}
);
registryPackage = parseNpmJsonOutput(
registryOutput = parseNpmJsonOutput(
stdout,
`${ name }@${ version } metadata`
);
Expand All @@ -794,11 +794,21 @@ async function runNpmPublishPreflight(
throw error;
}

/* npm v12 always returns an array; older versions return an object for a single match. */
const registryPackages = Array.isArray( registryOutput )
? registryOutput
: [ registryOutput ];
if ( registryPackages.length !== 1 ) {
throw new Error(
`Expected npm registry lookup for ${ name }@${ version } to return one version, got ${ registryPackages.length }.`
);
}

const {
version: registryVersion,
gitHead: registryGitHead,
'dist-tags': distTags = {},
} = registryPackage;
} = registryPackages[ 0 ];
Comment thread
manzoorwanijk marked this conversation as resolved.
Resolved
if ( registryVersion !== version ) {
throw new Error(
`Expected npm registry lookup for ${ name }@${ version } to return version ${ version }, got ${ registryVersion }.`
Expand Down
90 changes: 67 additions & 23 deletions tools/release/commands/test/packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -549,30 +549,74 @@ describe( 'runNpmPublishPreflight', () => {
expect( console ).toHaveLogged();
} );

it( 'accepts a published version from the prepared commit with the expected dist-tag', async () => {
const commandFn = vi
.fn()
.mockResolvedValueOnce( WHOAMI )
.mockResolvedValueOnce( {
stdout: '{"version":"4.50.0","gitHead":"publish-sha","dist-tags":{"latest":"4.50.0"}}',
} );
it.each( [
Comment thread
manzoorwanijk marked this conversation as resolved.
Resolved
[
'an object, as npm v11 returns',
'{"version":"4.50.0","gitHead":"publish-sha","dist-tags":{"latest":"4.50.0"}}',
],
[
'an array, as npm v12 returns',
'[{"version":"4.50.0","gitHead":"publish-sha","dist-tags":{"latest":"4.50.0"}}]',
],
] )(
'accepts a published version from the prepared commit with the expected dist-tag when npm view returns %s',
async ( _description, stdout ) => {
const commandFn = vi
.fn()
.mockResolvedValueOnce( WHOAMI )
.mockResolvedValueOnce( { stdout } );

await expect(
runNpmPublishPreflight(
{
distTag: 'latest',
gitWorkingDirectoryPath: '/repo',
publishCommit: 'publish-sha',
releasePackages: [
{ name: '@wordpress/a11y', version: '4.50.0' },
],
},
{ commandFn }
)
).resolves.toEqual( [ '@wordpress/a11y' ] );
expect( commandFn ).toHaveBeenCalledTimes( 2 );
expect( console ).toHaveLogged();
} );
await expect(
runNpmPublishPreflight(
{
distTag: 'latest',
gitWorkingDirectoryPath: '/repo',
publishCommit: 'publish-sha',
releasePackages: [
{ name: '@wordpress/a11y', version: '4.50.0' },
],
},
{ commandFn }
)
).resolves.toEqual( [ '@wordpress/a11y' ] );
expect( commandFn ).toHaveBeenCalledTimes( 2 );
expect( console ).toHaveLogged();
}
);

it.each( [
[ 'no versions', '[]', 0 ],
[
'more than one version',
'[{"version":"4.50.0","gitHead":"publish-sha"},{"version":"4.50.1","gitHead":"other-sha"}]',
2,
],
] )(
'fails when npm view returns %s',
async ( _description, stdout, count ) => {
const commandFn = vi
.fn()
.mockResolvedValueOnce( WHOAMI )
.mockResolvedValueOnce( { stdout } );

await expect(
runNpmPublishPreflight(
{
distTag: 'latest',
gitWorkingDirectoryPath: '/repo',
publishCommit: 'publish-sha',
releasePackages: [
{ name: '@wordpress/a11y', version: '4.50.0' },
],
},
{ commandFn }
)
).rejects.toThrow(
`Expected npm registry lookup for @wordpress/a11y@4.50.0 to return one version, got ${ count }.`
);
expect( console ).toHaveLogged();
}
);

it( 'fails when a published version came from another commit', async () => {
const commandFn = vi
Expand Down
28 changes: 28 additions & 0 deletions tools/validation/read-npm-pack-result.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Reads the single package result from `npm pack --json` output.
*
* @param {string} stdout The `npm pack --json` stdout.
* @param {string} packageName Package name for error messages.
*
* @return {Object} The pack result for the package.
*/
export function readNpmPackResult( stdout, packageName ) {
let output;
try {
output = JSON.parse( stdout );
} catch {
throw new Error(
`Could not parse npm pack output for ${ packageName }:\n${ stdout }`
);
}
/* npm v12 keys `pack --json` output by package name; older versions return an array. */
const results = Array.isArray( output )
? output
: Object.values( output ?? {} );
if ( results.length !== 1 ) {
throw new Error(
`Expected one npm pack result for ${ packageName }, got ${ results.length }.`
);
}
return results[ 0 ];
Comment thread
manzoorwanijk marked this conversation as resolved.
Resolved
}
31 changes: 31 additions & 0 deletions tools/validation/read-npm-pack-result.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { expect, test } from 'vitest';
import { readNpmPackResult } from './read-npm-pack-result.mjs';

const result = { name: 'test-package', filename: 'test-package-1.0.0.tgz' };

test.each( [
[ 'an array, as npm v11 returns', [ result ] ],
[
'an object keyed by name, as npm v12 returns',
{ 'test-package': result },
],
] )( 'reads %s', ( _description, output ) => {
expect(
readNpmPackResult( JSON.stringify( output ), 'test-package' )
).toEqual( result );
} );

test.each( [
Comment thread
manzoorwanijk marked this conversation as resolved.
Resolved
[ 'no results', '[]', 0 ],
[ 'more than one result', JSON.stringify( [ result, result ] ), 2 ],
] )( 'fails on %s', ( _description, stdout, count ) => {
expect( () => readNpmPackResult( stdout, 'test-package' ) ).toThrow(
`Expected one npm pack result for test-package, got ${ count }.`
);
} );

test( 'fails on output that is not JSON', () => {
expect( () =>
readNpmPackResult( 'npm warn something', 'test-package' )
).toThrow( 'Could not parse npm pack output for test-package' );
} );
34 changes: 19 additions & 15 deletions tools/validation/test-vitest-consumers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { tmpdir } from 'node:os';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { parseArgs } from 'node:util';
import { readNpmPackResult } from './read-npm-pack-result.mjs';

const { values } = parseArgs( {
options: {
Expand Down Expand Up @@ -45,7 +46,7 @@ const env = {
delete env.NODE_OPTIONS;
delete env.NODE_PATH;

function run( command, args, cwd = consumer ) {
function execute( command, args, cwd ) {
const result = spawnSync( command, args, {
cwd,
env,
Expand All @@ -62,6 +63,10 @@ function run( command, args, cwd = consumer ) {
result.stderr
}`
);
return result;
}
function run( command, args, cwd = consumer ) {
const result = execute( command, args, cwd );
return result.stdout + result.stderr;
}
function write( name, content ) {
Expand Down Expand Up @@ -104,21 +109,20 @@ function pack( name ) {
path.join( staging, 'package.json' ),
JSON.stringify( manifest )
);
const output = JSON.parse(
run(
'npm',
[
'pack',
staging,
'--json',
'--ignore-scripts',
'--pack-destination',
directory,
],
directory
)
const { stdout } = execute(
'npm',
[
'pack',
staging,
'--json',
'--ignore-scripts',
'--pack-destination',
directory,
],
directory
);
return `file:${ path.join( directory, output[ 0 ].filename ) }`;
const packed = readNpmPackResult( stdout, manifest.name );
return `file:${ path.join( directory, packed.filename ) }`;
}
const scripts = values.scripts ?? pack( 'scripts' );
const eslintPlugin = values[ 'eslint-plugin' ] ?? pack( 'eslint-plugin' );
Expand Down
7 changes: 2 additions & 5 deletions tools/validation/validate-package-contents.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { readFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join, resolve } from 'node:path';
import { parseArgs } from 'node:util';
import { readNpmPackResult } from './read-npm-pack-result.mjs';

const usage =
'Usage: node tools/validation/validate-package-contents.mjs <package-directory> [--disallow-path <path>]';
Expand Down Expand Up @@ -67,11 +68,7 @@ if ( packResult.status !== 0 ) {
process.exit( packResult.status ?? 1 );
}

/* npm v12 keys `pack --json` output by package name; older versions return an array. */
const packOutput = JSON.parse( packResult.stdout );
const [ pack ] = Array.isArray( packOutput )
? packOutput
: Object.values( packOutput );
const pack = readNpmPackResult( packResult.stdout, packageJson.name );
const packedPaths = pack.files.map( ( { path } ) => path );
const packedPathSet = new Set( packedPaths );
const disallowedPathPatterns = [
Expand Down
Loading