Add some unit tests to codify assumptions

Signed-off-by: Sebastian Malton <sebastian@malton.name>

package.json

@@ -329,6 +329,7 @@
     "@types/lodash": "^4.14.187",
     "@types/marked": "^4.0.7",
     "@types/md5-file": "^4.0.2",
+    "@types/memorystream": "^0.3.0",
     "@types/mini-css-extract-plugin": "^2.4.0",
     "@types/mock-fs": "^4.13.1",
     "@types/node": "^16.18.3",
@@ -396,6 +397,7 @@
     "jest-environment-jsdom": "^28.1.3",
     "jest-mock-extended": "^2.0.9",
     "make-plural": "^6.2.2",
+    "memorystream": "^0.3.1",
     "mini-css-extract-plugin": "^2.6.1",
     "mock-http": "^1.1.0",
     "node-gyp": "^8.3.0",

src/main/utils/shell-env/compute-unix-shell-environment.injectable.ts

@@ -8,6 +8,9 @@ import getBasenameOfPathInjectable from "../../../common/path/get-basename.injec
 import spawnInjectable from "../../child-process/spawn.injectable";
 import randomUUIDInjectable from "../../crypto/random-uuid.injectable";
 import loggerInjectable from "../../../common/logger.injectable";
+import processExecPathInjectable from "./execPath.injectable";
+import processEnvInjectable from "./env.injectable";
+import { object } from "../../../common/utils";
 
 export interface UnixShellEnvOptions {
   signal: AbortSignal;
@@ -15,17 +18,26 @@ export interface UnixShellEnvOptions {
 
 export type ComputeUnixShellEnvironment = (shell: string, opts: UnixShellEnvOptions) => Promise<EnvironmentVariables>;
 
-const getResetProcessEnv = (src: Partial<Record<string, string>>, names: string[]): ((target: Partial<Record<string, string>>) => void) => {
-  const pairs = names.map(name => ([name, src[name]] as const));
+const getResetProcessEnv = (src: Partial<Record<string, string>>, overrides: Partial<Record<string, string>>): {
+  resetEnvPairs: (target: Partial<Record<string, string>>) => void;
+  env: Partial<Record<string, string>>;
+} => {
+  const originals = object.entries(overrides).map(([name]) => [name, src[name]] as const);
 
-  return (target) => {
-    for (const [name, orginalValue] of pairs) {
-      if (orginalValue) {
-        target[name] = orginalValue;
-      } else {
-        delete target[name];
+  return {
+    env: {
+      ...src,
+      ...overrides,
+    },
+    resetEnvPairs: (target) => {
+      for (const [name, orginalValue] of originals) {
+        if (orginalValue) {
+          target[name] = orginalValue;
+        } else {
+          delete target[name];
+        }
       }
-    }
+    },
   };
 };
 
@@ -40,54 +52,52 @@ const computeUnixShellEnvironmentInjectable = getInjectable({
     const spawn = di.inject(spawnInjectable);
     const logger = di.inject(loggerInjectable);
     const randomUUID = di.inject(randomUUIDInjectable);
+    const processExecPath = di.inject(processExecPathInjectable);
+    const processEnv = di.inject(processEnvInjectable);
+
+    const getShellSpecifices = (shellName: string) => {
+      const mark = randomUUID().replace(/-/g, "");
+      const regex = new RegExp(`${mark}(\\{.*\\})${mark}`);
 
-    const getShellSpecifices = (shellName: string, mark: string) => {
       if (powerShellName.test(shellName)) {
         // Older versions of PowerShell removes double quotes sometimes so we use "double single quotes" which is how
         // you escape single quotes inside of a single quoted string.
         return {
-          command: `Command '${process.execPath}' -p '\\"${mark}\\" + JSON.stringify(process.env) + \\"${mark}\\"'`,
+          command: `Command '${processExecPath}' -p '\\"${mark}\\" + JSON.stringify(process.env) + \\"${mark}\\"'`,
           shellArgs: ["-Login"],
+          regex,
         };
       }
 
-      return {
-        command: `'${process.execPath}' -p '"${mark}" + JSON.stringify(process.env) + "${mark}"'`,
-        shellArgs: cshLikeShellName.test(shellName) || fishLikeShellName.test(shellName)
-          // Some shells don't support any other options when providing the -l (login) shell option
-          ? ["-l"]
-          // zsh (at least, maybe others) don't load RC files when in non-interactive mode, even when using -l (login) option
-          : ["-li"],
-      };
+      let command = `'${processExecPath}' -p '"${mark}" + JSON.stringify(process.env) + "${mark}"'`;
+      const shellArgs = ["-l"];
+
+      if (fishLikeShellName.test(shellName)) {
+        shellArgs.push("-c", command);
+        command = "";
+      } else if (!cshLikeShellName.test(shellName)) {
+        // zsh (at least, maybe others) don't load RC files when in non-interactive mode, even when using -l (login) option
+        shellArgs.push("-i");
+      } else {
+        // Some shells don't support any other options when providing the -l (login) shell option
+      }
+
+      return { command, shellArgs, regex };
     };
 
 
     return async (shellPath, opts) => {
-      const resetEnvPairs = getResetProcessEnv(process.env, [
-        "ELECTRON_RUN_AS_NODE",
-        "ELECTRON_NO_ATTACH_CONSOLE",
-        "TERM",
-      ]);
-      const env = {
-        ...process.env,
+      const { resetEnvPairs, env } = getResetProcessEnv(processEnv, {
         ELECTRON_RUN_AS_NODE: "1",
         ELECTRON_NO_ATTACH_CONSOLE: "1",
         TERM: "screen-256color-bce", // required for fish
-      };
-      const mark = randomUUID().replace(/-/g, "");
-      const regex = new RegExp(`${mark}(\\{.*\\})${mark}`);
-      const { command, shellArgs } = getShellSpecifices(shellPath, mark);
+      });
+      const shellName = getBasenameOfPath(shellPath);
+      const { command, shellArgs, regex } = getShellSpecifices(shellName);
 
       logger.info(`[UNIX-SHELL-ENV]: running against ${shellPath}`, { command, shellArgs });
 
       return new Promise((resolve, reject) => {
-        const shellName = getBasenameOfPath(shellPath);
-        const isFishShellLike = fishLikeShellName.test(shellName);
-
-        if (isFishShellLike) {
-          shellArgs.push("-c", command);
-        }
-
         const shellProcess = spawn(shellPath, shellArgs, {
           signal: opts.signal,
           env,
@@ -122,11 +132,7 @@ const computeUnixShellEnvironmentInjectable = getInjectable({
           }
         });
 
-        if (isFishShellLike) {
-          shellProcess.stdin.end();
-        } else {
-          shellProcess.stdin.end(command);
-        }
+        shellProcess.stdin.end(command);
       });
     };
   },

src/main/utils/shell-env/compute-unix-shell-environment.test.ts

@@ -0,0 +1,432 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+
+import type { DiContainer } from "@ogre-tools/injectable";
+import type { ChildProcessWithoutNullStreams } from "child_process";
+import EventEmitter from "events";
+import { flushPromises } from "../../../common/test-utils/flush-promises";
+import type { Spawn } from "../../child-process/spawn.injectable";
+import spawnInjectable from "../../child-process/spawn.injectable";
+import randomUUIDInjectable from "../../crypto/random-uuid.injectable";
+import { getDiForUnitTesting } from "../../getDiForUnitTesting";
+import type { ComputeUnixShellEnvironment } from "./compute-unix-shell-environment.injectable";
+import computeUnixShellEnvironmentInjectable from "./compute-unix-shell-environment.injectable";
+import processEnvInjectable from "./env.injectable";
+import processExecPathInjectable from "./execPath.injectable";
+import MemoryStream from "memorystream";
+import type { EnvironmentVariables } from "./compute-shell-environment.injectable";
+
+const expectedEnv = {
+  SOME_ENV_VAR: "some-env-value",
+  ELECTRON_RUN_AS_NODE: "1",
+  ELECTRON_NO_ATTACH_CONSOLE: "1",
+  TERM: "screen-256color-bce",
+};
+
+describe("computeUnixShellEnvironment technical tests", () => {
+  let di: DiContainer;
+  let computeUnixShellEnvironment: ComputeUnixShellEnvironment;
+  let spawnMock: jest.MockedFunction<Spawn>;
+  let shellProcessFake: ChildProcessWithoutNullStreams;
+  let stdinValue: string;
+  let shellStdin: MemoryStream;
+  let shellStdout: MemoryStream;
+  let shellStderr: MemoryStream;
+  let unixShellEnv: Promise<EnvironmentVariables>;
+
+  beforeEach(() => {
+    di = getDiForUnitTesting({
+      doGeneralOverrides: true,
+    });
+
+    spawnMock = jest.fn().mockImplementation((spawnfile, spawnargs) => {
+      shellStdin = new MemoryStream();
+      shellStdout = new MemoryStream();
+      shellStderr = new MemoryStream();
+      stdinValue = "";
+
+      shellStdin.on("data", (chunk) => {
+        stdinValue += chunk.toString();
+      });
+
+      return shellProcessFake = Object.assign(new EventEmitter(), {
+        stdin: shellStdin,
+        stdout: shellStdout,
+        stderr: shellStderr,
+        stdio: [
+          shellStdin,
+          shellStdout,
+          shellStderr,
+        ] as any,
+        killed: false,
+        kill: jest.fn(),
+        send: jest.fn(),
+        disconnect: jest.fn(),
+        unref: jest.fn(),
+        ref: jest.fn(),
+        connected: false,
+        exitCode: null,
+        signalCode: null,
+        spawnargs,
+        spawnfile,
+      });
+    });
+    di.override(spawnInjectable, () => spawnMock);
+    di.override(randomUUIDInjectable, () => () => "deadbeef");
+
+    di.override(processEnvInjectable, () => ({
+      SOME_ENV_VAR: "some-env-value",
+      TERM: "some-other-value",
+    }));
+    di.override(processExecPathInjectable, () => "/some/process/exec/path");
+
+    di.unoverride(computeUnixShellEnvironmentInjectable);
+    di.permitSideEffects(computeUnixShellEnvironmentInjectable);
+    computeUnixShellEnvironment = di.inject(computeUnixShellEnvironmentInjectable);
+  });
+
+  describe.each([
+    "/bin/csh",
+    "/bin/tcsh",
+  ])("when shell is %s", (shellPath) => {
+    beforeEach(async () => {
+      unixShellEnv = computeUnixShellEnvironment(shellPath, { signal: new AbortSignal() });
+      await flushPromises();
+    });
+
+    it("should spawn a process with the correct arguments", () => {
+      expect(spawnMock).toBeCalledWith(
+        shellPath,
+        [
+          "-l",
+        ],
+        {
+          env: expectedEnv,
+        },
+      );
+    });
+
+    it("should send the command via stdin", () => {
+      expect(stdinValue).toBe(`'/some/process/exec/path' -p '"deadbeef" + JSON.stringify(process.env) + "deadbeef"'`);
+    });
+
+    it("should close stdin", () => {
+      expect(shellStdin.readableEnded).toBe(true);
+    });
+
+    describe("when process errors", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("error", new Error("some-error"));
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("some-error");
+      });
+    });
+
+    describe("when process exits with non-zero exit code", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("close", 1, null);
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("Unexpected return code from spawned shell (code: 1, signal: null)");
+      });
+    });
+
+    describe("when process exits with a signal", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("close", 0, "SIGKILL");
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("Unexpected return code from spawned shell (code: 0, signal: SIGKILL)");
+      });
+    });
+
+    describe("when process stdout emits some data", () => {
+      beforeEach(() => {
+        const fakeInnerEnv = {
+          PATH: "/bin",
+          ...expectedEnv,
+        };
+
+        shellStdout.emit("data", Buffer.from(`some-other-datadeadbeef${JSON.stringify(fakeInnerEnv)}deadbeefsome-third-other-data`));
+      });
+
+      describe("when process successfully exits", () => {
+        beforeEach(() => {
+          shellProcessFake.emit("close", 0);
+        });
+
+        it("should resolve the env", async () => {
+          await expect(unixShellEnv).resolves.toMatchObject({
+            PATH: "/bin",
+            SOME_ENV_VAR: "some-env-value",
+            TERM: "some-other-value",
+          });
+        });
+      });
+    });
+  });
+
+  describe.each([
+    "/bin/bash",
+    "/bin/sh",
+    "/bin/zsh",
+  ])("when shell is %s", (shellPath) => {
+    beforeEach(async () => {
+      unixShellEnv = computeUnixShellEnvironment(shellPath, { signal: new AbortSignal() });
+      await flushPromises();
+    });
+
+    it("should spawn a process with the correct arguments", () => {
+      expect(spawnMock).toBeCalledWith(
+        shellPath,
+        [
+          "-l",
+          "-i",
+        ],
+        {
+          env: expectedEnv,
+        },
+      );
+    });
+
+    it("should send the command via stdin", () => {
+      expect(stdinValue).toBe(`'/some/process/exec/path' -p '"deadbeef" + JSON.stringify(process.env) + "deadbeef"'`);
+    });
+
+    it("should close stdin", () => {
+      expect(shellStdin.readableEnded).toBe(true);
+    });
+
+    describe("when process errors", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("error", new Error("some-error"));
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("some-error");
+      });
+    });
+
+    describe("when process exits with non-zero exit code", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("close", 1, null);
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("Unexpected return code from spawned shell (code: 1, signal: null)");
+      });
+    });
+
+    describe("when process exits with a signal", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("close", 0, "SIGKILL");
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("Unexpected return code from spawned shell (code: 0, signal: SIGKILL)");
+      });
+    });
+
+    describe("when process stdout emits some data", () => {
+      beforeEach(() => {
+        const fakeInnerEnv = {
+          PATH: "/bin",
+          ...expectedEnv,
+        };
+
+        shellStdout.emit("data", Buffer.from(`some-other-datadeadbeef${JSON.stringify(fakeInnerEnv)}deadbeefsome-third-other-data`));
+      });
+
+      describe("when process successfully exits", () => {
+        beforeEach(() => {
+          shellProcessFake.emit("close", 0);
+        });
+
+        it("should resolve the env", async () => {
+          await expect(unixShellEnv).resolves.toMatchObject({
+            PATH: "/bin",
+            SOME_ENV_VAR: "some-env-value",
+            TERM: "some-other-value",
+          });
+        });
+      });
+    });
+  });
+
+  describe.each([
+    "/usr/local/bin/fish",
+  ])("when shell is %s", (shellPath) => {
+    beforeEach(async () => {
+      unixShellEnv = computeUnixShellEnvironment(shellPath, { signal: new AbortSignal() });
+      await flushPromises();
+    });
+
+    it("should spawn a process with the correct arguments", () => {
+      expect(spawnMock).toBeCalledWith(
+        shellPath,
+        [
+          "-l",
+          "-c",
+          `'/some/process/exec/path' -p '"deadbeef" + JSON.stringify(process.env) + "deadbeef"'`,
+        ],
+        {
+          env: expectedEnv,
+        },
+      );
+    });
+
+    it("should not send anything via stdin", () => {
+      expect(stdinValue).toBe("");
+    });
+
+    it("should close stdin", () => {
+      expect(shellStdin.readableEnded).toBe(true);
+    });
+
+    describe("when process errors", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("error", new Error("some-error"));
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("some-error");
+      });
+    });
+
+    describe("when process exits with non-zero exit code", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("close", 1, null);
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("Unexpected return code from spawned shell (code: 1, signal: null)");
+      });
+    });
+
+    describe("when process exits with a signal", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("close", 0, "SIGKILL");
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("Unexpected return code from spawned shell (code: 0, signal: SIGKILL)");
+      });
+    });
+
+    describe("when process stdout emits some data", () => {
+      beforeEach(() => {
+        const fakeInnerEnv = {
+          PATH: "/bin",
+          ...expectedEnv,
+        };
+
+        shellStdout.emit("data", Buffer.from(`some-other-datadeadbeef${JSON.stringify(fakeInnerEnv)}deadbeefsome-third-other-data`));
+      });
+
+      describe("when process successfully exits", () => {
+        beforeEach(() => {
+          shellProcessFake.emit("close", 0);
+        });
+
+        it("should resolve the env", async () => {
+          await expect(unixShellEnv).resolves.toMatchObject({
+            PATH: "/bin",
+            SOME_ENV_VAR: "some-env-value",
+            TERM: "some-other-value",
+          });
+        });
+      });
+    });
+  });
+
+  describe.each([
+    "/usr/local/bin/pwsh",
+    "/usr/local/bin/pwsh-preview",
+  ])("when shell is %s", (shellPath) => {
+    beforeEach(async () => {
+      unixShellEnv = computeUnixShellEnvironment(shellPath, { signal: new AbortSignal() });
+      await flushPromises();
+    });
+
+    it("should spawn a process with the correct arguments", () => {
+      expect(spawnMock).toBeCalledWith(
+        shellPath,
+        [
+          "-Login",
+        ],
+        {
+          env: expectedEnv,
+        },
+      );
+    });
+
+    it("should send the command via stdin", () => {
+      expect(stdinValue).toBe(`Command '/some/process/exec/path' -p '\\"deadbeef\\" + JSON.stringify(process.env) + \\"deadbeef\\"'`);
+    });
+
+    it("should close stdin", () => {
+      expect(shellStdin.readableEnded).toBe(true);
+    });
+
+    describe("when process errors", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("error", new Error("some-error"));
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("some-error");
+      });
+    });
+
+    describe("when process exits with non-zero exit code", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("close", 1, null);
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("Unexpected return code from spawned shell (code: 1, signal: null)");
+      });
+    });
+
+    describe("when process exits with a signal", () => {
+      beforeEach(() => {
+        shellProcessFake.emit("close", 0, "SIGKILL");
+      });
+
+      it("should reject the promise with the error", async () => {
+        await expect(unixShellEnv).rejects.toThrow("Unexpected return code from spawned shell (code: 0, signal: SIGKILL)");
+      });
+    });
+
+    describe("when process stdout emits some data", () => {
+      beforeEach(() => {
+        const fakeInnerEnv = {
+          PATH: "/bin",
+          ...expectedEnv,
+        };
+
+        shellStdout.emit("data", Buffer.from(`some-other-datadeadbeef${JSON.stringify(fakeInnerEnv)}deadbeefsome-third-other-data`));
+      });
+
+      describe("when process successfully exits", () => {
+        beforeEach(() => {
+          shellProcessFake.emit("close", 0);
+        });
+
+        it("should resolve the env", async () => {
+          await expect(unixShellEnv).resolves.toMatchObject({
+            PATH: "/bin",
+            SOME_ENV_VAR: "some-env-value",
+            TERM: "some-other-value",
+          });
+        });
+      });
+    });
+  });
+});

src/main/utils/shell-env/env.injectable.ts

@@ -0,0 +1,14 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getInjectable } from "@ogre-tools/injectable";
+import process from "process";
+
+const processEnvInjectable = getInjectable({
+  id: "process-env",
+  instantiate: () => process.env,
+  causesSideEffects: true,
+});
+
+export default processEnvInjectable;

src/main/utils/shell-env/execPath.injectable.ts

@@ -0,0 +1,14 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getInjectable } from "@ogre-tools/injectable";
+import process from "process";
+
+const processExecPathInjectable = getInjectable({
+  id: "process-exec-path",
+  instantiate: () => process.execPath,
+  causesSideEffects: true,
+});
+
+export default processExecPathInjectable;

yarn.lock

@@ -2217,6 +2217,13 @@
   resolved "https://registry.yarnpkg.com/@types/md5-file/-/md5-file-4.0.2.tgz#c7241e88f4aa17218c774befb0fc34f33f21fe36"
   integrity sha512-8gacRfEqLrmZ6KofpFfxyjsm/LYepeWUWUJGaf5A9W9J5B2/dRZMdkDqFDL6YDa9IweH12IO76jO7mpsK2B3wg==
 
+"@types/memorystream@^0.3.0":
+  version "0.3.0"
+  resolved "https://registry.yarnpkg.com/@types/memorystream/-/memorystream-0.3.0.tgz#7616df4c42a479805d052a058d990b879d5e368f"
+  integrity sha512-gzh6mqZcLryYHn4g2MuMWjo9J1+Py/XYwITyZmUxV7ZoBIi7bTbBgSiuC5tcm3UL3gmaiYssQFDlXr/3fK94cw==
+  dependencies:
+    "@types/node" "*"
+
 "@types/mime@^1":
   version "1.3.2"
   resolved "https://registry.yarnpkg.com/@types/mime/-/mime-1.3.2.tgz#93e25bf9ee75fe0fd80b594bc4feb0e862111b5a"
@@ -8735,6 +8742,11 @@ memoize-one@^6.0.0:
   resolved "https://registry.yarnpkg.com/memoize-one/-/memoize-one-6.0.0.tgz#b2591b871ed82948aee4727dc6abceeeac8c1045"
   integrity sha512-rkpe71W0N0c0Xz6QD0eJETuWAJGnJ9afsl1srmwPrI+yBCkge5EycXXbYRyvL29zZVUWQCY7InPRCv3GDXuZNw==
 
+memorystream@^0.3.1:
+  version "0.3.1"
+  resolved "https://registry.yarnpkg.com/memorystream/-/memorystream-0.3.1.tgz#86d7090b30ce455d63fbae12dda51a47ddcaf9b2"
+  integrity sha512-S3UwM3yj5mtUSEfP41UZmt/0SCoVYUcU1rkXv+BQ5Ig8ndL4sPoJNBUJERafdPb5jjHJGuMgytgKvKIf58XNBw==
+
 merge-descriptors@1.0.1:
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/merge-descriptors/-/merge-descriptors-1.0.1.tgz#b00aaa556dd8b44568150ec9d1b953f3f90cbb61"