From 8de3e751912fd795e176133d22b2c9fca0f9b908 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Mon, 5 Dec 2022 15:36:01 -0500 Subject: [PATCH] Fix sidebar-and-tab-navigation-tests - Move enabling extensions in tests to a proper location - Fix flushing promises Signed-off-by: Sebastian Malton --- ...h-exists.global-override-for-injectable.ts | 11 ------- .../file-system-provisioner-store.ts | 1 - ...and-tab-navigation-for-extensions.test.tsx | 3 ++ .../main/init-store.injectable.ts | 22 +++++++++++++ .../renderer/init-store.injectable.ts | 24 ++++++++++++++ .../test-utils/get-application-builder.tsx | 28 +++++++++++++--- .../utils/__tests__/storageHelper.test.ts | 32 ++++++++++++++++--- .../utils/create-storage/create-storage.ts | 32 ++++++++++--------- src/renderer/utils/storageHelper.ts | 23 +++++++------ 9 files changed, 132 insertions(+), 44 deletions(-) delete mode 100644 src/common/fs/path-exists.global-override-for-injectable.ts create mode 100644 src/features/file-system-provisioner/main/init-store.injectable.ts create mode 100644 src/features/file-system-provisioner/renderer/init-store.injectable.ts diff --git a/src/common/fs/path-exists.global-override-for-injectable.ts b/src/common/fs/path-exists.global-override-for-injectable.ts deleted file mode 100644 index 1b9b96c8dd..0000000000 --- a/src/common/fs/path-exists.global-override-for-injectable.ts +++ /dev/null @@ -1,11 +0,0 @@ -/** - * Copyright (c) OpenLens Authors. All rights reserved. - * Licensed under MIT License. See LICENSE in root directory for more information. - */ - -import { getGlobalOverride } from "../test-utils/get-global-override"; -import pathExistsInjectable from "./path-exists.injectable"; - -export default getGlobalOverride(pathExistsInjectable, () => async () => { - throw new Error("Tried to check if a path exists without override"); -}); diff --git a/src/extensions/extension-loader/file-system-provisioner-store/file-system-provisioner-store.ts b/src/extensions/extension-loader/file-system-provisioner-store/file-system-provisioner-store.ts index df1b9af4c0..5655a0cf06 100644 --- a/src/extensions/extension-loader/file-system-provisioner-store/file-system-provisioner-store.ts +++ b/src/extensions/extension-loader/file-system-provisioner-store/file-system-provisioner-store.ts @@ -34,7 +34,6 @@ export class FileSystemProvisionerStore extends BaseStore { }); makeObservable(this); - this.load(); } /** diff --git a/src/features/cluster/sidebar-and-tab-navigation-for-extensions.test.tsx b/src/features/cluster/sidebar-and-tab-navigation-for-extensions.test.tsx index a228cc880e..2441afa000 100644 --- a/src/features/cluster/sidebar-and-tab-navigation-for-extensions.test.tsx +++ b/src/features/cluster/sidebar-and-tab-navigation-for-extensions.test.tsx @@ -20,6 +20,7 @@ import type { IObservableValue } from "mobx"; import { runInAction, computed, observable } from "mobx"; import storageSaveDelayInjectable from "../../renderer/utils/create-storage/storage-save-delay.injectable"; import type { DiContainer } from "@ogre-tools/injectable"; +import { flushPromises } from "../../common/test-utils/flush-promises"; describe("cluster - sidebar and tab navigation for extensions", () => { let applicationBuilder: ApplicationBuilder; @@ -399,6 +400,8 @@ describe("cluster - sidebar and tab navigation for extensions", () => { const readJsonFileFake = windowDi.inject(readJsonFileInjectable); + await flushPromises(); // Needed because of several async calls + const actual = await readJsonFileFake( "/some-directory-for-lens-local-storage/some-cluster-id.json", ); diff --git a/src/features/file-system-provisioner/main/init-store.injectable.ts b/src/features/file-system-provisioner/main/init-store.injectable.ts new file mode 100644 index 0000000000..0fe3d4f77b --- /dev/null +++ b/src/features/file-system-provisioner/main/init-store.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 fileSystemProvisionerStoreInjectable from "../../../extensions/extension-loader/file-system-provisioner-store/file-system-provisioner-store.injectable"; +import { onLoadOfApplicationInjectionToken } from "../../../main/start-main-application/runnable-tokens/on-load-of-application-injection-token"; + +const initFileSystemProvisionerStoreInjectable = getInjectable({ + id: "init-file-system-provisioner-store", + instantiate: (di) => ({ + id: "init-file-system-provisioner-store", + run: () => { + const store = di.inject(fileSystemProvisionerStoreInjectable); + + store.load(); + }, + }), + injectionToken: onLoadOfApplicationInjectionToken, +}); + +export default initFileSystemProvisionerStoreInjectable; diff --git a/src/features/file-system-provisioner/renderer/init-store.injectable.ts b/src/features/file-system-provisioner/renderer/init-store.injectable.ts new file mode 100644 index 0000000000..95925270aa --- /dev/null +++ b/src/features/file-system-provisioner/renderer/init-store.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 fileSystemProvisionerStoreInjectable from "../../../extensions/extension-loader/file-system-provisioner-store/file-system-provisioner-store.injectable"; +import setupAppPathsInjectable from "../../../renderer/app-paths/setup-app-paths.injectable"; +import { beforeFrameStartsInjectionToken } from "../../../renderer/before-frame-starts/tokens"; + +const initFileSystemProvisionerStoreInjectable = getInjectable({ + id: "init-file-system-provisioner-store", + instantiate: (di) => ({ + id: "init-file-system-provisioner-store", + run: () => { + const store = di.inject(fileSystemProvisionerStoreInjectable); + + store.load(); + }, + runAfter: di.inject(setupAppPathsInjectable), + }), + injectionToken: beforeFrameStartsInjectionToken, +}); + +export default initFileSystemProvisionerStoreInjectable; diff --git a/src/renderer/components/test-utils/get-application-builder.tsx b/src/renderer/components/test-utils/get-application-builder.tsx index f4c98e07ad..87c916ced8 100644 --- a/src/renderer/components/test-utils/get-application-builder.tsx +++ b/src/renderer/components/test-utils/get-application-builder.tsx @@ -114,6 +114,7 @@ export interface ApplicationBuilder { allowKubeResource: (resourceName: KubeResource) => ApplicationBuilder; beforeApplicationStart: (callback: Callback) => ApplicationBuilder; + afterApplicationStart: (callback: Callback) => ApplicationBuilder; beforeWindowStart: (callback: Callback) => ApplicationBuilder; afterWindowStart: (callback: Callback) => ApplicationBuilder; @@ -170,6 +171,7 @@ export const getApplicationBuilder = () => { const { overrideForWindow, sendToWindow } = overrideChannels(mainDi); const beforeApplicationStartCallbacks: Callback[] = []; + const afterApplicationStartCallbacks: Callback[] = []; const beforeWindowStartCallbacks: Callback[] = []; const afterWindowStartCallbacks: Callback[] = []; @@ -556,7 +558,7 @@ export const getApplicationBuilder = () => { }, enable: (...extensions) => { - builder.beforeWindowStart((windowDi) => { + builder.afterWindowStart((windowDi) => { const rendererExtensionInstances = extensions.map((options) => getExtensionFakeForRenderer( windowDi, @@ -571,7 +573,7 @@ export const getApplicationBuilder = () => { ); }); - builder.beforeApplicationStart((mainDi) => { + builder.afterApplicationStart((mainDi) => { const mainExtensionInstances = extensions.map((extension) => getExtensionFakeForMain(mainDi, extension.id, extension.name, extension.mainOptions || {}), ); @@ -585,7 +587,7 @@ export const getApplicationBuilder = () => { }, disable: (...extensions) => { - builder.beforeWindowStart(windowDi => { + builder.afterWindowStart(windowDi => { extensions .map((ext) => ext.id) .forEach( @@ -593,7 +595,7 @@ export const getApplicationBuilder = () => { ); }); - builder.beforeApplicationStart(mainDi => { + builder.afterApplicationStart(mainDi => { extensions .map((ext) => ext.id) .forEach( @@ -623,6 +625,16 @@ export const getApplicationBuilder = () => { return builder; }, + afterApplicationStart(callback) { + if (applicationHasStarted) { + callback(mainDi); + } + + afterApplicationStartCallbacks.push(callback); + + return builder; + }, + beforeWindowStart(callback) { const alreadyRenderedWindows = builder.applicationWindow.getAll(); @@ -657,6 +669,10 @@ export const getApplicationBuilder = () => { mainDi.override(shouldStartHiddenInjectable, () => true); await mainDi.inject(startMainApplicationInjectable); + for (const callback of afterApplicationStartCallbacks) { + await callback(mainDi); + } + applicationHasStarted = true; }, @@ -670,6 +686,10 @@ export const getApplicationBuilder = () => { mainDi.override(shouldStartHiddenInjectable, () => false); await mainDi.inject(startMainApplicationInjectable); + for (const callback of afterApplicationStartCallbacks) { + await callback(mainDi); + } + applicationHasStarted = true; return builder diff --git a/src/renderer/utils/__tests__/storageHelper.test.ts b/src/renderer/utils/__tests__/storageHelper.test.ts index 5bb59c8205..af34ab4322 100644 --- a/src/renderer/utils/__tests__/storageHelper.test.ts +++ b/src/renderer/utils/__tests__/storageHelper.test.ts @@ -6,7 +6,7 @@ import { observable, reaction } from "mobx"; import { StorageHelper } from "../storageHelper"; import { delay } from "../../../common/utils/delay"; -import { toJS } from "../../../common/utils"; +import { noop, toJS } from "../../../common/utils"; interface StorageModel { [prop: string]: any /*json-serializable*/; @@ -26,7 +26,15 @@ describe("renderer/utils/StorageHelper", () => { message: "saved-before", // pretending as previously saved data }); - storageHelper = new StorageHelper(storageKey, { + storageHelper = new StorageHelper({ + logger: { + debug: noop, + error: noop, + info: noop, + silly: noop, + warn: noop, + }, + }, storageKey, { autoInit: false, defaultValue: { message: "blabla", @@ -48,7 +56,15 @@ describe("renderer/utils/StorageHelper", () => { }, }); - storageHelperAsync = new StorageHelper(storageKey, { + storageHelperAsync = new StorageHelper({ + logger: { + debug: noop, + error: noop, + info: noop, + silly: noop, + warn: noop, + }, + }, storageKey, { autoInit: false, defaultValue: storageHelper.defaultValue, storage: { @@ -118,7 +134,15 @@ describe("renderer/utils/StorageHelper", () => { beforeEach(() => { observedChanges.length = 0; - storageHelper = new StorageHelper("some-key", { + storageHelper = new StorageHelper({ + logger: { + debug: noop, + error: noop, + info: noop, + silly: noop, + warn: noop, + }, + }, "some-key", { autoInit: true, defaultValue, storage: { diff --git a/src/renderer/utils/create-storage/create-storage.ts b/src/renderer/utils/create-storage/create-storage.ts index 1149d479cc..04a0339ef1 100755 --- a/src/renderer/utils/create-storage/create-storage.ts +++ b/src/renderer/utils/create-storage/create-storage.ts @@ -7,17 +7,19 @@ // 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 type { JsonObject, JsonValue } from "type-fest"; +import { storageHelperLogPrefix, StorageHelper } from "../storageHelper"; +import type { JsonObject } from "type-fest"; import type { Logger } from "../../../common/logger"; import type { JoinPaths } from "../../../common/path/join-paths.injectable"; +import type { WriteJson } from "../../../common/fs/write-json-file.injectable"; +import type { ReadJson } from "../../../common/fs/read-json-file.injectable"; interface Dependencies { - storage: { initialized: boolean; loaded: boolean; data: Record }; + storage: { initialized: boolean; loaded: boolean; data: Partial> }; logger: Logger; directoryForLensLocalStorage: string; - readJsonFile: (filePath: string) => Promise; - writeJsonFile: (filePath: string, contentObject: JsonObject) => Promise; + readJsonFile: ReadJson; + writeJsonFile: WriteJson; joinPaths: JoinPaths; hostedClusterId: string | undefined; saveDelay: number; @@ -37,9 +39,7 @@ export const createStorage = ({ writeJsonFile, hostedClusterId, saveDelay, -}: Dependencies): CreateStorage => (key, defaultValue) => { - const { logPrefix } = StorageHelper; - +}: Dependencies): CreateStorage => (key: string, defaultValue: T) => { if (!storage.initialized) { storage.initialized = true; @@ -51,7 +51,7 @@ export const createStorage = ({ } catch { // do nothing } finally { - logger.info(`${logPrefix} loading finished for ${filePath}`); + logger.info(`${storageHelperLogPrefix} loading finished for ${filePath}`); storage.loaded = true; } @@ -62,30 +62,32 @@ export const createStorage = ({ }); async function saveFile(state: Record = {}) { - logger.info(`${logPrefix} saving ${filePath}`); + logger.info(`${storageHelperLogPrefix} saving ${filePath}`); try { await writeJsonFile(filePath, state); } catch (error) { - logger.error(`${logPrefix} saving failed: ${error}`, { + logger.error(`${storageHelperLogPrefix} saving failed: ${error}`, { json: state, jsonFilePath: filePath, }); } } })() - .catch(error => logger.error(`${logPrefix} Failed to initialize storage: ${error}`)); + .catch(error => logger.error(`${storageHelperLogPrefix} Failed to initialize storage: ${error}`)); } - return new StorageHelper(key, { + return new StorageHelper({ + logger, + }, key, { autoInit: true, defaultValue, storage: { async getItem(key: string) { await when(() => storage.loaded); - return storage.data[key]; + return storage.data[key] as T; }, - setItem(key: string, value: any) { + setItem(key: string, value: T) { storage.data[key] = value; }, removeItem(key: string) { diff --git a/src/renderer/utils/storageHelper.ts b/src/renderer/utils/storageHelper.ts index 613778cc3d..4f6dcec88a 100755 --- a/src/renderer/utils/storageHelper.ts +++ b/src/renderer/utils/storageHelper.ts @@ -8,8 +8,8 @@ import { action, comparer, computed, makeObservable, observable, observe, toJS, import type { Draft } from "immer"; import { produce, isDraft } from "immer"; import { isEqual, isPlainObject } from "lodash"; -import logger from "../../main/logger"; import assert from "assert"; +import type { Logger } from "../../common/logger"; export interface StorageChange { key: string; @@ -26,9 +26,9 @@ export interface StorageAdapter { } export interface StorageHelperOptions { - autoInit?: boolean; // start preloading data immediately, default: true - storage: StorageAdapter; - defaultValue: T; + readonly autoInit?: boolean; // start preloading data immediately, default: true + readonly storage: StorageAdapter; + readonly defaultValue: T; } export interface StorageLayer { @@ -41,11 +41,16 @@ export interface StorageLayer { merge(value: Partial | ((draft: Draft) => Partial | void)): void; } +export const storageHelperLogPrefix = "[STORAGE-HELPER]:"; + +interface Dependencies { + readonly logger: Logger; +} + export class StorageHelper implements StorageLayer { - static logPrefix = "[StorageHelper]:"; readonly storage: StorageAdapter; - private data = observable.box(undefined, { + private readonly data = observable.box(undefined, { deep: true, equals: comparer.structural, }); @@ -61,7 +66,7 @@ export class StorageHelper implements StorageLayer { return this.options.defaultValue; } - constructor(readonly key: string, private options: StorageHelperOptions) { + constructor(private readonly dependencies: Dependencies, readonly key: string, private readonly options: StorageHelperOptions) { makeObservable(this); const { storage, autoInit = true } = options; @@ -89,7 +94,7 @@ export class StorageHelper implements StorageLayer { }; private onError = (error: any): void => { - logger.error(`${StorageHelper.logPrefix} loading error: ${error}`, this); + this.dependencies.logger.error(`${storageHelperLogPrefix} loading error: ${error}`, this); }; @action @@ -127,7 +132,7 @@ export class StorageHelper implements StorageLayer { this.storage.onChange?.({ value, oldValue, key: this.key }); } catch (error) { - logger.error(`${StorageHelper.logPrefix} updating storage: ${error}`, this, { value, oldValue }); + this.dependencies.logger.error(`${storageHelperLogPrefix} updating storage: ${error}`, this, { value, oldValue }); } }