Technical requirements for behavioural unit tests (#5084)

* Implement a lot of technical requirements for behavioural unit tests

Note: the crux of this was to make routing env-agnostic, and not based on URLs as magic strings, but instead something type-enforced.

Note: extension-based routes comply to same exact interface by "late-registering" their routes when installed. Routes are just injectables.

Note: another chunk of global shared state is no more.

Note: a lot of explicit side effects have been cornered to injectables.

Note: a lot of stuff has become reactive as part if this.

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

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

* Make a directory commonly available

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Require id for <Select /> to prevent non-deterministic renders

This was caused by global state in a 3rd party lib: "react-select".

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Specify id for all <Select /> to satisfy previous commit

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Prevent explicit side effect in component by using existing dependency instead

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Extract instantiation of "conf" as injectables for causing side effects

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Introduce a legacy-helper to make gradual refactoring of inheritors of Singleton easier

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Make legacy unit tests for hotbar green and more simple by using the new legacy helper

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Temporarily kludge all unit tests green with a disclaimer about allowing side-effects

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Remove kludge in previous commit by explicitly permitting specific side effects where old unit tests require it

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Prevent old unit test with side effects from accessing file system

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Migrate to actual typing for di.permitSideEffects

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Prevent unit tests from failing because of non-standard method of HTML-element not present in js-dom

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Adapt integration tests to recent changes

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Fix code style

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Fix artifact from bad rebase

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Add a deprecation from a review comment

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Remove change that is not required

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

* Remove redundant comment

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

* Fix code style

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

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

* Remove redundant file

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

* Fix bad merge

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Improve variable name

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Tweak logger interface to be more descriptive

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

* Make injecting legacy singleton always provide new instance

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

* Remove conditional typing when not needed

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

* Improve naming of variable

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

* Remove unnecessary code style changes

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

* Remove flag for causing side effects from too broad scope

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

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

* Override side-effects in unit test using injectable instead of monkey patching

Co-authored-by: Janne Savolainen <janne.savolainen@live.fi>

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Flag some side-effects and add general overrides

Co-authored-by: Janne Savolainen <janne.savolainen@live.fi>

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Fix unit tests in CI by removing explicit side-effect

Co-authored-by: Janne Savolainen <janne.savolainen@live.fi>

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Remove explicit side-effect from getting default shell

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

* Introduce abstraction for getting absolute paths

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

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

* Switch to using abstraction for getting absolute path to control explicit side effect

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

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

* Introduce abstraction for joining paths

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

* Switch to using abstraction for joining paths to control explicit side effect

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

* Fix fake implementation for join paths

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

* Fix test after removing explicit side effect

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

* Remove explicit side effects from kubeconfig-syncs

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

* Fix arguments after removing explicit side effect

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

* Make registrators not async for not being needed anymore

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

* Make generalCatalogEntities non-observable, as there is no requirement

Co-authored-by: Janne Savolainen <janne.savolainen@live.fi>

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Remove redundant code

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

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Simplify logic for registering general catalog entity sources

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

* Add TODO

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

* Replace function for getting application menu items with reactive solution

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

* Fix typo in interface name

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

* Remove global shared state usages of hot bar store

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

* Remove redundant enum

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

Co-authored-by: Janne Savolainen <janne.savolainen@live.fi>

src/renderer/components/delete-cluster-dialog/__tests__/delete-cluster-dialog.test.tsx

@@ -4,7 +4,7 @@
  */
 import "@testing-library/jest-dom/extend-expect";
 import { KubeConfig } from "@kubernetes/client-node";
-import { fireEvent, render } from "@testing-library/react";
+import { fireEvent } from "@testing-library/react";
 import mockFs from "mock-fs";
 import React from "react";
 import * as selectEvent from "react-select-event";
@@ -16,6 +16,11 @@ import type { ClusterModel } from "../../../../common/cluster-types";
 import { getDisForUnitTesting } from "../../../../test-utils/get-dis-for-unit-testing";
 import { createClusterInjectionToken } from "../../../../common/cluster/create-cluster-injection-token";
 import createContextHandlerInjectable from "../../../../main/context-handler/create-context-handler.injectable";
+import deleteClusterDialogModelInjectable from "../delete-cluster-dialog-model/delete-cluster-dialog-model.injectable";
+import type { DeleteClusterDialogModel } from "../delete-cluster-dialog-model/delete-cluster-dialog-model";
+import type { DiRender } from "../../test-utils/renderFor";
+import { renderFor } from "../../test-utils/renderFor";
+import hotbarStoreInjectable from "../../../../common/hotbar-store.injectable";
 
 jest.mock("electron", () => ({
   app: {
@@ -88,16 +93,23 @@ let config: KubeConfig;
 
 describe("<DeleteClusterDialog />", () => {
   let createCluster: (model: ClusterModel) => Cluster;
+  let deleteClusterDialogModel: DeleteClusterDialogModel;
+  let render: DiRender;
 
   beforeEach(async () => {
-    const { mainDi, runSetups } = getDisForUnitTesting({ doGeneralOverrides: true });
+    const { mainDi, rendererDi, runSetups } = getDisForUnitTesting({ doGeneralOverrides: true });
+
+    render = renderFor(rendererDi);
 
     mainDi.override(createContextHandlerInjectable, () => () => undefined);
 
     mockFs();
 
+    rendererDi.override(hotbarStoreInjectable, () => ({}));
+
     await runSetups();
 
+    deleteClusterDialogModel = rendererDi.inject(deleteClusterDialogModelInjectable);
     createCluster = mainDi.inject(createClusterInjectionToken);
   });
 
@@ -137,7 +149,7 @@ describe("<DeleteClusterDialog />", () => {
         kubeConfigPath: "./temp-kube-config",
       });
 
-      DeleteClusterDialog.open({ cluster, config });
+      deleteClusterDialogModel.open({ cluster, config });
       const { getByText } = render(<DeleteClusterDialog />);
 
       const message = "The contents of kubeconfig file will be changed!";
@@ -155,7 +167,7 @@ describe("<DeleteClusterDialog />", () => {
         kubeConfigPath: "./temp-kube-config",
       });
 
-      DeleteClusterDialog.open({ cluster, config });
+      deleteClusterDialogModel.open({ cluster, config });
 
       const { getByTestId } = render(<DeleteClusterDialog />);
 
@@ -172,7 +184,7 @@ describe("<DeleteClusterDialog />", () => {
         kubeConfigPath: "./temp-kube-config",
       });
 
-      DeleteClusterDialog.open({ cluster, config });
+      deleteClusterDialogModel.open({ cluster, config });
 
       const { getByText } = render(<DeleteClusterDialog />);
 
@@ -193,7 +205,7 @@ describe("<DeleteClusterDialog />", () => {
         kubeConfigPath: "./temp-kube-config",
       });
 
-      DeleteClusterDialog.open({ cluster, config });
+      deleteClusterDialogModel.open({ cluster, config });
 
       const { getByText, getByTestId } = render(<DeleteClusterDialog />);
       const link = getByTestId("context-switch");
@@ -220,7 +232,7 @@ describe("<DeleteClusterDialog />", () => {
 
       const spy = jest.spyOn(cluster, "isInLocalKubeconfig").mockImplementation(() => true);
 
-      DeleteClusterDialog.open({ cluster, config });
+      deleteClusterDialogModel.open({ cluster, config });
 
       const { getByTestId } = render(<DeleteClusterDialog />);
 
@@ -256,7 +268,7 @@ describe("<DeleteClusterDialog />", () => {
         kubeConfigPath: "./temp-kube-config",
       });
 
-      DeleteClusterDialog.open({ cluster, config });
+      deleteClusterDialogModel.open({ cluster, config });
 
       const { getByTestId } = render(<DeleteClusterDialog />);
 

src/renderer/components/delete-cluster-dialog/delete-cluster-dialog-model/delete-cluster-dialog-model.injectable.ts

@@ -0,0 +1,13 @@
+/**
+ * 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 { DeleteClusterDialogModel } from "./delete-cluster-dialog-model";
+
+const deleteClusterDialogModelInjectable = getInjectable({
+  id: "delete-cluster-dialog-model",
+  instantiate: () => new DeleteClusterDialogModel(),
+});
+
+export default deleteClusterDialogModelInjectable;

src/renderer/components/delete-cluster-dialog/delete-cluster-dialog-model/delete-cluster-dialog-model.ts

@@ -0,0 +1,33 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import type { KubeConfig } from "@kubernetes/client-node";
+import { observable, makeObservable, action } from "mobx";
+import type { Cluster } from "../../../../common/cluster/cluster";
+
+export class DeleteClusterDialogModel {
+  isOpen = false;
+
+  constructor() {
+    makeObservable(this, {
+      isOpen: observable,
+      open: action,
+      close: action,
+    });
+  }
+
+  cluster: Cluster;
+  config: KubeConfig;
+
+  open = ({ cluster, config }: { cluster: Cluster; config: KubeConfig }) => {
+    this.isOpen = true;
+
+    this.cluster = cluster;
+    this.config = config;
+  };
+
+  close = () => {
+    this.isOpen = false;
+  };
+}

src/renderer/components/delete-cluster-dialog/delete-cluster-dialog.tsx

@@ -9,52 +9,35 @@ import { observer } from "mobx-react";
 import React from "react";
 
 import { Button } from "../button";
-import type { KubeConfig } from "@kubernetes/client-node";
-import type { Cluster } from "../../../common/cluster/cluster";
 import { saveKubeconfig } from "./save-config";
 import { Notifications } from "../notifications";
-import { HotbarStore } from "../../../common/hotbar-store";
 import { boundMethod } from "autobind-decorator";
 import { Dialog } from "../dialog";
 import { Icon } from "../icon";
 import { Select } from "../select";
 import { Checkbox } from "../checkbox";
 import { requestClearClusterAsDeleting, requestDeleteCluster, requestSetClusterAsDeleting } from "../../ipc";
+import { withInjectables } from "@ogre-tools/injectable-react";
+import hotbarStoreInjectable from "../../../common/hotbar-store.injectable";
+import type { DeleteClusterDialogModel } from "./delete-cluster-dialog-model/delete-cluster-dialog-model";
+import deleteClusterDialogModelInjectable from "./delete-cluster-dialog-model/delete-cluster-dialog-model.injectable";
+import type { HotbarStore } from "../../../common/hotbar-store";
 
-interface DialogState {
-  isOpen: boolean;
-  config?: KubeConfig;
-  cluster?: Cluster;
+interface Dependencies {
+  hotbarStore: HotbarStore;
+  model: DeleteClusterDialogModel;
 }
 
-const dialogState: DialogState = observable({
-  isOpen: false,
-});
-
-export interface DeleteClusterDialogProps {}
-
 @observer
-export class DeleteClusterDialog extends React.Component<DeleteClusterDialogProps> {
+class NonInjectedDeleteClusterDialog extends React.Component<Dependencies> {
   @observable showContextSwitch = false;
   @observable newCurrentContext = "";
 
-  constructor(props: DeleteClusterDialogProps) {
+  constructor(props: Dependencies) {
     super(props);
     makeObservable(this);
   }
 
-  static open({ config, cluster }: Partial<DialogState>) {
-    dialogState.isOpen = true;
-    dialogState.config = config;
-    dialogState.cluster = cluster;
-  }
-
-  static close() {
-    dialogState.isOpen = false;
-    dialogState.cluster = null;
-    dialogState.config = null;
-  }
-
   @boundMethod
   onOpen() {
     this.newCurrentContext = "";
@@ -66,25 +49,25 @@ export class DeleteClusterDialog extends React.Component<DeleteClusterDialogProp
 
   @boundMethod
   onClose() {
-    DeleteClusterDialog.close();
+    this.props.model.close();
     this.showContextSwitch = false;
   }
 
   removeContext() {
-    dialogState.config.contexts = dialogState.config.contexts.filter(item =>
-      item.name !== dialogState.cluster.contextName,
+    this.props.model.config.contexts = this.props.model.config.contexts.filter(item =>
+      item.name !== this.props.model.cluster.contextName,
     );
   }
 
   changeCurrentContext() {
     if (this.newCurrentContext && this.showContextSwitch) {
-      dialogState.config.currentContext = this.newCurrentContext;
+      this.props.model.config.currentContext = this.newCurrentContext;
     }
   }
 
   @boundMethod
   async onDelete() {
-    const { cluster, config } = dialogState;
+    const { cluster, config } = this.props.model;
 
     await requestSetClusterAsDeleting(cluster.id);
     this.removeContext();
@@ -92,7 +75,7 @@ export class DeleteClusterDialog extends React.Component<DeleteClusterDialogProp
 
     try {
       await saveKubeconfig(config, cluster.kubeConfigPath);
-      HotbarStore.getInstance().removeAllHotbarItems(cluster.id);
+      this.props.hotbarStore.removeAllHotbarItems(cluster.id);
       await requestDeleteCluster(cluster.id);
     } catch(error) {
       Notifications.error(`Cannot remove cluster, failed to process config file. ${error}`);
@@ -104,7 +87,7 @@ export class DeleteClusterDialog extends React.Component<DeleteClusterDialogProp
   }
 
   @computed get disableDelete() {
-    const { cluster, config } = dialogState;
+    const { cluster, config } = this.props.model;
     const noContextsAvailable = config.contexts.filter(context => context.name !== cluster.contextName).length == 0;
     const newContextNotSelected = this.newCurrentContext === "";
 
@@ -116,12 +99,12 @@ export class DeleteClusterDialog extends React.Component<DeleteClusterDialogProp
   }
 
   isCurrentContext() {
-    return dialogState.config.currentContext == dialogState.cluster.contextName;
+    return this.props.model.config.currentContext == this.props.model.cluster.contextName;
   }
 
   renderCurrentContextSwitch() {
     if (!this.showContextSwitch) return null;
-    const { cluster, config } = dialogState;
+    const { cluster, config } = this.props.model;
     const contexts = config.contexts.filter(context => context.name !== cluster.contextName);
 
     const options = [
@@ -134,6 +117,7 @@ export class DeleteClusterDialog extends React.Component<DeleteClusterDialogProp
     return (
       <div className="mt-4">
         <Select
+          id="delete-cluster-input"
           options={options}
           value={this.newCurrentContext}
           onChange={({ value }) => this.newCurrentContext = value}
@@ -145,7 +129,7 @@ export class DeleteClusterDialog extends React.Component<DeleteClusterDialogProp
   }
 
   renderDeleteMessage() {
-    const { cluster } = dialogState;
+    const { cluster } = this.props.model;
 
     if (cluster.isInLocalKubeconfig()) {
       return (
@@ -163,7 +147,7 @@ export class DeleteClusterDialog extends React.Component<DeleteClusterDialogProp
   }
 
   getWarningMessage() {
-    const { cluster, config } = dialogState;
+    const { cluster, config } = this.props.model;
     const contexts = config.contexts.filter(context => context.name !== cluster.contextName);
 
     if (!contexts.length) {
@@ -205,7 +189,7 @@ export class DeleteClusterDialog extends React.Component<DeleteClusterDialogProp
   }
 
   render() {
-    const { cluster, config, isOpen } = dialogState;
+    const { cluster, config, isOpen } = this.props.model;
 
     if (!cluster || !config) return null;
 
@@ -257,3 +241,14 @@ export class DeleteClusterDialog extends React.Component<DeleteClusterDialogProp
     );
   }
 }
+
+export const DeleteClusterDialog = withInjectables<Dependencies>(
+  NonInjectedDeleteClusterDialog,
+
+  {
+    getProps: (di) => ({
+      hotbarStore: di.inject(hotbarStoreInjectable),
+      model: di.inject(deleteClusterDialogModelInjectable),
+    }),
+  },
+);