fix(cli): --app command does not work when executing a command without arguments (#7249)

If the application passed with the `--app` parameter is executable, we were failing on a `fs.stat` call as it was not a file that was provided. This change swallows up that error and returns the provided command without modification.

Also added some tests (thank you rix0rrr@ for getting this started) and some other minor improvements.

Closes #6930

Author: shivlaks

packages/aws-cdk/lib/api/cxapp/exec.ts

@@ -14,37 +14,26 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom
   const context = config.context.all;
   await populateDefaultEnvironmentIfNeeded(aws, env);
 
-  let pathMetadata: boolean = config.settings.get(['pathMetadata']);
-  if (pathMetadata === undefined) {
-    pathMetadata = true; // defaults to true
-  }
+  const pathMetadata: boolean = config.settings.get(['pathMetadata']) ?? true;
 
   if (pathMetadata) {
     context[cxapi.PATH_METADATA_ENABLE_CONTEXT] = true;
   }
 
-  let assetMetadata: boolean = config.settings.get(['assetMetadata']);
-  if (assetMetadata === undefined) {
-    assetMetadata = true; // defaults to true
-  }
+  const assetMetadata: boolean = config.settings.get(['assetMetadata']) ?? true;
 
   if (assetMetadata) {
     context[cxapi.ASSET_RESOURCE_METADATA_ENABLED_CONTEXT] = true;
   }
 
-  let versionReporting: boolean = config.settings.get(['versionReporting']);
-  if (versionReporting === undefined) {
-    versionReporting = true; // defaults to true
-  }
+  const versionReporting: boolean = config.settings.get(['versionReporting']) ?? true;
 
   if (!versionReporting) {
     context[cxapi.DISABLE_VERSION_REPORTING] = true;
   }
 
-  let stagingEnabled = config.settings.get(['staging']);
-  if (stagingEnabled === undefined) {
-    stagingEnabled = true;
-  }
+  const stagingEnabled = config.settings.get(['staging']) ?? true;
+
   if (!stagingEnabled) {
     context[cxapi.DISABLE_ASSET_STAGING_CONTEXT] = true;
   }
@@ -57,9 +46,9 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom
     throw new Error(`--app is required either in command-line, in ${PROJECT_CONFIG} or in ${USER_DEFAULTS}`);
   }
 
-  // by pass "synth" if app points to a cloud assembly
+  // bypass "synth" if app points to a cloud assembly
   if (await fs.pathExists(app) && (await fs.stat(app)).isDirectory()) {
-    debug('--app points to a cloud assembly, so we by pass synth');
+    debug('--app points to a cloud assembly, so we bypass synth');
     return new cxapi.CloudAssembly(app);
   }
 
@@ -93,7 +82,7 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom
       //   (which would be different between Linux and Windows).
       //
       // - Inherit stderr from controlling terminal. We don't use the captured value
-      //   anway, and if the subprocess is printing to it for debugging purposes the
+      //   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), {
@@ -121,10 +110,10 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom
 
 /**
  * If we don't have region/account defined in context, we fall back to the default SDK behavior
- * where region is retreived from ~/.aws/config and account is based on default credentials provider
+ * where region is retrieved from ~/.aws/config and account is based on default credentials provider
  * chain and then STS is queried.
  *
- * This is done opportunistically: for example, if we can't acccess STS for some reason or the region
+ * This is done opportunistically: for example, if we can't access STS for some reason or the region
  * is not configured, the context value will be 'null' and there could failures down the line. In
  * some cases, synthesis does not require region/account information at all, so that might be perfectly
  * fine in certain scenarios.
@@ -179,7 +168,15 @@ const EXTENSION_MAP = new Map<string, CommandGenerator>([
  */
 async function guessExecutable(commandLine: string[]) {
   if (commandLine.length === 1) {
-    const fstat = await fs.stat(commandLine[0]);
+    let fstat;
+
+    try {
+      fstat = await fs.stat(commandLine[0]);
+    } catch (error) {
+      debug(`Not a file: '${commandLine[0]}'. Using '${commandLine}' as command-line`);
+      return commandLine;
+    }
+
     // tslint:disable-next-line:no-bitwise
     const isExecutable = (fstat.mode & fs.constants.X_OK) !== 0;
     const isWindows = process.platform === 'win32';

packages/aws-cdk/test/api/exec.test.ts

@@ -0,0 +1,122 @@
+jest.mock('child_process');
+import * as sinon from 'sinon';
+import { execProgram } from '../../lib/api/cxapp/exec';
+import { setVerbose } from '../../lib/logging';
+import { Configuration } from '../../lib/settings';
+import * as bockfs from '../bockfs';
+import { testAssembly } from '../util';
+import { mockSpawn } from '../util/mock-child_process';
+import { MockSdkProvider } from '../util/mock-sdk';
+
+setVerbose(true);
+
+let sdkProvider: MockSdkProvider;
+let config: Configuration;
+beforeEach(() => {
+  sdkProvider = new MockSdkProvider();
+  config = new Configuration();
+
+  config.settings.set(['output'], 'cdk.out');
+
+  // insert contents in fake filesystem
+  bockfs({
+    '/home/project/cloud-executable': 'ARBITRARY',
+    '/home/project/windows.js': 'ARBITRARY',
+    'home/project/executable-app.js': 'ARBITRARY'
+  });
+  bockfs.workingDirectory('/home/project');
+  bockfs.executable('/home/project/cloud-executable');
+  bockfs.executable('/home/project/executable-app.js');
+});
+
+afterEach(() => {
+  sinon.restore();
+  bockfs.restore();
+});
+
+test('validates --app key is present', async () => {
+  // GIVEN no config key for `app`
+  await expect(execProgram(sdkProvider, config)).rejects.toThrow(
+    '--app is required either in command-line, in cdk.json or in ~/.cdk.json'
+  );
+
+});
+
+test('bypasses synth when app points to a cloud assembly', async () => {
+  // GIVEN
+  config.settings.set(['app'], 'cdk.out');
+  writeOutputAssembly();
+
+  // WHEN
+  const cloudAssembly = await execProgram(sdkProvider, config);
+  expect(cloudAssembly.artifacts).toEqual([]);
+  expect(cloudAssembly.directory).toEqual('cdk.out');
+});
+
+test('the application set in --app is executed', async () => {
+  // GIVEN
+  config.settings.set(['app'], 'cloud-executable');
+  mockSpawn({
+    commandLine: ['cloud-executable'],
+    sideEffect: () => writeOutputAssembly(),
+  });
+
+  // WHEN
+  await execProgram(sdkProvider, config);
+});
+
+test('the application set in --app is executed as-is if it contains a filename that does not exist', async () => {
+  // GIVEN
+  config.settings.set(['app'], 'does-not-exist');
+  mockSpawn({
+    commandLine: ['does-not-exist'],
+    sideEffect: () => writeOutputAssembly(),
+  });
+
+  // WHEN
+  await execProgram(sdkProvider, config);
+});
+
+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'],
+    sideEffect: () => writeOutputAssembly(),
+  });
+
+  // WHEN
+  await execProgram(sdkProvider, config);
+});
+
+test('application set in --app as `*.js` always uses handler on windows', async () => {
+  // GIVEN
+  sinon.stub(process, 'platform').value('win32');
+  config.settings.set(['app'], 'windows.js');
+  mockSpawn({
+    commandLine: [process.execPath, 'windows.js'],
+    sideEffect: () => writeOutputAssembly(),
+  });
+
+  // WHEN
+  await execProgram(sdkProvider, config);
+});
+
+test('application set in --app is `*.js` and executable', async () => {
+  // GIVEN
+  config.settings.set(['app'], 'executable-app.js');
+  mockSpawn({
+    commandLine: ['executable-app.js'],
+    sideEffect: () => writeOutputAssembly(),
+  });
+
+  // WHEN
+  await execProgram(sdkProvider, config);
+});
+
+function writeOutputAssembly() {
+  const asm = testAssembly({
+    stacks: []
+  });
+  bockfs.write('/home/project/cdk.out/manifest.json', JSON.stringify(asm));
+}

packages/aws-cdk/test/bockfs.ts

@@ -13,26 +13,53 @@ import * as os from 'os';
 import * as path_ from 'path';
 
 const bockFsRoot = path_.join(os.tmpdir(), 'bockfs');
+let oldCwd: string | undefined;
 
 function bockfs(files: Record<string, string>) {
+  oldCwd = process.cwd();
   for (const [fileName, contents] of Object.entries(files)) {
     bockfs.write(fileName, contents);
   }
 }
 
 namespace bockfs {
-  export function write(fileName: string, contents: string) {
-    const fullPath = path(fileName);
+  /**
+   * Write contents to a fake file
+   */
+  export function write(fakeFilename: string, contents: string) {
+    const fullPath = path(fakeFilename);
     fs.mkdirSync(path_.dirname(fullPath), { recursive: true });
     fs.writeFileSync(fullPath, contents, { encoding: 'utf-8' });
   }
 
-  export function path(x: string) {
-    if (x.startsWith('/')) { x = x.substr(1); } // Force path to be non-absolute
-    return path_.join(bockFsRoot, x);
+  /**
+   * Turn a fake path into a real path
+   */
+  export function path(fakePath: string) {
+    if (fakePath.startsWith('/')) { fakePath = fakePath.substr(1); } // Force path to be non-absolute
+    return path_.join(bockFsRoot, fakePath);
   }
 
+  /**
+   * Change to a fake directory
+   */
+  export function workingDirectory(fakePath: string) {
+    process.chdir(path(fakePath));
+  }
+
+  export function executable(...fakePaths: string[]) {
+    for (const fakepath of fakePaths) {
+      fs.chmodSync(path(fakepath), '755');
+    }
+  }
+
+  /**
+   * Remove all files and restore working directory
+   */
   export function restore() {
+    if (oldCwd) {
+      process.chdir(oldCwd);
+    }
     fs.removeSync(bockFsRoot);
   }
 }

packages/aws-cdk/test/util.ts

@@ -38,7 +38,7 @@ export class MockCloudExecutable extends CloudExecutable {
   }
 }
 
-function testAssembly(assembly: TestAssembly): cxapi.CloudAssembly {
+export function testAssembly(assembly: TestAssembly): cxapi.CloudAssembly {
   const builder = new cxapi.CloudAssemblyBuilder();
 
   for (const stack of assembly.stacks) {

packages/aws-cdk/test/util/mock-child_process.ts

@@ -0,0 +1,75 @@
+import * as child_process from 'child_process';
+import * as events from 'events';
+
+if (!(child_process as any).spawn.mockImplementationOnce) {
+  throw new Error('Call "jest.mock(\'child_process\');" at the top of the test file!');
+}
+
+export interface Invocation {
+  commandLine: string[];
+  cwd?: string;
+  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
+   */
+  sideEffect?: () => void;
+}
+
+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) => {
+      if (invocation.prefix) {
+        // Match command line prefix
+        expect([binary, ...args].slice(0, invocation.commandLine.length)).toEqual(invocation.commandLine);
+      } else {
+        // Match full command line
+        expect([binary, ...args]).toEqual(invocation.commandLine);
+      }
+
+      if (invocation.cwd != null) {
+        expect(options.cwd).toBe(invocation.cwd);
+      }
+
+      if (invocation.sideEffect) {
+        invocation.sideEffect();
+      }
+
+      const child: any = new events.EventEmitter();
+      child.stdin = new events.EventEmitter();
+      child.stdin.write = jest.fn();
+      child.stdin.end = jest.fn();
+      child.stdout = new events.EventEmitter();
+      child.stderr = new events.EventEmitter();
+
+      if (invocation.stdout) {
+        mockEmit(child.stdout, 'data', invocation.stdout);
+      }
+      mockEmit(child, 'close', invocation.exitCode ?? 0);
+      mockEmit(child, 'exit', invocation.exitCode ?? 0);
+
+      return child;
+    });
+  }
+
+  mock.mockImplementation((binary: string, args: string[], _options: any) => {
+    throw new Error(`Did not expect call of ${JSON.stringify([binary, ...args])}`);
+  });
+}
+
+/**
+ * Must do this on the next tick, as emitter.emit() expects all listeners to have been attached already
+ */
+function mockEmit(emitter: events.EventEmitter, event: string, data: any) {
+  setImmediate(() => {
+    emitter.emit(event, data);
+  });
+}