From 68837b7c272f3a0fccf9a15080827b499ffad53a Mon Sep 17 00:00:00 2001 From: Janne Savolainen Date: Mon, 27 Jun 2022 15:22:02 +0300 Subject: [PATCH] Make root frame child components comply with open closed principle and include it in the behavioural unit tests Signed-off-by: Janne Savolainen --- ...navigation-using-application-menu.test.tsx | 4 - .../navigation-using-application-menu.test.ts | 34 ++++++-- src/main/getDiForUnitTesting.ts | 9 +- ...-root-frame-child-component.injectable.tsx | 30 +++++++ ...r-root-frame-child-component.injectable.ts | 24 ++++++ ...g-root-frame-child-component.injectable.ts | 22 +++++ .../__tests__/delete-cluster-dialog.test.tsx | 3 - ...s-root-frame-child-component.injectable.ts | 22 +++++ .../test-utils/get-application-builder.tsx | 85 +++++++++---------- ...t-frame-child-component-injection-token.ts | 17 ++++ src/renderer/frames/root-frame/root-frame.tsx | 24 +++--- src/renderer/getDiForUnitTesting.tsx | 16 +++- 12 files changed, 212 insertions(+), 78 deletions(-) create mode 100644 src/renderer/components/cluster-manager/cluster-manager-root-frame-child-component.injectable.tsx create mode 100644 src/renderer/components/command-palette/command-container-root-frame-child-component.injectable.ts create mode 100644 src/renderer/components/confirm-dialog/confirm-dialog-root-frame-child-component.injectable.ts create mode 100644 src/renderer/components/notifications/notifications-root-frame-child-component.injectable.ts create mode 100644 src/renderer/frames/root-frame/root-frame-child-component-injection-token.ts diff --git a/src/behaviours/add-cluster/navigation-using-application-menu.test.tsx b/src/behaviours/add-cluster/navigation-using-application-menu.test.tsx index bb68918c1a..334dd193a6 100644 --- a/src/behaviours/add-cluster/navigation-using-application-menu.test.tsx +++ b/src/behaviours/add-cluster/navigation-using-application-menu.test.tsx @@ -9,10 +9,6 @@ import { getApplicationBuilder } from "../../renderer/components/test-utils/get- import React from "react"; // TODO: Make components free of side effects by making them deterministic -jest.mock("../../renderer/components/tooltip/tooltip", () => ({ - Tooltip: () => null, -})); - jest.mock("../../renderer/components/tooltip/withTooltip", () => ({ withTooltip: (Target: any) => ({ tooltip, tooltipOverrideDisabled, ...props }: any) => , })); diff --git a/src/behaviours/welcome/navigation-using-application-menu.test.ts b/src/behaviours/welcome/navigation-using-application-menu.test.ts index 805e99efb4..1be9aa1d3a 100644 --- a/src/behaviours/welcome/navigation-using-application-menu.test.ts +++ b/src/behaviours/welcome/navigation-using-application-menu.test.ts @@ -21,25 +21,41 @@ describe("welcome - navigation using application menu", () => { expect(rendered.container).toMatchSnapshot(); }); - it("does not show welcome page yet", () => { - const actual = rendered.queryByTestId("welcome-page"); + it("shows welcome page being front page", () => { + const actual = rendered.getByTestId("welcome-page"); - expect(actual).toBeNull(); + expect(actual).not.toBeNull(); }); - describe("when navigating to welcome using application menu", () => { + describe("when navigated somewhere else", () => { beforeEach(() => { - applicationBuilder.applicationMenu.click("help.welcome"); + applicationBuilder.applicationMenu.click("root.preferences"); }); it("renders", () => { - expect(rendered.container).toMatchSnapshot(); + expect(rendered.baseElement).toMatchSnapshot(); }); - it("shows welcome page", () => { - const actual = rendered.getByTestId("welcome-page"); + it("does not show welcome page", () => { + const actual = rendered.queryByTestId("welcome-page"); - expect(actual).not.toBeNull(); + expect(actual).toBeNull(); + }); + + describe("when navigated to welcome using application menu", () => { + beforeEach(() => { + applicationBuilder.applicationMenu.click("help.welcome"); + }); + + it("renders", () => { + expect(rendered.container).toMatchSnapshot(); + }); + + it("shows welcome page", () => { + const actual = rendered.getByTestId("welcome-page"); + + expect(actual).not.toBeNull(); + }); }); }); }); diff --git a/src/main/getDiForUnitTesting.ts b/src/main/getDiForUnitTesting.ts index c09142d83d..61d1f0e2d8 100644 --- a/src/main/getDiForUnitTesting.ts +++ b/src/main/getDiForUnitTesting.ts @@ -99,6 +99,7 @@ import updateHelmReleaseInjectable from "./helm/helm-service/update-helm-release import waitUntilBundledExtensionsAreLoadedInjectable from "./start-main-application/lens-window/application-window/wait-until-bundled-extensions-are-loaded.injectable"; import { registerMobX } from "@ogre-tools/injectable-extension-for-mobx"; import electronInjectable from "./utils/resolve-system-proxy/electron.injectable"; +import type { HotbarStore } from "../common/hotbars/store"; export function getDiForUnitTesting(opts: { doGeneralOverrides?: boolean } = {}) { const { @@ -127,7 +128,13 @@ export function getDiForUnitTesting(opts: { doGeneralOverrides?: boolean } = {}) di.override(electronInjectable, () => ({})); di.override(waitUntilBundledExtensionsAreLoadedInjectable, () => async () => {}); di.override(getRandomIdInjectable, () => () => "some-irrelevant-random-id"); - di.override(hotbarStoreInjectable, () => ({ load: () => {} })); + + di.override(hotbarStoreInjectable, () => ({ + load: () => {}, + getActive: () => ({ name: "some-hotbar", items: [] }), + getDisplayIndex: () => "0", + }) as unknown as HotbarStore); + di.override(userStoreInjectable, () => ({ startMainReactions: () => {}, extensionRegistryUrl: { customUrl: "some-custom-url" }}) as UserStore); di.override(extensionsStoreInjectable, () => ({ isEnabled: (opts) => (void opts, false) }) as ExtensionsStore); di.override(clusterStoreInjectable, () => ({ provideInitialFromMain: () => {}, getById: (id) => (void id, {}) as Cluster }) as ClusterStore); diff --git a/src/renderer/components/cluster-manager/cluster-manager-root-frame-child-component.injectable.tsx b/src/renderer/components/cluster-manager/cluster-manager-root-frame-child-component.injectable.tsx new file mode 100644 index 0000000000..9db066e1ad --- /dev/null +++ b/src/renderer/components/cluster-manager/cluster-manager-root-frame-child-component.injectable.tsx @@ -0,0 +1,30 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import React from "react"; +import { getInjectable } from "@ogre-tools/injectable"; +import { rootFrameChildComponentInjectionToken } from "../../frames/root-frame/root-frame-child-component-injection-token"; +import { ClusterManager } from "./cluster-manager"; +import { computed } from "mobx"; +import { ErrorBoundary } from "../error-boundary"; + +const clusterManagerRootFrameChildComponentInjectable = getInjectable({ + id: "cluster-manager-root-frame-child-component", + + instantiate: () => ({ + id: "cluster-manager", + + shouldRender: computed(() => true), + + Component: () => ( + + + + ), + }), + + injectionToken: rootFrameChildComponentInjectionToken, +}); + +export default clusterManagerRootFrameChildComponentInjectable; diff --git a/src/renderer/components/command-palette/command-container-root-frame-child-component.injectable.ts b/src/renderer/components/command-palette/command-container-root-frame-child-component.injectable.ts new file mode 100644 index 0000000000..f6c5105cc3 --- /dev/null +++ b/src/renderer/components/command-palette/command-container-root-frame-child-component.injectable.ts @@ -0,0 +1,24 @@ +/** + * 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 { rootFrameChildComponentInjectionToken } from "../../frames/root-frame/root-frame-child-component-injection-token"; +import { computed } from "mobx"; +import { CommandContainer } from "./command-container"; + +const commandContainerRootFrameChildComponentInjectable = getInjectable({ + id: "command-container-root-frame-child-component", + + instantiate: () => ({ + id: "command-container", + shouldRender: computed(() => true), + Component: CommandContainer, + }), + + causesSideEffects: true, + + injectionToken: rootFrameChildComponentInjectionToken, +}); + +export default commandContainerRootFrameChildComponentInjectable; diff --git a/src/renderer/components/confirm-dialog/confirm-dialog-root-frame-child-component.injectable.ts b/src/renderer/components/confirm-dialog/confirm-dialog-root-frame-child-component.injectable.ts new file mode 100644 index 0000000000..0132040818 --- /dev/null +++ b/src/renderer/components/confirm-dialog/confirm-dialog-root-frame-child-component.injectable.ts @@ -0,0 +1,22 @@ +/** + * 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 { rootFrameChildComponentInjectionToken } from "../../frames/root-frame/root-frame-child-component-injection-token"; +import { computed } from "mobx"; +import { ConfirmDialog } from "./confirm-dialog"; + +const confirmDialogRootFrameChildComponentInjectable = getInjectable({ + id: "confirm-dialog-root-frame-child-component", + + instantiate: () => ({ + id: "confirm-dialog", + shouldRender: computed(() => true), + Component: ConfirmDialog, + }), + + injectionToken: rootFrameChildComponentInjectionToken, +}); + +export default confirmDialogRootFrameChildComponentInjectable; diff --git a/src/renderer/components/delete-cluster-dialog/__tests__/delete-cluster-dialog.test.tsx b/src/renderer/components/delete-cluster-dialog/__tests__/delete-cluster-dialog.test.tsx index 09bf3a0700..5b27db3fa1 100644 --- a/src/renderer/components/delete-cluster-dialog/__tests__/delete-cluster-dialog.test.tsx +++ b/src/renderer/components/delete-cluster-dialog/__tests__/delete-cluster-dialog.test.tsx @@ -24,7 +24,6 @@ import { getInjectable } from "@ogre-tools/injectable"; import { computed } from "mobx"; import { routeSpecificComponentInjectionToken } from "../../../routes/route-specific-component-injection-token"; import { navigateToRouteInjectionToken } from "../../../../common/front-end-routing/navigate-to-route-injection-token"; -import hotbarStoreInjectable from "../../../../common/hotbars/store.injectable"; import normalizedPlatformInjectable from "../../../../common/vars/normalized-platform.injectable"; import kubectlBinaryNameInjectable from "../../../../main/kubectl/binary-name.injectable"; import kubectlDownloadingNormalizedArchInjectable from "../../../../main/kubectl/normalized-arch.injectable"; @@ -111,8 +110,6 @@ describe("", () => { mainDi.override(kubectlBinaryNameInjectable, () => "kubectl"); mainDi.override(kubectlDownloadingNormalizedArchInjectable, () => "amd64"); mainDi.override(normalizedPlatformInjectable, () => "darwin"); - - rendererDi.override(hotbarStoreInjectable, () => ({})); rendererDi.override(storesAndApisCanBeCreatedInjectable, () => true); }); diff --git a/src/renderer/components/notifications/notifications-root-frame-child-component.injectable.ts b/src/renderer/components/notifications/notifications-root-frame-child-component.injectable.ts new file mode 100644 index 0000000000..eec5a1962a --- /dev/null +++ b/src/renderer/components/notifications/notifications-root-frame-child-component.injectable.ts @@ -0,0 +1,22 @@ +/** + * 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 { rootFrameChildComponentInjectionToken } from "../../frames/root-frame/root-frame-child-component-injection-token"; +import { computed } from "mobx"; +import { Notifications } from "./notifications"; + +const notificationsRootFrameChildComponentInjectable = getInjectable({ + id: "notifications-root-frame-child-component", + + instantiate: () => ({ + id: "notifications", + shouldRender: computed(() => true), + Component: Notifications, + }), + + injectionToken: rootFrameChildComponentInjectionToken, +}); + +export default notificationsRootFrameChildComponentInjectable; diff --git a/src/renderer/components/test-utils/get-application-builder.tsx b/src/renderer/components/test-utils/get-application-builder.tsx index 25101ae634..ca288c01b6 100644 --- a/src/renderer/components/test-utils/get-application-builder.tsx +++ b/src/renderer/components/test-utils/get-application-builder.tsx @@ -7,7 +7,6 @@ import rendererExtensionsInjectable from "../../../extensions/renderer-extension import currentlyInClusterFrameInjectable from "../../routes/currently-in-cluster-frame.injectable"; import type { IObservableArray, ObservableSet } from "mobx"; import { computed, observable, runInAction } from "mobx"; -import { renderFor } from "./renderFor"; import React from "react"; import { Router } from "react-router"; import { Observer } from "mobx-react"; @@ -45,7 +44,6 @@ import type { MinimalTrayMenuItem } from "../../../main/tray/electron-tray/elect import electronTrayInjectable from "../../../main/tray/electron-tray/electron-tray.injectable"; import applicationWindowInjectable from "../../../main/start-main-application/lens-window/application-window/application-window.injectable"; import { Notifications } from "../notifications/notifications"; -import broadcastThatRootFrameIsRenderedInjectable from "../../frames/root-frame/broadcast-that-root-frame-is-rendered.injectable"; import { getDiForUnitTesting as getRendererDi } from "../../getDiForUnitTesting"; import { getDiForUnitTesting as getMainDi } from "../../../main/getDiForUnitTesting"; import { overrideChannels } from "../../../test-utils/channel-fakes/override-channels"; @@ -53,7 +51,6 @@ import trayIconPathsInjectable from "../../../main/tray/tray-icon-path.injectabl import assert from "assert"; import { openMenu } from "react-select-event"; import userEvent from "@testing-library/user-event"; -import { StatusBar } from "../status-bar/status-bar"; import lensProxyPortInjectable from "../../../main/lens-proxy/lens-proxy-port.injectable"; import type { Route } from "../../../common/front-end-routing/front-end-route-injection-token"; import type { NavigateToRouteOptions } from "../../../common/front-end-routing/navigate-to-route-injection-token"; @@ -62,7 +59,8 @@ import type { LensMainExtension } from "../../../extensions/lens-main-extension" import type { LensExtension } from "../../../extensions/lens-extension"; import extensionInjectable from "../../../extensions/extension-loader/extension/extension.injectable"; -import { TopBar } from "../layout/top-bar/top-bar"; +import { renderFor } from "./renderFor"; +import { RootFrame } from "../../frames/root-frame/root-frame"; type Callback = (dis: DiContainers) => void | Promise; @@ -125,10 +123,7 @@ interface DiContainers { } interface Environment { - renderSidebar: () => React.ReactNode; - renderTopBar: () => React.ReactNode; - renderStatusBar: () => React.ReactNode; - beforeRender: () => void; + render: () => RenderResult; onAllowKubeResource: () => void; } @@ -166,16 +161,16 @@ export const getApplicationBuilder = () => { const environments = { application: { - renderSidebar: () => null, + render: () => { + const history = rendererDi.inject(historyInjectable); - renderTopBar: () => , + const render = renderFor(rendererDi); - renderStatusBar: () => , - - beforeRender: () => { - const nofifyThatRootFrameIsRendered = rendererDi.inject(broadcastThatRootFrameIsRenderedInjectable); - - nofifyThatRootFrameIsRendered(); + return render( + + + , + ); }, onAllowKubeResource: () => { @@ -186,10 +181,33 @@ export const getApplicationBuilder = () => { } as Environment, clusterFrame: { - renderSidebar: () => , - renderStatusBar: () => null, - renderTopBar: () => null, - beforeRender: () => {}, + render: () => { + const currentRouteComponent = rendererDi.inject(currentRouteComponentInjectable); + const history = rendererDi.inject(historyInjectable); + + const render = renderFor(rendererDi); + + return render( + + + + + {() => { + const Component = currentRouteComponent.get(); + + if (!Component) { + return null; + } + + return ; + }} + + + + , + ); + }, + onAllowKubeResource: () => {}, } as Environment, }; @@ -474,37 +492,12 @@ export const getApplicationBuilder = () => { await startFrame(); - const render = renderFor(rendererDi); - const history = rendererDi.inject(historyInjectable); - const currentRouteComponent = rendererDi.inject(currentRouteComponentInjectable); for (const callback of beforeRenderCallbacks) { await callback(dis); } - environment.beforeRender(); - - rendered = render( - - {environment.renderSidebar()} - {environment.renderTopBar()} - {environment.renderStatusBar()} - - - {() => { - const Component = currentRouteComponent.get(); - - if (!Component) { - return null; - } - - return ; - }} - - - - , - ); + rendered = environment.render(); return rendered; }, diff --git a/src/renderer/frames/root-frame/root-frame-child-component-injection-token.ts b/src/renderer/frames/root-frame/root-frame-child-component-injection-token.ts new file mode 100644 index 0000000000..5f9aca7c38 --- /dev/null +++ b/src/renderer/frames/root-frame/root-frame-child-component-injection-token.ts @@ -0,0 +1,17 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import { getInjectionToken } from "@ogre-tools/injectable"; +import type { IComputedValue } from "mobx"; + +export interface RootFrameChildComponent { + id: string; + Component: React.ElementType; + shouldRender: IComputedValue; +} + +export const rootFrameChildComponentInjectionToken = getInjectionToken({ + id: "root-frame-child-component", +}); diff --git a/src/renderer/frames/root-frame/root-frame.tsx b/src/renderer/frames/root-frame/root-frame.tsx index 6cb4d016c5..f4abe41a3f 100644 --- a/src/renderer/frames/root-frame/root-frame.tsx +++ b/src/renderer/frames/root-frame/root-frame.tsx @@ -5,23 +5,20 @@ import { injectSystemCAs } from "../../../common/system-ca"; import React from "react"; -import { observer } from "mobx-react"; -import { ClusterManager } from "../../components/cluster-manager"; -import { ErrorBoundary } from "../../components/error-boundary"; -import { Notifications } from "../../components/notifications"; -import { ConfirmDialog } from "../../components/confirm-dialog"; -import { CommandContainer } from "../../components/command-palette/command-container"; +import { Observer } from "mobx-react"; import { withInjectables } from "@ogre-tools/injectable-react"; import broadcastThatRootFrameIsRenderedInjectable from "./broadcast-that-root-frame-is-rendered.injectable"; +import type { RootFrameChildComponent } from "./root-frame-child-component-injection-token"; +import { rootFrameChildComponentInjectionToken } from "./root-frame-child-component-injection-token"; // Todo: remove import-time side-effect. injectSystemCAs(); interface Dependencies { broadcastThatRootFrameIsRendered: () => void; + childComponents: RootFrameChildComponent[]; } -@observer class NonInjectedRootFrame extends React.Component { static displayName = "RootFrame"; @@ -32,12 +29,12 @@ class NonInjectedRootFrame extends React.Component { render() { return ( <> - - - - - - + {this.props.childComponents + .map((child) => ( + + {() => (child.shouldRender.get() ? : null) } + + ))} ); } @@ -49,6 +46,7 @@ export const RootFrame = withInjectables( { getProps: (di, props) => ({ broadcastThatRootFrameIsRendered: di.inject(broadcastThatRootFrameIsRenderedInjectable), + childComponents: di.injectMany(rootFrameChildComponentInjectionToken), ...props, }), }, diff --git a/src/renderer/getDiForUnitTesting.tsx b/src/renderer/getDiForUnitTesting.tsx index 8851629cd5..f0bac6ca91 100644 --- a/src/renderer/getDiForUnitTesting.tsx +++ b/src/renderer/getDiForUnitTesting.tsx @@ -41,7 +41,7 @@ import apiManagerInjectable from "../common/k8s-api/api-manager/manager.injectab import ipcRendererInjectable from "./utils/channel/ipc-renderer.injectable"; import type { IpcRenderer } from "electron"; import setupOnApiErrorListenersInjectable from "./api/setup-on-api-errors.injectable"; -import { observable } from "mobx"; +import { observable, computed } from "mobx"; import defaultShellInjectable from "./components/+preferences/default-shell.injectable"; import appVersionInjectable from "../common/get-configuration-file-model/app-version/app-version.injectable"; import provideInitialValuesForSyncBoxesInjectable from "./utils/sync-box/provide-initial-values-for-sync-boxes.injectable"; @@ -59,6 +59,8 @@ import goForwardInjectable from "./components/layout/top-bar/go-forward.injectab import closeWindowInjectable from "./components/layout/top-bar/close-window.injectable"; import maximizeWindowInjectable from "./components/layout/top-bar/maximize-window.injectable"; import toggleMaximizeWindowInjectable from "./components/layout/top-bar/toggle-maximize-window.injectable"; +import commandContainerRootFrameChildComponentInjectable from "./components/command-palette/command-container-root-frame-child-component.injectable"; +import type { HotbarStore } from "../common/hotbars/store"; export const getDiForUnitTesting = (opts: { doGeneralOverrides?: boolean } = {}) => { const { @@ -103,6 +105,12 @@ export const getDiForUnitTesting = (opts: { doGeneralOverrides?: boolean } = {}) di.override(lensResourcesDirInjectable, () => "/irrelevant"); + di.override(commandContainerRootFrameChildComponentInjectable, () => ({ + Component: () => null, + id: "command-container", + shouldRender: computed(() => false), + })); + di.override(watchHistoryStateInjectable, () => () => () => {}); di.override(openAppContextMenuInjectable, () => () => {}); @@ -126,7 +134,11 @@ export const getDiForUnitTesting = (opts: { doGeneralOverrides?: boolean } = {}) // eslint-disable-next-line unused-imports/no-unused-vars-ts di.override(extensionsStoreInjectable, () => ({ isEnabled: ({ id, isBundled }) => false }) as ExtensionsStore); - di.override(hotbarStoreInjectable, () => ({})); + di.override(hotbarStoreInjectable, () => ({ + getActive: () => ({ name: "some-hotbar", items: [] }), + getDisplayIndex: () => "0", + }) as unknown as HotbarStore); + di.override(fileSystemProvisionerStoreInjectable, () => ({}) as FileSystemProvisionerStore);