From a5856e2f027a4a6f75a03a999cc3336be8949a0c Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 26 Apr 2026 21:51:47 +0200 Subject: [PATCH] =?UTF-8?q?test(persons):=20fix=20E2E=20flakiness=20?= =?UTF-8?q?=E2=80=94=20replace=20waitForTimeout=20with=20waitForListbox,?= =?UTF-8?q?=20remove=20conditional=20assertions,=20fix=20data-hydrated=20s?= =?UTF-8?q?elector?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses three blockers raised in PR #350 review (Felix, Sara, Tobias): 1. Replace all waitForTimeout(400) calls with waitForListbox() which uses waitForSelector('[role="listbox"]', { state: 'visible' }) — auto-waits for the debounce to resolve, faster on fast machines and reliable under CI. 2. Remove all conditional if (hasResults) / if (hasDropdown) wrappers. Tests now use unconditional expect(dropdown).toBeVisible() assertions so a missing-data condition causes an explicit failure instead of a silent green run. 3. Replace waitForSelector('[data-hydrated]') with waitForLoadState('networkidle') in getDocumentEditUrl — the data-hydrated attribute does not exist in the app markup and would cause a 30s timeout on every test. 4. Extract page: Page type import from @playwright/test and introduce waitForListbox(page: Page) helper to avoid repeating the selector pattern. Co-Authored-By: Claude Sonnet 4.6 --- frontend/e2e/person-typeahead.spec.ts | 124 +++++++++++--------------- 1 file changed, 50 insertions(+), 74 deletions(-) diff --git a/frontend/e2e/person-typeahead.spec.ts b/frontend/e2e/person-typeahead.spec.ts index 3d472431..17baed2c 100644 --- a/frontend/e2e/person-typeahead.spec.ts +++ b/frontend/e2e/person-typeahead.spec.ts @@ -7,17 +7,15 @@ * The tests run at both desktop (1280×720) and tablet (768×1024) viewports * as required by the acceptance criteria. */ -import { test, expect } from '@playwright/test'; +import { test, expect, type Page } from '@playwright/test'; /** * Find a document edit URL to use as the test page. * Falls back to /documents/new if no existing document is found. */ -async function getDocumentEditUrl( - page: Parameters[1] extends (args: { page: infer P }) => unknown ? P : never -): Promise { +async function getDocumentEditUrl(page: Page): Promise { await page.goto('/'); - await page.waitForSelector('[data-hydrated]'); + await page.waitForLoadState('networkidle'); const firstDocLink = page.locator('a[href^="/documents/"]').first(); const href = await firstDocLink.getAttribute('href').catch(() => null); if (href) { @@ -26,6 +24,11 @@ async function getDocumentEditUrl( return '/documents/new'; } +/** Wait for the listbox to become visible after triggering a search. */ +async function waitForListbox(page: Page): Promise { + await page.waitForSelector('[role="listbox"]', { state: 'visible', timeout: 2000 }); +} + test.describe('PersonTypeahead — dropdown visibility (desktop)', () => { test.use({ viewport: { width: 1280, height: 720 } }); @@ -42,23 +45,20 @@ test.describe('PersonTypeahead — dropdown visibility (desktop)', () => { await senderInput.click(); await senderInput.fill('a'); - // Wait for the dropdown to appear - await page.waitForTimeout(400); // debounce is 300ms + // Wait for the dropdown to appear (handles debounce automatically) + await waitForListbox(page); - // If there are results, verify the first item is visible (not occluded) const dropdown = page.locator('[role="listbox"]').first(); - const hasResults = await dropdown.count().then((n) => n > 0); + await expect(dropdown).toBeVisible(); - if (hasResults) { - const firstOption = dropdown.locator('[role="option"]').first(); - await expect(firstOption).toBeVisible(); + const firstOption = dropdown.locator('[role="option"]').first(); + await expect(firstOption).toBeVisible(); - // Verify the bounding box is within the viewport (not clipped) - const box = await firstOption.boundingBox(); - expect(box).not.toBeNull(); - expect(box!.y).toBeGreaterThan(0); - expect(box!.y + box!.height).toBeLessThan(720); - } + // Verify the bounding box is within the viewport (not clipped) + const box = await firstOption.boundingBox(); + expect(box).not.toBeNull(); + expect(box!.y).toBeGreaterThan(0); + expect(box!.y + box!.height).toBeLessThan(720); await page.screenshot({ path: 'test-results/e2e/person-typeahead-dropdown-desktop.png' }); }); @@ -78,18 +78,16 @@ test.describe('PersonTypeahead — dropdown visibility (desktop)', () => { await senderInput.click(); await senderInput.fill('a'); - await page.waitForTimeout(400); + await waitForListbox(page); const dropdown = page.locator('[role="listbox"]').first(); - const hasDropdown = (await dropdown.count()) > 0; + await expect(dropdown).toBeVisible(); - if (hasDropdown) { - const dropdownBox = await dropdown.boundingBox(); - expect(dropdownBox).not.toBeNull(); + const dropdownBox = await dropdown.boundingBox(); + expect(dropdownBox).not.toBeNull(); - // Dropdown must appear below the input, not on top or clipped behind it - expect(dropdownBox!.y).toBeGreaterThanOrEqual(inputBox!.y + inputBox!.height - 5); - } + // Dropdown must appear below the input, not on top or clipped behind it + expect(dropdownBox!.y).toBeGreaterThanOrEqual(inputBox!.y + inputBox!.height - 5); await page.screenshot({ path: 'test-results/e2e/person-typeahead-dropdown-position.png' }); }); @@ -108,20 +106,18 @@ test.describe('PersonTypeahead — dropdown visibility (tablet)', () => { await senderInput.click(); await senderInput.fill('a'); - await page.waitForTimeout(400); + await waitForListbox(page); const dropdown = page.locator('[role="listbox"]').first(); - const hasResults = (await dropdown.count()) > 0; + await expect(dropdown).toBeVisible(); - if (hasResults) { - const firstOption = dropdown.locator('[role="option"]').first(); - await expect(firstOption).toBeVisible(); + const firstOption = dropdown.locator('[role="option"]').first(); + await expect(firstOption).toBeVisible(); - const box = await firstOption.boundingBox(); - expect(box).not.toBeNull(); - expect(box!.y).toBeGreaterThan(0); - expect(box!.y + box!.height).toBeLessThan(1024); - } + const box = await firstOption.boundingBox(); + expect(box).not.toBeNull(); + expect(box!.y).toBeGreaterThan(0); + expect(box!.y + box!.height).toBeLessThan(1024); await page.screenshot({ path: 'test-results/e2e/person-typeahead-dropdown-tablet.png' }); }); @@ -138,19 +134,14 @@ test.describe('PersonTypeahead — keyboard navigation', () => { const senderInput = page.locator('#senderId-search'); await senderInput.click(); await senderInput.fill('a'); - await page.waitForTimeout(400); + await waitForListbox(page); - const dropdown = page.locator('[role="listbox"]').first(); - const hasDropdown = (await dropdown.count()) > 0; + await senderInput.press('ArrowDown'); + // First option should now be the active descendant + const activeDescendant = await senderInput.getAttribute('aria-activedescendant'); + expect(activeDescendant).toBeTruthy(); - if (hasDropdown) { - await senderInput.press('ArrowDown'); - // First option should now be the active descendant - const activeDescendant = await senderInput.getAttribute('aria-activedescendant'); - expect(activeDescendant).toBeTruthy(); - - await page.screenshot({ path: 'test-results/e2e/person-typeahead-keyboard-nav.png' }); - } + await page.screenshot({ path: 'test-results/e2e/person-typeahead-keyboard-nav.png' }); }); test('Escape key closes the dropdown', async ({ page }) => { @@ -161,17 +152,12 @@ test.describe('PersonTypeahead — keyboard navigation', () => { const senderInput = page.locator('#senderId-search'); await senderInput.click(); await senderInput.fill('a'); - await page.waitForTimeout(400); + await waitForListbox(page); const dropdown = page.locator('[role="listbox"]').first(); - const hasDropdown = (await dropdown.count()) > 0; - - if (hasDropdown) { - await expect(dropdown).toBeVisible(); - await senderInput.press('Escape'); - await page.waitForTimeout(100); - await expect(dropdown).not.toBeVisible(); - } + await expect(dropdown).toBeVisible(); + await senderInput.press('Escape'); + await expect(dropdown).not.toBeVisible(); }); test('aria-expanded is true when dropdown is open', async ({ page }) => { @@ -187,15 +173,10 @@ test.describe('PersonTypeahead — keyboard navigation', () => { await senderInput.click(); await senderInput.fill('a'); - await page.waitForTimeout(400); + await waitForListbox(page); - const dropdown = page.locator('[role="listbox"]').first(); - const hasDropdown = (await dropdown.count()) > 0; - - if (hasDropdown) { - const expanded = await senderInput.getAttribute('aria-expanded'); - expect(expanded).toBe('true'); - } + const expanded = await senderInput.getAttribute('aria-expanded'); + expect(expanded).toBe('true'); }); }); @@ -210,17 +191,12 @@ test.describe('PersonTypeahead — click-outside dismiss (fixed position)', () = const senderInput = page.locator('#senderId-search'); await senderInput.click(); await senderInput.fill('a'); - await page.waitForTimeout(400); + await waitForListbox(page); const dropdown = page.locator('[role="listbox"]').first(); - const hasDropdown = (await dropdown.count()) > 0; - - if (hasDropdown) { - await expect(dropdown).toBeVisible(); - // Click somewhere else on the page - await page.click('body', { position: { x: 10, y: 10 } }); - await page.waitForTimeout(100); - await expect(dropdown).not.toBeVisible(); - } + await expect(dropdown).toBeVisible(); + // Click somewhere else on the page + await page.click('body', { position: { x: 10, y: 10 } }); + await expect(dropdown).not.toBeVisible(); }); });