From a6a3d9c29e385babf2dc410a911687e5dbb3ab47 Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Thu, 14 May 2026 15:42:08 +1000 Subject: [PATCH 1/2] fix: rewrite flaky anvil pagination e2e to test-id locators (#4830) --- e2e/anvil/anvil-pagination.spec.ts | 280 +++++++++++++++++++++++++---- e2e/testFunctions.ts | 255 +------------------------- 2 files changed, 249 insertions(+), 286 deletions(-) diff --git a/e2e/anvil/anvil-pagination.spec.ts b/e2e/anvil/anvil-pagination.spec.ts index 349943721..2264d76ef 100644 --- a/e2e/anvil/anvil-pagination.spec.ts +++ b/e2e/anvil/anvil-pagination.spec.ts @@ -1,36 +1,252 @@ -import { test } from "@playwright/test"; -import { - filterAndTestLastPagePagination, - testFirstPagePagination, - testPaginationContent, -} from "../testFunctions"; -import { - ANVIL_FILTER_NAMES, - ANVIL_TABS, - FILE_FORMAT_INDEX, -} from "./anvil-tabs"; - -test("Check first page has disabled back and enabled forward pagination buttons on Donors Page", async ({ - page, -}) => { - await testFirstPagePagination(page, ANVIL_TABS.DONORS); -}); +import type { Locator, Page } from "@playwright/test"; +import { expect, test } from "@playwright/test"; +import { MUI_CLASSES, TEST_IDS } from "../features/common/constants"; +import { closeAutocompletePopper } from "../features/common/filters"; + +const ENTITY_URL = "/files"; +// Used for the row-content-diff test. On Files the first cell renders only an +// icon download button (no text), so `textContent()` returns "" and can't be +// used as a change anchor; this entity has a first cell with real text. +const ENTITY_URL_WITH_FIRST_CELL = "/biosamples"; +const PAGE_SIZE = 25; +const PAGES_TO_NAVIGATE = 4; +const SAMPLE_NAVIGATIONS = 3; +// Filter-count bounds for the "applying a filter" tests. Lower bound must +// exceed PAGE_SIZE so the filtered result spans more than one page; upper +// bound keeps the navigation-to-last-page loop fast in CI. +const FILTER_COUNT_MIN = PAGE_SIZE; +const FILTER_COUNT_MAX = 120; + +test.describe("AnVIL CMG pagination", () => { + test.beforeEach(async ({ page }) => { + await page.goto(ENTITY_URL); + await paginationResults(page).waitFor(); + }); + + test("shows the page counter on first load", async ({ page }) => { + await expect(pagination(page).getByText(paginationRegex(1))).toBeVisible(); + }); -test("Paginate through the entire Files tab to confirm that the page number stays consistent and that paginating forwards is disabled on the last page. Uses filters to reduce the amount of calls necessary", async ({ - page, -}) => { - const result = await filterAndTestLastPagePagination( + test("renders both pagination buttons", async ({ page }) => { + await expect(paginationButtons(page)).toHaveCount(2); + }); + + test("disables the back button and enables the next button on the first page", async ({ page, - ANVIL_TABS.FILES, - ANVIL_FILTER_NAMES[FILE_FORMAT_INDEX] - ); - if (!result) { - test.fail(); - } -}); + }) => { + const buttons = paginationButtons(page); + await expect(buttons.first()).toBeDisabled(); + await expect(buttons.last()).toBeEnabled(); + }); -test("Check forward and backwards pagination causes the page content to change on the Biosamples page", async ({ - page, -}) => { - await testPaginationContent(page, ANVIL_TABS.BIOSAMPLES); + test("enables the back button after navigating to page 2", async ({ + page, + }) => { + const buttons = paginationButtons(page); + await triggerActionAndWaitForUpdate(paginationResults(page), () => + buttons.last().click() + ); + await expect(buttons.first()).toBeEnabled(); + }); + + test("increments the page counter on each forward navigation", async ({ + page, + }) => { + const buttons = paginationButtons(page); + const results = paginationResults(page); + for (let pageNo = 1; pageNo <= PAGES_TO_NAVIGATE; pageNo++) { + await expect(pagination(page)).toHaveText(paginationRegex(pageNo)); + if (pageNo < PAGES_TO_NAVIGATE) { + await triggerActionAndWaitForUpdate(results, () => + buttons.last().click() + ); + } + } + }); + + test("shows different table content on every page", async ({ page }) => { + // Override the Files navigation from beforeEach — on Files the first cell + // is an icon-only download button with no text content (`textContent()` + // returns ""), so it can't anchor a per-row content diff. See the + // ENTITY_URL_WITH_FIRST_CELL note at the top of the file. + await page.goto(ENTITY_URL_WITH_FIRST_CELL); + await paginationResults(page).waitFor(); + + const buttons = paginationButtons(page); + const firstCell = tableFirstCell(page); + const values: (string | null)[] = [await firstCell.textContent()]; + for (let i = 0; i < SAMPLE_NAVIGATIONS; i++) { + await triggerActionAndWaitForUpdate(firstCell, () => + buttons.last().click() + ); + values.push(await firstCell.textContent()); + } + expect(new Set(values).size).toEqual(values.length); + }); + + test("keeps the total-pages value constant while navigating", async ({ + page, + }) => { + const buttons = paginationButtons(page); + const results = paginationResults(page); + const values: (number | undefined)[] = [ + parseTotalPages(await pagination(page).textContent()), + ]; + for (let i = 0; i < SAMPLE_NAVIGATIONS; i++) { + await triggerActionAndWaitForUpdate(results, () => + buttons.last().click() + ); + values.push(parseTotalPages(await pagination(page).textContent())); + } + expect(new Set(values).size).toEqual(1); + }); + + test("updates total pages after applying a filter", async ({ page }) => { + const results = paginationResults(page); + await openSearchAllFilters(page); + const { button, count } = await findFilterInRange(page); + await triggerActionAndWaitForUpdate(results, async () => { + await button.dispatchEvent("click"); + await closeAutocompletePopper(page); + }); + const totalPages = parseTotalPages(await pagination(page).textContent()); + expect(totalPages).toEqual(Math.ceil(count / PAGE_SIZE)); + }); + + test("disables the forward button on the last page", async ({ page }) => { + const buttons = paginationButtons(page); + const results = paginationResults(page); + await openSearchAllFilters(page); + const { button } = await findFilterInRange(page); + await triggerActionAndWaitForUpdate(results, async () => { + await button.dispatchEvent("click"); + await closeAutocompletePopper(page); + }); + const totalPages = + parseTotalPages(await pagination(page).textContent()) ?? 1; + for (let i = 1; i < totalPages; i++) { + await triggerActionAndWaitForUpdate(results, () => + buttons.last().click() + ); + } + await expect(buttons.last()).toBeDisabled(); + }); }); + +/* ——————————————————————————— helpers ——————————————————————————— */ + +/** + * Finds the first filter item in the open search-all-filters dropdown whose + * count is strictly between `min` and `max`. + * + * "Required" entries are skipped — those represent the consent-group facet's + * controlled-access marker and may collide with the filter-mechanics under + * test in unrelated ways. + * @param page - Page. + * @param min - Exclusive lower bound on the filter's count. + * @param max - Exclusive upper bound on the filter's count. + * @returns The matching filter-item locator and its numeric count. + */ +async function findFilterInRange( + page: Page, + min = FILTER_COUNT_MIN, + max = FILTER_COUNT_MAX +): Promise<{ button: Locator; count: number }> { + const items = page + .getByTestId(TEST_IDS.FILTER_ITEM) + .filter({ hasNotText: "Required" }); + const itemCount = await items.count(); + for (let i = 0; i < itemCount; i++) { + const item = items.nth(i); + const text = await item.getByTestId(TEST_IDS.FILTER_COUNT).textContent(); + const count = Number(text); + if (count > min && count < max) { + return { button: item, count }; + } + } + throw new Error(`No filter found with count between ${min} and ${max}`); +} + +/** + * Opens the search-all-filters dropdown and waits for it to be ready. + * @param page - Page. + */ +async function openSearchAllFilters(page: Page): Promise { + const filter = page.getByTestId(TEST_IDS.SEARCH_ALL_FILTERS); + await expect(filter).toBeVisible(); + await filter.click(); +} + +/** + * Returns the pagination container locator. + * @param page - Page. + * @returns The pagination container locator. + */ +function pagination(page: Page): Locator { + return page.getByTestId(TEST_IDS.TABLE_PAGINATION); +} + +/** + * Returns a locator for both pagination icon buttons (back and next). + * @param page - Page. + * @returns A locator selecting the two pagination buttons. + */ +function paginationButtons(page: Page): Locator { + return pagination(page).locator(MUI_CLASSES.ICON_BUTTON); +} + +/** + * Returns a regex matching "Page N of ". + * @param n - The expected current page number. + * @returns A RegExp matching the formatted page counter text. + */ +function paginationRegex(n: number): RegExp { + return new RegExp(`^Page\\s+${n}\\s+of\\s+\\d+$`); +} + +/** + * Returns the pagination results locator (the row-count summary cell). Used + * as a content-change anchor for `triggerActionAndWaitForUpdate`. + * @param page - Page. + * @returns The pagination results locator. + */ +function paginationResults(page: Page): Locator { + return page.getByTestId(TEST_IDS.TABLE_PAGINATION_RESULTS); +} + +/** + * Parses the total-pages value from a pagination text like "Page X of Y". + * @param text - Pagination text. + * @returns Y as a number, or undefined if the text doesn't match. + */ +function parseTotalPages(text: string | null): number | undefined { + const match = text?.match(/Page\s+\d+\s+of\s+(\d+)/); + if (!match?.[1]) return; + return Number(match[1]); +} + +/** + * Returns the table's first-cell locator. + * @param page - Page. + * @returns The first-cell locator. + */ +function tableFirstCell(page: Page): Locator { + return page.getByTestId(TEST_IDS.TABLE_FIRST_CELL); +} + +/** + * Captures the target locator's text content, runs the action, then polls + * until the target's text differs from the captured value. Use this to wait + * for the post-fetch row swap after pagination or filter clicks rather than + * relying on `waitForLoadState`, which doesn't track in-place data updates. + * @param target - Locator whose text content changes after the action. + * @param action - The action to trigger (click, filter selection, etc.). + */ +async function triggerActionAndWaitForUpdate( + target: Locator, + action: () => Promise +): Promise { + const before = await target.textContent(); + await action(); + await expect.poll(() => target.textContent()).not.toEqual(before); +} diff --git a/e2e/testFunctions.ts b/e2e/testFunctions.ts index cac090774..32020a3b3 100644 --- a/e2e/testFunctions.ts +++ b/e2e/testFunctions.ts @@ -1,11 +1,7 @@ import { expect, Locator, Page } from "@playwright/test"; import { ANVIL_TABS } from "./anvil/anvil-tabs"; import { TITLE_TEXT_REQUEST_FILE_MANIFEST } from "./anvil/common/constants"; -import { - KEYBOARD_KEYS, - MUI_CLASSES, - TEST_IDS, -} from "./features/common/constants"; +import { MUI_CLASSES, TEST_IDS } from "./features/common/constants"; import { ColumnDescription, TabDescription } from "./testInterfaces"; /* eslint-disable sonarjs/no-duplicate-string -- ignoring duplicate strings here */ @@ -14,9 +10,6 @@ import { ColumnDescription, TabDescription } from "./testInterfaces"; const TIMEOUT_EXPORT_REQUEST = 60000; const TIMEOUT_DOWNLOAD = 10000; -// Filter length const for regexes -const MAX_FILTER_LENGTH = 256; - /** * Get an array of all visible column header names * @param page - a Playwright page object @@ -514,252 +507,6 @@ export async function testBulkDownloadFileManifestWorkflow( return true; } -const PAGE_COUNT_REGEX = /Page \d+ of \d+/; -const MAX_PAGINATIONS = 200; - -/** - * Test that the forward pagination button is enabled and the back button is disabled on the first page of the selected tab - * @param page - a Playwright page object - * @param tab - the tab object to test on - */ -export async function testFirstPagePagination( - page: Page, - tab: TabDescription -): Promise { - await page.goto(tab.url); - await expect(getFirstRowNthColumnCellLocator(page, 0)).toBeVisible(); - // Should start on first page - await expect(page.getByText(PAGE_COUNT_REGEX, { exact: true })).toHaveText( - /Page 1 of \d+/ - ); - // Forward button should start enabled - await expect( - page - .getByTestId(TEST_IDS.TABLE_PAGINATION) - .locator(MUI_CLASSES.ICON_BUTTON) - .last() - ).toBeEnabled(); - // Back Button should start disabled - await expect( - page - .getByTestId(TEST_IDS.TABLE_PAGINATION) - .locator(MUI_CLASSES.ICON_BUTTON) - .first() - ).toBeDisabled(); -} - -/** - * Filter the current tab to reduce the number of pages to select, then go to the last page and test that the forward button is disabled and the back button is enabled - * @param page - a Playwright page object - * @param tab - the tab object to test on - * @param filterName - the name of the filter top to use to reduce the number of pages - * @returns - true if the test passes, false if it fails due to configuration issues - */ -export async function filterAndTestLastPagePagination( - page: Page, - tab: TabDescription, - filterName: string -): Promise { - // Filter to reduce the number of pages that must be selected - await page.goto(tab.url); - await page - .getByTestId(TEST_IDS.FILTERS) - .getByText(filterRegex(filterName)) - .click(); - await expect(page.getByRole("checkbox").first()).toBeVisible(); - const filterTexts = await page - .getByRole("button") - .filter({ hasText: RegExp("(\\d{1," + MAX_FILTER_LENGTH + "})[\\s]*$") }) - .allInnerTexts(); - // Get the filter with the lowest associated count - const validFilterCounts = filterTexts - .map( - // Get filter counts - (filterOption) => - filterOption - .split("\n") - .reverse() - .map((x) => Number(x)) - .find((x) => !isNaN(x) && x !== 0) ?? -1 - ) - .filter( - /// Filter for counts that will produce a useful number of paginations - (n) => - !isNaN(n) && - n > (tab.maxPages ?? 0) * 3 && - n < (tab.maxPages ?? 0) * MAX_PAGINATIONS - ); - if (validFilterCounts.length == 0) { - console.log( - "PAGINATION LAST PAGE: Test would involve too many or too few paginations, so halting" - ); - return false; - } - const minFilterValue = Math.min(...validFilterCounts); - - const results = await page - .getByTestId(TEST_IDS.TABLE_PAGINATION_RESULTS) - .textContent(); - - await page - .getByRole("button") - .filter({ hasText: RegExp(`${minFilterValue}[\\s]*$`) }) - .click(); - - await expect - .poll( - async () => - await page.getByTestId(TEST_IDS.TABLE_PAGINATION_RESULTS).textContent() - ) - .not.toEqual(results); - - await page.keyboard.press(KEYBOARD_KEYS.ESCAPE); - - // Should start on first page, and there should be multiple pages available - await expect( - page - .getByTestId(TEST_IDS.TABLE_PAGINATION_PAGE) - .getByText(PAGE_COUNT_REGEX, { exact: true }) - ).toHaveText(/Page 1 of \d+/); - await expect( - page - .getByTestId(TEST_IDS.TABLE_PAGINATION_PAGE) - .getByText(PAGE_COUNT_REGEX, { exact: true }) - ).not.toHaveText("Page 1 of 1"); - - // Detect number of pages - const splitStartingPageText = ( - await page - .getByTestId(TEST_IDS.TABLE_PAGINATION_PAGE) - .getByText(PAGE_COUNT_REGEX, { exact: true }) - .innerText() - ).split(" "); - const maxPages = parseInt( - splitStartingPageText[splitStartingPageText.length - 1] - ); - // Paginate forwards - for (let i = 2; i < maxPages + 1; i++) { - // (dispatchevent necessary because the filter menu sometimes interrupts the click event) - await page - .getByTestId(TEST_IDS.TABLE_PAGINATION) - .locator(MUI_CLASSES.ICON_BUTTON) - .last() - .dispatchEvent("click"); - await expect(getFirstRowNthColumnCellLocator(page, 0)).toBeVisible(); - // Expect the page count to have incremented - await expect( - page - .getByTestId(TEST_IDS.TABLE_PAGINATION_PAGE) - .getByText(PAGE_COUNT_REGEX, { exact: true }) - ).toHaveText(`Page ${i} of ${maxPages}`); - } - // Expect to be on the last page - await expect( - page - .getByTestId(TEST_IDS.TABLE_PAGINATION_PAGE) - .getByText(PAGE_COUNT_REGEX, { exact: true }) - ).toContainText(`Page ${maxPages} of ${maxPages}`); - // Expect the back button to be enabled on the last page - await expect( - page - .getByTestId(TEST_IDS.TABLE_PAGINATION) - .locator(MUI_CLASSES.ICON_BUTTON) - .first() - ).toBeEnabled(); - // Expect the forward button to be disabled - await expect( - page - .getByTestId(TEST_IDS.TABLE_PAGINATION) - .locator(MUI_CLASSES.ICON_BUTTON) - .last() - ).toBeDisabled(); - return true; -} - -/** - * Test that paginating changes the content on the page - * @param page - a Playwright page object - * @param tab - the tab to test on - */ -export async function testPaginationContent( - page: Page, - tab: TabDescription -): Promise { - // Navigate to the correct tab - await page.goto(tab.url); - await expect(getTabByText(page, tab.tabName)).toHaveAttribute( - "aria-selected", - "true" - ); - - await page.getByTestId(TEST_IDS.TABLE_FIRST_CELL).waitFor(); - - const firstElementTextLocator = page.getByTestId(TEST_IDS.TABLE_FIRST_CELL); - - // Should start on first page - await expect(page.getByText(PAGE_COUNT_REGEX, { exact: true })).toHaveText( - /Page 1 of \d+/ - ); - const maxPages = 5; - const FirstTableEntries = []; - - // Paginate forwards - for (let i = 2; i < maxPages + 1; i++) { - await expect(firstElementTextLocator).not.toHaveText(""); - const OriginalFirstTableEntry = await firstElementTextLocator.innerText(); - // Click the next button - // dispatchEvent necessary because the table loading component sometimes interrupts a click event - await page - .getByTestId(TEST_IDS.TABLE_PAGINATION) - .locator(MUI_CLASSES.ICON_BUTTON) - .last() - .dispatchEvent("click"); - // Expect the page count to have incremented - await expect(page.getByText(PAGE_COUNT_REGEX, { exact: true })).toHaveText( - RegExp(`Page ${i} of \\d+`) - ); - // Expect the back button to be enabled - await expect( - page - .getByTestId(TEST_IDS.TABLE_PAGINATION) - .locator(MUI_CLASSES.ICON_BUTTON) - .first() - ).toBeEnabled(); - // Expect the forwards button to be enabled - if (i != maxPages) { - await expect( - page - .getByTestId(TEST_IDS.TABLE_PAGINATION) - .locator(MUI_CLASSES.ICON_BUTTON) - .last() - ).toBeEnabled(); - } - // Expect the first entry to have changed on the new page - await expect(firstElementTextLocator).not.toHaveText( - OriginalFirstTableEntry - ); - // Remember the first entry - FirstTableEntries.push(OriginalFirstTableEntry); - } - - // Paginate backwards - for (let i = 0; i < maxPages - 1; i++) { - const OldFirstTableEntry = FirstTableEntries[maxPages - i - 2]; - await page - .getByTestId(TEST_IDS.TABLE_PAGINATION) - .locator(MUI_CLASSES.ICON_BUTTON) - .first() - .click(); - await page.getByTestId(TEST_IDS.TABLE_PAGINATION_RESULTS).waitFor(); - // Expect page number to be correct - await expect(page.getByText(PAGE_COUNT_REGEX, { exact: true })).toHaveText( - RegExp(`Page ${maxPages - i - 1} of \\d+`) - ); - // Expect page entry to be consistent with forward pagination - await expect(firstElementTextLocator).toHaveText(OldFirstTableEntry); - } -} - /** * Return the card with the given heading text. * @param page - Playwright page object. From 1a4016b9d85adb7433e314b16d4ca947347eda46 Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Thu, 14 May 2026 16:04:54 +1000 Subject: [PATCH 2/2] refactor: hoist openSearchAllFilters to shared e2e helpers (#4830) --- e2e/anvil/anvil-pagination.spec.ts | 15 ++++----------- e2e/features/common/filters.ts | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/e2e/anvil/anvil-pagination.spec.ts b/e2e/anvil/anvil-pagination.spec.ts index 2264d76ef..9e10cd67e 100644 --- a/e2e/anvil/anvil-pagination.spec.ts +++ b/e2e/anvil/anvil-pagination.spec.ts @@ -1,7 +1,10 @@ import type { Locator, Page } from "@playwright/test"; import { expect, test } from "@playwright/test"; import { MUI_CLASSES, TEST_IDS } from "../features/common/constants"; -import { closeAutocompletePopper } from "../features/common/filters"; +import { + closeAutocompletePopper, + openSearchAllFilters, +} from "../features/common/filters"; const ENTITY_URL = "/files"; // Used for the row-content-diff test. On Files the first cell renders only an @@ -167,16 +170,6 @@ async function findFilterInRange( throw new Error(`No filter found with count between ${min} and ${max}`); } -/** - * Opens the search-all-filters dropdown and waits for it to be ready. - * @param page - Page. - */ -async function openSearchAllFilters(page: Page): Promise { - const filter = page.getByTestId(TEST_IDS.SEARCH_ALL_FILTERS); - await expect(filter).toBeVisible(); - await filter.click(); -} - /** * Returns the pagination container locator. * @param page - Page. diff --git a/e2e/features/common/filters.ts b/e2e/features/common/filters.ts index 4852efd6b..e20446806 100644 --- a/e2e/features/common/filters.ts +++ b/e2e/features/common/filters.ts @@ -194,6 +194,21 @@ export async function openFilterDropdown( await expectFilterPopoverOpen(filters.page()); } +/** + * Opens the search-all-filters dropdown by clicking its trigger and waits for + * the autocomplete popper to mount. Use this when a test wants to iterate + * over the dropdown's filter items without typing a search term (for the + * typing case see `fillSearchAllFilters`). + * @param page - Page. + */ +export async function openSearchAllFilters(page: Page): Promise { + await expectAutocompletePopperClosed(page); + const filter = page.getByTestId(TEST_IDS.SEARCH_ALL_FILTERS); + await expect(filter).toBeVisible(); + await filter.click(); + await expectAutocompletePopperOpen(page); +} + /** * Opens a sidebar filter dropdown, selects its first option, and returns the * option name. Waits for the item to be selected before returning.