From 199f1425594a41b1eea56fb0116a44b0538f0d5e Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Fri, 8 Apr 2022 10:15:57 -0400 Subject: [PATCH] fix dock-store.test.ts test by overriding createStorage to not touch file system Signed-off-by: Sebastian Malton --- .../cluster-overview-store.ts | 4 +- .../namespace-store/namespace.store.ts | 8 ++-- .../dock/__test__/dock-store.test.ts | 16 ++----- .../dock/dock-tab-store/dock-tab.store.ts | 9 ++-- src/renderer/components/dock/dock/store.ts | 4 +- .../components/dock/logs/tab-store.ts | 5 +-- src/renderer/components/drawer/drawer.tsx | 4 +- .../item-object-list/list-layout.tsx | 10 +++-- .../components/layout/main-layout.tsx | 11 ++--- .../components/layout/sidebar-item.tsx | 8 ++-- .../table/table-model/table-model.ts | 4 +- src/renderer/getDiForUnitTesting.tsx | 42 ++++++++++++++++++- .../port-forward-store/port-forward-store.ts | 4 +- .../utils/__tests__/storageHelper.test.ts | 3 +- .../create-storage.injectable.ts | 25 ++++++----- .../utils/create-storage/create-storage.ts | 7 +++- src/renderer/utils/storageHelper.ts | 18 +++++--- 17 files changed, 109 insertions(+), 73 deletions(-) diff --git a/src/renderer/components/+cluster/cluster-overview-store/cluster-overview-store.ts b/src/renderer/components/+cluster/cluster-overview-store/cluster-overview-store.ts index 20cf9c6c2b..ab33c36b3f 100644 --- a/src/renderer/components/+cluster/cluster-overview-store/cluster-overview-store.ts +++ b/src/renderer/components/+cluster/cluster-overview-store/cluster-overview-store.ts @@ -7,7 +7,7 @@ import { action, observable, reaction, when, makeObservable } from "mobx"; import { KubeObjectStore } from "../../../../common/k8s-api/kube-object.store"; import type { Cluster, ClusterApi } from "../../../../common/k8s-api/endpoints"; import { getMetricsByNodeNames, type ClusterMetricData } from "../../../../common/k8s-api/endpoints"; -import type { StorageHelper } from "../../../utils"; +import type { StorageLayer } from "../../../utils"; import { autoBind } from "../../../utils"; import { type IMetricsReqParams, normalizeMetrics } from "../../../../common/k8s-api/endpoints/metrics.api"; import { nodesStore } from "../../+nodes/nodes.store"; @@ -28,7 +28,7 @@ export interface ClusterOverviewStorageState { } interface Dependencies { - storage: StorageHelper; + storage: StorageLayer; } export class ClusterOverviewStore extends KubeObjectStore implements ClusterOverviewStorageState { diff --git a/src/renderer/components/+namespaces/namespace-store/namespace.store.ts b/src/renderer/components/+namespaces/namespace-store/namespace.store.ts index abf89979a2..f3731c5aee 100644 --- a/src/renderer/components/+namespaces/namespace-store/namespace.store.ts +++ b/src/renderer/components/+namespaces/namespace-store/namespace.store.ts @@ -5,7 +5,7 @@ import type { IReactionDisposer } from "mobx"; import { action, comparer, computed, makeObservable, reaction } from "mobx"; -import type { StorageHelper } from "../../../utils"; +import type { StorageLayer } from "../../../utils"; import { autoBind, noop, toggle } from "../../../utils"; import type { KubeObjectStoreLoadingParams } from "../../../../common/k8s-api/kube-object.store"; import { KubeObjectStore } from "../../../../common/k8s-api/kube-object.store"; @@ -13,7 +13,7 @@ import type { NamespaceApi } from "../../../../common/k8s-api/endpoints/namespac import { Namespace } from "../../../../common/k8s-api/endpoints/namespaces.api"; interface Dependencies { - storage: StorageHelper; + storage: StorageLayer; } export class NamespaceStore extends KubeObjectStore { @@ -70,7 +70,7 @@ export class NamespaceStore extends KubeObjectStore { * @private * The current value (list of namespaces names) in the storage layer */ - @computed private get selectedNamespaces(): string[] { + @computed private get selectedNamespaces() { return this.dependencies.storage.get() ?? []; } @@ -84,7 +84,7 @@ export class NamespaceStore extends KubeObjectStore { /** * The list of selected namespace names (for filtering) */ - @computed get contextNamespaces(): string[] { + @computed get contextNamespaces() { if (!this.selectedNamespaces.length) { return this.allowedNamespaces; // show all namespaces when nothing selected } diff --git a/src/renderer/components/dock/__test__/dock-store.test.ts b/src/renderer/components/dock/__test__/dock-store.test.ts index 88f6b8d11a..89d6f059f9 100644 --- a/src/renderer/components/dock/__test__/dock-store.test.ts +++ b/src/renderer/components/dock/__test__/dock-store.test.ts @@ -3,12 +3,11 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ -import directoryForUserDataInjectable from "../../../../common/app-paths/directory-for-user-data/directory-for-user-data.injectable"; +import hostedClusterIdInjectable from "../../../../common/cluster-store/hosted-cluster-id.injectable"; import { getDiForUnitTesting } from "../../../getDiForUnitTesting"; import type { DockStore, DockTab } from "../dock/store"; import { TabKind } from "../dock/store"; import dockStoreInjectable from "../dock/store.injectable"; -import fse from "fs-extra"; const initialTabs: DockTab[] = [ { id: "terminal", kind: TabKind.TERMINAL, title: "Terminal", pinned: false }, @@ -24,20 +23,11 @@ describe("DockStore", () => { beforeEach(async () => { const di = getDiForUnitTesting({ doGeneralOverrides: true }); - di.override( - directoryForUserDataInjectable, - () => "some-test-suite-specific-directory-for-user-data", - ); + di.override(hostedClusterIdInjectable, () => "some-cluster-id"); + await di.runSetups(); dockStore = di.inject(dockStoreInjectable); - - await dockStore.whenReady; - }); - - afterEach(() => { - fse.remove("some-test-suite-specific-directory-for-user-data"); - dockStore.closeAllTabs(); }); it("closes tab and selects one from right", () => { diff --git a/src/renderer/components/dock/dock-tab-store/dock-tab.store.ts b/src/renderer/components/dock/dock-tab-store/dock-tab.store.ts index 55ca4f075e..f4ac99c6cb 100644 --- a/src/renderer/components/dock/dock-tab-store/dock-tab.store.ts +++ b/src/renderer/components/dock/dock-tab-store/dock-tab.store.ts @@ -4,8 +4,9 @@ */ import { action, observable, reaction } from "mobx"; -import type { StorageHelper } from "../../../utils"; +import type { StorageLayer } from "../../../utils"; import { autoBind, toJS } from "../../../utils"; +import type { CreateStorage } from "../../../utils/create-storage/create-storage"; import type { TabId } from "../dock/store"; export interface DockTabStoreOptions { @@ -16,11 +17,11 @@ export interface DockTabStoreOptions { export type DockTabStorageState = Record; interface DockTabStoreDependencies { - createStorage: (storageKey: string, options: DockTabStorageState) => StorageHelper>; + createStorage: CreateStorage; } export class DockTabStore { - protected storage?: StorageHelper>; + protected storage?: StorageLayer>; private data = observable.map(); constructor(protected dependencies: DockTabStoreDependencies, protected options: DockTabStoreOptions) { @@ -41,7 +42,7 @@ export class DockTabStore { // auto-save to local-storage if (storageKey) { - const storage = this.storage = this.dependencies.createStorage(storageKey, {}); + const storage = this.storage = this.dependencies.createStorage(storageKey, {}); storage.whenReady.then(() => { this.data.replace(storage.value); diff --git a/src/renderer/components/dock/dock/store.ts b/src/renderer/components/dock/dock/store.ts index 6b6078d23b..88560632cc 100644 --- a/src/renderer/components/dock/dock/store.ts +++ b/src/renderer/components/dock/dock/store.ts @@ -5,7 +5,7 @@ import * as uuid from "uuid"; import { action, comparer, computed, makeObservable, observable, reaction, runInAction } from "mobx"; -import type { StorageHelper } from "../../../utils"; +import type { StorageLayer } from "../../../utils"; import { autoBind } from "../../../utils"; import throttle from "lodash/throttle"; @@ -99,7 +99,7 @@ export interface DockTabCloseEvent { } interface Dependencies { - readonly storage: StorageHelper; + readonly storage: StorageLayer; readonly tabDataClearers: Record void>; readonly tabDataValidator: Partial boolean>>; } diff --git a/src/renderer/components/dock/logs/tab-store.ts b/src/renderer/components/dock/logs/tab-store.ts index 7310e71881..9cc83b9610 100644 --- a/src/renderer/components/dock/logs/tab-store.ts +++ b/src/renderer/components/dock/logs/tab-store.ts @@ -3,11 +3,10 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ -import type { DockTabStorageState } from "../dock-tab-store/dock-tab.store"; import { DockTabStore } from "../dock-tab-store/dock-tab.store"; -import type { StorageHelper } from "../../../utils"; import type { TabId } from "../dock/store"; import { logTabDataValidator } from "./log-tab-data.validator"; +import type { CreateStorage } from "../../../utils/create-storage/create-storage"; export interface LogTabOwnerRef { /** @@ -58,7 +57,7 @@ export interface LogTabData { } interface Dependencies { - createStorage: (storageKey: string, options: DockTabStorageState) => StorageHelper>; + createStorage: CreateStorage; } export class LogTabStore extends DockTabStore { diff --git a/src/renderer/components/drawer/drawer.tsx b/src/renderer/components/drawer/drawer.tsx index efb4ec5433..0f20d9188e 100644 --- a/src/renderer/components/drawer/drawer.tsx +++ b/src/renderer/components/drawer/drawer.tsx @@ -8,7 +8,7 @@ import "./drawer.scss"; import React from "react"; import { clipboard } from "electron"; import { createPortal } from "react-dom"; -import type { StorageHelper } from "../../utils"; +import type { StorageLayer } from "../../utils"; import { cssNames, noop } from "../../utils"; import { Icon } from "../icon"; import type { AnimateName } from "../animate"; @@ -63,7 +63,7 @@ export const resizingAnchorProps: Record; + drawerStorage: StorageLayer<{ width: number }>; } class NonInjectedDrawer extends React.Component { diff --git a/src/renderer/components/item-object-list/list-layout.tsx b/src/renderer/components/item-object-list/list-layout.tsx index 5f96d5e47e..4407327c98 100644 --- a/src/renderer/components/item-object-list/list-layout.tsx +++ b/src/renderer/components/item-object-list/list-layout.tsx @@ -10,7 +10,7 @@ import React from "react"; import { computed, makeObservable, untracked } from "mobx"; import type { ConfirmDialogParams } from "../confirm-dialog"; import type { TableCellProps, TableProps, TableRowProps, TableSortCallbacks } from "../table"; -import type { IClassName, SingleOrMany, StorageHelper } from "../../utils"; +import type { IClassName, SingleOrMany, StorageLayer } from "../../utils"; import { autoBind, cssNames, noop } from "../../utils"; import type { AddRemoveButtonsProps } from "../add-remove-buttons"; import type { ItemObject } from "../../../common/item.store"; @@ -65,7 +65,7 @@ export type ItemListStore = }) & ( PreLoadStores extends true ? { - loadAll: (selectedNamespaces: string[]) => Promise; + loadAll: (selectedNamespaces: readonly string[]) => Promise; } : { loadAll?: unknown; @@ -149,9 +149,13 @@ const defaultProps: Partial> = { failedToLoadMessage: "Failed to load items", }; +export interface ItemListLayoutStorage { + showFilters: boolean; +} + interface Dependencies { namespaceStore: NamespaceStore; - itemListLayoutStorage: StorageHelper<{ showFilters: boolean }>; + itemListLayoutStorage: StorageLayer; } @observer diff --git a/src/renderer/components/layout/main-layout.tsx b/src/renderer/components/layout/main-layout.tsx index 87a7af04d4..4ce8aabdb5 100755 --- a/src/renderer/components/layout/main-layout.tsx +++ b/src/renderer/components/layout/main-layout.tsx @@ -7,16 +7,13 @@ import styles from "./main-layout.module.scss"; import React from "react"; import { observer } from "mobx-react"; -import type { StorageHelper } from "../../utils"; +import type { StorageLayer } from "../../utils"; import { cssNames } from "../../utils"; import { ErrorBoundary } from "../error-boundary"; import { ResizeDirection, ResizeGrowthDirection, ResizeSide, ResizingAnchor } from "../resizing-anchor"; import { withInjectables } from "@ogre-tools/injectable-react"; -import type { - SidebarStorageState } from "./sidebar-storage/sidebar-storage.injectable"; -import sidebarStorageInjectable, { - defaultSidebarWidth, -} from "./sidebar-storage/sidebar-storage.injectable"; +import type { SidebarStorageState } from "./sidebar-storage/sidebar-storage.injectable"; +import sidebarStorageInjectable, { defaultSidebarWidth } from "./sidebar-storage/sidebar-storage.injectable"; export interface MainLayoutProps { sidebar: React.ReactNode; @@ -31,7 +28,7 @@ export interface MainLayoutProps { */ interface Dependencies { - sidebarStorage: StorageHelper; + sidebarStorage: StorageLayer; } @observer diff --git a/src/renderer/components/layout/sidebar-item.tsx b/src/renderer/components/layout/sidebar-item.tsx index 96e9ea513d..36570131bc 100644 --- a/src/renderer/components/layout/sidebar-item.tsx +++ b/src/renderer/components/layout/sidebar-item.tsx @@ -7,20 +7,18 @@ import "./sidebar-item.scss"; import React from "react"; import { computed, makeObservable } from "mobx"; -import type { StorageHelper } from "../../utils"; +import type { StorageLayer } from "../../utils"; import { cssNames } from "../../utils"; import { observer } from "mobx-react"; import { NavLink } from "react-router-dom"; import { Icon } from "../icon"; import { withInjectables } from "@ogre-tools/injectable-react"; -import type { - SidebarStorageState, -} from "./sidebar-storage/sidebar-storage.injectable"; +import type { SidebarStorageState } from "./sidebar-storage/sidebar-storage.injectable"; import sidebarStorageInjectable from "./sidebar-storage/sidebar-storage.injectable"; import type { HierarchicalSidebarItem } from "./sidebar-items.injectable"; interface Dependencies { - sidebarStorage: StorageHelper; + sidebarStorage: StorageLayer; } export interface SidebarItemProps { diff --git a/src/renderer/components/table/table-model/table-model.ts b/src/renderer/components/table/table-model/table-model.ts index 341d7b8f26..4349ada20c 100644 --- a/src/renderer/components/table/table-model/table-model.ts +++ b/src/renderer/components/table/table-model/table-model.ts @@ -2,7 +2,7 @@ * Copyright (c) OpenLens Authors. All rights reserved. * Licensed under MIT License. See LICENSE in root directory for more information. */ -import type { StorageHelper } from "../../../utils"; +import type { StorageLayer } from "../../../utils"; import type { TableSortParams } from "../table"; export interface TableStorageModel { @@ -12,7 +12,7 @@ export interface TableStorageModel { } interface Dependencies { - storage: StorageHelper; + storage: StorageLayer; } export class TableModel { diff --git a/src/renderer/getDiForUnitTesting.tsx b/src/renderer/getDiForUnitTesting.tsx index f82565516c..5521a657f1 100644 --- a/src/renderer/getDiForUnitTesting.tsx +++ b/src/renderer/getDiForUnitTesting.tsx @@ -4,7 +4,7 @@ */ import glob from "glob"; -import { memoize, noop } from "lodash/fp"; +import { isEqual, isPlainObject, memoize, noop } from "lodash/fp"; import { createContainer } from "@ogre-tools/injectable"; import { Environments, setLegacyGlobalDiForExtensionApi } from "../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; import getValueFromRegisteredChannelInjectable from "./app-paths/get-value-from-registered-channel/get-value-from-registered-channel.injectable"; @@ -35,6 +35,10 @@ import { joinPathsFake } from "../common/test-utils/join-paths-fake"; import hotbarStoreInjectable from "../common/hotbar-store.injectable"; import terminalSpawningPoolInjectable from "./components/dock/terminal/terminal-spawning-pool.injectable"; import hostedClusterIdInjectable from "../common/cluster-store/hosted-cluster-id.injectable"; +import createStorageInjectable from "./utils/create-storage/create-storage.injectable"; +import { observable, toJS } from "mobx"; +import type { Draft } from "immer"; +import { produce, isDraft } from "immer"; export const getDiForUnitTesting = ( { doGeneralOverrides } = { doGeneralOverrides: false }, @@ -59,7 +63,41 @@ export const getDiForUnitTesting = ( di.override(isWindowsInjectable, () => false); di.override(isLinuxInjectable, () => false); - di.override(terminalSpawningPoolInjectable, () => new HTMLElement()); + di.override(terminalSpawningPoolInjectable, () => document.createElement("div")); + di.override(createStorageInjectable, () => function (key: string, defaultValue: MockT) { + const srcValue = observable.box(defaultValue); + + return { + get: () => srcValue.get(), + isDefaultValue: val => isEqual(val, defaultValue), + merge: (value: Partial | ((draft: Draft) => void | Partial)) => { + const nextValue = produce(toJS(srcValue.get()), (draft) => { + + if (typeof value == "function") { + const newValue = value(draft); + + // merge returned plain objects from `value-as-callback` usage + // otherwise `draft` can be just modified inside a callback without returning any value (void) + if (newValue && !isDraft(newValue)) { + Object.assign(draft, newValue); + } + } else if (isPlainObject(value)) { + Object.assign(draft, value); + } + + return draft; + }); + + srcValue.set(nextValue); + }, + reset: () => srcValue.set(defaultValue), + set: (val: MockT) => srcValue.set(val), + get value() { + return srcValue.get(); + }, + whenReady: Promise.resolve(), + }; + }); di.override(hostedClusterIdInjectable, () => undefined); di.override(getAbsolutePathInjectable, () => getAbsolutePathFake); diff --git a/src/renderer/port-forward/port-forward-store/port-forward-store.ts b/src/renderer/port-forward/port-forward-store/port-forward-store.ts index 956a355a1c..542996e00b 100644 --- a/src/renderer/port-forward/port-forward-store/port-forward-store.ts +++ b/src/renderer/port-forward/port-forward-store/port-forward-store.ts @@ -5,7 +5,7 @@ import { action, makeObservable, observable, reaction } from "mobx"; import { ItemStore } from "../../../common/item.store"; -import type { StorageHelper } from "../../utils"; +import type { StorageLayer } from "../../utils"; import { autoBind, disposer } from "../../utils"; import type { ForwardedPort } from "../port-forward-item"; import { PortForwardItem } from "../port-forward-item"; @@ -14,7 +14,7 @@ import { waitUntilFree } from "tcp-port-used"; import logger from "../../../common/logger"; interface Dependencies { - storage: StorageHelper; + storage: StorageLayer; notifyErrorPortForwarding: (message: string) => void; } diff --git a/src/renderer/utils/__tests__/storageHelper.test.ts b/src/renderer/utils/__tests__/storageHelper.test.ts index 7adb49d2c5..5bb59c8205 100644 --- a/src/renderer/utils/__tests__/storageHelper.test.ts +++ b/src/renderer/utils/__tests__/storageHelper.test.ts @@ -6,6 +6,7 @@ import { observable, reaction } from "mobx"; import { StorageHelper } from "../storageHelper"; import { delay } from "../../../common/utils/delay"; +import { toJS } from "../../../common/utils"; interface StorageModel { [prop: string]: any /*json-serializable*/; @@ -131,7 +132,7 @@ describe("renderer/utils/StorageHelper", () => { it("storage.get() is observable", () => { expect(storageHelper.get()).toEqual(defaultValue); - reaction(() => storageHelper.toJSON(), change => { + reaction(() => toJS(storageHelper.get()), change => { observedChanges.push(change); }); diff --git a/src/renderer/utils/create-storage/create-storage.injectable.ts b/src/renderer/utils/create-storage/create-storage.injectable.ts index b9b7fae39b..798e763dff 100644 --- a/src/renderer/utils/create-storage/create-storage.injectable.ts +++ b/src/renderer/utils/create-storage/create-storage.injectable.ts @@ -15,20 +15,19 @@ import hostedClusterIdInjectable from "../../../common/cluster-store/hosted-clus const createStorageInjectable = getInjectable({ id: "create-storage", - instantiate: (di) => - createStorage({ - storage: observable({ - initialized: false, - loaded: false, - data: {} as Record, // json-serializable - }), - readJsonFile: di.inject(readJsonFileInjectable), - writeJsonFile: di.inject(writeJsonFileInjectable), - logger: di.inject(loggerInjectable), - directoryForLensLocalStorage: di.inject(directoryForLensLocalStorageInjectable), - getAbsolutePath: di.inject(getAbsolutePathInjectable), - hostedClusterId: di.inject(hostedClusterIdInjectable), + instantiate: (di) => createStorage({ + storage: observable({ + initialized: false, + loaded: false, + data: {}, }), + readJsonFile: di.inject(readJsonFileInjectable), + writeJsonFile: di.inject(writeJsonFileInjectable), + logger: di.inject(loggerInjectable), + directoryForLensLocalStorage: di.inject(directoryForLensLocalStorageInjectable), + getAbsolutePath: di.inject(getAbsolutePathInjectable), + hostedClusterId: di.inject(hostedClusterIdInjectable), + }), }); export default createStorageInjectable; diff --git a/src/renderer/utils/create-storage/create-storage.ts b/src/renderer/utils/create-storage/create-storage.ts index 0bafec2800..f2d7b47e05 100755 --- a/src/renderer/utils/create-storage/create-storage.ts +++ b/src/renderer/utils/create-storage/create-storage.ts @@ -6,6 +6,7 @@ // Keeps window.localStorage state in external JSON-files. // Because app creates random port between restarts => storage session wiped out each time. import { comparer, reaction, toJS, when } from "mobx"; +import type { StorageLayer } from "../storageHelper"; import { StorageHelper } from "../storageHelper"; import { isTestEnv } from "../../../common/vars"; import type { JsonObject, JsonValue } from "type-fest"; @@ -22,6 +23,8 @@ interface Dependencies { hostedClusterId: string | undefined; } +export type CreateStorage = (key: string, defaultValue: T) => StorageLayer; + /** * Creates a helper for saving data under the "key" intended for window.localStorage */ @@ -33,7 +36,7 @@ export const createStorage = ({ readJsonFile, writeJsonFile, hostedClusterId, -}: Dependencies) => (key: string, defaultValue: T) => { +}: Dependencies): CreateStorage => (key, defaultValue) => { const { logPrefix } = StorageHelper; if (!storage.initialized) { @@ -78,7 +81,7 @@ export const createStorage = ({ .catch(error => logger.error(`${logPrefix} Failed to initialize storage: ${error}`)); } - return new StorageHelper(key, { + return new StorageHelper(key, { autoInit: true, defaultValue, storage: { diff --git a/src/renderer/utils/storageHelper.ts b/src/renderer/utils/storageHelper.ts index 41cca0b854..99a5486448 100755 --- a/src/renderer/utils/storageHelper.ts +++ b/src/renderer/utils/storageHelper.ts @@ -24,7 +24,17 @@ export interface StorageHelperOptions { defaultValue: T; } -export class StorageHelper { +export interface StorageLayer { + isDefaultValue(val: T): boolean; + get(): T; + readonly value: T; + readonly whenReady: Promise; + set(value: T): void; + reset(): void; + merge(value: Partial | ((draft: Draft) => Partial | void)): void; +} + +export class StorageHelper implements StorageLayer { static logPrefix = "[StorageHelper]:"; readonly storage: StorageAdapter; @@ -140,7 +150,7 @@ export class StorageHelper { @action merge(value: Partial | ((draft: Draft) => Partial | void)) { - const nextValue = produce(this.toJSON(), (draft: Draft) => { + const nextValue = produce(toJS(this.get()), (draft) => { if (typeof value == "function") { const newValue = value(draft); @@ -159,8 +169,4 @@ export class StorageHelper { this.set(nextValue); } - - toJSON(): T { - return toJS(this.get()); - } }