From d0ed4e46b1147c98d3f0196ce4857d2d7550ed54 Mon Sep 17 00:00:00 2001 From: Janne Savolainen Date: Fri, 17 Jun 2022 16:21:18 +0300 Subject: [PATCH] Fix tray bugs (#5666) * Make unit tests for tray behave correctly when item is disabled Signed-off-by: Janne Savolainen * Make multiple separators in tray originating from same extension not throw up Signed-off-by: Janne Savolainen --- ...arators-originating-from-extension.test.ts | 54 +++++++++++++++++++ .../tray-menu-item-registrator.injectable.ts | 10 ++-- .../test-utils/get-application-builder.tsx | 4 ++ 3 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 src/behaviours/tray/multiple-separators-originating-from-extension.test.ts diff --git a/src/behaviours/tray/multiple-separators-originating-from-extension.test.ts b/src/behaviours/tray/multiple-separators-originating-from-extension.test.ts new file mode 100644 index 0000000000..d2e44c24e2 --- /dev/null +++ b/src/behaviours/tray/multiple-separators-originating-from-extension.test.ts @@ -0,0 +1,54 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { LensMainExtension } from "../../extensions/lens-main-extension"; +import type { TrayMenuRegistration } from "../../main/tray/tray-menu-registration"; +import type { ApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder"; +import { getApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder"; +import getRandomIdInjectable from "../../common/utils/get-random-id.injectable"; + +describe("multiple separators originating from extension", () => { + let applicationBuilder: ApplicationBuilder; + + beforeEach(async () => { + applicationBuilder = getApplicationBuilder(); + + applicationBuilder.beforeApplicationStart(({ mainDi }) => { + mainDi.unoverride(getRandomIdInjectable); + mainDi.permitSideEffects(getRandomIdInjectable); + }); + + await applicationBuilder.render(); + }); + + it("given extension with multiple separators, when extension is enabled, does not throw", () => { + const someExtension = new SomeTestExtension({ + id: "some-extension-id", + trayMenus: [{ type: "separator" }, { type: "separator" } ], + }); + + return expect( + applicationBuilder.extensions.main.enable(someExtension), + ).resolves.toBeUndefined(); + }); +}); + +class SomeTestExtension extends LensMainExtension { + constructor({ id, trayMenus }: { + id: string; + trayMenus: TrayMenuRegistration[]; + }) { + super({ + id, + absolutePath: "irrelevant", + isBundled: false, + isCompatible: false, + isEnabled: false, + manifest: { name: id, version: "some-version", engines: { lens: "^5.5.0" }}, + manifestPath: "irrelevant", + }); + + this.trayMenus = trayMenus; + } +} diff --git a/src/main/tray/tray-menu-item/tray-menu-item-registrator.injectable.ts b/src/main/tray/tray-menu-item/tray-menu-item-registrator.injectable.ts index 86d57c541c..fa9914b4b6 100644 --- a/src/main/tray/tray-menu-item/tray-menu-item-registrator.injectable.ts +++ b/src/main/tray/tray-menu-item/tray-menu-item-registrator.injectable.ts @@ -15,6 +15,7 @@ import type { TrayMenuRegistration } from "../tray-menu-registration"; import { withErrorSuppression } from "../../../common/utils/with-error-suppression/with-error-suppression"; import type { WithErrorLoggingFor } from "../../../common/utils/with-error-logging/with-error-logging.injectable"; import withErrorLoggingInjectable from "../../../common/utils/with-error-logging/with-error-logging.injectable"; +import getRandomIdInjectable from "../../../common/utils/get-random-id.injectable"; const trayMenuItemRegistratorInjectable = getInjectable({ id: "tray-menu-item-registrator", @@ -22,11 +23,12 @@ const trayMenuItemRegistratorInjectable = getInjectable({ instantiate: (di) => (extension, installationCounter) => { const mainExtension = extension as LensMainExtension; const withErrorLoggingFor = di.inject(withErrorLoggingInjectable); + const getRandomId = di.inject(getRandomIdInjectable); pipeline( mainExtension.trayMenus, - flatMap(toItemInjectablesFor(mainExtension, installationCounter, withErrorLoggingFor)), + flatMap(toItemInjectablesFor(mainExtension, installationCounter, withErrorLoggingFor, getRandomId)), (injectables) => di.register(...injectables), ); @@ -37,9 +39,9 @@ const trayMenuItemRegistratorInjectable = getInjectable({ export default trayMenuItemRegistratorInjectable; -const toItemInjectablesFor = (extension: LensMainExtension, installationCounter: number, withErrorLoggingFor: WithErrorLoggingFor) => { +const toItemInjectablesFor = (extension: LensMainExtension, installationCounter: number, withErrorLoggingFor: WithErrorLoggingFor, getRandomId: () => string) => { const _toItemInjectables = (parentId: string | null) => (registration: TrayMenuRegistration): Injectable[] => { - const trayItemId = registration.id || kebabCase(registration.label || ""); + const trayItemId = registration.id || kebabCase(registration.label || getRandomId()); const id = `${trayItemId}-tray-menu-item-for-extension-${extension.sanitizedExtensionId}-instance-${installationCounter}`; const parentInjectable = getInjectable({ @@ -72,7 +74,7 @@ const toItemInjectablesFor = (extension: LensMainExtension, installationCounter: return decorated(registration); }, - enabled: computed(() => !!registration.enabled), + enabled: computed(() => registration.enabled ?? true), visible: computed(() => true), extension, diff --git a/src/renderer/components/test-utils/get-application-builder.tsx b/src/renderer/components/test-utils/get-application-builder.tsx index 7b526123d9..9a1b3ea4a4 100644 --- a/src/renderer/components/test-utils/get-application-builder.tsx +++ b/src/renderer/components/test-utils/get-application-builder.tsx @@ -317,6 +317,10 @@ export const getApplicationBuilder = () => { throw new Error(`Tried to click tray menu item with ID ${id} which does not exist. Available IDs are: "${availableIds}"`); } + if (!menuItem.enabled.get()) { + throw new Error(`Tried to click tray menu item with ID ${id} which is disabled.`); + } + await menuItem.click?.(); }, },