Merge pull request #153 from Microsoft/commandRefactor

Command refactor

src/common/commandExecutor.ts

@@ -28,7 +28,7 @@ export class CommandExecutor {
     private currentWorkingDirectory: string;
 
     constructor(currentWorkingDirectory?: string) {
-        this.currentWorkingDirectory = currentWorkingDirectory;
+        this.currentWorkingDirectory = currentWorkingDirectory || process.cwd();
     }
 
     public execute(command: string, options: Options = {}): Q.Promise<void> {
@@ -42,17 +42,6 @@ export class CommandExecutor {
                 this.generateRejectionForCommand(command, reason));
     }
 
-    /**
-     * Spawns a child process with the params passed and returns promise of the spawned ChildProcess
-     * This method does not wait for the spawned process to finish execution
-     * {command} - The command to be invoked in the child process
-     * {args} - Arguments to be passed to the command
-     * {options} - additional options with which the child process needs to be spawned
-     */
-    public spawn(command: string, args: string[], options: Options = {}): ChildProcess {
-        return this.spawnChildProcess(command, args, options).spawnedProcess;
-    }
-
     /**
      * Spawns a child process with the params passed
      * This method waits until the spawned process finishes execution
@@ -60,15 +49,14 @@ export class CommandExecutor {
      * {args} - Arguments to be passed to the command
      * {options} - additional options with which the child process needs to be spawned
      */
-    public spawnAndWaitForCompletion(command: string, args: string[], options: Options = {}): Q.Promise<void> {
-        return this.spawnChildProcess(command, args, options).outcome;
+    public spawn(command: string, args: string[], options: Options = {}): Q.Promise<any> {
+        return this.spawnChildProcess(command, args, true, options).outcome;
     }
 
     /**
      * Spawns the React Native packager in a child process.
      */
-    public spawnReactPackager(args?: string[], options: Options = {}): Q.Promise<ChildProcess> {
-        let deferred = Q.defer<ChildProcess>();
+    public spawnReactPackager(args?: string[], options: Options = {}): Q.Promise<ISpawnResult> {
         let command = HostPlatform.getNpmCliCommand(CommandExecutor.ReactNativeCommand);
         let runArguments = ["start"];
 
@@ -77,23 +65,8 @@ export class CommandExecutor {
         }
 
         let spawnOptions = Object.assign({}, { cwd: this.currentWorkingDirectory }, options);
-
-        let result = new Node.ChildProcess().spawn(command, runArguments, spawnOptions);
-        result.spawnedProcess.once("error", (error: any) => {
-            deferred.reject(ErrorHelper.getNestedError(error, InternalErrorCode.PackagerStartFailed));
-        });
-
-        result.stderr.on("data", (data: Buffer) => {
-            Log.logStreamData(data, process.stderr);
-        });
-
-        result.stdout.on("data", (data: Buffer) => {
-            Log.logStreamData(data, process.stdout);
-        });
-
-        // TODO #83 - PROMISE: We need to consume result.outcome here
-        Q.delay(300).done(() => deferred.resolve(result.spawnedProcess));
-        return deferred.promise;
+        let result = this.spawnChildProcess(command, runArguments, false, spawnOptions);
+        return Q.resolve(result);
     }
 
     /**
@@ -122,26 +95,27 @@ export class CommandExecutor {
     /**
      * Executes a react native command and waits for its completion.
      */
-    public spawnAndWaitReactCommand(command: string, args?: string[], options: Options = {}): Q.Promise<void> {
-        return this.spawnChildReactCommandProcess(command, args, options).outcome;
+    public spawnReactCommand(command: string, args?: string[], waitForExit: boolean = true, options: Options = {}): ISpawnResult {
+        return this.spawnChildReactCommandProcess(command, args, waitForExit, options);
     }
 
-    public spawnChildReactCommandProcess(command: string, args?: string[], options: Options = {}): ISpawnResult {
+    public spawnChildReactCommandProcess(command: string, args?: string[], waitForExit: boolean = true, options: Options = {}): ISpawnResult {
         let runArguments = [command];
         if (args) {
             runArguments = runArguments.concat(args);
         }
 
         let reactCommand = HostPlatform.getNpmCliCommand(CommandExecutor.ReactNativeCommand);
-        return this.spawnChildProcess(reactCommand, runArguments, options);
+        return this.spawnChildProcess(reactCommand, runArguments, waitForExit, options);
     }
 
-    private spawnChildProcess(command: string, args: string[], options: Options = {}): ISpawnResult {
+    private spawnChildProcess(command: string, args: string[], waitForExit: boolean = true, options: Options = {}): ISpawnResult {
         let spawnOptions = Object.assign({}, { cwd: this.currentWorkingDirectory }, options);
         let commandWithArgs = command + " " + args.join(" ");
+        let childProcessHelper = new Node.ChildProcess();
 
         Log.logCommandStatus(commandWithArgs, CommandStatus.Start);
-        let result = new Node.ChildProcess().spawnWithExitHandler(command, args, spawnOptions);
+        let result = waitForExit ? childProcessHelper.spawnWaitUntilFinished(command, args, spawnOptions) : childProcessHelper.spawnWaitUntilStarted(command, args, spawnOptions);
 
         result.stderr.on("data", (data: Buffer) => {
             Log.logStreamData(data, process.stderr);
@@ -156,7 +130,6 @@ export class CommandExecutor {
                 Log.logCommandStatus(commandWithArgs, CommandStatus.End),
             reason =>
                 this.generateRejectionForCommand(commandWithArgs, reason));
-
         return result;
     }
 

src/common/node/childProcess.ts

@@ -37,6 +37,7 @@ interface ISpawnOptions {
 }
 
 export class ChildProcess {
+    public static ERROR_TIMEOUT_MILLISECONDS = 300;
     public exec(command: string, options: IExecOptions = {}): IExecResult {
         let outcome = Q.defer<Buffer>();
 
@@ -55,7 +56,25 @@ export class ChildProcess {
         return this.exec(command, options).outcome.then(stdout => stdout.toString());
     }
 
-    public spawnWithExitHandler(command: string, args?: string[], options: ISpawnOptions = {}): ISpawnResult {
+    public spawnWaitUntilStarted(command: string, args: string[] = [], options: ISpawnOptions = {}): ISpawnResult {
+        let outcome = Q.defer<void>();
+        let spawnedProcess = child_process.spawn(command, args, options);
+        spawnedProcess.once("error", (error: any) => {
+            outcome.reject(error);
+        });
+
+        Q.delay(ChildProcess.ERROR_TIMEOUT_MILLISECONDS).done(() => outcome.resolve(void 0));
+
+        return {
+              spawnedProcess: spawnedProcess,
+              stdin: spawnedProcess.stdin,
+              stdout: spawnedProcess.stdout,
+              stderr: spawnedProcess.stderr,
+              outcome: outcome.promise
+       };
+    }
+
+    public spawnWaitUntilFinished(command: string, args: string[] = [], options: ISpawnOptions = {}): ISpawnResult {
         let outcome = Q.defer<void>();
         let commandWithArgs = command + " " + args.join(" ");
 
@@ -63,36 +82,21 @@ export class ChildProcess {
         spawnedProcess.once("error", (error: any) => {
             outcome.reject(error);
         });
+
         spawnedProcess.once("exit", (code: number) => {
             if (code === 0) {
                 outcome.resolve(void 0);
             } else {
-                outcome.reject(ErrorHelper.getInternalError(InternalErrorCode.CommandFailedWithErrorCode, commandWithArgs, code));
+                outcome.reject(ErrorHelper.getInternalError(InternalErrorCode.CommandFailed, commandWithArgs, code));
             }
         });
 
-        return {
-              spawnedProcess: spawnedProcess,
-              stdin: spawnedProcess.stdin,
-              stdout: spawnedProcess.stdout,
-              stderr: spawnedProcess.stderr,
-              outcome: outcome.promise };
-    }
-
-    public spawn(command: string, args?: string[], options: ISpawnOptions = {}): ISpawnResult {
-        let outcome = Q.defer<void>();
-        let commandWithArgs = command + " " + args.join(" ");
-        let spawnedProcess = child_process.spawn(command, args, options);
-        spawnedProcess.once("error", (error: any) => {
-            outcome.reject(ErrorHelper.getNestedError(error, InternalErrorCode.CommandFailed, commandWithArgs));
-        });
-
         return {
               spawnedProcess: spawnedProcess,
               stdin: spawnedProcess.stdin,
               stdout: spawnedProcess.stdout,
               stderr: spawnedProcess.stderr,
               outcome: outcome.promise
-        };
+       };
     }
 }

src/common/packager.ts

@@ -50,10 +50,13 @@ export class Packager {
 
                             let spawnOptions = { env: childEnvForDebugging };
 
-                            // TODO #83 - PROMISE: We need to consume the result of this spawn
-                            new CommandExecutor(this.projectPath).spawnReactPackager(args, spawnOptions).then((packagerProcess) => {
-                                this.packagerProcess = packagerProcess;
-                                executedStartPackagerCmd = true;
+                            return new CommandExecutor(this.projectPath).spawnReactPackager(args, spawnOptions).then((packagerSpawnResult) => {
+                                return packagerSpawnResult.outcome.then(() => {
+                                    this.packagerProcess = packagerSpawnResult.spawnedProcess;
+                                    executedStartPackagerCmd = true;
+                                }).done(() => { }, reason => {
+                                    return Q.reject(reason);
+                                });
                             });
                         });
                 }

src/debugger/ios/compiler.ts

@@ -16,7 +16,7 @@ export class Compiler {
 
     public compile(): Q.Promise<void> {
         return this.xcodeBuildArguments().then((xcodeArguments: string[]) => {
-            return new CommandExecutor(this.projectRoot).spawnAndWaitForCompletion("xcodebuild", xcodeArguments);
+            return new CommandExecutor(this.projectRoot).spawn("xcodebuild", xcodeArguments);
         });
     }
 

src/debugger/ios/deviceDeployer.ts

@@ -22,7 +22,7 @@ export class DeviceDeployer {
             const pathToCompiledApp = path.join(this.projectRoot, "ios", "build",
                 "Build", "Products", "Debug-iphoneos", `${projectName}.app`);
             return new CommandExecutor(this.projectRoot)
-                .spawnAndWaitForCompletion("ideviceinstaller", ["-i", pathToCompiledApp]).catch((err) => {
+                .spawn("ideviceinstaller", ["-i", pathToCompiledApp]).catch((err) => {
                     if ((<any>err).code === "ENOENT") {
                         throw ErrorHelper.getNestedError(err, InternalErrorCode.IDeviceInstallerNotFound);
                     }

src/debugger/ios/deviceRunner.ts

@@ -148,23 +148,15 @@ export class DeviceRunner {
     private startNativeDebugProxy(proxyPort: number): Q.Promise<void> {
         this.cleanup();
 
-        return this.mountDeveloperImage().then(function(): Q.Promise<void> {
-            const {spawnedProcess} = new Node.ChildProcess().spawnWithExitHandler("idevicedebugserverproxy", [proxyPort.toString()]);
-            this.nativeDebuggerProxyInstance = spawnedProcess;
-            const deferred = Q.defer<ChildProcess>();
-
-            spawnedProcess.on("error", (err: Error) => {
-                deferred.reject(err);
-            });
-
-            // Allow 200ms for the spawn to error out
-            return Q.delay(200);
+        return this.mountDeveloperImage().then(function(): Q.Promise<any> {
+            let result = new Node.ChildProcess().spawnWaitUntilStarted("idevicedebugserverproxy",  [proxyPort.toString()]);
+            return result.outcome.then(() => this.nativeDebuggerProxyInstance = result.spawnedProcess);
         });
     }
 
     private mountDeveloperImage(): Q.Promise<void> {
         return this.getDiskImage().then(function(path: string): Q.Promise<void> {
-            const imagemounter = new Node.ChildProcess().spawnWithExitHandler("ideviceimagemounter", [path]).spawnedProcess;
+            const imagemounter = new Node.ChildProcess().spawnWaitUntilFinished("ideviceimagemounter", [path]).spawnedProcess;
             const deferred = Q.defer<void>();
             let stdout: string = "";
             imagemounter.stdout.on("data", function(data: any): void {
@@ -207,7 +199,7 @@ export class DeviceRunner {
 
         // Attempt to find the developer disk image for the appropriate
         return Q.all([versionInfo, pathInfo]).spread<string>(function(version: string, sdkpath: string): Q.Promise<string> {
-            const find = nodeChildProcess.spawn("find", [sdkpath, "-path", "*" + version + "*", "-name", "DeveloperDiskImage.dmg"]).spawnedProcess;
+            const find = nodeChildProcess.spawnWaitUntilFinished("find", [sdkpath, "-path", "*" + version + "*", "-name", "DeveloperDiskImage.dmg"]).spawnedProcess;
             const deferred = Q.defer<string>();
 
             find.stdout.on("data", function(data: any): void {

src/extension/android/logCatMonitor.ts

@@ -37,7 +37,7 @@ export class LogCatMonitor implements vscode.Disposable {
         const adbParameters = ["-s", this._deviceId, "logcat"].concat(logCatArguments);
         this._logger.logMessage(`Monitoring LogCat for device ${this._deviceId} with arguments: ${logCatArguments}`);
 
-        this._logCatSpawn = new ChildProcess().spawnWithExitHandler("adb", adbParameters);
+        this._logCatSpawn = new ChildProcess().spawnWaitUntilFinished("adb", adbParameters);
 
         /* LogCat has a buffer and prints old messages when first called. To ignore them,
             we won't print messages for the first 0.5 seconds */

src/extension/commandPaletteHandler.ts

@@ -96,7 +96,7 @@ export class CommandPaletteHandler {
 
         return this.reactNativePackager.start()
             .then(() => {
-                return new CommandExecutor(this.workspaceRoot).spawnAndWaitReactCommand(command, args, null);
+                return new CommandExecutor(this.workspaceRoot).spawnReactCommand(command, args, null);
             }).then(() => {
                 return Q.resolve<void>(void 0);
             });

src/extension/intellisenseHelper.ts

@@ -112,7 +112,7 @@ export class IntellisenseHelper {
                     runArguments.push("--prefix " + installPath);
                     runArguments.push("typescript@" + IntellisenseHelper.s_typeScriptVersion);
 
-                    return new CommandExecutor(installPath).spawnAndWaitForCompletion(npmCommand, runArguments)
+                    return new CommandExecutor(installPath).spawn(npmCommand, runArguments)
                         .then(() => {
                             return true;
                         })

src/test/common/commandExecutor.test.ts

@@ -3,7 +3,6 @@
 
 import {CommandExecutor} from "../../common/commandExecutor";
 import {Log} from "../../common/log/log";
-import {ChildProcess} from "child_process";
 
 import * as assert from "assert";
 import * as semver from "semver";
@@ -75,38 +74,13 @@ suite("commandExecutor", function() {
                 console.log(message);
             });
 
-            Q({})
-                .then(function() {
-                    let process: ChildProcess = ce.spawn("node", ["-v"]);
-                    let deferred = Q.defer<string>();
-
-                    process.stdout.on("data", function(data: any) {
-                        deferred.resolve(data.toString());
-                    });
-
-                    return deferred.promise;
-                })
-                .then(function(output: string) {
-                    assert(semver.clean(output));
-                }).done(() => done(), done);
-        });
-
-        test("should spawnAndWaitForCompletion a command", function(done: MochaDone) {
-            let ce = new CommandExecutor();
-            let loggedOutput: string = "";
-
-            sinon.stub(Log, "logMessage", function(message: string, formatMessage: boolean = true) {
-                loggedOutput += message;
-                console.log(message);
-            });
-
             Q({})
                 .then(function () {
-                    return ce.spawnAndWaitForCompletion("node", ["-v"]);
+                    return ce.spawn("node", ["-v"]);
                 }).done(() => done(), done);
         });
 
-        test("spawnAndWaitForCompletion should reject a bad command", function(done: MochaDone) {
+        test("spawn should reject a bad command", function(done: MochaDone) {
             let ce = new CommandExecutor();
             let loggedOutput: string = "";
 
@@ -117,7 +91,7 @@ suite("commandExecutor", function() {
 
             Q({})
                 .then(function() {
-                    return ce.spawnAndWaitForCompletion("bar", ["-v"]);
+                    return ce.spawn("bar", ["-v"]);
                 })
                 .catch((reason) => {
                     console.log(reason.message);