Fix sidebar-and-tab-navigation-tests

- Move enabling extensions in tests to a proper location
- Fix flushing promises

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/common/fs/path-exists.global-override-for-injectable.ts

@@ -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");
-});

src/extensions/extension-loader/file-system-provisioner-store/file-system-provisioner-store.ts

@@ -34,7 +34,6 @@ export class FileSystemProvisionerStore extends BaseStore<FSProvisionModel> {
     });
 
     makeObservable(this);
-    this.load();
   }
 
   /**

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",
             );

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;

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;

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

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<StorageModel>(storageKey, {
+      storageHelper = new StorageHelper<StorageModel>({
+        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<typeof defaultValue>("some-key", {
+      storageHelper = new StorageHelper<typeof defaultValue>({
+        logger: {
+          debug: noop,
+          error: noop,
+          info: noop,
+          silly: noop,
+          warn: noop,
+        },
+      }, "some-key", {
         autoInit: true,
         defaultValue,
         storage: {

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 { storageHelperLogPrefix, StorageHelper } from "../storageHelper";
-import type { JsonObject, JsonValue } from "type-fest";
+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<string, any> };
+  storage: { initialized: boolean; loaded: boolean; data: Partial<Record<string, unknown>> };
   logger: Logger;
   directoryForLensLocalStorage: string;
-  readJsonFile: (filePath: string) => Promise<JsonValue>;
+  readJsonFile: ReadJson;
-  writeJsonFile: (filePath: string, contentObject: JsonObject) => Promise<void>;
+  writeJsonFile: WriteJson;
   joinPaths: JoinPaths;
   hostedClusterId: string | undefined;
   saveDelay: number;
@@ -37,9 +39,7 @@ export const createStorage = ({
   writeJsonFile,
   hostedClusterId,
   saveDelay,
-}: Dependencies): CreateStorage => (key, defaultValue) => {
+}: Dependencies): CreateStorage => <T>(key: string, defaultValue: T) => {
-  const { logPrefix } = StorageHelper;
-
   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<string, any> = {}) {
-        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) {

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<T> {
   key: string;
@@ -26,9 +26,9 @@ export interface StorageAdapter<T> {
 }
 
 export interface StorageHelperOptions<T> {
-  autoInit?: boolean; // start preloading data immediately, default: true
+  readonly autoInit?: boolean; // start preloading data immediately, default: true
-  storage: StorageAdapter<T>;
+  readonly storage: StorageAdapter<T>;
-  defaultValue: T;
+  readonly defaultValue: T;
 }
 
 export interface StorageLayer<T> {
@@ -41,11 +41,16 @@ export interface StorageLayer<T> {
   merge(value: Partial<T> | ((draft: Draft<T>) => Partial<T> | void)): void;
 }
 
+export const storageHelperLogPrefix = "[STORAGE-HELPER]:";
+
+interface Dependencies {
+  readonly logger: Logger;
+}
+
 export class StorageHelper<T> implements StorageLayer<T> {
-  static logPrefix = "[StorageHelper]:";
   readonly storage: StorageAdapter<T>;
 
-  private data = observable.box<T | undefined>(undefined, {
+  private readonly data = observable.box<T | undefined>(undefined, {
     deep: true,
     equals: comparer.structural,
   });
@@ -61,7 +66,7 @@ export class StorageHelper<T> implements StorageLayer<T> {
     return this.options.defaultValue;
   }
 
-  constructor(readonly key: string, private options: StorageHelperOptions<T>) {
+  constructor(private readonly dependencies: Dependencies, readonly key: string, private readonly options: StorageHelperOptions<T>) {
     makeObservable(this);
 
     const { storage, autoInit = true } = options;
@@ -89,7 +94,7 @@ export class StorageHelper<T> implements StorageLayer<T> {
   };
 
   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<T> implements StorageLayer<T> {
 
       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 });
     }
   }