From 6b8382f815c4d9a209cb26b2438facf27ba9ac67 Mon Sep 17 00:00:00 2001 From: Janne Savolainen Date: Mon, 13 Jun 2022 15:50:50 +0300 Subject: [PATCH] Add unit test to make sure that enabling, disabling and re-enabling extensions does not make tray menu items throw Signed-off-by: Janne Savolainen --- ...and-tab-navigation-for-extensions.test.tsx | 2 +- ...-characters-in-page-registrations.test.tsx | 2 +- .../navigate-to-extension-page.test.tsx | 2 +- ...to-extension-specific-preferences.test.tsx | 2 +- ...vigation-to-telemetry-preferences.test.tsx | 4 +- ...nu-item-originating-from-extension.test.ts | 34 +++- .../test-utils/get-application-builder.tsx | 146 +++++++++--------- 7 files changed, 109 insertions(+), 83 deletions(-) diff --git a/src/behaviours/cluster/sidebar-and-tab-navigation-for-extensions.test.tsx b/src/behaviours/cluster/sidebar-and-tab-navigation-for-extensions.test.tsx index eb7fd0bc05..8ef472f20d 100644 --- a/src/behaviours/cluster/sidebar-and-tab-navigation-for-extensions.test.tsx +++ b/src/behaviours/cluster/sidebar-and-tab-navigation-for-extensions.test.tsx @@ -46,7 +46,7 @@ describe("cluster - sidebar and tab navigation for extensions", () => { const getRendererExtensionFake = getRendererExtensionFakeFor(applicationBuilder); const testExtension = getRendererExtensionFake(extensionStubWithSidebarItems); - await applicationBuilder.addExtensions(testExtension); + await applicationBuilder.extensions.renderer.enable(testExtension); }); describe("given no state for expanded sidebar items exists, and navigated to child sidebar item, when rendered", () => { diff --git a/src/behaviours/extension-special-characters-in-page-registrations.test.tsx b/src/behaviours/extension-special-characters-in-page-registrations.test.tsx index 2320d28a1b..20c29d82b5 100644 --- a/src/behaviours/extension-special-characters-in-page-registrations.test.tsx +++ b/src/behaviours/extension-special-characters-in-page-registrations.test.tsx @@ -23,7 +23,7 @@ describe("extension special characters in page registrations", () => { extensionWithPagesHavingSpecialCharacters, ); - await applicationBuilder.addExtensions(testExtension); + await applicationBuilder.extensions.renderer.enable(testExtension); rendered = await applicationBuilder.render(); }); diff --git a/src/behaviours/navigate-to-extension-page.test.tsx b/src/behaviours/navigate-to-extension-page.test.tsx index 35c47b9076..255a8951ba 100644 --- a/src/behaviours/navigate-to-extension-page.test.tsx +++ b/src/behaviours/navigate-to-extension-page.test.tsx @@ -27,7 +27,7 @@ describe("navigate to extension page", () => { extensionWithPagesHavingParameters, ); - await applicationBuilder.addExtensions(testExtension); + await applicationBuilder.extensions.renderer.enable(testExtension); rendered = await applicationBuilder.render(); diff --git a/src/behaviours/preferences/navigation-to-extension-specific-preferences.test.tsx b/src/behaviours/preferences/navigation-to-extension-specific-preferences.test.tsx index f44fb9bf20..5eb9b2f384 100644 --- a/src/behaviours/preferences/navigation-to-extension-specific-preferences.test.tsx +++ b/src/behaviours/preferences/navigation-to-extension-specific-preferences.test.tsx @@ -48,7 +48,7 @@ describe("preferences - navigation to extension specific preferences", () => { const getRendererExtensionFake = getRendererExtensionFakeFor(applicationBuilder); const testExtension = getRendererExtensionFake(extensionStubWithExtensionSpecificPreferenceItems); - applicationBuilder.addExtensions(testExtension); + applicationBuilder.extensions.renderer.enable(testExtension); }); it("renders", () => { diff --git a/src/behaviours/preferences/navigation-to-telemetry-preferences.test.tsx b/src/behaviours/preferences/navigation-to-telemetry-preferences.test.tsx index f16c2d809b..42ea181ddc 100644 --- a/src/behaviours/preferences/navigation-to-telemetry-preferences.test.tsx +++ b/src/behaviours/preferences/navigation-to-telemetry-preferences.test.tsx @@ -50,7 +50,7 @@ describe("preferences - navigation to telemetry preferences", () => { const getRendererExtensionFake = getRendererExtensionFakeFor(applicationBuilder); const testExtensionWithTelemetryPreferenceItems = getRendererExtensionFake(extensionStubWithTelemetryPreferenceItems); - applicationBuilder.addExtensions( + applicationBuilder.extensions.renderer.enable( testExtensionWithTelemetryPreferenceItems, ); }); @@ -105,7 +105,7 @@ describe("preferences - navigation to telemetry preferences", () => { ], }); - applicationBuilder.addExtensions( + applicationBuilder.extensions.renderer.enable( testExtensionWithTelemetryPreferenceItems, ); diff --git a/src/behaviours/tray/clicking-tray-menu-item-originating-from-extension.test.ts b/src/behaviours/tray/clicking-tray-menu-item-originating-from-extension.test.ts index c9589b1dab..bc6feadfcf 100644 --- a/src/behaviours/tray/clicking-tray-menu-item-originating-from-extension.test.ts +++ b/src/behaviours/tray/clicking-tray-menu-item-originating-from-extension.test.ts @@ -37,7 +37,7 @@ describe("clicking tray menu item originating from extension", () => { trayMenus: [{ label: "some-label", click: clickMock }], }); - await applicationBuilder.addMainExtensions(someExtension); + await applicationBuilder.extensions.main.enable(someExtension); }); it("when item is clicked, triggers the click handler", () => { @@ -84,15 +84,33 @@ describe("clicking tray menu item originating from extension", () => { }); }); - it("when disabling extension, does not have tray menu items", () => { - applicationBuilder.removeMainExtensions(someExtension); + describe("when disabling extension", () => { + beforeEach(() => { + applicationBuilder.extensions.main.disable(someExtension); + }); - expect( - applicationBuilder.tray.get( - "some-label-tray-menu-item-for-extension-some-extension-id-instance-1", - ), - ).toBeNull(); + it("does not have the tray menu item from extension", () => { + applicationBuilder.extensions.main.disable(someExtension); + + expect( + applicationBuilder.tray.get( + "some-label-tray-menu-item-for-extension-some-extension-id-instance-1", + ), + ).toBeNull(); + }); + + // Note: Motivation here is to make sure that enabling same extension does not throw + it("when extension is re-enabled, has the tray menu item from extension", async () => { + await applicationBuilder.extensions.main.enable(someExtension); + + expect( + applicationBuilder.tray.get( + "some-label-tray-menu-item-for-extension-some-extension-id-instance-2", + ), + ).not.toBeNull(); + }); }); + }); }); diff --git a/src/renderer/components/test-utils/get-application-builder.tsx b/src/renderer/components/test-utils/get-application-builder.tsx index 031b3a34fb..de28b39c51 100644 --- a/src/renderer/components/test-utils/get-application-builder.tsx +++ b/src/renderer/components/test-utils/get-application-builder.tsx @@ -6,7 +6,7 @@ import type { LensRendererExtension } from "../../../extensions/lens-renderer-ex import rendererExtensionsInjectable from "../../../extensions/renderer-extensions.injectable"; import currentlyInClusterFrameInjectable from "../../routes/currently-in-cluster-frame.injectable"; import { extensionRegistratorInjectionToken } from "../../../extensions/extension-loader/extension-registrator-injection-token"; -import type { IObservableArray } from "mobx"; +import type { IObservableArray, ObservableSet } from "mobx"; import { computed, observable, runInAction } from "mobx"; import { renderFor } from "./renderFor"; import React from "react"; @@ -58,15 +58,29 @@ import { StatusBar } from "../status-bar/status-bar"; import lensProxyPortInjectable from "../../../main/lens-proxy/lens-proxy-port.injectable"; import type { LensMainExtension } from "../../../extensions/lens-main-extension"; import trayMenuItemsInjectable from "../../../main/tray/tray-menu-item/tray-menu-items.injectable"; +import type { LensExtension } from "../../../extensions/lens-extension"; type Callback = (dis: DiContainers) => void | Promise; +type EnableExtensions = (...extensions: T[]) => Promise; +type DisableExtensions = (...extensions: T[]) => void; + export interface ApplicationBuilder { dis: DiContainers; setEnvironmentToClusterFrame: () => ApplicationBuilder; - addExtensions: (...extensions: LensRendererExtension[]) => Promise; - addMainExtensions: (...extensions: LensMainExtension[]) => Promise; - removeMainExtensions: (...extensions: LensMainExtension[]) => ApplicationBuilder; + + extensions: { + renderer: { + enable: EnableExtensions; + disable: DisableExtensions; + }; + + main: { + enable: EnableExtensions; + disable: DisableExtensions; + }; + }; + allowKubeResource: (resourceName: KubeResource) => ApplicationBuilder; beforeApplicationStart: (callback: Callback) => ApplicationBuilder; beforeRender: (callback: Callback) => ApplicationBuilder; @@ -139,7 +153,7 @@ export const getApplicationBuilder = () => { const beforeApplicationStartCallbacks: Callback[] = []; const beforeRenderCallbacks: Callback[] = []; - const extensionsState = observable.array(); + const rendererExtensionsState = observable.set(); const mainExtensionsState = observable.set(); rendererDi.override(subscribeStoresInjectable, () => () => () => {}); @@ -179,7 +193,7 @@ export const getApplicationBuilder = () => { ); rendererDi.override(rendererExtensionsInjectable, () => - computed(() => extensionsState), + computed(() => [...rendererExtensionsState]), ); mainDi.override(mainExtensionsInjectable, () => @@ -204,6 +218,43 @@ export const getApplicationBuilder = () => { let allowedResourcesState: IObservableArray; let rendered: RenderResult; + const enableExtensionsFor = ( + extensionState: T, + di: DiContainer, + ) => { + let index = 0; + + return async (...extensions: LensExtension[]) => { + const extensionRegistrators = di.injectMany( + extensionRegistratorInjectionToken, + ); + + const addAndEnableExtensions = async () => { + index++; + + const registratorPromises = extensions.flatMap((extension) => + extensionRegistrators.map((registrator) => + registrator(extension, index), + ), + ); + + await Promise.all(registratorPromises); + + runInAction(() => { + extensions.forEach((extension) => { + extensionState.add(extension); + }); + }); + }; + + if (rendered) { + await addAndEnableExtensions(); + } else { + builder.beforeRender(addAndEnableExtensions); + } + }; + }; + const builder: ApplicationBuilder = { dis, @@ -349,70 +400,16 @@ export const getApplicationBuilder = () => { return builder; }, - addExtensions: async (...extensions) => { - const extensionRegistrators = rendererDi.injectMany( - extensionRegistratorInjectionToken, - ); + extensions: { + renderer: { + enable: enableExtensionsFor(rendererExtensionsState, rendererDi), + disable: disableExtensionsFor(rendererExtensionsState), + }, - const addAndEnableExtensions = async () => { - const registratorPromises = extensions.flatMap((extension) => - extensionRegistrators.map((registrator) => registrator(extension, 1)), - ); - - await Promise.all(registratorPromises); - - runInAction(() => { - extensions.forEach((extension) => { - extensionsState.push(extension); - }); - }); - }; - - if (rendered) { - await addAndEnableExtensions(); - } else { - builder.beforeRender(addAndEnableExtensions); - } - - return builder; - }, - - addMainExtensions: async (...extensions) => { - const extensionRegistrators = mainDi.injectMany( - extensionRegistratorInjectionToken, - ); - - const addAndEnableExtensions = async () => { - const registratorPromises = extensions.flatMap((extension) => - extensionRegistrators.map((registrator) => registrator(extension, 1)), - ); - - await Promise.all(registratorPromises); - - runInAction(() => { - extensions.forEach((extension) => { - mainExtensionsState.add(extension); - }); - }); - }; - - if (rendered) { - await addAndEnableExtensions(); - } else { - builder.beforeRender(addAndEnableExtensions); - } - - return builder; - }, - - removeMainExtensions: (...extensions) => { - extensions.forEach(extension => { - runInAction(() => { - mainExtensionsState.delete(extension); - }); - }); - - return builder; + main: { + enable: enableExtensionsFor(mainExtensionsState, mainDi), + disable: disableExtensionsFor(mainExtensionsState), + }, }, allowKubeResource: (resourceName) => { @@ -536,3 +533,14 @@ function toFlatChildren(parentId: string | null | undefined): ToFlatChildren { ), ]; } + +const disableExtensionsFor = + (extensionState: T) => + + (...extensions: LensExtension[]) => { + extensions.forEach((extension) => { + runInAction(() => { + extensionState.delete(extension); + }); + }); + };