Handle return from yargs command modules

Prevent the application from reaching the code path in `main` when the
command was handled directly by a `commandDir` module. This prevents
the operation from succeeding, while displaying a message such as
*"Unknown command: docs"* (#256).

In order to achieve this, the module decorator sets `argv.result` on
successful execution of the command module's handler; and if
`argv.result` is set, `main` does not get called.

Fixes #256

CHANGELOG.md

@@ -1,3 +1,7 @@
+## [UNRELEASED]
+
+* [FIXED] `cdk docs` works but a message __Unknown command: docs__ is printed ([#256])
+
 ## 0.7.3 - 2018-07-09
 
 ### Highlights

packages/aws-cdk/bin/cdk.ts

@@ -93,7 +93,7 @@ function decorateCommand(commandObject: yargs.CommandModule): yargs.CommandModul
             if (args.verbose) {
                 setVerbose();
             }
-            return commandObject.handler(args);
+            return args.result = commandObject.handler(args);
         }
     };
 }
@@ -151,7 +151,7 @@ async function initCommandLine() {
 
     const cmd = argv._[0];
 
-    const returnValue = await main(cmd, argv);
+    const returnValue = await (argv.result || main(cmd, argv));
     if (typeof returnValue === 'object') {
         return toJsonOrYaml(returnValue);
     } else if (typeof returnValue === 'string') {

packages/aws-cdk/lib/commands/docs.ts

@@ -20,26 +20,25 @@ export interface Arguments extends yargs.Arguments {
     browser: string
 }
 
-export async function handler(argv: Arguments) {
+export async function handler(argv: Arguments): Promise<number> {
     let documentationIndexPath: string;
     try {
         // tslint:disable-next-line:no-var-require Taking an un-declared dep on aws-cdk-docs, to avoid a dependency circle
         const docs = require('aws-cdk-docs');
         documentationIndexPath = docs.documentationIndexPath;
     } catch (err) {
         error('Unable to open CDK documentation - the aws-cdk-docs package appears to be missing. Please run `npm install -g aws-cdk-docs`');
-        process.exit(-1);
-        return;
+        return -1;
     }
 
     const browserCommand = argv.browser.replace(/%u/g, documentationIndexPath);
     debug(`Opening documentation ${green(browserCommand)}`);
-    process.exit(await new Promise<number>((resolve, reject) => {
+    return await new Promise<number>((resolve, reject) => {
         exec(browserCommand, (err, stdout, stderr) => {
             if (err) { return reject(err); }
             if (stdout) { debug(stdout); }
             if (stderr) { warning(stderr); }
             resolve(0);
         });
-    }));
+    });
 }

packages/aws-cdk/lib/commands/doctor.ts

@@ -7,14 +7,14 @@ export const command = 'doctor';
 export const describe = 'Check your set-up for potential problems';
 export const builder = {};
 
-export async function handler(): Promise<never> {
+export async function handler(): Promise<number> {
     let exitStatus: number = 0;
     for (const verification of verifications) {
         if (!await verification()) {
             exitStatus = -1;
         }
     }
-    return process.exit(exitStatus);
+    return exitStatus;
 }
 
 const verifications: Array<() => boolean | Promise<boolean>> = [

packages/aws-cdk/test/test.cdk-docs.ts

@@ -1,20 +1,11 @@
 import * as mockery from 'mockery';
 import { ICallbackFunction, Test, testCase } from 'nodeunit';
 
-let exitCalled: boolean = false;
-let exitStatus: undefined | number;
-function fakeExit(status?: number) {
-    exitCalled = true;
-    exitStatus = status;
-}
-
 const argv = { browser: 'echo %u' };
 
 module.exports = testCase({
     '`cdk docs`': {
         'setUp'(cb: ICallbackFunction) {
-            exitCalled = false;
-            exitStatus = undefined;
             mockery.registerMock('../../lib/logging', {
                 debug() { return; },
                 error() { return; },
@@ -26,32 +17,25 @@ module.exports = testCase({
         'tearDown'(cb: ICallbackFunction) {
             mockery.disable();
             mockery.deregisterAll();
-
             cb();
         },
         async 'exits with 0 when everything is OK'(test: Test) {
-            mockery.registerMock('process', { ...process, exit: fakeExit });
             mockery.registerMock('aws-cdk-docs', { documentationIndexPath: 'index.html' });
-
             try {
-                await require('../lib/commands/docs').handler(argv);
-                test.ok(exitCalled, 'process.exit() was called');
-                test.equal(exitStatus, 0, 'exit status was 0');
+                const result = await require('../lib/commands/docs').handler(argv);
+                test.equal(result, 0, 'exit status was 0');
             } catch (e) {
-                test.ifError(e);
+                test.doesNotThrow(() => { throw e; });
             } finally {
                 test.done();
             }
         },
         async 'exits with non-0 when documentation is missing'(test: Test) {
-            mockery.registerMock('process', { ...process, exit: fakeExit });
-
             try {
-                await require('../lib/commands/docs').handler(argv);
-                test.ok(exitCalled, 'process.exit() was called');
-                test.notEqual(exitStatus, 0, 'exit status was non-0');
+                const result = await require('../lib/commands/docs').handler(argv);
+                test.notEqual(result, 0, 'exit status was non-0');
             } catch (e) {
-                test.ifError(e);
+                test.doesNotThrow(() => { throw e; });
             } finally {
                 test.done();
             }

packages/aws-cdk/test/test.cdk-doctor.ts

@@ -1,18 +1,9 @@
 import * as mockery from 'mockery';
 import { ICallbackFunction, Test, testCase } from 'nodeunit';
 
-let exitCalled: boolean = false;
-let exitStatus: undefined | number;
-function fakeExit(status?: number) {
-    exitCalled = true;
-    exitStatus = status;
-}
-
 module.exports = testCase({
     '`cdk doctor`': {
         'setUp'(cb: ICallbackFunction) {
-            exitCalled = false;
-            exitStatus = undefined;
             mockery.registerMock('../../lib/logging', {
                 print: () => undefined
             });
@@ -22,32 +13,26 @@ module.exports = testCase({
         'tearDown'(cb: ICallbackFunction) {
             mockery.disable();
             mockery.deregisterAll();
-
             cb();
         },
         async 'exits with 0 when everything is OK'(test: Test) {
-            mockery.registerMock('process', { ...process, exit: fakeExit });
             mockery.registerMock('aws-cdk-docs/package.json', { version: 'x.y.z' });
 
             try {
-                await require('../lib/commands/doctor').handler();
-                test.ok(exitCalled, 'process.exit() was called');
-                test.equal(exitStatus, 0, 'exit status was 0');
+                const result = await require('../lib/commands/doctor').handler();
+                test.equal(result, 0, 'exit status was 0');
             } catch (e) {
-                test.ifError(e);
+                test.doesNotThrow(() => e);
             } finally {
                 test.done();
             }
         },
         async 'exits with non-0 when documentation is missing'(test: Test) {
-            mockery.registerMock('process', { ...process, exit: fakeExit });
-
             try {
-                await require('../lib/commands/doctor').handler();
-                test.ok(exitCalled, 'process.exit() was called');
-                test.notEqual(exitStatus, 0, 'exit status was non-0');
+                const result = await require('../lib/commands/doctor').handler();
+                test.notEqual(result, 0, 'exit status was non-0');
             } catch (e) {
-                test.ifError(e);
+                test.doesNotThrow(() => e);
             } finally {
                 test.done();
             }