Use fork instead of spawn for AppWorker (#760)

Use fork instead of spawn for AppWorker
microsoft · Jul 31, 2018 · 2df05fa · 2df05fa
1 parent 46e2461
commit 2df05fa
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 11 deletions.
diff --git a/src/debugger/forkedAppWorker.ts b/src/debugger/forkedAppWorker.ts
@@ -75,11 +75,9 @@ export class ForkedAppWorker implements IDebuggeeWorker {
         // Note that we set --debug-brk flag to pause the process on the first line - this is
         // required for debug adapter to set the breakpoints BEFORE the debuggee has started.
         // The adapter will continue execution once it's done with breakpoints.
-        const nodeArgs = [`--inspect=${port}`, "--debug-brk", scriptToRunPath];
+        const nodeArgs = [`--inspect=${port}`, "--debug-brk"];
         // Start child Node process in debugging mode
-        this.debuggeeProcess = child_process.spawn("node", nodeArgs, {
-            stdio: ["pipe", "pipe", "pipe", "ipc"],
-        })
+        this.debuggeeProcess = child_process.fork(scriptToRunPath, nodeArgs)
         .on("message", (message: any) => {
             // 'workerLoaded' is a special message that indicates that worker is done with loading.
             // We need to wait for it before doing any IPC because process.send doesn't seems to care

diff --git a/src/debugger/nodeDebugWrapper.ts b/src/debugger/nodeDebugWrapper.ts
@@ -76,7 +76,7 @@ export function makeSession(
         private launch(request: DebugProtocol.Request): void {
             this.requestSetup(request.arguments)
                 .then(() => {
-                    logger.verbose(`Handle launch request: ${request.arguments}`);
+                    logger.verbose(`Handle launch request: ${JSON.stringify(request.arguments, null , 2)}`);
                     return this.remoteExtension.launch(request);
                 })
                 .then(() => {

diff --git a/test/debugger/appWorker.test.ts b/test/debugger/appWorker.test.ts
@@ -19,30 +19,30 @@ suite("appWorker", function() {
         const packagerPort = 8081;
 
         suite("SandboxedAppWorker", function() {
-            const originalSpawn = child_process.spawn;
             const sourcesStoragePath = path.resolve(__dirname, "assets");
 
             // Inject 5 sec delay before shutting down to worker to give tests some time to execute
             const WORKER_DELAY_SHUTDOWN = `setTimeout(() => {console.log("Shutting down")}, 5000)`;
 
             let testWorker: ForkedAppWorker;
-            let spawnStub: Sinon.SinonStub;
+            let forkStub: Sinon.SinonStub;
             let postReplyFunction = sinon.stub();
 
             function workerWithScript(scriptBody: string): ForkedAppWorker {
                 const wrappedBody = [ MultipleLifetimesAppWorker.WORKER_BOOTSTRAP,
                     scriptBody, MultipleLifetimesAppWorker.WORKER_DONE, WORKER_DELAY_SHUTDOWN ].join("\n");
 
-                spawnStub = sinon.stub(child_process, "spawn", () =>
-                    originalSpawn("node", ["-e", wrappedBody], {stdio: ["pipe", "pipe", "pipe", "ipc"]}));
+                forkStub = sinon.stub(child_process, "fork", () => {
+                    return child_process.spawn("node", ["-e", wrappedBody], { stdio: ["inherit", "inherit", "inherit", "ipc"] });
+                });
 
                 testWorker = new ForkedAppWorker("localhost", packagerPort, sourcesStoragePath, "", postReplyFunction);
                 return testWorker;
             }
 
             teardown(function() {
                 // Reset everything
-                spawnStub.restore();
+                forkStub.restore();
                 postReplyFunction.reset();
                 if (testWorker) {
                     testWorker.stop();
@@ -63,7 +63,7 @@ suite("appWorker", function() {
             test("should be able to import scripts", function() {
                 // NOTE: we're not able to mock reading script for import since this is performed by a
                 // separate node process and is out of control so we must provide a real script file
-                const scriptImportPath = path.resolve(sourcesStoragePath, "importScriptsTest.js");
+                const scriptImportPath = path.resolve(sourcesStoragePath, "importScriptsTest.js").replace(/\\/g, "/");
                 const startScriptContents = `importScripts("${scriptImportPath}"); postMessage("postImport");`;
 
                 return workerWithScript(startScriptContents).start().then(() => {