fix: using concurrently programmatically (#71)

* fix: check if func binaries exist before starting Closes #38 * chore: update tests parsePort() and isWindows() * moving to use concurrently programmatically * fixing broken tests * using npx for http-server * refactor of how we detect if the api exists to be better xplat * adding debug back in * removing an undefined and using a default message when no API at all * reintroducing the build option on start * cleanup of usings * Fix proxy startup command Co-authored-by: Wassim Chegham <github@wassim.dev> Co-authored-by: Anthony Chu <anthony@anthonychu.ca>
2021-03-01 20:26:18 +11:00 · 2021-03-01 20:26:18 +11:00 · d0d1969291
--- a/.gitignore
+++ b/.gitignore
@ -44,3 +44,4 @@ __pycache__/

 dist
 *.tgz
+*.orig
--- a/package-lock.json
+++ b/package-lock.json
--- a/package.json
+++ b/package.json
@ -14,8 +14,8 @@
    "copy-assets": "node ./scripts/copy-assets.js"
  },
  "bin": {
-    "swa": "./dist/cli/index.js",
-    "swa-emu": "./dist/cli/index.js"
+    "swa": "dist/cli/index.js",
+    "swa-emu": "dist/cli/index.js"
  },
  "author": "Microsoft Corporation",
  "dependencies": {
@ -32,6 +32,7 @@
    "@commitlint/cli": "^11.0.0",
    "@commitlint/config-angular": "^11.0.0",
    "@types/blessed": "^0.1.17",
+    "@types/concurrently": "^5.2.1",
    "@types/cookie": "^0.4.0",
    "@types/http-proxy": "^1.17.4",
    "@types/jest": "^26.0.16",
--- a/readme.md
+++ b/readme.md
@ -95,7 +95,7 @@ Here is a list of the default ports used by popular dev servers:

 #### Serve with a local API backend dev server

-When developing your backend locally, it's often useful to use the local API backend dev server to serve your API backend content. Using the backend server allows you to use built-in features like debugging and rich editor support. 
+When developing your backend locally, it's often useful to use the local API backend dev server to serve your API backend content. Using the backend server allows you to use built-in features like debugging and rich editor support.

 To use the CLI with your local API backend dev server, follow these two steps:

@ -144,8 +144,7 @@ The CLI allows you to mock and read authentication & authorization credentials.

 When requesting `http://localhost:4280/.auth/login/<PROVIDER_NAME>`, you have access a local authentication UI allowing you to set fake user information.

-
-### Reading credentials 
+### Reading credentials

 When requesting the `http://localhost:4280/.auth/me` endpoint, a `clientPrincipal` containing the fake information will be returned by the authentication API.

--- a/src/cli/commands/start.ts
+++ b/src/cli/commands/start.ts
@ -1,9 +1,9 @@
+import concurrently from "concurrently";
 import fs from "fs";
 import path from "path";
-import shell from "shelljs";
 import builder from "../../core/builder";
 import { createRuntimeHost } from "../../core/runtimeHost";
-import { getBin, isHttpUrl, isPortAvailable, parseUrl, readConfigFile, validateDevServerConfig } from "../../core/utils";
+import { isHttpUrl, isPortAvailable, parseUrl, readConfigFile, validateDevServerConfig } from "../../core/utils";
 import { DEFAULT_CONFIG } from "../../config";

 export async function start(startContext: string, program: CLIConfig) {
@ -99,15 +99,18 @@ export async function start(startContext: string, program: CLIConfig) {
  let apiPort = (program.apiPort || DEFAULT_CONFIG.apiPort) as number;

  // handle the API location config
-  let serveApiContent = undefined;
+  let serveApiContent = "echo No API found. Skipping";
  if (useApiDevServer) {
    serveApiContent = `echo 'using api dev server at ${useApiDevServer}'`;
    const { port } = parseUrl(useApiDevServer);
    apiPort = port;
  } else {
-    // serve the api if and only if the user provide the --api-location flag
    if (program.apiLocation && configFile?.apiLocation) {
-      serveApiContent = `([ -d '${configFile?.apiLocation}' ] && (cd ${configFile?.apiLocation}; func start --cors * --port ${program.apiPort})) || echo 'No API found. Skipping.'`;
+      const funcBinary = "npx func";
+      // serve the api if and only if the user provides a folder via the --api-location flag
+      if (fs.existsSync(configFile.apiLocation)) {
+        serveApiContent = `cd ${configFile.apiLocation} && ${funcBinary} start --cors * --port ${program.apiPort}`;
+      }
    }
  }

@ -124,53 +127,40 @@ export async function start(startContext: string, program: CLIConfig) {
    SWA_CLI_PORT: `${program.port}`,
  };

-  const concurrentlyBin = getBin("concurrently");
-
-  const startCommand = [
-    // run concurrent commands
-    `${concurrentlyBin}`,
-    `--restart-tries 1`,
-    `--names " swa","auth"," app"," api"`, // 4 characters each
-    `-c 'bgYellow.bold,bgMagenta.bold,bgCyan.bold,bgGreen.bold'`,
-
+  const concurrentlyEnv = { ...process.env, ...envVarsObj };
+  const concurrentlyCommands = [
    // start the reverse proxy
-    `"node ../proxy/server.js"`,
+    { command: `node ${path.join(__dirname, "..", "..", "proxy", "server.js")}`, name: " swa", env: concurrentlyEnv, prefixColor: "bgYellow.bold" },

    // emulate auth
-    `"node ../auth/server.js --host=${program.host} --port=${authPort}"`,
+    {
+      command: `node ${path.join(__dirname, "..", "..", "auth", "server.js")} --host=${program.host} --port=${authPort}`,
+      name: "auth",
+      env: concurrentlyEnv,
+      prefixColor: "bgMagenta.bold",
+    },

    // serve the app
-    `"${serveStaticContent}"`,
+    { command: serveStaticContent, name: " app", env: concurrentlyEnv, prefixColor: "bgCyan.bold" },

    // serve the api, if it's available
-    serveApiContent && `"${serveApiContent}"`,
-
-    `--color=always`,
+    { command: serveApiContent, name: " api", env: concurrentlyEnv, prefixColor: "bgGreen.bold" },
  ];

  if (process.env.DEBUG) {
    console.log({ env: envVarsObj });
-    console.log({ startCommand });
+    console.log({ concurrentlyCommands });
  }

  if (program.build) {
    // run the app/api builds
-    builder({
+    await builder({
      config: configFile as GithubActionSWAConfig,
    });
  }
-  // run concurrent commands
-  shell.exec(
-    startCommand.join(" "),
-    {
-      // set the cwd to the installation folder
-      cwd: path.resolve(__dirname, ".."),
-      env: { ...process.env, ...envVarsObj },
-    },
-    (_code, _stdout, stderr) => {
-      if (stderr.length) {
-        console.error(stderr);
-      }
-    }
-  );
+
+  await concurrently(concurrentlyCommands, {
+    restartTries: 1,
+    prefix: "name",
+  });
 }
--- a/src/core/builder.ts
+++ b/src/core/builder.ts
@ -1,45 +1,49 @@
+import concurrently from "concurrently";
 import fs from "fs";
 import path from "path";
 import shell from "shelljs";
 import { detectRuntime, RuntimeType } from "./runtimes";
-import { getBin, readConfigFile } from "./utils";
-
-const exec = (command: string, options = {}) => shell.exec(command, { async: false, ...options });
-
-// use the concurrently binary provided by this emulator
-const concurrentlyBin = getBin("concurrently");
+import { readConfigFile } from "./utils";

 const nodeBuilder = (location: string, buildCommand: string, name: string, colour: string) => {
-  const appBuildCommand = [
-    "CI=1",
-    concurrentlyBin,
-    `--names ${name}`,
-    `-c '${colour}'`,
-    `--kill-others-on-fail`,
-    `"npm install && ${buildCommand}"`,
-    `--color=always`,
-  ].join(" ");
-  exec(appBuildCommand, {
-    cwd: location,
-  });
+  return concurrently(
+    [
+      {
+        command: `npm install && ${buildCommand}`,
+        name: name,
+        env: {
+          CI: "1",
+          cwd: location,
+        },
+        prefixColor: colour,
+      },
+    ],
+    {
+      killOthers: ["failure"],
+    }
+  );
 };

 const dotnetBuilder = (location: string, name: string, colour: string) => {
-  const appBuildCommand = [
-    "CI=1",
-    concurrentlyBin,
-    `--names ${name}`,
-    `-c '${colour}'`,
-    `--kill-others-on-fail`,
-    `"dotnet build"`,
-    `--color=always`,
-  ].join(" ");
-  exec(appBuildCommand, {
-    cwd: location,
-  });
+  return concurrently(
+    [
+      {
+        command: `dotnet build`,
+        name: name,
+        env: {
+          CI: "1",
+          cwd: location,
+        },
+        prefixColor: colour,
+      },
+    ],
+    {
+      killOthers: ["failure"],
+    }
+  );
 };

-const builder = ({ config }: { config: Partial<GithubActionSWAConfig> }) => {
+const builder = async ({ config }: { config: Partial<GithubActionSWAConfig> }) => {
  const configFile = readConfigFile();
  if (configFile) {
    let { appLocation, apiLocation, appBuildCommand, apiBuildCommand } = config as GithubActionSWAConfig;
@ -50,14 +54,14 @@ const builder = ({ config }: { config: Partial<GithubActionSWAConfig> }) => {
        case RuntimeType.dotnet:
          {
            // build app
-            dotnetBuilder(appLocation as string, "app_build", "bgGreen.bold");
+            await dotnetBuilder(appLocation as string, "app_build", "bgGreen.bold");

            // NOTE: API is optional. Build it only if it exists
            // This may result in a double-compile of some libraries if they are shared between the
            // Blazor app and the API, but it's an acceptable outcome
            apiLocation = path.resolve(process.cwd(), apiLocation as string);
            if (fs.existsSync(apiLocation) === true && fs.existsSync(path.join(apiLocation, "host.json"))) {
-              dotnetBuilder(apiLocation, "api_build", "bgYellow.bold");
+              await dotnetBuilder(apiLocation, "api_build", "bgYellow.bold");
            }
          }
          break;
@ -71,12 +75,12 @@ const builder = ({ config }: { config: Partial<GithubActionSWAConfig> }) => {
            }

            // build app
-            nodeBuilder(appLocation as string, appBuildCommand as string, "app_build", "bgGreen.bold");
+            await nodeBuilder(appLocation as string, appBuildCommand as string, "app_build", "bgGreen.bold");

            // NOTE: API is optional. Build it only if it exists
            apiLocation = path.resolve(process.cwd(), apiLocation as string);
            if (fs.existsSync(apiLocation) === true && fs.existsSync(path.join(apiLocation, "host.json"))) {
-              nodeBuilder(apiLocation, apiBuildCommand as string, "api_build", "bgYellow.bold");
+              await nodeBuilder(apiLocation, apiBuildCommand as string, "api_build", "bgYellow.bold");
            }
          }
          break;
--- a/src/core/runtimeHost.spec.ts
+++ b/src/core/runtimeHost.spec.ts
@ -26,7 +26,7 @@ describe("runtimeHost", () => {
      });

      expect(spyDetectRuntime).toHaveBeenCalledWith(`.${path.sep}`);
-      expect(rh.command).toContain(`.bin${path.sep}http-server`);
+      expect(rh.command).toContain(`http-server`);
      expect(rh.args).toEqual([
        "./foobar",
        "-d",
@ -49,7 +49,7 @@ describe("runtimeHost", () => {
      });

      expect(spyDetectRuntime).toHaveBeenCalledWith(`.${path.sep}`);
-      expect(rh.command).toContain(`.bin${path.sep}http-server`);
+      expect(rh.command).toContain(`http-server`);
      expect(rh.args).toEqual([
        `.${path.sep}`,
        "-d",
@ -72,7 +72,7 @@ describe("runtimeHost", () => {
      });

      expect(spyDetectRuntime).toHaveBeenCalledWith(`.${path.sep}`);
-      expect(rh.command).toContain(`.bin${path.sep}http-server`);
+      expect(rh.command).toContain(`http-server`);
      expect(rh.args).toEqual([
        `.${path.sep}`,
        "-d",
@ -95,7 +95,7 @@ describe("runtimeHost", () => {
      });

      expect(spyDetectRuntime).toHaveBeenCalledWith(`.${path.sep}`);
-      expect(rh.command).toContain(`.bin${path.sep}http-server`);
+      expect(rh.command).toContain(`http-server`);
      expect(rh.args).toEqual([
        `.${path.sep}`,
        "-d",
@ -118,7 +118,7 @@ describe("runtimeHost", () => {
      });

      expect(spyDetectRuntime).toHaveBeenCalledWith("./foobar");
-      expect(rh.command).toContain(`.bin${path.sep}http-server`);
+      expect(rh.command).toContain(`http-server`);
      expect(rh.args).toEqual([
        `.${path.sep}`,
        "-d",
--- a/src/core/runtimeHost.ts
+++ b/src/core/runtimeHost.ts
@ -1,12 +1,11 @@
 import { DEFAULT_CONFIG } from "../config";
 import { detectRuntime, RuntimeType } from "./runtimes";
-import { getBin } from "./utils";

-const httpServerBin = getBin("http-server");
+const httpServerBin = "npx http-server";
 export const createRuntimeHost = ({ appPort, proxyHost, proxyPort, appLocation, appArtifactLocation }: RuntimeHostConfig) => {
  const runtimeType = detectRuntime(appLocation);

-  console.log(">> detected runtime:", runtimeType);
+  console.info("INFO: Detected runtime:", runtimeType);

  switch (runtimeType) {
    // .NET runtime
--- a/src/core/utils.test.ts
+++ b/src/core/utils.test.ts
@ -1,8 +1,12 @@
 import mockFs from "mock-fs";
 import path from "path";
-import { response, validateCookie, readConfigFile, argv } from "./utils";
+import { response, validateCookie, readConfigFile, argv, parsePort } from "./utils";

 describe("Utils", () => {
+  const mockExit = jest.spyOn(process, "exit").mockImplementation(() => {
+    return undefined as never;
+  });
+
  beforeEach(() => {
    process.env.DEBUG = "";
    process.argv = [];
@ -640,4 +644,23 @@ jobs:
      });
    });
  });
+
+  describe("parsePort()", () => {
+    it("Ports below 1024 should be invalid", () => {
+      parsePort("0");
+      expect(mockExit).toBeCalledWith(-1);
+    });
+    it("Ports above 49151 should be invalid", () => {
+      parsePort("98765");
+      expect(mockExit).toBeCalledWith(-1);
+    });
+    it("Non-number ports should be invalid", () => {
+      parsePort("not a number");
+      expect(mockExit).toBeCalledWith(-1);
+    });
+    it("Ports between 1024 - 49151 should be valid", () => {
+      const port = parsePort("1984");
+      expect(port).toBe(1984);
+    });
+  });
 });
--- a/src/core/utils.ts
+++ b/src/core/utils.ts
@ -1,4 +1,3 @@
-import { spawnSync } from "child_process";
 import cookie from "cookie";
 import fs from "fs";
 import net from "net";
@ -121,10 +120,12 @@ function validateUserConfig(userConfig: Partial<GithubActionSWAConfig>) {
 }

 export const readConfigFile = ({ userConfig }: { userConfig?: Partial<GithubActionSWAConfig> } = {}): Partial<GithubActionSWAConfig> | undefined => {
+  const infoMessage = `INFO: GitHub Actions configuration was not found under ".github/workflows/"`;
  const githubActionFolder = path.resolve(process.cwd(), ".github/workflows/");

  // does the config folder exist?
  if (fs.existsSync(githubActionFolder) === false) {
+    console.info(infoMessage);
    return userConfig && validateUserConfig(userConfig);
  }

@ -137,6 +138,7 @@ export const readConfigFile = ({ userConfig }: { userConfig?: Partial<GithubActi

  // does the config file exist?
  if (!githubActionFile || fs.existsSync(githubActionFile)) {
+    console.info(infoMessage);
    return userConfig && validateUserConfig(userConfig);
  }

@ -377,24 +379,6 @@ export function computeAppLocationFromArtifactLocation(appArtifactLocation: stri
  return undefined;
 }

-export function getBin(binary: string) {
-  if (binary.indexOf(path.sep) >= 0) {
-    return path.isAbsolute(binary) ? binary : path.resolve(binary);
-  }
-
-  const binDirOutput = spawnSync(isWindows() ? "npm.cmd" : "npm", ["bin"], { cwd: __dirname });
-  const binDirErr = binDirOutput.stderr.toString();
-  if (binDirErr) {
-    console.error({ binDirErr });
-  }
-  const binDirOut = binDirOutput.stdout.toString().trim();
-  return path.resolve(binDirOut, isWindows() ? `${binary}.cmd` : binary);
-}
-
-export function isWindows() {
-  return process.platform === "win32" || /^(msys|cygwin)$/.test(process.env?.OSTYPE as string);
-}
-
 export function parsePort(port: string) {
  const portNumber = parseInt(port, 10);
  if (isNaN(portNumber)) {