From 3c2d384dcf367c6411c6286a312404251a0014c1 Mon Sep 17 00:00:00 2001 From: Hatton Date: Thu, 3 Sep 2026 11:24:48 -0600 Subject: [PATCH] Click a workspace tab by a test id, not by its localized label TopBar.tsx rendered each tab as an with a localized label and no id, class or data-testid, so automation had to match the English text. Two costs, both already paid. src/BloomE2E/helpers/workspace.ts mapped tab ids to "Collections", "Edit" and "Publish", so the suite silently only worked in an English UI, which rules out automating the UI-language cases. And the component-tester's bloomExeCdp.ts drove "#main-tabs button", a selector that exists nowhere in the source, so bloom-exe-tabs.uitest.ts cannot have worked for some time: it needs a Bloom the developer already has running, and nothing ran it. Each tab now carries data-testid="workspace-tab-", built from Bloom's own API name for the tab, and the tab strip carries data-testid="workspace-tabs". So switchTab and clickWorkspaceTab both take a tab id and need no map, and neither reads a label. Retires the AUTOMATION-DEBT.md entry "The top bar has no stable test ids, so tests match on localized text". Its "seen again" note stays: the Edit tab's page thumbnail menu still has no test ids, and that is not fixed here. Co-Authored-By: Claude Opus 5 (1M context) --- .../bloom-exe-collection-topbar.uitest.ts | 2 +- .../react_components/TopBar/TopBar.tsx | 4 +++ .../component-tests/bloom-exe-tabs.uitest.ts | 10 ++++---- .../component-tester/bloomExeCdp.ts | 25 ++++++++----------- src/BloomE2E/AUTOMATION-DEBT.md | 25 +++---------------- src/BloomE2E/helpers/workspace.ts | 17 ++++++------- 6 files changed, 33 insertions(+), 50 deletions(-) diff --git a/src/BloomBrowserUI/react_components/TopBar/CollectionTopBarControls/component-tests/bloom-exe-collection-topbar.uitest.ts b/src/BloomBrowserUI/react_components/TopBar/CollectionTopBarControls/component-tests/bloom-exe-collection-topbar.uitest.ts index 3d955de146cf..bf34d6f1ce96 100644 --- a/src/BloomBrowserUI/react_components/TopBar/CollectionTopBarControls/component-tests/bloom-exe-collection-topbar.uitest.ts +++ b/src/BloomBrowserUI/react_components/TopBar/CollectionTopBarControls/component-tests/bloom-exe-collection-topbar.uitest.ts @@ -10,7 +10,7 @@ test.describe("CollectionTopBarControls on Bloom.exe", () => { const connection = await connectToBloomExe(); try { - await clickWorkspaceTab(connection.page, "Collections"); + await clickWorkspaceTab(connection.page, "collection"); await waitForActiveWorkspaceTab("collection"); await expect( diff --git a/src/BloomBrowserUI/react_components/TopBar/TopBar.tsx b/src/BloomBrowserUI/react_components/TopBar/TopBar.tsx index 7824f44894dc..6ca83547343e 100644 --- a/src/BloomBrowserUI/react_components/TopBar/TopBar.tsx +++ b/src/BloomBrowserUI/react_components/TopBar/TopBar.tsx @@ -166,6 +166,9 @@ const Tab: React.FunctionComponent<{
  • { const connection = await connectToBloomExe(); try { - await clickWorkspaceTab(connection.page, "Collections"); + await clickWorkspaceTab(connection.page, "collection"); await waitForActiveWorkspaceTab("collection"); await expect(connection.page.locator("body")).toHaveClass( /collection-mode/, ); - await clickWorkspaceTab(connection.page, "Publish"); + await clickWorkspaceTab(connection.page, "publish"); await waitForActiveWorkspaceTab("publish"); await expect(connection.page.locator("body")).toHaveClass( /publish-mode/, ); - await clickWorkspaceTab(connection.page, "Edit"); + await clickWorkspaceTab(connection.page, "edit"); await waitForActiveWorkspaceTab("edit"); await expect(connection.page.locator("body")).toHaveClass( /edit-mode/, @@ -56,7 +56,7 @@ test.describe("Bloom exe CDP top bar", () => { ) .toBe(true); - await clickWorkspaceTab(connection.page, "Publish"); + await clickWorkspaceTab(connection.page, "publish"); await waitForActiveWorkspaceTab("publish"); await expect @@ -67,7 +67,7 @@ test.describe("Bloom exe CDP top bar", () => { ) .toBe(true); - await clickWorkspaceTab(connection.page, "Edit"); + await clickWorkspaceTab(connection.page, "edit"); await waitForActiveWorkspaceTab("edit"); } finally { await connection.browser.close(); diff --git a/src/BloomBrowserUI/react_components/component-tester/bloomExeCdp.ts b/src/BloomBrowserUI/react_components/component-tester/bloomExeCdp.ts index beac61f2d2f6..3efa65c53db8 100644 --- a/src/BloomBrowserUI/react_components/component-tester/bloomExeCdp.ts +++ b/src/BloomBrowserUI/react_components/component-tester/bloomExeCdp.ts @@ -70,23 +70,20 @@ export const connectToBloomExe = async (): Promise<{ return { browser, page }; }; +/** + * Click a workspace tab in the real top bar. The tab is found by the test id that + * react_components/TopBar/TopBar.tsx puts on it, so this works in any UI language. + * + * Bloom hides the Edit and Publish tabs until a book is selected, so a caller that wants either + * of them must select a book first. + */ export const clickWorkspaceTab = async ( page: Page, - name: WorkspaceTabId extends infer _T - ? "Collections" | "Edit" | "Publish" - : never, + tab: WorkspaceTabId, ): Promise => { - await page.waitForSelector("#main-tabs button", { - timeout: 10000, - }); - - await page.locator("#main-tabs button").filter({ hasText: name }).first(); - - await page - .locator("#main-tabs button") - .filter({ hasText: name }) - .first() - .click(); + const target = page.getByTestId(`workspace-tab-${tab}`); + await target.waitFor({ state: "visible", timeout: 10000 }); + await target.click(); }; export const getWorkspaceTabs = async (): Promise<{ diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index 7a909784f998..d59e6d24c38d 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -21,7 +21,6 @@ the identity here. Before you start on a marked entry, ask the owner of its bran | Branch | What it pays down | | --- | --- | -| `BL-16799-tab-test-ids` | `data-testid` on the workspace tabs, so no test matches a localized label. | | `BL-16799-page-change` | `editView/jumpToPage` refuses a jump it cannot do, and every page-changing helper waits for the Edit tab to settle. | | `BL-16799-collection-languages` | The `e2e/setCollectionLanguages` hook, so no test composes `.bloomCollection` XML. | @@ -133,29 +132,13 @@ than the source and naming the file that is newer. Bloom needs an explicit "no d option before the second half of that can be trusted. (Found 2026-09-01 while fixing the top-bar test ids.) -## The top bar has no stable test ids, so tests match on localized text - -`TopBar.tsx` renders the workspace tabs as `` with a localized `` -label and no id, class, or `data-testid`. Two costs, both already paid: the -component-tester's `bloomExeCdp.ts` drives `#main-tabs button`, a selector that exists -nowhere in the source, so `bloom-exe-tabs.uitest.ts` cannot have worked for some time -(it needs a developer's Bloom already running, and nothing runs it in CI — see the entry -below); and `src/BloomE2E/helpers/workspace.ts` has to map tab ids to the English labels -"Collections"/"Edit"/"Publish", so the suite silently only works in an English UI — -which rules out automating the UI-language cases. Fix direction: -`data-testid="workspace-tab-collection"` (etc.) on each tab, and drop the label matching. -The shell root has its own test id as of 2026-09-01, so the fixture no longer identifies -Bloom's shell document by `[role="tablist"]`. -(Found 2026-09-01 while scaffolding src/BloomE2E.) - -being fixed on `BL-16799-tab-test-ids`, which puts a test id on each tab and drops the -label matching. +## The Edit tab's page thumbnail menu has no stable test ids, so tests match on localized text -seen again 2026-09-01, in the Edit tab's page thumbnail menu: the items -`pageThumbnailList.tsx` renders carry no id, class or `data-testid` (all their styling is -inline), so `src/BloomE2E/helpers/pageThumbnails.ts` has to find "Copy Page" and "Paste Page" +The items `pageThumbnailList.tsx` renders carry no id, class or `data-testid` (all their +styling is inline), so `src/BloomE2E/helpers/pageThumbnails.ts` has to find "Copy Page" and "Paste Page" by their English labels, exactly as the top bar does. Same fix: a `data-testid` per command, taken from the `commandId` the menu already has. +(Found 2026-09-01 while scaffolding src/BloomE2E.) ## One toolbox harness test asserts on classes that do not exist diff --git a/src/BloomE2E/helpers/workspace.ts b/src/BloomE2E/helpers/workspace.ts index 6d6464c7e49d..4a3d1d13a7ba 100644 --- a/src/BloomE2E/helpers/workspace.ts +++ b/src/BloomE2E/helpers/workspace.ts @@ -23,14 +23,11 @@ export interface IWorkspaceTabs { navigationLocked: boolean; } -// The visible label on each tab in the top bar. These differ from the API's tab ids, which is why -// a test names the id and this map does the translating. The labels are localized, so a run in -// another UI language would need this to come from the l10n data instead. -const TAB_LABEL: Record = { - collection: "Collections", - edit: "Edit", - publish: "Publish", -}; +// The test id on each tab in the top bar, set in react_components/TopBar/TopBar.tsx. The tab ids +// here are Bloom's own API names, and the test ids are built from them, so this needs no map. +function tabTestId(tab: WorkspaceTabId): string { + return `workspace-tab-${tab}`; +} /** Ask Bloom which workspace tab is active and what state the others are in. */ export async function getTabs(page: Page): Promise { @@ -60,13 +57,15 @@ export async function waitForActiveTab( * * Bloom hides the Edit and Publish tabs entirely until a book is selected, so a test that wants * either of them must select a book first (see helpers/collection.ts). + * + * The tab is found by its test id, not by its label, so this works in any UI language. */ export async function switchTab( page: Page, tab: WorkspaceTabId, timeoutMs = 30000, ): Promise { - const target = page.getByRole("tab", { name: TAB_LABEL[tab] }); + const target = page.getByTestId(tabTestId(tab)); await target.waitFor({ state: "visible", timeout: timeoutMs }); await target.click(); await waitForActiveTab(page, tab, timeoutMs);