Cascade different refactorings to eliminate global shared state from app paths

Co-authored-by: Mikko Aspiala <mikko.aspiala@gmail.com>

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>

src/main/__test__/cluster.test.ts

@@ -45,25 +45,29 @@ jest.mock("winston", () => ({
 }));
 
 jest.mock("../../common/ipc");
-jest.mock("../context-handler");
 jest.mock("request");
 jest.mock("request-promise-native");
 
 import { Console } from "console";
 import mockFs from "mock-fs";
-import { Cluster } from "../cluster";
-import { Kubectl } from "../kubectl";
+import type { Cluster } from "../../common/cluster/cluster";
+import { Kubectl } from "../kubectl/kubectl";
+import { getDiForUnitTesting } from "../getDiForUnitTesting";
+import type { ClusterModel } from "../../common/cluster-types";
+import { createClusterInjectionToken } from "../../common/cluster/create-cluster-injection-token";
 
 console = new Console(process.stdout, process.stderr); // fix mockFS
 
 describe("create clusters", () => {
-  beforeEach(() => {
+  let cluster: Cluster;
+  let createCluster: (model: ClusterModel) => Cluster;
+
+  beforeEach(async () => {
     jest.clearAllMocks();
-  });
 
-  let c: Cluster;
+    const di = getDiForUnitTesting({ doGeneralOverrides: true });
+
 
-  beforeEach(() => {
     const mockOpts = {
       "minikube-config.yml": JSON.stringify({
         apiVersion: "v1",
@@ -89,8 +93,14 @@ describe("create clusters", () => {
     };
 
     mockFs(mockOpts);
+    
+    await di.runSetups();
+
+    createCluster = di.inject(createClusterInjectionToken);
+    
     jest.spyOn(Kubectl.prototype, "ensureKubectl").mockReturnValue(Promise.resolve(true));
-    c = new Cluster({
+
+    cluster = createCluster({
       id: "foo",
       contextName: "minikube",
       kubeConfigPath: "minikube-config.yml",
@@ -102,48 +112,39 @@ describe("create clusters", () => {
   });
 
   it("should be able to create a cluster from a cluster model and apiURL should be decoded", () => {
-    expect(c.apiUrl).toBe("https://192.168.64.3:8443");
+    expect(cluster.apiUrl).toBe("https://192.168.64.3:8443");
   });
 
   it("reconnect should not throw if contextHandler is missing", () => {
-    expect(() => c.reconnect()).not.toThrowError();
+    expect(() => cluster.reconnect()).not.toThrowError();
   });
 
   it("disconnect should not throw if contextHandler is missing", () => {
-    expect(() => c.disconnect()).not.toThrowError();
+    expect(() => cluster.disconnect()).not.toThrowError();
   });
 
   it("activating cluster should try to connect to cluster and do a refresh", async () => {
-
-    const c = new class extends Cluster {
-      // only way to mock protected methods, without these we leak promises
-      protected bindEvents() {
-        return;
-      }
-      async ensureKubectl() {
-        return Promise.resolve(null);
-      }
-    }({
+    const cluster = createCluster({
       id: "foo",
       contextName: "minikube",
       kubeConfigPath: "minikube-config.yml",
     });
 
-    c.contextHandler = {
+    cluster.contextHandler = {
       ensureServer: jest.fn(),
       stopServer: jest.fn(),
     } as any;
 
-    jest.spyOn(c, "reconnect");
-    jest.spyOn(c, "canI");
-    jest.spyOn(c, "refreshConnectionStatus");
+    jest.spyOn(cluster, "reconnect");
+    jest.spyOn(cluster, "canI");
+    jest.spyOn(cluster, "refreshConnectionStatus");
 
-    await c.activate();
+    await cluster.activate();
 
-    expect(c.reconnect).toBeCalled();
-    expect(c.refreshConnectionStatus).toBeCalled();
+    expect(cluster.reconnect).toBeCalled();
+    expect(cluster.refreshConnectionStatus).toBeCalled();
 
-    c.disconnect();
+    cluster.disconnect();
     jest.resetAllMocks();
   });
 });

src/main/__test__/context-handler.test.ts

@@ -20,10 +20,12 @@
  */
 
 import { UserStore } from "../../common/user-store";
-import { ContextHandler } from "../context-handler";
+import type { ContextHandler } from "../context-handler/context-handler";
 import { PrometheusProvider, PrometheusProviderRegistry, PrometheusService } from "../prometheus";
 import mockFs from "mock-fs";
-import { AppPaths } from "../../common/app-paths";
+import { getDiForUnitTesting } from "../getDiForUnitTesting";
+import createContextHandlerInjectable from "../context-handler/create-context-handler.injectable";
+import type { Cluster } from "../../common/cluster/cluster";
 
 jest.mock("electron", () => ({
   app: {
@@ -77,25 +79,28 @@ class TestProvider extends PrometheusProvider {
   }
 }
 
-function getHandler() {
-  return new ContextHandler(({
-    getProxyKubeconfig: (): any => ({
-      makeApiClient: (): any => undefined,
-    }),
-    apiUrl: "http://localhost:81",
-  }) as any);
-}
-
-AppPaths.init();
+const clusterStub = {
+  getProxyKubeconfig: (): any => ({
+    makeApiClient: (): any => undefined,
+  }),
+  apiUrl: "http://localhost:81",
+} as Cluster;
 
 describe("ContextHandler", () => {
-  beforeEach(() => {
+  let createContextHandler: (cluster: Cluster) => ContextHandler;
+  
+  beforeEach(async () => {
+    const di = getDiForUnitTesting({ doGeneralOverrides: true });
+
     mockFs({
       "tmp": {},
     });
 
+    await di.runSetups();
+
+    createContextHandler = di.inject(createContextHandlerInjectable);
+
     PrometheusProviderRegistry.createInstance();
-    UserStore.createInstance();
   });
 
   afterEach(() => {
@@ -124,7 +129,12 @@ describe("ContextHandler", () => {
         reg.registerProvider(new TestProvider(`id_${count++}`, ServiceResult.Success));
       }
 
-      expect(() => (getHandler() as any).getPrometheusService()).rejects.toBeDefined();
+      expect(() => {
+        // TODO: Unit test shouldn't access protected or private methods
+        const contextHandler = createContextHandler(clusterStub) as any;
+        
+        return contextHandler.getPrometheusService();
+      }).rejects.toBeDefined();
     });
 
     it.each([
@@ -150,7 +160,10 @@ describe("ContextHandler", () => {
         reg.registerProvider(new TestProvider(`id_${count++}`, ServiceResult.Success));
       }
 
-      const service = await (getHandler() as any).getPrometheusService();
+      // TODO: Unit test shouldn't access protected or private methods
+      const contextHandler = createContextHandler(clusterStub) as any;
+      
+      const service = await contextHandler.getPrometheusService();
 
       expect(service.id === `id_${failures}`);
     });
@@ -178,7 +191,10 @@ describe("ContextHandler", () => {
         reg.registerProvider(new TestProvider(`id_${count++}`, serviceResult));
       }
 
-      const service = await (getHandler() as any).getPrometheusService();
+      // TODO: Unit test shouldn't access protected or private methods
+      const contextHandler = createContextHandler(clusterStub) as any;
+      
+      const service = await contextHandler.getPrometheusService();
 
       expect(service.id === "id_0");
     });
@@ -211,8 +227,11 @@ describe("ContextHandler", () => {
       for (let i = 0; i < afterSuccesses; i += 1) {
         reg.registerProvider(new TestProvider(`id_${count++}`, ServiceResult.Success));
       }
-
-      const service = await (getHandler() as any).getPrometheusService();
+      
+      // TODO: Unit test shouldn't access protected or private methods
+      const contextHandler = createContextHandler(clusterStub) as any;
+      
+      const service = await contextHandler.getPrometheusService();
 
       expect(service.id === "id_0");
     });
@@ -224,8 +243,11 @@ describe("ContextHandler", () => {
       reg.registerProvider(new TestProvider(`id_${count++}`, ServiceResult.Failure));
       reg.registerProvider(new TestProvider(`id_${count++}`, ServiceResult.Success));
       reg.registerProvider(new TestProvider(`id_${count++}`, ServiceResult.Success));
+      
+      // TODO: Unit test shouldn't access protected or private methods
+      const contextHandler = createContextHandler(clusterStub) as any;
 
-      const service = await (getHandler() as any).getPrometheusService();
+      const service = await contextHandler.getPrometheusService();
 
       expect(service.id).not.toBe("id_2");
     });

src/main/__test__/kube-auth-proxy.test.ts

@@ -19,6 +19,8 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
+import type { ClusterModel } from "../../common/cluster-types";
+
 jest.mock("winston", () => ({
   format: {
     colorize: jest.fn(),
@@ -48,11 +50,11 @@ jest.mock("../../common/ipc");
 jest.mock("child_process");
 jest.mock("tcp-port-used");
 
-import { Cluster } from "../cluster";
-import { KubeAuthProxy } from "../kube-auth-proxy";
+import type { Cluster } from "../../common/cluster/cluster";
+import type { KubeAuthProxy } from "../kube-auth-proxy/kube-auth-proxy";
 import { broadcastMessage } from "../../common/ipc";
 import { ChildProcess, spawn } from "child_process";
-import { bundledKubectlPath, Kubectl } from "../kubectl";
+import { bundledKubectlPath, Kubectl } from "../kubectl/kubectl";
 import { mock, MockProxy } from "jest-mock-extended";
 import { waitUntilUsed } from "tcp-port-used";
 import { EventEmitter, Readable } from "stream";
@@ -60,7 +62,9 @@ import { UserStore } from "../../common/user-store";
 import { Console } from "console";
 import { stdout, stderr } from "process";
 import mockFs from "mock-fs";
-import { AppPaths } from "../../common/app-paths";
+import { getDiForUnitTesting } from "../getDiForUnitTesting";
+import createKubeAuthProxyInjectable from "../kube-auth-proxy/create-kube-auth-proxy.injectable";
+import { createClusterInjectionToken } from "../../common/cluster/create-cluster-injection-token";
 
 console = new Console(stdout, stderr);
 
@@ -68,25 +72,11 @@ const mockBroadcastIpc = broadcastMessage as jest.MockedFunction<typeof broadcas
 const mockSpawn = spawn as jest.MockedFunction<typeof spawn>;
 const mockWaitUntilUsed = waitUntilUsed as jest.MockedFunction<typeof waitUntilUsed>;
 
-jest.mock("electron", () => ({
-  app: {
-    getVersion: () => "99.99.99",
-    getName: () => "lens",
-    setName: jest.fn(),
-    setPath: jest.fn(),
-    getPath: () => "tmp",
-    getLocale: () => "en",
-    setLoginItemSettings: jest.fn(),
-  },
-  ipcMain: {
-    on: jest.fn(),
-    handle: jest.fn(),
-  },
-}));
-AppPaths.init();
-
 describe("kube auth proxy tests", () => {
-  beforeEach(() => {
+  let createCluster: (model: ClusterModel) => Cluster;
+  let createKubeAuthProxy: (cluster: Cluster, environmentVariables: NodeJS.ProcessEnv) => KubeAuthProxy;
+
+  beforeEach(async () => {
     jest.clearAllMocks();
 
     const mockMinikubeConfig = {
@@ -115,7 +105,16 @@ describe("kube auth proxy tests", () => {
       "tmp": {},
     };
 
+    const di = getDiForUnitTesting({ doGeneralOverrides: true });
+
     mockFs(mockMinikubeConfig);
+    
+    await di.runSetups();
+
+    createCluster = di.inject(createClusterInjectionToken);
+
+    createKubeAuthProxy = di.inject(createKubeAuthProxyInjectable);
+
     UserStore.createInstance();
   });
 
@@ -125,7 +124,13 @@ describe("kube auth proxy tests", () => {
   });
 
   it("calling exit multiple times shouldn't throw", async () => {
-    const kap = new KubeAuthProxy(new Cluster({ id: "foobar", kubeConfigPath: "minikube-config.yml", contextName: "minikube" }), {});
+    const cluster = createCluster({
+      id: "foobar",
+      kubeConfigPath: "minikube-config.yml",
+      contextName: "minikube",
+    });
+
+    const kap = createKubeAuthProxy(cluster, {});
 
     kap.exit();
     kap.exit();
@@ -211,9 +216,13 @@ describe("kube auth proxy tests", () => {
       });
       mockWaitUntilUsed.mockReturnValueOnce(Promise.resolve());
 
-      const cluster = new Cluster({ id: "foobar", kubeConfigPath: "minikube-config.yml", contextName: "minikube" });
+      const cluster = createCluster({
+        id: "foobar",
+        kubeConfigPath: "minikube-config.yml",
+        contextName: "minikube",
+      });
 
-      proxy = new KubeAuthProxy(cluster, {});
+      proxy = createKubeAuthProxy(cluster, {});
     });
 
     it("should call spawn and broadcast errors", async () => {

src/main/__test__/kubeconfig-manager.test.ts

@@ -18,6 +18,7 @@
  * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
+import { getDiForUnitTesting } from "../getDiForUnitTesting";
 
 const logger = {
   silly: jest.fn(),
@@ -46,41 +47,28 @@ jest.mock("winston", () => ({
   },
 }));
 
-import { KubeconfigManager } from "../kubeconfig-manager";
+import { KubeconfigManager } from "../kubeconfig-manager/kubeconfig-manager";
 import mockFs from "mock-fs";
-import { Cluster } from "../cluster";
-import type { ContextHandler } from "../context-handler";
+import type { Cluster } from "../../common/cluster/cluster";
 import fse from "fs-extra";
 import { loadYaml } from "@kubernetes/client-node";
 import { Console } from "console";
 import * as path from "path";
-import { AppPaths } from "../../common/app-paths";
-
-jest.mock("electron", () => ({
-  app: {
-    getVersion: () => "99.99.99",
-    getName: () => "lens",
-    setName: jest.fn(),
-    setPath: jest.fn(),
-    getPath: () => "tmp",
-    getLocale: () => "en",
-    setLoginItemSettings: jest.fn(),
-  },
-  ipcMain: {
-    on: jest.fn(),
-    handle: jest.fn(),
-  },
-}));
-
-AppPaths.init();
+import createKubeconfigManagerInjectable from "../kubeconfig-manager/create-kubeconfig-manager.injectable";
+import { createClusterInjectionToken } from "../../common/cluster/create-cluster-injection-token";
+import directoryForTempInjectable from "../../common/app-paths/directory-for-temp/directory-for-temp.injectable";
 
 console = new Console(process.stdout, process.stderr); // fix mockFS
 
 describe("kubeconfig manager tests", () => {
   let cluster: Cluster;
-  let contextHandler: ContextHandler;
+  let createKubeconfigManager: (cluster: Cluster) => KubeconfigManager;
+
+  beforeEach(async () => {
+    const di = getDiForUnitTesting({ doGeneralOverrides: true });
+
+    di.override(directoryForTempInjectable, () => "some-directory-for-temp");
 
-  beforeEach(() => {
     const mockOpts = {
       "minikube-config.yml": JSON.stringify({
         apiVersion: "v1",
@@ -107,14 +95,22 @@ describe("kubeconfig manager tests", () => {
 
     mockFs(mockOpts);
 
-    cluster = new Cluster({
+    await di.runSetups();
+
+    const createCluster = di.inject(createClusterInjectionToken);
+
+    createKubeconfigManager = di.inject(createKubeconfigManagerInjectable);
+
+    cluster = createCluster({
       id: "foo",
       contextName: "minikube",
       kubeConfigPath: "minikube-config.yml",
     });
-    contextHandler = {
+
+    cluster.contextHandler = {
       ensureServer: () => Promise.resolve(),
     } as any;
+    
     jest.spyOn(KubeconfigManager.prototype, "resolveProxyUrl", "get").mockReturnValue("http://127.0.0.1:9191/foo");
   });
 
@@ -123,10 +119,10 @@ describe("kubeconfig manager tests", () => {
   });
 
   it("should create 'temp' kube config with proxy", async () => {
-    const kubeConfManager = new KubeconfigManager(cluster, contextHandler);
+    const kubeConfManager = createKubeconfigManager(cluster);
 
     expect(logger.error).not.toBeCalled();
-    expect(await kubeConfManager.getPath()).toBe(`tmp${path.sep}kubeconfig-foo`);
+    expect(await kubeConfManager.getPath()).toBe(`some-directory-for-temp${path.sep}kubeconfig-foo`);
     // this causes an intermittent "ENXIO: no such device or address, read" error
     //    const file = await fse.readFile(await kubeConfManager.getPath());
     const file = fse.readFileSync(await kubeConfManager.getPath());
@@ -138,7 +134,8 @@ describe("kubeconfig manager tests", () => {
   });
 
   it("should remove 'temp' kube config on unlink and remove reference from inside class", async () => {
-    const kubeConfManager = new KubeconfigManager(cluster, contextHandler);
+    const kubeConfManager = createKubeconfigManager(cluster);
+    
     const configPath = await kubeConfManager.getPath();
 
     expect(await fse.pathExists(configPath)).toBe(true);

src/main/__test__/router.test.ts

@@ -19,7 +19,6 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-import { AppPaths } from "../../common/app-paths";
 import { Router } from "../router";
 
 jest.mock("electron", () => ({
@@ -38,8 +37,6 @@ jest.mock("electron", () => ({
   },
 }));
 
-AppPaths.init();
-
 describe("Router", () => {
   it("blocks path traversal attacks", async () => {
     const response: any = {