From 150706152e9557c18ee6f753544ff9afe6f4fa4d Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Mon, 29 Jul 2019 17:57:06 -0400 Subject: [PATCH 1/4] fix: Inject current environment to spawn process, add additional error log statements --- src/services/offlineService.ts | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/services/offlineService.ts b/src/services/offlineService.ts index 40ac8551..16a2289c 100644 --- a/src/services/offlineService.ts +++ b/src/services/offlineService.ts @@ -1,4 +1,4 @@ -import { spawn } from "child_process"; +import { spawn, SpawnOptions } from "child_process"; import fs from "fs"; import Serverless from "serverless"; import configConstants from "../config"; @@ -62,18 +62,19 @@ export class OfflineService extends BaseService { /** * Spawn a Node child process with predefined environment variables * @param command CLI Command - NO ARGS - * @param args Array of arguments for CLI command - * @param env Additional environment variables to be set in addition to - * predefined variables in `serverless.yml` + * @param spawnArgs Array of arguments for CLI command */ - private spawn(command: string, args?: string[], env?: any): Promise { - env = { + private spawn(command: string, spawnArgs?: string[]): Promise { + const env = { + // Inherit environment from current process, most importantly, the PATH + ...process.env, + // Override any custom environment variables from serverless configuration ...this.serverless.service.provider["environment"], - ...env } - this.log(`Spawning process '${command} ${args.join(" ")}'`); + this.log(`Spawning process '${command} ${spawnArgs.join(" ")}'`); return new Promise((resolve, reject) => { - const childProcess = spawn(command, args, {env}); + const spawnOptions: SpawnOptions = { env }; + const childProcess = spawn(command, spawnArgs, spawnOptions); childProcess.stdout.on("data", (data) => { this.log(data, { @@ -94,7 +95,12 @@ export class OfflineService extends BaseService { }); childProcess.on("error", (err) => { - this.log(`${err}`, { + this.log(`${err}\n + Command: ${command} + Arguments: "${spawnArgs.join(" ")}" + Options: ${JSON.stringify(spawnOptions, null, 2)}\n + Make sure you've installed azure-func-core-tools. Run:\n + npm i azure-func-core-tools -g`, { color: "red" }, command); reject(err); From e0e27e475fe2445d16e7781e9d28828e5cfa2db8 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Thu, 1 Aug 2019 12:56:57 -0700 Subject: [PATCH 2/4] fix: Enabled pass-through for stdio and append .cmd on commands in windows --- src/services/offlineService.test.ts | 36 ++++++++++++++---- src/services/offlineService.ts | 59 ++++++----------------------- 2 files changed, 41 insertions(+), 54 deletions(-) diff --git a/src/services/offlineService.test.ts b/src/services/offlineService.test.ts index c8fc4f49..df392ba4 100644 --- a/src/services/offlineService.test.ts +++ b/src/services/offlineService.test.ts @@ -7,10 +7,7 @@ import { MockFactory } from "../test/mockFactory"; import { OfflineService } from "./offlineService"; describe("Offline Service", () => { - - const mySpawn = mockSpawn(); - require("child_process").spawn = mySpawn; - mySpawn.setDefault(mySpawn.simple(0, "Exit code")); + let mySpawn; function createService(sls?: Serverless): OfflineService { return new OfflineService( @@ -21,12 +18,16 @@ describe("Offline Service", () => { beforeEach(() => { // Mocking the file system so that files are not created in project directory - mockFs({}) + mockFs({}); + + mySpawn = mockSpawn(); + require("child_process").spawn = mySpawn; + mySpawn.setDefault(mySpawn.simple(0, "Exit code")); }); afterEach(() => { mockFs.restore(); - }) + }); it("builds required files for offline execution", async () => { const sls = MockFactory.createTestServerless(); @@ -113,7 +114,12 @@ describe("Offline Service", () => { rmdirSpy.mockRestore(); }); - it("instructs users how to run locally", async () => { + it("calls func host start on Mac OS", async () => { + Object.defineProperty(process, "platform", { + value: "darwin", + writable: true, + }); + const sls = MockFactory.createTestServerless(); const service = createService(sls); await service.start(); @@ -123,4 +129,20 @@ describe("Offline Service", () => { expect(call.command).toEqual("func"); expect(call.args).toEqual(["host", "start"]); }); + + it("calls func host start on windows", async () => { + Object.defineProperty(process, "platform", { + value: "win32", + writable: true, + }); + + const sls = MockFactory.createTestServerless(); + const service = createService(sls); + await service.start(); + const calls = mySpawn.calls; + expect(calls).toHaveLength(1); + const call = calls[0]; + expect(call.command).toEqual("func.cmd"); + expect(call.args).toEqual(["host", "start"]); + }); }); diff --git a/src/services/offlineService.ts b/src/services/offlineService.ts index 16a2289c..ec77c174 100644 --- a/src/services/offlineService.ts +++ b/src/services/offlineService.ts @@ -29,7 +29,7 @@ export class OfflineService extends BaseService { await this.packageService.createBindings(); const filenames = Object.keys(this.localFiles); for (const filename of filenames) { - if (!fs.existsSync(filename)){ + if (!fs.existsSync(filename)) { fs.writeFileSync( filename, this.localFiles[filename] @@ -44,7 +44,7 @@ export class OfflineService extends BaseService { await this.packageService.cleanUp(); const filenames = Object.keys(this.localFiles); for (const filename of filenames) { - if (fs.existsSync(filename)){ + if (fs.existsSync(filename)) { this.log(`Removing file '${filename}'`); fs.unlinkSync(filename) } @@ -65,6 +65,10 @@ export class OfflineService extends BaseService { * @param spawnArgs Array of arguments for CLI command */ private spawn(command: string, spawnArgs?: string[]): Promise { + if (process.platform === "win32") { + command += ".cmd"; + } + const env = { // Inherit environment from current process, most importantly, the PATH ...process.env, @@ -73,54 +77,15 @@ export class OfflineService extends BaseService { } this.log(`Spawning process '${command} ${spawnArgs.join(" ")}'`); return new Promise((resolve, reject) => { - const spawnOptions: SpawnOptions = { env }; + const spawnOptions: SpawnOptions = { env, stdio: "inherit" }; const childProcess = spawn(command, spawnArgs, spawnOptions); - childProcess.stdout.on("data", (data) => { - this.log(data, { - color: configConstants.funcConsoleColor, - }, command); - }); - - childProcess.stderr.on("data", (data) => { - this.log(data, { - color: "red", - }, command); - }) - - childProcess.on("message", (message) => { - this.log(message, { - color: configConstants.funcConsoleColor, - }, command); - }); - - childProcess.on("error", (err) => { - this.log(`${err}\n - Command: ${command} - Arguments: "${spawnArgs.join(" ")}" - Options: ${JSON.stringify(spawnOptions, null, 2)}\n - Make sure you've installed azure-func-core-tools. Run:\n - npm i azure-func-core-tools -g`, { - color: "red" - }, command); - reject(err); - }); - childProcess.on("exit", (code) => { - this.log(`Exited with code: ${code}`, { - color: (code === 0) ? "green" : "red", - }, command); - }); - - childProcess.on("close", (code) => { - this.log(`Closed with code: ${code}`, { - color: (code === 0) ? "green" : "red", - }, command); - resolve(); - }); - - childProcess.on("disconnect", () => { - this.log("Process disconnected"); + if (code === 0) { + resolve(); + } else { + reject(); + } }); }); } From d16b80ddcd4c5dafbfb20b33fec9b3c61e648df9 Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Fri, 2 Aug 2019 08:38:41 -0700 Subject: [PATCH 3/4] Respond to PR feedback --- src/services/offlineService.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/services/offlineService.ts b/src/services/offlineService.ts index ec77c174..9f429f1b 100644 --- a/src/services/offlineService.ts +++ b/src/services/offlineService.ts @@ -72,7 +72,7 @@ export class OfflineService extends BaseService { const env = { // Inherit environment from current process, most importantly, the PATH ...process.env, - // Override any custom environment variables from serverless configuration + // Environment variables from serverless config are king ...this.serverless.service.provider["environment"], } this.log(`Spawning process '${command} ${spawnArgs.join(" ")}'`); @@ -80,6 +80,18 @@ export class OfflineService extends BaseService { const spawnOptions: SpawnOptions = { env, stdio: "inherit" }; const childProcess = spawn(command, spawnArgs, spawnOptions); + childProcess.on("error", (err) => { + this.log(`${err}\n + Command: ${command} + Arguments: "${spawnArgs.join(" ")}" + Options: ${JSON.stringify(spawnOptions, null, 2)}\n + Make sure you've installed azure-func-core-tools. Run:\n + npm i azure-func-core-tools -g`, { + color: "red" + }, command); + reject(err); + }); + childProcess.on("exit", (code) => { if (code === 0) { resolve(); From c6c582f5769fb53c0fdb00853e37ebe4d7309ca9 Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Fri, 2 Aug 2019 10:04:49 -0700 Subject: [PATCH 4/4] Remove on error log --- src/services/offlineService.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/services/offlineService.ts b/src/services/offlineService.ts index 9f429f1b..996a1cc4 100644 --- a/src/services/offlineService.ts +++ b/src/services/offlineService.ts @@ -80,18 +80,6 @@ export class OfflineService extends BaseService { const spawnOptions: SpawnOptions = { env, stdio: "inherit" }; const childProcess = spawn(command, spawnArgs, spawnOptions); - childProcess.on("error", (err) => { - this.log(`${err}\n - Command: ${command} - Arguments: "${spawnArgs.join(" ")}" - Options: ${JSON.stringify(spawnOptions, null, 2)}\n - Make sure you've installed azure-func-core-tools. Run:\n - npm i azure-func-core-tools -g`, { - color: "red" - }, command); - reject(err); - }); - childProcess.on("exit", (code) => { if (code === 0) { resolve();