Replace jest.mock's with overriding dependencies (#6014)

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

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

src/renderer/components/+catalog/catalog.test.tsx

@@ -7,10 +7,9 @@ import React from "react";
 import { screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { Catalog } from "./catalog";
-import { mockWindow } from "../../../../__mocks__/windowMock";
 import type { CatalogEntityActionContext, CatalogEntityData } from "../../../common/catalog";
 import { CatalogEntity } from "../../../common/catalog";
-import type { CatalogEntityRegistry } from "../../api/catalog/entity/registry";
+import type { CatalogEntityOnBeforeRun, CatalogEntityRegistry } from "../../api/catalog/entity/registry";
 import { CatalogEntityDetailRegistry } from "../../../extensions/registries";
 import type { CatalogEntityStore } from "./catalog-entity-store/catalog-entity.store";
 import { getDiForUnitTesting } from "../../getDiForUnitTesting";
@@ -19,38 +18,15 @@ import catalogEntityStoreInjectable from "./catalog-entity-store/catalog-entity-
 import catalogEntityRegistryInjectable from "../../api/catalog/entity/registry.injectable";
 import type { DiRender } from "../test-utils/renderFor";
 import { renderFor } from "../test-utils/renderFor";
-import mockFs from "mock-fs";
 import directoryForUserDataInjectable from "../../../common/app-paths/directory-for-user-data/directory-for-user-data.injectable";
 import getConfigurationFileModelInjectable from "../../../common/get-configuration-file-model/get-configuration-file-model.injectable";
 import type { AppEvent } from "../../../common/app-event-bus/event-bus";
 import appEventBusInjectable from "../../../common/app-event-bus/app-event-bus.injectable";
 import { computed } from "mobx";
 import broadcastMessageInjectable from "../../../common/ipc/broadcast-message.injectable";
-
-mockWindow();
-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(),
-  },
-  ipcRenderer: {
-    on: jest.fn(),
-    invoke: jest.fn(),
-  },
-}));
-
-jest.mock("./hotbar-toggle-menu-item", () => ({
-  HotbarToggleMenuItem: () => <div>menu item</div>,
-}));
+import type { AsyncFnMock } from "@async-fn/jest";
+import asyncFn from "@async-fn/jest";
+import { flushPromises } from "../../../common/test-utils/flush-promises";
 
 class MockCatalogEntity extends CatalogEntity {
   public apiVersion = "api";
@@ -95,7 +71,6 @@ describe("<Catalog />", () => {
 
     di.permitSideEffects(getConfigurationFileModelInjectable);
 
-    mockFs();
     CatalogEntityDetailRegistry.createInstance();
 
     render = renderFor(di);
@@ -103,8 +78,6 @@ describe("<Catalog />", () => {
     catalogEntityItem = createMockCatalogEntity(onRun);
     catalogEntityRegistry = di.inject(catalogEntityRegistryInjectable);
 
-    di.override(catalogEntityRegistryInjectable, () => catalogEntityRegistry);
-
     emitEvent = jest.fn();
 
     di.override(appEventBusInjectable, () => ({
@@ -119,60 +92,71 @@ describe("<Catalog />", () => {
 
   afterEach(() => {
     CatalogEntityDetailRegistry.resetInstance();
-    jest.clearAllMocks();
-    jest.restoreAllMocks();
-    mockFs.restore();
   });
 
-  it("can use catalogEntityRegistry.addOnBeforeRun to add hooks for catalog entities", (done) => {
-    catalogEntityRegistry.addOnBeforeRun(
-      (event) => {
-        expect(event.target.getId()).toBe("a_catalogEntity_uid");
-        expect(event.target.getName()).toBe("a catalog entity");
+  describe("can use catalogEntityRegistry.addOnBeforeRun to add hooks for catalog entities", () => {
+    let onBeforeRunMock: AsyncFnMock<CatalogEntityOnBeforeRun>;
 
-        setTimeout(() => {
-          expect(onRun).toHaveBeenCalled();
-          done();
-        }, 500);
-      },
-    );
+    beforeEach(() => {
+      onBeforeRunMock = asyncFn();
+
+      catalogEntityRegistry.addOnBeforeRun(onBeforeRunMock);
+
+      render(<Catalog />);
+
+      userEvent.click(screen.getByTestId("detail-panel-hot-bar-icon"));
+    });
+
+    it("calls on before run event", () => {
+      const target = onBeforeRunMock.mock.calls[0][0].target;
+
+      const actual = { id: target.getId(), name: target.getName() };
+
+      expect(actual).toEqual({
+        id: "a_catalogEntity_uid",
+        name: "a catalog entity",
+      });
+    });
+
+    it("does not call onRun yet", () => {
+      expect(onRun).not.toHaveBeenCalled();
+    });
+
+    it("when before run event resolves, calls onRun", async () => {
+      await onBeforeRunMock.resolve();
+
+      expect(onRun).toHaveBeenCalled();
+    });
+  });
+
+  it("onBeforeRun prevents event => onRun wont be triggered", async () => {
+    const onBeforeRunMock = jest.fn((event) => event.preventDefault());
+
+    catalogEntityRegistry.addOnBeforeRun(onBeforeRunMock);
 
     render(<Catalog />);
 
     userEvent.click(screen.getByTestId("detail-panel-hot-bar-icon"));
+
+    await flushPromises();
+
+    expect(onRun).not.toHaveBeenCalled();
   });
 
-  it("onBeforeRun prevents event => onRun wont be triggered", (done) => {
-    catalogEntityRegistry.addOnBeforeRun(
-      (e) => {
-        setTimeout(() => {
-          expect(onRun).not.toHaveBeenCalled();
-          done();
-        }, 500);
-        e.preventDefault();
-      },
-    );
+  it("addOnBeforeRun throw an exception => onRun will be triggered", async () => {
+    const onBeforeRunMock = jest.fn(() => {
+      throw new Error("some error");
+    });
+
+    catalogEntityRegistry.addOnBeforeRun(onBeforeRunMock);
 
     render(<Catalog />);
 
     userEvent.click(screen.getByTestId("detail-panel-hot-bar-icon"));
-  });
 
-  it("addOnBeforeRun throw an exception => onRun will be triggered", (done) => {
-    catalogEntityRegistry.addOnBeforeRun(
-      () => {
-        setTimeout(() => {
-          expect(onRun).toHaveBeenCalled();
-          done();
-        }, 500);
+    await flushPromises();
 
-        throw new Error("error!");
-      },
-    );
-
-    render(<Catalog />);
-
-    userEvent.click(screen.getByTestId("detail-panel-hot-bar-icon"));
+    expect(onRun).toHaveBeenCalled();
   });
 
   it("addOnRunHook return a promise and does not prevent run event => onRun()", (done) => {
@@ -189,40 +173,34 @@ describe("<Catalog />", () => {
     userEvent.click(screen.getByTestId("detail-panel-hot-bar-icon"));
   });
 
-  it("addOnRunHook return a promise and prevents event wont be triggered", (done) => {
-    catalogEntityRegistry.addOnBeforeRun(
-      async (e) => {
-        expect(onRun).not.toBeCalled();
+  it("addOnRunHook return a promise and prevents event wont be triggered", async () => {
+    const onBeforeRunMock = asyncFn();
 
-        setTimeout(() => {
-          expect(onRun).not.toBeCalled();
-          done();
-        }, 500);
-
-        e.preventDefault();
-      },
-    );
+    catalogEntityRegistry.addOnBeforeRun(onBeforeRunMock);
 
     render(<Catalog />);
 
     userEvent.click(screen.getByTestId("detail-panel-hot-bar-icon"));
+
+    onBeforeRunMock.mock.calls[0][0].preventDefault();
+
+    await onBeforeRunMock.resolve();
+
+    expect(onRun).not.toHaveBeenCalled();
   });
 
-  it("addOnRunHook return a promise and reject => onRun will be triggered", (done) => {
-    catalogEntityRegistry.addOnBeforeRun(
-      async () => {
-        setTimeout(() => {
-          expect(onRun).toHaveBeenCalled();
-          done();
-        }, 500);
+  it("addOnRunHook return a promise and reject => onRun will be triggered", async () => {
+    const onBeforeRunMock = asyncFn();
 
-        throw new Error("rejection!");
-      },
-    );
+    catalogEntityRegistry.addOnBeforeRun(onBeforeRunMock);
 
     render(<Catalog />);
 
     userEvent.click(screen.getByTestId("detail-panel-hot-bar-icon"));
+
+    await onBeforeRunMock.reject();
+
+    expect(onRun).toHaveBeenCalled();
   });
 
   it("emits catalog open AppEvent", () => {

src/renderer/components/+config-secrets/__tests__/secret-details.test.tsx

@@ -4,17 +4,20 @@
  */
 
 import React from "react";
-import { render } from "@testing-library/react";
 import { SecretDetails } from "../secret-details";
 import { Secret, SecretType } from "../../../../common/k8s-api/endpoints";
+import { getDiForUnitTesting } from "../../../getDiForUnitTesting";
+import { renderFor } from "../../test-utils/renderFor";
 
 jest.mock("../../kube-object-meta/kube-object-meta", () => ({
   KubeObjectMeta: () => null,
 }));
 
-
 describe("SecretDetails tests", () => {
   it("should show the visibility toggle when the secret value is ''", () => {
+    const di = getDiForUnitTesting({ doGeneralOverrides: true });
+    const render = renderFor(di);
+
     const secret = new Secret({
       apiVersion: "v1",
       kind: "secret",

src/renderer/components/+network-policies/__tests__/network-policy-details.test.tsx

@@ -4,15 +4,22 @@
  */
 
 import React from "react";
-import { findByTestId, findByText, render } from "@testing-library/react";
+import { findByTestId, findByText } from "@testing-library/react";
 import { NetworkPolicy } from "../../../../common/k8s-api/endpoints";
 import { NetworkPolicyDetails } from "../network-policy-details";
-
-jest.mock("../../kube-object-meta/kube-object-meta", () => ({
-  KubeObjectMeta: () => null,
-}));
+import { getDiForUnitTesting } from "../../../getDiForUnitTesting";
+import type { DiRender } from "../../test-utils/renderFor";
+import { renderFor } from "../../test-utils/renderFor";
 
 describe("NetworkPolicyDetails", () => {
+  let render: DiRender;
+
+  beforeEach(() => {
+    const di = getDiForUnitTesting({ doGeneralOverrides: true });
+
+    render = renderFor(di);
+  });
+
   it("should render w/o errors", () => {
     const policy = new NetworkPolicy({
       metadata: {} as never,

src/renderer/components/cluster-settings/components/__tests__/cluster-local-terminal-settings.test.tsx

@@ -4,28 +4,38 @@
  */
 
 import React from "react";
-import { render, waitFor } from "@testing-library/react";
+import { waitFor } from "@testing-library/react";
 import { ClusterLocalTerminalSetting } from "../cluster-local-terminal-settings";
 import userEvent from "@testing-library/user-event";
-import { stat } from "fs/promises";
-import { Notifications } from "../../../notifications";
 import type { Stats } from "fs";
 import type { Cluster } from "../../../../../common/cluster/cluster";
-
-const mockStat = stat as jest.MockedFunction<typeof stat>;
-
-jest.mock("fs", () => {
-  const actual = jest.requireActual("fs");
-
-  actual.promises.stat = jest.fn();
-
-  return actual;
-});
-
-jest.mock("../../../notifications");
+import { getDiForUnitTesting } from "../../../../getDiForUnitTesting";
+import type { DiRender } from "../../../test-utils/renderFor";
+import { renderFor } from "../../../test-utils/renderFor";
+import showErrorNotificationInjectable from "../../../notifications/show-error-notification.injectable";
+import statInjectable from "../../../../../common/fs/stat/stat.injectable";
 
 describe("ClusterLocalTerminalSettings", () => {
+  let render: DiRender;
+  let showErrorNotificationMock: jest.Mock;
+  let statMock: jest.Mock;
+
   beforeEach(() => {
+    const di = getDiForUnitTesting({ doGeneralOverrides: true });
+
+    showErrorNotificationMock = jest.fn();
+
+    statMock = jest.fn();
+
+    di.override(statInjectable, () => statMock);
+
+    di.override(
+      showErrorNotificationInjectable,
+      () => showErrorNotificationMock,
+    );
+
+    render = renderFor(di);
+
     jest.resetAllMocks();
   });
 
@@ -89,7 +99,7 @@ describe("ClusterLocalTerminalSettings", () => {
   });
 
   it("should save the new CWD if path is a directory", async () => {
-    mockStat.mockImplementation(async (path) => {
+    statMock.mockImplementation(async (path) => {
       expect(path).toBe("/foobar");
 
       return {
@@ -114,7 +124,7 @@ describe("ClusterLocalTerminalSettings", () => {
   });
 
   it("should not save the new CWD if path is a file", async () => {
-    mockStat.mockImplementation(async (path) => {
+    statMock.mockImplementation(async (path) => {
       expect(path).toBe("/foobar");
 
       return {
@@ -136,6 +146,6 @@ describe("ClusterLocalTerminalSettings", () => {
     userEvent.type(dn, "/foobar");
     userEvent.click(dom.baseElement);
 
-    await waitFor(() => expect(Notifications.error).toBeCalled());
+    await waitFor(() => expect(showErrorNotificationMock).toHaveBeenCalled());
   });
 });

src/renderer/components/cluster-settings/components/cluster-local-terminal-settings.tsx

@@ -8,79 +8,25 @@ import { observer } from "mobx-react";
 import type { Cluster } from "../../../../common/cluster/cluster";
 import { Input } from "../../input";
 import { SubTitle } from "../../layout/sub-title";
-import { stat } from "fs/promises";
-import { Notifications } from "../../notifications";
-import { isErrnoException, resolveTilde } from "../../../utils";
+import type { ShowNotification } from "../../notifications";
+import { resolveTilde } from "../../../utils";
 import { Icon } from "../../icon";
 import { PathPicker } from "../../path-picker";
 import { isWindows } from "../../../../common/vars";
-import type { Stats } from "fs";
-import logger from "../../../../common/logger";
-import { lowerFirst } from "lodash";
+import { withInjectables } from "@ogre-tools/injectable-react";
+import showErrorNotificationInjectable from "../../notifications/show-error-notification.injectable";
+import type { ValidateDirectory } from "../../../../common/fs/validate-directory.injectable";
+import validateDirectoryInjectable from "../../../../common/fs/validate-directory.injectable";
 
 export interface ClusterLocalTerminalSettingProps {
   cluster: Cluster;
 }
-
-function getUserReadableFileType(stats: Stats): string {
-  if (stats.isFile()) {
-    return "a file";
-  }
-
-  if (stats.isFIFO()) {
-    return "a pipe";
-  }
-
-  if (stats.isSocket()) {
-    return "a socket";
-  }
-
-  if (stats.isBlockDevice()) {
-    return "a block device";
-  }
-
-  if (stats.isCharacterDevice()) {
-    return "a character device";
-  }
-
-  return "an unknown file type";
+interface Dependencies {
+  showErrorNotification: ShowNotification;
+  validateDirectory: ValidateDirectory;
 }
 
-/**
- * Validate that `dir` currently points to a directory. If so return `false`.
- * Otherwise, return a user readable error message string for displaying.
- * @param dir The path to be validated
- */
-async function validateDirectory(dir: string): Promise<string | false> {
-  try {
-    const stats = await stat(dir);
-
-    if (stats.isDirectory()) {
-      return false;
-    }
-
-    return `the provided path is ${getUserReadableFileType(stats)} and not a directory.`;
-  } catch (error) {
-    switch (isErrnoException(error) ? error.code : undefined) {
-      case "ENOENT":
-        return `the provided path does not exist.`;
-      case "EACCES":
-        return `search permissions is denied for one of the directories in the prefix of the provided path.`;
-      case "ELOOP":
-        return `the provided path is a sym-link which points to a chain of sym-links that is too long to resolve. Perhaps it is cyclic.`;
-      case "ENAMETOOLONG":
-        return `the pathname is too long to be used.`;
-      case "ENOTDIR":
-        return `a prefix of the provided path is not a directory.`;
-      default:
-        logger.warn(`[CLUSTER-LOCAL-TERMINAL-SETTINGS]: unexpected error in validateDirectory for resolved path=${dir}`, error);
-
-        return error ? lowerFirst(String(error)) : "of an unknown error, please try again.";
-    }
-  }
-}
-
-export const ClusterLocalTerminalSetting = observer(({ cluster }: ClusterLocalTerminalSettingProps) => {
+const NonInjectedClusterLocalTerminalSetting = observer(({ cluster, showErrorNotification, validateDirectory }: Dependencies & ClusterLocalTerminalSettingProps) => {
   if (!cluster) {
     return null;
   }
@@ -109,15 +55,15 @@ export const ClusterLocalTerminalSetting = observer(({ cluster }: ClusterLocalTe
       cluster.preferences.terminalCWD = undefined;
     } else {
       const dir = resolveTilde(directory);
-      const errorMessage = await validateDirectory(dir);
+      const result = await validateDirectory(dir);
 
-      if (errorMessage) {
-        Notifications.error(
+      if (!result.callWasSuccessful) {
+        showErrorNotification(
           <>
             <b>Terminal Working Directory</b>
             <p>
               {"Your changes were not saved because "}
-              {errorMessage}
+              {result.error}
             </p>
           </>,
         );
@@ -200,3 +146,16 @@ export const ClusterLocalTerminalSetting = observer(({ cluster }: ClusterLocalTe
     </>
   );
 });
+
+export const ClusterLocalTerminalSetting = withInjectables<Dependencies, ClusterLocalTerminalSettingProps>(
+  NonInjectedClusterLocalTerminalSetting,
+
+  {
+    getProps: (di, props) => ({
+      showErrorNotification: di.inject(showErrorNotificationInjectable),
+      validateDirectory: di.inject(validateDirectoryInjectable),
+      ...props,
+    }),
+  },
+);
+

src/renderer/components/kube-object-menu/__snapshots__/kube-object-menu.test.tsx.snap

@@ -19,7 +19,6 @@ exports[`kube-object-menu given kube object renders 1`] = `
           <i
             class="Icon material interactive focusable"
             tabindex="0"
-            tooltip="Delete"
           >
             <span
               class="icon"
@@ -28,6 +27,9 @@ exports[`kube-object-menu given kube object renders 1`] = `
               delete
             </span>
           </i>
+          <div>
+            Delete
+          </div>
           <span
             class="title"
           >
@@ -59,7 +61,6 @@ exports[`kube-object-menu given kube object when removing kube object renders 1`
           <i
             class="Icon material interactive focusable"
             tabindex="0"
-            tooltip="Delete"
           >
             <span
               class="icon"
@@ -68,6 +69,9 @@ exports[`kube-object-menu given kube object when removing kube object renders 1`
               delete
             </span>
           </i>
+          <div>
+            Delete
+          </div>
           <span
             class="title"
           >
@@ -148,7 +152,6 @@ exports[`kube-object-menu given kube object when rerendered with different kube
           <i
             class="Icon material interactive focusable"
             tabindex="0"
-            tooltip="Delete"
           >
             <span
               class="icon"
@@ -157,6 +160,9 @@ exports[`kube-object-menu given kube object when rerendered with different kube
               delete
             </span>
           </i>
+          <div>
+            Delete
+          </div>
           <span
             class="title"
           >
@@ -185,7 +191,6 @@ exports[`kube-object-menu given kube object when rerendered with different kube
           <i
             class="Icon material interactive focusable"
             tabindex="0"
-            tooltip="Delete"
           >
             <span
               class="icon"
@@ -194,6 +199,9 @@ exports[`kube-object-menu given kube object when rerendered with different kube
               delete
             </span>
           </i>
+          <div>
+            Delete
+          </div>
           <span
             class="title"
           >
@@ -277,7 +285,6 @@ exports[`kube-object-menu given kube object with namespace when removing kube ob
           <i
             class="Icon material interactive focusable"
             tabindex="0"
-            tooltip="Delete"
           >
             <span
               class="icon"
@@ -286,6 +293,9 @@ exports[`kube-object-menu given kube object with namespace when removing kube ob
               delete
             </span>
           </i>
+          <div>
+            Delete
+          </div>
           <span
             class="title"
           >
@@ -369,7 +379,6 @@ exports[`kube-object-menu given kube object without namespace when removing kube
           <i
             class="Icon material interactive focusable"
             tabindex="0"
-            tooltip="Delete"
           >
             <span
               class="icon"
@@ -378,6 +387,9 @@ exports[`kube-object-menu given kube object without namespace when removing kube
               delete
             </span>
           </i>
+          <div>
+            Delete
+          </div>
           <span
             class="title"
           >

src/renderer/components/kube-object-menu/kube-object-menu.test.tsx

@@ -26,12 +26,6 @@ import createEditResourceTabInjectable from "../dock/edit-resource/edit-resource
 import hideDetailsInjectable from "../kube-detail-params/hide-details.injectable";
 import { kubeObjectMenuItemInjectionToken } from "./kube-object-menu-item-injection-token";
 
-// TODO: Make tooltips free of side effects by making it deterministic
-jest.mock("../tooltip/tooltip");
-jest.mock("../tooltip/withTooltip", () => ({
-  withTooltip: (target: any) => target,
-}));
-
 // TODO: make `animated={false}` not required to make tests deterministic
 describe("kube-object-menu", () => {
   let di: DiContainer;