Skip to content

Commit

Permalink
refactor: make node-bundle tests executable using ts-jest (#32022)
Browse files Browse the repository at this point in the history
Make `node-bundle` easier to test (in-process instead of using a subcommand that requires `.js` to have been compiled), and fix a bug in the tests that used `--license` instead of `--allowed-license` (configure `yargs` to be `strict`).

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Nov 5, 2024
1 parent ae29bb5 commit 5a3a32f
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 138 deletions.
1 change: 1 addition & 0 deletions tools/@aws-cdk/node-bundle/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion tools/@aws-cdk/node-bundle/src/api/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ export class Bundle {

if (this.test) {
const command = `${path.join(bundleDir, this.test)}`;
console.log(`Running santiy test: ${command}`);
console.log(`Running sanity test: ${command}`);
shell(command, { cwd: bundleDir });
}

Expand Down
115 changes: 115 additions & 0 deletions tools/@aws-cdk/node-bundle/src/cli-main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import * as path from 'path';
import * as fs from 'fs-extra';
import * as yargs from 'yargs';
import { Bundle, BundleProps, BundleValidateOptions } from './api';

function versionNumber(): string {
return fs.readJSONSync(path.join(__dirname, '..', 'package.json')).version;
}

export async function cliMain(cliArgs: string[]) {
const argv = await yargs
.usage('Usage: node-bundle COMMAND')
.option('entrypoint', { type: 'array', nargs: 1, desc: 'List of entrypoints to bundle' })
.option('external', { type: 'array', nargs: 1, default: [], desc: 'Packages in this list will be excluded from the bundle and added as dependencies (example: fsevents:optional)' })
.option('allowed-license', { type: 'array', nargs: 1, default: [], desc: 'List of valid licenses' })
.option('resource', { type: 'array', nargs: 1, default: [], desc: 'List of resources that need to be explicitly copied to the bundle (example: node_modules/proxy-agent/contextify.js:bin/contextify.js)' })
.option('dont-attribute', { type: 'string', desc: 'Dependencies matching this regular expressions wont be added to the notice file' })
.option('test', { type: 'string', desc: 'Validation command to sanity test the bundle after its created' })
.command('validate', 'Validate the package is ready for bundling', args => args
.option('fix', { type: 'boolean', default: false, alias: 'f', desc: 'Fix any fixable violations' }),
)
.command('write', 'Write the bundled version of the project to a temp directory')
.command('pack', 'Write the bundle and create the tarball')
.demandCommand() // require a subcommand
.strict() // require a VALID subcommand, and only supported options
.fail((msg, err) => {
// Throw an error in test mode, exit with an error code otherwise
if (err) { throw err; }
if (process.env.NODE_ENV === 'test') {
throw new Error(msg);
}
console.error(msg);
process.exit(1); // exit() not exitCode, we must not return.
})
.help()
.version(versionNumber())
.parse(cliArgs);

const command = argv._[0];

function undefinedIfEmpty(arr?: any[]): string[] | undefined {
if (!arr || arr.length === 0) return undefined;
return arr as string[];
}

const resources: any = {};
for (const resource of (argv.resource as string[])) {
const parts = resource.split(':');
resources[parts[0]] = parts[1];
}

const optionalExternals = [];
const runtimeExternals = [];

for (const external of (argv.external as string[])) {
const parts = external.split(':');
const name = parts[0];
const type = parts[1];
switch (type) {
case 'optional':
optionalExternals.push(name);
break;
case 'runtime':
runtimeExternals.push(name);
break;
default:
throw new Error(`Unsupported dependency type '${type}' for external package '${name}'. Supported types are: ['optional', 'runtime']`);
}
}

const props: BundleProps = {
packageDir: process.cwd(),
entryPoints: undefinedIfEmpty(argv.entrypoint),
externals: { dependencies: runtimeExternals, optionalDependencies: optionalExternals },
allowedLicenses: undefinedIfEmpty(argv['allowed-license']),
resources: resources,
dontAttribute: argv['dont-attribute'],
test: argv.test,
};

const bundle = new Bundle(props);

switch (command) {
case 'validate':
// When using `yargs.command(command, builder [, handler])` without the handler
// as we do here, there is no typing for command-specific options. So force a cast.
const fix = argv.fix as boolean | undefined;
validate(bundle, { fix });
break;
case 'write':
write(bundle);
break;
case 'pack':
pack(bundle);
break;
default:
throw new Error(`Unknown command: ${command}`);
}
}

function write(bundle: Bundle) {
const bundleDir = bundle.write();
console.log(bundleDir);
}

function validate(bundle: Bundle, options: BundleValidateOptions = {}) {
const report = bundle.validate(options);
if (!report.success) {
throw new Error(report.summary);
}
}

function pack(bundle: Bundle) {
bundle.pack();
}
105 changes: 2 additions & 103 deletions tools/@aws-cdk/node-bundle/src/cli.ts
Original file line number Diff line number Diff line change
@@ -1,107 +1,6 @@
import * as path from 'path';
import * as fs from 'fs-extra';
import * as yargs from 'yargs';
import { Bundle, BundleProps, BundleValidateOptions } from './api';
import { cliMain } from './cli-main';

function versionNumber(): string {
return fs.readJSONSync(path.join(__dirname, '..', 'package.json')).version;
}

async function buildCommands() {

const argv = yargs
.usage('Usage: node-bundle COMMAND')
.option('entrypoint', { type: 'array', nargs: 1, desc: 'List of entrypoints to bundle' })
.option('external', { type: 'array', nargs: 1, default: [], desc: 'Packages in this list will be excluded from the bundle and added as dependencies (example: fsevents:optional)' })
.option('allowed-license', { type: 'array', nargs: 1, default: [], desc: 'List of valid licenses' })
.option('resource', { type: 'array', nargs: 1, default: [], desc: 'List of resources that need to be explicitly copied to the bundle (example: node_modules/proxy-agent/contextify.js:bin/contextify.js)' })
.option('dont-attribute', { type: 'string', desc: 'Dependencies matching this regular expressions wont be added to the notice file' })
.option('test', { type: 'string', desc: 'Validation command to sanity test the bundle after its created' })
.command('validate', 'Validate the package is ready for bundling', args => args
.option('fix', { type: 'boolean', default: false, alias: 'f', desc: 'Fix any fixable violations' }),
)
.command('write', 'Write the bundled version of the project to a temp directory')
.command('pack', 'Write the bundle and create the tarball')
.help()
.version(versionNumber())
.argv;

const command = argv._[0];

function undefinedIfEmpty(arr?: any[]): string[] | undefined {
if (!arr || arr.length === 0) return undefined;
return arr as string[];
}

const resources: any = {};
for (const resource of (argv.resource as string[])) {
const parts = resource.split(':');
resources[parts[0]] = parts[1];
}

const optionalExternals = [];
const runtimeExternals = [];

for (const external of (argv.external as string[])) {
const parts = external.split(':');
const name = parts[0];
const type = parts[1];
switch (type) {
case 'optional':
optionalExternals.push(name);
break;
case 'runtime':
runtimeExternals.push(name);
break;
default:
throw new Error(`Unsupported dependency type '${type}' for external package '${name}'. Supported types are: ['optional', 'runtime']`);
}
}

const props: BundleProps = {
packageDir: process.cwd(),
entryPoints: undefinedIfEmpty(argv.entrypoint),
externals: { dependencies: runtimeExternals, optionalDependencies: optionalExternals },
allowedLicenses: undefinedIfEmpty(argv['allowed-license']),
resources: resources,
dontAttribute: argv['dont-attribute'],
test: argv.test,
};

const bundle = new Bundle(props);

switch (command) {
case 'validate':
validate(bundle, { fix: argv.fix });
break;
case 'write':
write(bundle);
break;
case 'pack':
pack(bundle);
break;
default:
throw new Error(`Unknown command: ${command}`);
}
}

function write(bundle: Bundle) {
const bundleDir = bundle.write();
console.log(bundleDir);
}

function validate(bundle: Bundle, options: BundleValidateOptions = {}) {
const report = bundle.validate(options);
if (!report.success) {
throw new Error(report.summary);
}
}

function pack(bundle: Bundle) {
bundle.pack();
}

buildCommands()
cliMain(process.argv.slice(2))
.catch((err: Error) => {
console.error(`Error: ${err.message}`);
process.exitCode = 1;
Expand Down
79 changes: 46 additions & 33 deletions tools/@aws-cdk/node-bundle/test/cli.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import * as path from 'path';
import * as fs from 'fs-extra';
import { cliMain } from '../src/cli-main';
import { Package } from './_package';
import { shell } from '../src/api/_shell';
import * as util from 'util';

test('validate', () => {
test('validate', async () => {

const pkg = Package.create({ name: 'consumer', licenses: ['Apache-2.0'], circular: true });
const dep1 = pkg.addDependency({ name: 'dep1', licenses: ['INVALID'] });
Expand All @@ -14,15 +15,14 @@ test('validate', () => {

try {
const command = [
whereami(),
'--entrypoint', pkg.entrypoint,
'--resource', 'missing:bin/missing',
'--license', 'Apache-2.0',
'--allowed-license', 'Apache-2.0',
'validate',
].join(' ');
shell(command, { cwd: pkg.dir, quiet: true });
];
await runCliMain(pkg.dir, command);
} catch (e: any) {
const violations = new Set(e.stderr.toString().trim().split('\n').filter((l: string) => l.startsWith('-')));
const violations = new Set(e.message.trim().split('\n').filter((l: string) => l.startsWith('-')));
const expected = new Set([
`- invalid-license: Dependency ${dep1.name}@${dep1.version} has an invalid license: UNKNOWN`,
`- multiple-license: Dependency ${dep2.name}@${dep2.version} has multiple licenses: Apache-2.0,MIT`,
Expand All @@ -35,7 +35,7 @@ test('validate', () => {

});

test('write', () => {
test('write', async () => {

const pkg = Package.create({ name: 'consumer', licenses: ['Apache-2.0'] });
pkg.addDependency({ name: 'dep1', licenses: ['MIT'] });
Expand All @@ -45,13 +45,12 @@ test('write', () => {
pkg.install();

const command = [
whereami(),
'--entrypoint', pkg.entrypoint,
'--license', 'Apache-2.0',
'--license', 'MIT',
'--allowed-license', 'Apache-2.0',
'--allowed-license', 'MIT',
'write',
].join(' ');
const bundleDir = shell(command, { cwd: pkg.dir, quiet: true });
];
const bundleDir = await runCliMain(pkg.dir, command);

expect(fs.existsSync(path.join(bundleDir, pkg.entrypoint))).toBeTruthy();
expect(fs.existsSync(path.join(bundleDir, 'package.json'))).toBeTruthy();
Expand All @@ -67,7 +66,7 @@ test('write', () => {

});

test('validate and fix', () => {
test('validate and fix', async () => {

const pkg = Package.create({ name: 'consumer', licenses: ['Apache-2.0'] });
pkg.addDependency({ name: 'dep1', licenses: ['MIT'] });
Expand All @@ -76,33 +75,32 @@ test('validate and fix', () => {
pkg.write();
pkg.install();

const run = (sub: string) => {
const run = (sub: string[]) => {
const command = [
whereami(),
'--entrypoint', pkg.entrypoint,
'--license', 'Apache-2.0',
'--license', 'MIT',
sub,
].join(' ');
shell(command, { cwd: pkg.dir, quiet: true });
'--allowed-license', 'Apache-2.0',
'--allowed-license', 'MIT',
...sub,
];
return runCliMain(pkg.dir, command);
};

try {
run('pack');
await run(['pack']);
throw new Error('Expected packing to fail before fixing');
} catch {
// this should fix the fact we don't generate
// the project with the correct attributions
run('validate --fix');
await run(['validate', '--fix']);
}

run('pack');
await run(['pack']);
const tarball = path.join(pkg.dir, `${pkg.name}-${pkg.version}.tgz`);
expect(fs.existsSync(tarball)).toBeTruthy();

});

test('pack', () => {
test('pack', async () => {

const pkg = Package.create({ name: 'consumer', licenses: ['Apache-2.0'] });
const dep1 = pkg.addDependency({ name: 'dep1', licenses: ['MIT'] });
Expand All @@ -127,19 +125,34 @@ test('pack', () => {
pkg.install();

const command = [
whereami(),
'--entrypoint', pkg.entrypoint,
'--license', 'Apache-2.0',
'--license', 'MIT',
'--allowed-license', 'Apache-2.0',
'--allowed-license', 'MIT',
'pack',
].join(' ');
shell(command, { cwd: pkg.dir, quiet: true });
];
await runCliMain(pkg.dir, command);

const tarball = path.join(pkg.dir, `${pkg.name}-${pkg.version}.tgz`);
expect(fs.existsSync(tarball)).toBeTruthy();

});

function whereami() {
return path.join(path.join(__dirname, '..', 'bin', 'node-bundle'));
}
async function runCliMain(cwd: string, command: string[]): Promise<string> {
const log: string[] = []
const spy = jest
.spyOn(console, 'log')
.mockImplementation((...args) => {
log.push(util.format(...args));
});

const curdir = process.cwd();
process.chdir(cwd);
try {
await cliMain(command);

return log.join('\n');
} finally {
process.chdir(curdir);
spy.mockRestore();
}
}
Loading

0 comments on commit 5a3a32f

Please sign in to comment.