From b467be96863c1925d87fd85a8d466768d7745d51 Mon Sep 17 00:00:00 2001 From: "Claude frontend-developer (Opus 4.6)" Date: Mon, 23 Feb 2026 10:51:51 +0000 Subject: [PATCH 1/3] fix(e2e): improve robustness of flaky tablet/mobile E2E tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Root Causes ### search() / clearSearch() race condition (work-items-list.spec.ts tablet timeouts) The waitForResponse listener was registered AFTER searchInput.fill(), creating a race where the debounced 300ms request could complete before the listener was attached. On WebKit/tablet, this caused the listener to never resolve. Fix: register the listener BEFORE the fill action, then await it after. Also added waitForLoaded() after the response to ensure React has flushed filtered data into the DOM before callers read titles. ### confirmDelete() missing DELETE response wait (work-items-list.spec.ts mobile timeout) The confirmDelete() method clicked the button and waited for the modal to hide, but didn't wait for the DELETE API response. This caused a race where the test proceeded to assert list state before the server confirmed deletion and before React re-fetched the list. Fix: register a DELETE 204 response listener before the click. In the test, the post-delete list-refresh GET listener is also registered before confirmDelete() is called. ### waitForURL timeout too short for mobile WebKit (work-item-create.spec.ts) The waitForURL('**/work-items/**', { timeout: 7000 }) hardcoded a 7s timeout that was too short for mobile WebKit (project-level navigationTimeout is 15s). Removed the hardcoded timeout so the project-level setting applies. Same fix for the heading assertion. ### proxy login race condition (proxy-setup.spec.ts desktop) The login tests used `expect(page).not.toHaveURL(/\/login/, { timeout: 15000 })` after button click, which could time out if React's router update lagged behind session establishment. Replaced with a shared loginThroughProxy() helper that uses Promise.all to start the API response listener BEFORE clicking Submit, then waits for URL change with waitForURL() — a condition-based wait that is more reliable than polling not.toHaveURL. Eliminated duplicated login code across 3 tests. Co-Authored-By: Claude (Sonnet 4.6) --- e2e/pages/WorkItemsPage.ts | 43 +++++++++++-- e2e/tests/proxy/proxy-setup.spec.ts | 64 +++++++++++++------ e2e/tests/work-items/work-item-create.spec.ts | 8 ++- e2e/tests/work-items/work-items-list.spec.ts | 15 ++++- 4 files changed, 97 insertions(+), 33 deletions(-) diff --git a/e2e/pages/WorkItemsPage.ts b/e2e/pages/WorkItemsPage.ts index 331fd846f..fca43460b 100644 --- a/e2e/pages/WorkItemsPage.ts +++ b/e2e/pages/WorkItemsPage.ts @@ -152,23 +152,41 @@ export class WorkItemsPage { } /** - * Type a search query and wait for the debounced API response. + * Type a search query and wait for both the debounced API response and the + * DOM to re-render with the filtered results. + * + * The response listener must be registered BEFORE the fill action to avoid a + * race condition where the debounced request resolves before the listener is + * attached (especially common on WebKit/tablet where the 300ms debounce can + * fire and complete before the next line executes). + * + * After the network response is received we additionally call waitForLoaded() + * to ensure React has flushed the new data into the DOM before callers + * attempt to read titles or interact with list items. */ async search(query: string): Promise { - await this.searchInput.fill(query); - await this.page.waitForResponse( + const responsePromise = this.page.waitForResponse( (resp) => resp.url().includes('/api/work-items') && resp.status() === 200, ); + await this.searchInput.fill(query); + await responsePromise; + await this.waitForLoaded(); } /** - * Clear the search input and wait for the list to update. + * Clear the search input and wait for both the API response and the DOM to + * update. + * + * The response listener must be registered BEFORE the clear action for the + * same race-condition reason as search(). */ async clearSearch(): Promise { - await this.searchInput.clear(); - await this.page.waitForResponse( + const responsePromise = this.page.waitForResponse( (resp) => resp.url().includes('/api/work-items') && resp.status() === 200, ); + await this.searchInput.clear(); + await responsePromise; + await this.waitForLoaded(); } /** @@ -216,10 +234,21 @@ export class WorkItemsPage { /** * Confirm the deletion in the delete modal. - * No explicit timeout — uses project-level actionTimeout (15s for WebKit). + * + * Waits for both the DELETE API response and the modal to hide. Registering + * the response listener before the click prevents a race where the DELETE + * completes and the modal closes before the listener is attached (common on + * fast Chromium or heavily-loaded CI runners). */ async confirmDelete(): Promise { + const deleteResponsePromise = this.page.waitForResponse( + (resp) => + resp.url().includes('/api/work-items') && + resp.request().method() === 'DELETE' && + resp.status() === 204, + ); await this.deleteConfirmButton.click(); + await deleteResponsePromise; await this.deleteModal.waitFor({ state: 'hidden' }); } diff --git a/e2e/tests/proxy/proxy-setup.spec.ts b/e2e/tests/proxy/proxy-setup.spec.ts index 09b814b09..894f92964 100644 --- a/e2e/tests/proxy/proxy-setup.spec.ts +++ b/e2e/tests/proxy/proxy-setup.spec.ts @@ -25,6 +25,42 @@ test.beforeAll(async () => { } }); +/** + * Log in through the proxy and wait until the session is established. + * + * Uses Promise.all to start the API response listener before clicking Submit, + * preventing a race where the login response arrives before the listener is + * attached (especially relevant through the extra nginx hop). After the API + * response confirms success, we wait for the URL to change away from /login + * using waitForURL — this is a condition-based wait that is more reliable than + * the previous `expect(page).not.toHaveURL` pattern which could time out if + * React's router update lagged slightly behind the session establishment. + * + * @param page - Playwright Page within an unauthenticated context + * @param baseUrl - The proxy base URL to use + */ +async function loginThroughProxy(page: import('@playwright/test').Page, baseUrl: string) { + await page.goto(`${baseUrl}/login`); + await page.getByLabel(/email/i).fill(TEST_ADMIN.email); + await page.getByLabel(/password/i).fill(TEST_ADMIN.password); + + // Start the response listener BEFORE clicking so we don't miss the response. + const [loginResponse] = await Promise.all([ + page.waitForResponse( + (resp) => resp.url().includes('/api/auth/login') && resp.request().method() === 'POST', + ), + page.getByRole('button', { name: /sign in/i }).click(), + ]); + + if (!loginResponse.ok()) { + throw new Error(`Login through proxy failed: ${loginResponse.status()} ${loginResponse.statusText()}`); + } + + // Wait for the React router to navigate away from /login after session is set. + // The proxy adds an extra nginx hop so allow enough time for the redirect chain. + await page.waitForURL((url) => !url.pathname.includes('/login'), { timeout: 15000 }); +} + test.describe('Reverse Proxy Setup', { tag: '@responsive' }, () => { test('should return healthy status through proxy', async ({ request }) => { // Given: A reverse proxy forwarding to the Cornerstone app @@ -73,16 +109,12 @@ test.describe('Reverse Proxy Setup', { tag: '@responsive' }, () => { storageState: { cookies: [], origins: [] }, }); const page = await context.newPage(); - await page.goto(`${proxyBaseUrl}/login`); - // When: Logging in with valid credentials - await page.getByLabel(/email/i).fill(TEST_ADMIN.email); - await page.getByLabel(/password/i).fill(TEST_ADMIN.password); - await page.getByRole('button', { name: /sign in/i }).click(); + // When: Logging in with valid credentials through the proxy + await loginThroughProxy(page, proxyBaseUrl); - // Then: Should redirect away from login page (to dashboard or home) - // Proxy login goes through an extra nginx hop — give it more time than the default 5s - await expect(page).not.toHaveURL(/\/login/, { timeout: 15000 }); + // Then: Should have redirected away from login page (to dashboard or home) + expect(page.url()).not.toMatch(/\/login/); await context.close(); }); @@ -96,12 +128,8 @@ test.describe('Reverse Proxy Setup', { tag: '@responsive' }, () => { }); const page = await context.newPage(); - // When: Logging in through the proxy - await page.goto(`${proxyBaseUrl}/login`); - await page.getByLabel(/email/i).fill(TEST_ADMIN.email); - await page.getByLabel(/password/i).fill(TEST_ADMIN.password); - await page.getByRole('button', { name: /sign in/i }).click(); - await expect(page).not.toHaveURL(/\/login/, { timeout: 15000 }); + // When: Logging in through the proxy (helper waits for session to be established) + await loginThroughProxy(page, proxyBaseUrl); // And: Navigating to a protected route await page.goto(`${proxyBaseUrl}/profile`); @@ -121,12 +149,8 @@ test.describe('Reverse Proxy Setup', { tag: '@responsive' }, () => { }); const page = await context.newPage(); - // Login first - await page.goto(`${proxyBaseUrl}/login`); - await page.getByLabel(/email/i).fill(TEST_ADMIN.email); - await page.getByLabel(/password/i).fill(TEST_ADMIN.password); - await page.getByRole('button', { name: /sign in/i }).click(); - await expect(page).not.toHaveURL(/\/login/, { timeout: 15000 }); + // Login first — helper waits for the API response and URL to change + await loginThroughProxy(page, proxyBaseUrl); // When: Logging out through the proxy await page.goto(`${proxyBaseUrl}/profile`); diff --git a/e2e/tests/work-items/work-item-create.spec.ts b/e2e/tests/work-items/work-item-create.spec.ts index 072079201..b4ebde0d1 100644 --- a/e2e/tests/work-items/work-item-create.spec.ts +++ b/e2e/tests/work-items/work-item-create.spec.ts @@ -98,12 +98,14 @@ test.describe('Create with title only — happy path (Scenario 3)', { tag: '@res createdId = body.workItem?.id ?? body.id ?? null; // Should redirect to /work-items/:id - await page.waitForURL('**/work-items/**', { timeout: 7000 }); + // Use project-level navigationTimeout (15s for WebKit) — do not hardcode + // a timeout that is too short for mobile/tablet WebKit. + await page.waitForURL('**/work-items/**'); expect(page.url()).toMatch(/\/work-items\/[a-z0-9-]+$/); expect(page.url()).not.toContain('/work-items/new'); - // Detail page shows the correct title - await expect(page.getByRole('heading', { level: 1 })).toHaveText(title, { timeout: 7000 }); + // Detail page shows the correct title — use project-level expect timeout + await expect(page.getByRole('heading', { level: 1 })).toHaveText(title); } finally { if (createdId) await deleteWorkItemViaApi(page, createdId); } diff --git a/e2e/tests/work-items/work-items-list.spec.ts b/e2e/tests/work-items/work-items-list.spec.ts index 932a2faaa..3fb96d4ce 100644 --- a/e2e/tests/work-items/work-items-list.spec.ts +++ b/e2e/tests/work-items/work-items-list.spec.ts @@ -292,11 +292,20 @@ test.describe('Delete modal — confirm (Scenario 6)', { tag: '@responsive' }, ( const modalText = await workItemsPage.deleteModal.textContent(); expect(modalText).toContain(title); - // Confirm deletion + // Register the list-refresh response listener before confirming deletion so + // we don't miss the GET that fires immediately after the DELETE completes. + const listRefreshPromise = page.waitForResponse( + (resp) => resp.url().includes('/api/work-items') && resp.status() === 200, + ); + + // Confirm deletion — confirmDelete() waits for the DELETE API response and + // the modal to close. await workItemsPage.confirmDelete(); - // Work item no longer in list - await workItemsPage.waitForLoaded(); + // Wait for the list to refresh with the post-delete GET response. + await listRefreshPromise; + + // Work item no longer in list (assert on DOM, no need for another waitForLoaded) const titlesAfter = await workItemsPage.getWorkItemTitles(); expect(titlesAfter).not.toContain(title); From 8bcc4323b4c8439cb6bc12cfac10e337d5d9d96f Mon Sep 17 00:00:00 2001 From: "Claude product-architect (Opus 4.6)" Date: Mon, 23 Feb 2026 10:54:42 +0000 Subject: [PATCH 2/3] fix(e2e): fix ESLint consistent-type-imports in proxy-setup.spec.ts Use a proper `import type { Page }` declaration instead of an inline `import()` type annotation, which is forbidden by the `@typescript-eslint/consistent-type-imports` rule. Co-Authored-By: Claude (Sonnet 4.6) --- e2e/tests/proxy/proxy-setup.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/e2e/tests/proxy/proxy-setup.spec.ts b/e2e/tests/proxy/proxy-setup.spec.ts index 894f92964..e47a732a6 100644 --- a/e2e/tests/proxy/proxy-setup.spec.ts +++ b/e2e/tests/proxy/proxy-setup.spec.ts @@ -6,6 +6,7 @@ */ import { test, expect } from '@playwright/test'; +import type { Page } from '@playwright/test'; import { readFile } from 'fs/promises'; import { TEST_ADMIN, API } from '../../fixtures/testData.js'; @@ -39,7 +40,7 @@ test.beforeAll(async () => { * @param page - Playwright Page within an unauthenticated context * @param baseUrl - The proxy base URL to use */ -async function loginThroughProxy(page: import('@playwright/test').Page, baseUrl: string) { +async function loginThroughProxy(page: Page, baseUrl: string) { await page.goto(`${baseUrl}/login`); await page.getByLabel(/email/i).fill(TEST_ADMIN.email); await page.getByLabel(/password/i).fill(TEST_ADMIN.password); From 7e031ab8fd91ce4753365741e242bddedf10669e Mon Sep 17 00:00:00 2001 From: "Claude product-architect (Opus 4.6)" Date: Mon, 23 Feb 2026 10:58:22 +0000 Subject: [PATCH 3/3] style(e2e): apply Prettier formatting to proxy-setup.spec.ts Co-Authored-By: Claude (Sonnet 4.6) --- e2e/tests/proxy/proxy-setup.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/e2e/tests/proxy/proxy-setup.spec.ts b/e2e/tests/proxy/proxy-setup.spec.ts index e47a732a6..c9dda21e5 100644 --- a/e2e/tests/proxy/proxy-setup.spec.ts +++ b/e2e/tests/proxy/proxy-setup.spec.ts @@ -54,7 +54,9 @@ async function loginThroughProxy(page: Page, baseUrl: string) { ]); if (!loginResponse.ok()) { - throw new Error(`Login through proxy failed: ${loginResponse.status()} ${loginResponse.statusText()}`); + throw new Error( + `Login through proxy failed: ${loginResponse.status()} ${loginResponse.statusText()}`, + ); } // Wait for the React router to navigate away from /login after session is set.