From 4c31f4f6a783130077364d7fd7943ecbec5b5a60 Mon Sep 17 00:00:00 2001 From: Iku-turso Date: Fri, 21 Oct 2022 15:20:01 +0300 Subject: [PATCH] Adapt application builder and tests to array-like paths over string-like paths Array-like paths do not have weakness for special characters as part of id, such as ".". Also note: the error messaging for clicking of application menu in application builder is a bit worse now I think, but the simplification of the test code is worth it in this case IMHO. Co-authored-by: Janne Savolainen Signed-off-by: Iku-turso --- ...navigation-using-application-menu.test.tsx | 6 +++- ...ation-menu-in-legacy-extension-api.test.ts | 2 +- .../analytics-for-installing-update.test.ts | 2 +- .../navigation-using-application-menu.test.ts | 2 +- .../navigation-using-application-menu.test.ts | 6 +++- ...ing-the-app-using-application-menu.test.ts | 2 +- .../navigation-using-application-menu.test.ts | 12 +++++-- .../test-utils/get-application-builder.tsx | 34 +++++-------------- 8 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/features/add-cluster/navigation-using-application-menu.test.tsx b/src/features/add-cluster/navigation-using-application-menu.test.tsx index a114115300..19b11672c9 100644 --- a/src/features/add-cluster/navigation-using-application-menu.test.tsx +++ b/src/features/add-cluster/navigation-using-application-menu.test.tsx @@ -29,7 +29,11 @@ describe("add-cluster - navigation using application menu", () => { describe("when navigating to add cluster using application menu", () => { beforeEach(async () => { - await applicationBuilder.applicationMenu.click("root.file.add-cluster"); + await applicationBuilder.applicationMenu.click( + "root", + "file", + "add-cluster", + ); }); it("renders", () => { diff --git a/src/features/application-menu/application-menu-in-legacy-extension-api.test.ts b/src/features/application-menu/application-menu-in-legacy-extension-api.test.ts index 96bdaa8660..0f3899925e 100644 --- a/src/features/application-menu/application-menu-in-legacy-extension-api.test.ts +++ b/src/features/application-menu/application-menu-in-legacy-extension-api.test.ts @@ -107,7 +107,7 @@ describe("application-menu-in-legacy-extension-api", () => { it("when the extension-based clickable menu item is clicked, does so", () => { builder.applicationMenu.click( - "root.some-top-menu-item.some-extension-name/some-clickable-item", + "root", "some-top-menu-item", "some-extension-name/some-clickable-item", ); expect(onClickMock).toHaveBeenCalled(); diff --git a/src/features/application-update/analytics-for-installing-update.test.ts b/src/features/application-update/analytics-for-installing-update.test.ts index 3a38b0d842..2a7b8d19fd 100644 --- a/src/features/application-update/analytics-for-installing-update.test.ts +++ b/src/features/application-update/analytics-for-installing-update.test.ts @@ -142,7 +142,7 @@ describe("analytics for installing update", () => { it("when checking for updates using application menu, sends event to analytics for being checked from application menu", async () => { analyticsListenerMock.mockClear(); - builder.applicationMenu.click("root.mac.check-for-updates"); + builder.applicationMenu.click("root", "mac", "check-for-updates"); expect(analyticsListenerMock.mock.calls).toEqual([ [ diff --git a/src/features/extensions/navigation-using-application-menu.test.ts b/src/features/extensions/navigation-using-application-menu.test.ts index 36d33adb6c..c17653d7a3 100644 --- a/src/features/extensions/navigation-using-application-menu.test.ts +++ b/src/features/extensions/navigation-using-application-menu.test.ts @@ -48,7 +48,7 @@ describe("extensions - navigation using application menu", () => { describe("when navigating to extensions using application menu", () => { beforeEach(() => { - builder.applicationMenu.click("root.mac.navigate-to-extensions"); + builder.applicationMenu.click("root", "mac", "navigate-to-extensions"); }); it("focuses the window", () => { diff --git a/src/features/preferences/navigation-using-application-menu.test.ts b/src/features/preferences/navigation-using-application-menu.test.ts index 1767617462..ae143fe468 100644 --- a/src/features/preferences/navigation-using-application-menu.test.ts +++ b/src/features/preferences/navigation-using-application-menu.test.ts @@ -36,7 +36,11 @@ describe("preferences - navigation using application menu", () => { describe("when navigating to preferences using application menu", () => { beforeEach(() => { - applicationBuilder.applicationMenu.click("root.mac.navigate-to-preferences"); + applicationBuilder.applicationMenu.click( + "root", + "mac", + "navigate-to-preferences", + ); }); it("renders", () => { diff --git a/src/features/quitting-and-restarting-the-app/quitting-the-app-using-application-menu.test.ts b/src/features/quitting-and-restarting-the-app/quitting-the-app-using-application-menu.test.ts index 065bed8923..654b361eb5 100644 --- a/src/features/quitting-and-restarting-the-app/quitting-the-app-using-application-menu.test.ts +++ b/src/features/quitting-and-restarting-the-app/quitting-the-app-using-application-menu.test.ts @@ -45,7 +45,7 @@ describe("quitting the app using application menu", () => { describe("when application is quit", () => { beforeEach(() => { - builder.applicationMenu.click("root.mac.quit"); + builder.applicationMenu.click("root", "mac", "quit"); }); it("closes all windows", () => { diff --git a/src/features/welcome/navigation-using-application-menu.test.ts b/src/features/welcome/navigation-using-application-menu.test.ts index 6e1512680b..02ccf18e40 100644 --- a/src/features/welcome/navigation-using-application-menu.test.ts +++ b/src/features/welcome/navigation-using-application-menu.test.ts @@ -29,7 +29,11 @@ describe("welcome - navigation using application menu", () => { describe("when navigated somewhere else", () => { beforeEach(() => { - applicationBuilder.applicationMenu.click("root.mac.navigate-to-preferences"); + applicationBuilder.applicationMenu.click( + "root", + "mac", + "navigate-to-preferences", + ); }); it("renders", () => { @@ -44,7 +48,11 @@ describe("welcome - navigation using application menu", () => { describe("when navigated to welcome using application menu", () => { beforeEach(() => { - applicationBuilder.applicationMenu.click("root.help.navigate-to-welcome"); + applicationBuilder.applicationMenu.click( + "root", + "help", + "navigate-to-welcome", + ); }); it("renders", () => { diff --git a/src/renderer/components/test-utils/get-application-builder.tsx b/src/renderer/components/test-utils/get-application-builder.tsx index de3d873db5..b3373f562f 100644 --- a/src/renderer/components/test-utils/get-application-builder.tsx +++ b/src/renderer/components/test-utils/get-application-builder.tsx @@ -66,10 +66,8 @@ import { Namespace } from "../../../common/k8s-api/endpoints"; import { overrideFsWithFakes } from "../../../test-utils/override-fs-with-fakes"; import applicationMenuItemCompositeInjectable from "../../../features/application-menu/main/application-menu-item-composite.injectable"; import { getCompositePaths } from "../../../common/utils/composite/get-composite-paths/get-composite-paths"; -import { getCompositeNormalization } from "../../../common/utils/composite/get-composite-normalization/get-composite-normalization"; -import type { ClickableMenuItem } from "../../../features/application-menu/main/menu-items/application-menu-item-injection-token"; -import type { Composite } from "../../../common/utils/composite/get-composite/get-composite"; import { discoverFor } from "./discovery-of-html-elements"; +import { findComposite } from "../../../common/utils/composite/find-composite/find-composite"; type Callback = (di: DiContainer) => void | Promise; @@ -128,7 +126,7 @@ export interface ApplicationBuilder { }; applicationMenu: { - click: (path: string) => void; + click: (...path: string[]) => void; items: string[]; }; preferences: { @@ -354,26 +352,19 @@ export const getApplicationBuilder = () => { return getCompositePaths(composite); }, - click: (path: string) => { + click: (...path: string[]) => { const composite = mainDi.inject( applicationMenuItemCompositeInjectable, ).get(); - const clickableMenuItems = getCompositeNormalization(composite).filter(isClickableMenuItem); - const clickableMenuItemMap = new Map(clickableMenuItems); - // TODO: find out why this any!? The typing of above map is strict, so why map.get() isn't? - const clickableMenuItem = clickableMenuItemMap.get(path); + const clickableMenuItem = findComposite(...path)(composite).value; - if (clickableMenuItem === undefined) { - const clickableIds = [...clickableMenuItemMap.keys()]; - - throw new Error( - `Tried to click application menu item with unknown path "${path}". Available clickable paths are: \n"${clickableIds.join('",\n"')}"`, - ); + if(clickableMenuItem.kind === "clickable-menu-item") { + // Todo: prevent leaking of Electron + (clickableMenuItem.onClick as any)(); + } else { + throw new Error(`Tried to trigger clicking of an application menu item, but item at path '${path.join(" -> ")}' isn't clickable.`); } - - // Todo: prevent leaking of Electron. - (clickableMenuItem.value as any).onClick(); }, }, @@ -868,10 +859,3 @@ const disableExtensionFor = extensionsState.delete(id); }); }; - -const isClickableMenuItem = ( - pathAndComposite: readonly [path: string, composite: any], -): pathAndComposite is [string, Composite] => - pathAndComposite[1].value.kind === "clickable-menu-item"; - -