From f675f562c2cf73333de1b4d4b1754a622cdb308d Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 26 Oct 2021 17:02:15 -0700 Subject: [PATCH 01/17] added support for the field in cdk.json --- packages/aws-cdk/lib/api/cxapp/exec.ts | 11 ++++++++--- packages/aws-cdk/test/api/exec.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index facaf24a3bfe0..2d8b52595ba49 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -46,6 +46,11 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom debug('context:', context); env[cxapi.CONTEXT_ENV] = JSON.stringify(context); + const build = config.settings.get(['build']); + if (build) { + await exec(build); + } + const app = config.settings.get(['app']); if (!app) { throw new Error(`--app is required either in command-line, in ${PROJECT_CONFIG} or in ${USER_DEFAULTS}`); @@ -74,7 +79,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom debug('env:', env); - await exec(); + await exec(commandLine[0], commandLine.slice(1)); return createAssembly(outdir); @@ -91,7 +96,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom } } - async function exec() { + async function exec(command: string, args: string[] = []) { return new Promise((ok, fail) => { // We use a slightly lower-level interface to: // @@ -103,7 +108,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom // anyway, and if the subprocess is printing to it for debugging purposes the // user gets to see it sooner. Plus, capturing doesn't interact nicely with some // processes like Maven. - const proc = childProcess.spawn(commandLine[0], commandLine.slice(1), { + const proc = childProcess.spawn(command, args, { stdio: ['ignore', 'inherit', 'inherit'], detached: false, shell: true, diff --git a/packages/aws-cdk/test/api/exec.test.ts b/packages/aws-cdk/test/api/exec.test.ts index d2e65907527fa..f7773242a1d71 100644 --- a/packages/aws-cdk/test/api/exec.test.ts +++ b/packages/aws-cdk/test/api/exec.test.ts @@ -194,6 +194,28 @@ test('application set in --app is `*.js` and executable', async () => { await execProgram(sdkProvider, config); }); +test('cli throws when the `build` script fails', async () => { + config.settings.set(['build'], 'fake-command'); + const expectedError = 'Subprocess exited with error 127'; + mockSpawn({ + commandLine: ['fake-command'], + exitCode: 127, + }); + + await expect(execProgram(sdkProvider, config)).rejects.toEqual(new Error(expectedError)); +}, TEN_SECOND_TIMEOUT); + +test('cli does not throw when the `build` script succeeds', async () => { + config.settings.set(['build'], 'real-command'); + mockSpawn({ + commandLine: ['real-command'], + exitCode: 0, + }); + + await expect(execProgram(sdkProvider, config)).resolves; +}, TEN_SECOND_TIMEOUT); + + function writeOutputAssembly() { const asm = testAssembly({ stacks: [], From 62b813d2b303dd33bbc403d7635b752abb3d3748 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 26 Oct 2021 17:06:46 -0700 Subject: [PATCH 02/17] updated README --- packages/aws-cdk/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index e4566b7bbb690..29c5479bb4d7b 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -464,6 +464,7 @@ Some of the interesting keys that can be used in the JSON configuration files: ```json5 { "app": "node bin/main.js", // Command to start the CDK app (--app='node bin/main.js') + "build": "./build.sh", // Specify pre-synth build (no command line option) "context": { // Context entries (--context=key=value) "key": "value" }, From dad551a66812768cd226cbf58816e58f87603278 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 26 Oct 2021 18:11:50 -0700 Subject: [PATCH 03/17] addressed PR review --- packages/aws-cdk/README.md | 9 ++++++++- packages/aws-cdk/lib/api/cxapp/exec.ts | 8 +++++--- packages/aws-cdk/test/api/exec.test.ts | 3 +-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index 29c5479bb4d7b..0c9160d14afc9 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -464,7 +464,7 @@ Some of the interesting keys that can be used in the JSON configuration files: ```json5 { "app": "node bin/main.js", // Command to start the CDK app (--app='node bin/main.js') - "build": "./build.sh", // Specify pre-synth build (no command line option) + "build": "mvn package", // Specify pre-synth build (no command line option) "context": { // Context entries (--context=key=value) "key": "value" }, @@ -474,6 +474,13 @@ Some of the interesting keys that can be used in the JSON configuration files: } ``` +If specified, the command in the `build` key will be executed immediately before synthesis. +This can be used to build Lambda Functions, CDK Application code, or other assets. +`build` cannot be specified on the command line, and must be specified in either +the Project configuration or the User configuration. The command specified +in `build` will be executed by the "watch" process before deployment. + + ### Environment The following environment variables affect aws-cdk: diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index 2d8b52595ba49..1a7a5dba1cc6d 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -48,7 +48,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom const build = config.settings.get(['build']); if (build) { - await exec(build); + await exec([build]); } const app = config.settings.get(['app']); @@ -79,7 +79,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom debug('env:', env); - await exec(commandLine[0], commandLine.slice(1)); + await exec(commandLine); return createAssembly(outdir); @@ -96,7 +96,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom } } - async function exec(command: string, args: string[] = []) { + async function exec(commandAndArgs: string[]) { return new Promise((ok, fail) => { // We use a slightly lower-level interface to: // @@ -108,6 +108,8 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom // anyway, and if the subprocess is printing to it for debugging purposes the // user gets to see it sooner. Plus, capturing doesn't interact nicely with some // processes like Maven. + const command = commandAndArgs[0]; + const args = commandAndArgs.slice(1); const proc = childProcess.spawn(command, args, { stdio: ['ignore', 'inherit', 'inherit'], detached: false, diff --git a/packages/aws-cdk/test/api/exec.test.ts b/packages/aws-cdk/test/api/exec.test.ts index f7773242a1d71..c9a7ef687344b 100644 --- a/packages/aws-cdk/test/api/exec.test.ts +++ b/packages/aws-cdk/test/api/exec.test.ts @@ -196,13 +196,12 @@ test('application set in --app is `*.js` and executable', async () => { test('cli throws when the `build` script fails', async () => { config.settings.set(['build'], 'fake-command'); - const expectedError = 'Subprocess exited with error 127'; mockSpawn({ commandLine: ['fake-command'], exitCode: 127, }); - await expect(execProgram(sdkProvider, config)).rejects.toEqual(new Error(expectedError)); + await expect(execProgram(sdkProvider, config)).rejects.toEqual(new Error('Subprocess exited with error 127')); }, TEN_SECOND_TIMEOUT); test('cli does not throw when the `build` script succeeds', async () => { From 52099e1ac9d612012747309fa07adfed03c09c92 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 26 Oct 2021 18:16:27 -0700 Subject: [PATCH 04/17] removed blank line --- packages/aws-cdk/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index 0c9160d14afc9..b02e3f9153af0 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -480,7 +480,6 @@ This can be used to build Lambda Functions, CDK Application code, or other asset the Project configuration or the User configuration. The command specified in `build` will be executed by the "watch" process before deployment. - ### Environment The following environment variables affect aws-cdk: From 697b01660c415581a318e46f7a1041b6d01285f1 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 28 Oct 2021 10:27:00 -0700 Subject: [PATCH 05/17] updated test --- packages/aws-cdk/test/api/exec.test.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk/test/api/exec.test.ts b/packages/aws-cdk/test/api/exec.test.ts index c9a7ef687344b..2f11862704ded 100644 --- a/packages/aws-cdk/test/api/exec.test.ts +++ b/packages/aws-cdk/test/api/exec.test.ts @@ -195,23 +195,32 @@ test('application set in --app is `*.js` and executable', async () => { }); test('cli throws when the `build` script fails', async () => { + // GIVEN config.settings.set(['build'], 'fake-command'); mockSpawn({ commandLine: ['fake-command'], exitCode: 127, }); + // WHEN await expect(execProgram(sdkProvider, config)).rejects.toEqual(new Error('Subprocess exited with error 127')); }, TEN_SECOND_TIMEOUT); test('cli does not throw when the `build` script succeeds', async () => { - config.settings.set(['build'], 'real-command'); + // GIVEN + config.settings.set(['build'], 'real command'); + config.settings.set(['app'], 'executable-app.js'); mockSpawn({ - commandLine: ['real-command'], + commandLine: ['real command'], exitCode: 0, + }, + { + commandLine: ['executable-app.js'], + sideEffect: () => writeOutputAssembly(), }); - await expect(execProgram(sdkProvider, config)).resolves; + // WHEN + await execProgram(sdkProvider, config); }, TEN_SECOND_TIMEOUT); From f9fda40fbc77edc8910878bd1711961190574d08 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 28 Oct 2021 12:05:49 -0700 Subject: [PATCH 06/17] build key cannot be specified in user config --- packages/aws-cdk/lib/settings.ts | 4 ++++ packages/aws-cdk/test/usersettings.test.ts | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/packages/aws-cdk/lib/settings.ts b/packages/aws-cdk/lib/settings.ts index 4bfafd447d607..6182ecc26e0c3 100644 --- a/packages/aws-cdk/lib/settings.ts +++ b/packages/aws-cdk/lib/settings.ts @@ -113,6 +113,10 @@ export class Configuration { const readUserContext = this.props.readUserContext ?? true; + if (userConfig.get(['build'])) { + throw new Error('The `build` key cannot be specified in the user config (~/.cdk.json), specify it in the project config (cdk.json) instead'); + } + const contextSources = [ this.commandLineContext, this.projectConfig.subSettings([CONTEXT_KEY]).makeReadOnly(), diff --git a/packages/aws-cdk/test/usersettings.test.ts b/packages/aws-cdk/test/usersettings.test.ts index 948b3b3f907bc..92d7db4a44025 100644 --- a/packages/aws-cdk/test/usersettings.test.ts +++ b/packages/aws-cdk/test/usersettings.test.ts @@ -69,4 +69,24 @@ test('load context from all 3 files if available', async () => { expect(config.context.get('project')).toBe('foobar'); expect(config.context.get('foo')).toBe('bar'); expect(config.context.get('test')).toBe('bar'); +}); + +test('throws an error if the `build` key is specified in the user config', async () => { + // GIVEN + const GIVEN_CONFIG: Map = new Map([ + [USER_CONFIG, { + build: 'foobar', + }], + ]); + + // WHEN + mockedFs.pathExists.mockImplementation(path => { + return GIVEN_CONFIG.has(path); + }); + mockedFs.readJSON.mockImplementation(path => { + return GIVEN_CONFIG.get(path); + }); + + // THEN + await expect(new Configuration().load()).rejects.toEqual(new Error('The `build` key cannot be specified in the user config (~/.cdk.json), specify it in the project config (cdk.json) instead')); }); \ No newline at end of file From 7263fc5e6836ebbbb74b443d71859fe5fac560f0 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 28 Oct 2021 12:07:11 -0700 Subject: [PATCH 07/17] update readme --- packages/aws-cdk/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index b02e3f9153af0..0a2270b08fc90 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -476,8 +476,8 @@ Some of the interesting keys that can be used in the JSON configuration files: If specified, the command in the `build` key will be executed immediately before synthesis. This can be used to build Lambda Functions, CDK Application code, or other assets. -`build` cannot be specified on the command line, and must be specified in either -the Project configuration or the User configuration. The command specified +`build` cannot be specified on the command line or in the User configuration, +and must be specified in the Project configuration. The command specified in `build` will be executed by the "watch" process before deployment. ### Environment From ec29717a8699d62db094d2adc07c592c0ccace42 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 3 Nov 2021 11:33:43 -0700 Subject: [PATCH 08/17] string is now split on whitespace --- packages/aws-cdk/lib/api/cxapp/exec.ts | 8 ++++---- packages/aws-cdk/test/api/exec.test.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index 1a7a5dba1cc6d..b2cd1075f7817 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -48,7 +48,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom const build = config.settings.get(['build']); if (build) { - await exec([build]); + await exec(commandToArray(build)); } const app = config.settings.get(['app']); @@ -62,7 +62,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom return createAssembly(app); } - const commandLine = await guessExecutable(appToArray(app)); + const commandLine = await guessExecutable(commandToArray(app)); const outdir = config.settings.get(['output']); if (!outdir) { @@ -161,8 +161,8 @@ async function populateDefaultEnvironmentIfNeeded(aws: SdkProvider, env: { [key: * * If it's a string, split on spaces as a trivial way of tokenizing the command line. */ -function appToArray(app: any) { - return typeof app === 'string' ? app.split(' ') : app; +function commandToArray(command: any) { + return typeof command === 'string' ? command.split(' ') : command; } type CommandGenerator = (file: string) => string[]; diff --git a/packages/aws-cdk/test/api/exec.test.ts b/packages/aws-cdk/test/api/exec.test.ts index 2f11862704ded..00ac23f12b1c6 100644 --- a/packages/aws-cdk/test/api/exec.test.ts +++ b/packages/aws-cdk/test/api/exec.test.ts @@ -211,7 +211,7 @@ test('cli does not throw when the `build` script succeeds', async () => { config.settings.set(['build'], 'real command'); config.settings.set(['app'], 'executable-app.js'); mockSpawn({ - commandLine: ['real command'], + commandLine: ['real', 'command'], // `build` key is split on whitespace exitCode: 0, }, { From bee321aaa234ad5f69a430b56d57f161fa8f9b61 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 3 Nov 2021 11:40:28 -0700 Subject: [PATCH 09/17] updated docs --- packages/aws-cdk/lib/api/cxapp/exec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index b2cd1075f7817..b2e35ffa611d4 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -157,7 +157,7 @@ async function populateDefaultEnvironmentIfNeeded(aws: SdkProvider, env: { [key: } /** - * Make sure the 'app' is an array + * Make sure the 'command' is an array * * If it's a string, split on spaces as a trivial way of tokenizing the command line. */ From 55c348aa649dbaa7684c6ea656fe2e479f45c03b Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 3 Nov 2021 14:08:13 -0700 Subject: [PATCH 10/17] reworked exec to take a single string --- packages/aws-cdk/lib/api/cxapp/exec.ts | 37 +++++++++++++++++++------- packages/aws-cdk/test/api/exec.test.ts | 6 ++--- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index b2e35ffa611d4..89c12aeefefee 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -48,7 +48,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom const build = config.settings.get(['build']); if (build) { - await exec(commandToArray(build)); + await exec(build); } const app = config.settings.get(['app']); @@ -62,7 +62,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom return createAssembly(app); } - const commandLine = await guessExecutable(commandToArray(app)); + const commandLine = await guessExecutable(appToArray(app)); const outdir = config.settings.get(['output']); if (!outdir) { @@ -79,7 +79,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom debug('env:', env); - await exec(commandLine); + await exec(arrayToString(commandLine)); return createAssembly(outdir); @@ -96,7 +96,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom } } - async function exec(commandAndArgs: string[]) { + async function exec(commandAndArgs: string) { return new Promise((ok, fail) => { // We use a slightly lower-level interface to: // @@ -108,9 +108,9 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom // anyway, and if the subprocess is printing to it for debugging purposes the // user gets to see it sooner. Plus, capturing doesn't interact nicely with some // processes like Maven. - const command = commandAndArgs[0]; - const args = commandAndArgs.slice(1); - const proc = childProcess.spawn(command, args, { + //const command = commandAndArgs[0]; + //const args = commandAndArgs.slice(1); + const proc = childProcess.spawn(commandAndArgs, [], { stdio: ['ignore', 'inherit', 'inherit'], detached: false, shell: true, @@ -157,12 +157,29 @@ async function populateDefaultEnvironmentIfNeeded(aws: SdkProvider, env: { [key: } /** - * Make sure the 'command' is an array + * Make sure the 'app' is an array * * If it's a string, split on spaces as a trivial way of tokenizing the command line. */ -function commandToArray(command: any) { - return typeof command === 'string' ? command.split(' ') : command; +function appToArray(app: any) { + return typeof app === 'string' ? app.split(' ') : app; +} + +/** + * Turn the given array into a space-delimited string + * + * The first element of the returned string will be a space, ' '. + */ +function arrayToString(arr: any[]) { + let result = ''; + result += arr[0]; + arr.splice(0, 1); + + for (const member of arr) { + result += (' ' + member); + } + + return result; } type CommandGenerator = (file: string) => string[]; diff --git a/packages/aws-cdk/test/api/exec.test.ts b/packages/aws-cdk/test/api/exec.test.ts index 00ac23f12b1c6..6eebaf46fc594 100644 --- a/packages/aws-cdk/test/api/exec.test.ts +++ b/packages/aws-cdk/test/api/exec.test.ts @@ -161,7 +161,7 @@ test('the application set in --app is executed with arguments', async () => { // GIVEN config.settings.set(['app'], 'cloud-executable an-arg'); mockSpawn({ - commandLine: ['cloud-executable', 'an-arg'], + commandLine: ['cloud-executable an-arg'], sideEffect: () => writeOutputAssembly(), }); @@ -174,7 +174,7 @@ test('application set in --app as `*.js` always uses handler on windows', async sinon.stub(process, 'platform').value('win32'); config.settings.set(['app'], 'windows.js'); mockSpawn({ - commandLine: [process.execPath, 'windows.js'], + commandLine: [process.execPath + ' windows.js'], sideEffect: () => writeOutputAssembly(), }); @@ -211,7 +211,7 @@ test('cli does not throw when the `build` script succeeds', async () => { config.settings.set(['build'], 'real command'); config.settings.set(['app'], 'executable-app.js'); mockSpawn({ - commandLine: ['real', 'command'], // `build` key is split on whitespace + commandLine: ['real command'], // `build` key is not split on whitespace exitCode: 0, }, { From 67164821c270eabe1900750866f35718bb318851 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 3 Nov 2021 14:30:45 -0700 Subject: [PATCH 11/17] exec and spawn now deal with a single string, instead of a string[] --- packages/aws-cdk/test/api/exec.test.ts | 16 ++++++++-------- packages/aws-cdk/test/util/mock-child_process.ts | 10 ++++------ 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/aws-cdk/test/api/exec.test.ts b/packages/aws-cdk/test/api/exec.test.ts index 6eebaf46fc594..b9c5637acce1a 100644 --- a/packages/aws-cdk/test/api/exec.test.ts +++ b/packages/aws-cdk/test/api/exec.test.ts @@ -137,7 +137,7 @@ test('the application set in --app is executed', async () => { // GIVEN config.settings.set(['app'], 'cloud-executable'); mockSpawn({ - commandLine: ['cloud-executable'], + commandLine: 'cloud-executable', sideEffect: () => writeOutputAssembly(), }); @@ -149,7 +149,7 @@ test('the application set in --app is executed as-is if it contains a filename t // GIVEN config.settings.set(['app'], 'does-not-exist'); mockSpawn({ - commandLine: ['does-not-exist'], + commandLine: 'does-not-exist', sideEffect: () => writeOutputAssembly(), }); @@ -161,7 +161,7 @@ test('the application set in --app is executed with arguments', async () => { // GIVEN config.settings.set(['app'], 'cloud-executable an-arg'); mockSpawn({ - commandLine: ['cloud-executable an-arg'], + commandLine: 'cloud-executable an-arg', sideEffect: () => writeOutputAssembly(), }); @@ -174,7 +174,7 @@ test('application set in --app as `*.js` always uses handler on windows', async sinon.stub(process, 'platform').value('win32'); config.settings.set(['app'], 'windows.js'); mockSpawn({ - commandLine: [process.execPath + ' windows.js'], + commandLine: process.execPath + ' windows.js', sideEffect: () => writeOutputAssembly(), }); @@ -186,7 +186,7 @@ test('application set in --app is `*.js` and executable', async () => { // GIVEN config.settings.set(['app'], 'executable-app.js'); mockSpawn({ - commandLine: ['executable-app.js'], + commandLine: 'executable-app.js', sideEffect: () => writeOutputAssembly(), }); @@ -198,7 +198,7 @@ test('cli throws when the `build` script fails', async () => { // GIVEN config.settings.set(['build'], 'fake-command'); mockSpawn({ - commandLine: ['fake-command'], + commandLine: 'fake-command', exitCode: 127, }); @@ -211,11 +211,11 @@ test('cli does not throw when the `build` script succeeds', async () => { config.settings.set(['build'], 'real command'); config.settings.set(['app'], 'executable-app.js'); mockSpawn({ - commandLine: ['real command'], // `build` key is not split on whitespace + commandLine: 'real command', // `build` key is not split on whitespace exitCode: 0, }, { - commandLine: ['executable-app.js'], + commandLine: 'executable-app.js', sideEffect: () => writeOutputAssembly(), }); diff --git a/packages/aws-cdk/test/util/mock-child_process.ts b/packages/aws-cdk/test/util/mock-child_process.ts index 539fd06273399..2ebbd1467ee38 100644 --- a/packages/aws-cdk/test/util/mock-child_process.ts +++ b/packages/aws-cdk/test/util/mock-child_process.ts @@ -6,7 +6,7 @@ if (!(child_process as any).spawn.mockImplementationOnce) { } export interface Invocation { - commandLine: string[]; + commandLine: string; cwd?: string; exitCode?: number; stdout?: string; @@ -26,13 +26,11 @@ export function mockSpawn(...invocations: Invocation[]) { let mock = (child_process.spawn as any); for (const _invocation of invocations) { const invocation = _invocation; // Mirror into variable for closure - mock = mock.mockImplementationOnce((binary: string, args: string[], options: child_process.SpawnOptions) => { + mock = mock.mockImplementationOnce((binary: string, options: child_process.SpawnOptions) => { if (invocation.prefix) { - // Match command line prefix - expect([binary, ...args].slice(0, invocation.commandLine.length)).toEqual(invocation.commandLine); + expect([binary, []].slice(0, invocation.commandLine.length)).toEqual(invocation.commandLine); } else { - // Match full command line - expect([binary, ...args]).toEqual(invocation.commandLine); + expect(binary).toEqual(invocation.commandLine); } if (invocation.cwd != null) { From d0691ccce3e2b94c322c198a0ef7fd901801454e Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 3 Nov 2021 14:36:57 -0700 Subject: [PATCH 12/17] removed unused prefix property from interface --- packages/aws-cdk/test/util/mock-child_process.ts | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/aws-cdk/test/util/mock-child_process.ts b/packages/aws-cdk/test/util/mock-child_process.ts index 2ebbd1467ee38..d7645b4c2ce1a 100644 --- a/packages/aws-cdk/test/util/mock-child_process.ts +++ b/packages/aws-cdk/test/util/mock-child_process.ts @@ -11,11 +11,6 @@ export interface Invocation { exitCode?: number; stdout?: string; - /** - * Only match a prefix of the command (don't care about the details of the arguments) - */ - prefix?: boolean; - /** * Run this function as a side effect, if present */ @@ -27,11 +22,7 @@ export function mockSpawn(...invocations: Invocation[]) { for (const _invocation of invocations) { const invocation = _invocation; // Mirror into variable for closure mock = mock.mockImplementationOnce((binary: string, options: child_process.SpawnOptions) => { - if (invocation.prefix) { - expect([binary, []].slice(0, invocation.commandLine.length)).toEqual(invocation.commandLine); - } else { - expect(binary).toEqual(invocation.commandLine); - } + expect(binary).toEqual(invocation.commandLine); if (invocation.cwd != null) { expect(options.cwd).toBe(invocation.cwd); @@ -58,8 +49,8 @@ export function mockSpawn(...invocations: Invocation[]) { }); } - mock.mockImplementation((binary: string, args: string[], _options: any) => { - throw new Error(`Did not expect call of ${JSON.stringify([binary, ...args])}`); + mock.mockImplementation((binary: string, _options: any) => { + throw new Error(`Did not expect call of ${binary}`); }); } From 44d49f83a2f98b1a9606259a5c2e098381b50c98 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 3 Nov 2021 14:49:05 -0700 Subject: [PATCH 13/17] removed commented code --- packages/aws-cdk/lib/api/cxapp/exec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index 89c12aeefefee..609f1232e0606 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -108,8 +108,6 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom // anyway, and if the subprocess is printing to it for debugging purposes the // user gets to see it sooner. Plus, capturing doesn't interact nicely with some // processes like Maven. - //const command = commandAndArgs[0]; - //const args = commandAndArgs.slice(1); const proc = childProcess.spawn(commandAndArgs, [], { stdio: ['ignore', 'inherit', 'inherit'], detached: false, From 3949ae6213d5bcba9596f81fe32c46904edb1772 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 3 Nov 2021 14:52:55 -0700 Subject: [PATCH 14/17] removed uneeded [] --- packages/aws-cdk/lib/api/cxapp/exec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index 609f1232e0606..9a5f8f5933ab5 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -108,7 +108,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom // anyway, and if the subprocess is printing to it for debugging purposes the // user gets to see it sooner. Plus, capturing doesn't interact nicely with some // processes like Maven. - const proc = childProcess.spawn(commandAndArgs, [], { + const proc = childProcess.spawn(commandAndArgs, { stdio: ['ignore', 'inherit', 'inherit'], detached: false, shell: true, From 1be1185eebcad34457d981b47596cf63a02cbaf5 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 3 Nov 2021 14:55:40 -0700 Subject: [PATCH 15/17] updated arrayToString() to be cleaner --- packages/aws-cdk/lib/api/cxapp/exec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index 9a5f8f5933ab5..5793d434ae7ed 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -169,8 +169,8 @@ function appToArray(app: any) { * The first element of the returned string will be a space, ' '. */ function arrayToString(arr: any[]) { - let result = ''; - result += arr[0]; + // add the first element outside the loop, to prevent adding whitespace in front of the first character of the command + let result = arr[0]; arr.splice(0, 1); for (const member of arr) { From d0c0be650f888eaf068005fce24ada98412ea83b Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 3 Nov 2021 14:57:57 -0700 Subject: [PATCH 16/17] added comment to arrayToString() --- packages/aws-cdk/lib/api/cxapp/exec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index 5793d434ae7ed..b3b895bcc2357 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -171,6 +171,7 @@ function appToArray(app: any) { function arrayToString(arr: any[]) { // add the first element outside the loop, to prevent adding whitespace in front of the first character of the command let result = arr[0]; + // remove the first element, to not add it twice arr.splice(0, 1); for (const member of arr) { From eef80b9095016a39dc9e0c375a77d0ddc9881222 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 3 Nov 2021 15:28:17 -0700 Subject: [PATCH 17/17] removed unndeeded function, added Array.join() --- packages/aws-cdk/lib/api/cxapp/exec.ts | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index b3b895bcc2357..bcc298deaeea2 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -79,7 +79,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom debug('env:', env); - await exec(arrayToString(commandLine)); + await exec(commandLine.join(' ')); return createAssembly(outdir); @@ -163,24 +163,6 @@ function appToArray(app: any) { return typeof app === 'string' ? app.split(' ') : app; } -/** - * Turn the given array into a space-delimited string - * - * The first element of the returned string will be a space, ' '. - */ -function arrayToString(arr: any[]) { - // add the first element outside the loop, to prevent adding whitespace in front of the first character of the command - let result = arr[0]; - // remove the first element, to not add it twice - arr.splice(0, 1); - - for (const member of arr) { - result += (' ' + member); - } - - return result; -} - type CommandGenerator = (file: string) => string[]; /**