Skip to content

Commit

Permalink
feat(cdk-test): check API compatibility (#2356)
Browse files Browse the repository at this point in the history
Add a check to `cdk-test` to confirm that no breaking API changes
have been introduced with respect to the most recently published
version.

Because this will incur an NPM fetch, add a flag (`--quick`/`-q`) to
disable it.

Addresses #145.
  • Loading branch information
rix0rrr authored Apr 26, 2019
1 parent c8a19e5 commit 1642925
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 29 deletions.
4 changes: 2 additions & 2 deletions tools/cdk-build-tools/bin/cdk-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ async function main() {
const options = cdkBuildOptions();

if (options.pre) {
await shell(options.pre, timers);
await shell(options.pre, { timers });
}

// See if we need to call cfn2ts
Expand All @@ -37,7 +37,7 @@ async function main() {
// There can be multiple scopes, ensuring it's always an array.
options.cloudformation = [options.cloudformation];
}
await shell(['cfn2ts', ...options.cloudformation.map(scope => `--scope=${scope}`)], timers);
await shell(['cfn2ts', ...options.cloudformation.map(scope => `--scope=${scope}`)], { timers });
}

await compileCurrentPackage(timers, { jsii: args.jsii, tsc: args.tsc, tslint: args.tslint });
Expand Down
4 changes: 2 additions & 2 deletions tools/cdk-build-tools/bin/cdk-package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ async function main() {
args.verbose ? '-vvv' : '-v',
...args.targets ? flatMap(args.targets, (target: string) => ['-t', target]) : [],
'-o', outdir ];
await shell(command, timers);
await shell(command, { timers });
} else {
// just "npm pack" and deploy to "outdir"
const tarball = (await shell([ 'npm', 'pack' ], timers)).trim();
const tarball = (await shell([ 'npm', 'pack' ], { timers })).trim();
const target = path.join(outdir, 'js');
await fs.remove(target);
await fs.mkdirp(target);
Expand Down
28 changes: 23 additions & 5 deletions tools/cdk-build-tools/bin/cdk-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ async function main() {
const args = yargs
.env('CDK_TEST')
.usage('Usage: cdk-test')
.option('force', { type: 'boolean', alias: 'f', desc: 'Force a rebuild' })
.option('quick', { type: 'boolean', alias: 'q', desc: `Skip slow tests`, default: false })
.option('jsii-diff', {
type: 'string',
desc: 'Specify a different jsii-diff executable',
default: require.resolve('jsii-diff/bin/jsii-diff'),
defaultDescription: 'jsii-diff provided by node dependencies'
})
.option('jest', {
type: 'string',
desc: 'Specify a different jest executable',
Expand All @@ -33,7 +39,7 @@ async function main() {
const options = cdkBuildOptions();

if (options.test) {
await shell(options.test, timers);
await shell(options.test, { timers });
}

const testFiles = await unitTestFiles();
Expand All @@ -43,7 +49,7 @@ async function main() {
if (testFiles.length > 0) {
throw new Error(`Jest is enabled, but ${testFiles.length} nodeunit tests were found!`);
}
await shell([args.jest, '--testEnvironment=node', '--coverage'], timers);
await shell([args.jest, '--testEnvironment=node', '--coverage'], { timers });
} else if (testFiles.length > 0) {
const testCommand: string[] = [];

Expand All @@ -67,12 +73,24 @@ async function main() {
testCommand.push(args.nodeunit);
testCommand.push(...testFiles.map(f => f.path));

await shell(testCommand, timers);
await shell(testCommand, { timers });
}

// Run integration test if the package has integ test files
if (await hasIntegTests()) {
await shell(['cdk-integ-assert'], timers);
await shell(['cdk-integ-assert'], { timers });
}

// Run compatibility check if not disabled (against the latest
// published version)
if (!args.quick) {
try {
await shell([args["jsii-diff"], 'npm:'], { timers });
} catch (e) {
// If there was an exception running jsii-diff, swallow it
process.stderr.write(`The package seems to have undergone breaking API changes. Please revise and try to avoid.\n`);
process.stderr.write(`(This is just a warning for now but will soon become a build failure.)\n`);
}
}
}

Expand Down
8 changes: 4 additions & 4 deletions tools/cdk-build-tools/lib/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Timers } from "./timer";
* Run the compiler on the current package
*/
export async function compileCurrentPackage(timers: Timers, compilers: CompilerOverrides = {}): Promise<void> {
const stdout = await shell(packageCompiler(compilers), timers);
const stdout = await shell(packageCompiler(compilers), { timers });

// WORKAROUND: jsii 0.8.2 does not exit with non-zero on compilation errors
// until this is released: https://github.com/awslabs/jsii/pull/442
Expand All @@ -22,7 +22,7 @@ export async function compileCurrentPackage(timers: Timers, compilers: CompilerO
}

// Always call linters
await shell([compilers.tslint || require.resolve('tslint/bin/tslint'), '--project', '.'], timers);
await shell(['pkglint'], timers);
await shell([ path.join(__dirname, '..', 'bin', 'cdk-awslint') ], timers);
await shell([compilers.tslint || require.resolve('tslint/bin/tslint'), '--project', '.'], { timers });
await shell(['pkglint'], { timers });
await shell([ path.join(__dirname, '..', 'bin', 'cdk-awslint') ], { timers });
}
17 changes: 14 additions & 3 deletions tools/cdk-build-tools/lib/os.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,31 @@
import child_process = require("child_process");
import colors = require('colors/safe');
import fs = require('fs');
import util = require('util');
import { Timers } from "./timer";

interface ShellOptions {
timers?: Timers;
}

/**
* A shell command that does what you want
*
* Is platform-aware, handles errors nicely.
*/
export async function shell(command: string[], timers?: Timers): Promise<string> {
const timer = (timers || new Timers()).start(command[0]);
export async function shell(command: string[], options: ShellOptions = {}): Promise<string> {
const timer = (options.timers || new Timers()).start(command[0]);

await makeShellScriptExecutable(command[0]);

const child = child_process.spawn(command[0], command.slice(1), {
// Need this for Windows where we want .cmd and .bat to be found as well.
shell: true,
stdio: [ 'ignore', 'pipe', 'inherit' ]
stdio: [ 'ignore', 'pipe', 'pipe' ]
});

const makeRed = process.stderr.isTTY ? colors.red : (x: string) => x;

return new Promise<string>((resolve, reject) => {
const stdout = new Array<any>();

Expand All @@ -27,6 +34,10 @@ export async function shell(command: string[], timers?: Timers): Promise<string>
stdout.push(chunk);
});

child.stderr!.on('data', chunk => {
process.stderr.write(makeRed(chunk.toString()));
});

child.once('error', reject);

child.once('exit', code => {
Expand Down
74 changes: 61 additions & 13 deletions tools/cdk-build-tools/package-lock.json

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

2 changes: 2 additions & 0 deletions tools/cdk-build-tools/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@
},
"dependencies": {
"awslint": "^0.29.0",
"colors": "^1.3.3",
"fs-extra": "^7.0.1",
"jest": "^24.7.1",
"jsii": "^0.10.3",
"jsii-diff": "^0.10.3",
"jsii-pacmak": "^0.10.3",
"nodeunit": "^0.11.3",
"nyc": "^14.0.0",
Expand Down

0 comments on commit 1642925

Please sign in to comment.