From 86a0d7815711c802ef711821fa934eaee7d4868e Mon Sep 17 00:00:00 2001 From: QA Runner Date: Mon, 20 Jul 2026 11:44:00 -0700 Subject: [PATCH] test(e2e): drop transient-5xx retry scaffolding now that the fetch storm is fixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the directory fetch storm eliminated (previous commit: batched listProjects?include=documents instead of a getProject burst per modal open), the local Supabase gateway no longer collapses under modal-open load, so the specs' 5xx-oriented scaffolding is removed: - create-with-retry loops (project create x2, chat create x2, workflow create, tabular-review create x10-attempt, review add-document x6) - reload-guards for transient "Project not found" (waitForProjectLoaded, gotoProjectRow, project-assistant tab loop) - networkidle settle-waits whose purpose was to let the per-project fetch burst drain before submitting (NewProjectModal / NewTRModal) - oversized per-test timeouts justified by the storm (180s -> 60-120s) Kept, deliberately: - the title-generation settle logic in chat rename/delete - chat submit's model-seeding retry loop (guards a documented useSelectedModel localStorage hydration race, unrelated to the gateway) - account display-name toPass block (guards a documented profile hydration race, unrelated to the gateway) - generic short networkidle settles after login/logout navigation - Playwright's own retries: 2 on CI Verified: with the fix, 3 consecutive full-suite runs (plus 2 more before de-retry) were green with zero Playwright retries, running against an account seeded with ~40 projects — a larger directory fan-out than CI's fresh database ever produces. Co-Authored-By: Claude Fable 5 --- e2e/chat-management.spec.ts | 153 ++++++--------------------- e2e/critical-path.spec.ts | 72 +++---------- e2e/project-management.spec.ts | 138 ++++++------------------- e2e/tabular-reviews.spec.ts | 184 ++++++++++----------------------- e2e/workflows-account.spec.ts | 125 +++++++--------------- 5 files changed, 169 insertions(+), 503 deletions(-) diff --git a/e2e/chat-management.spec.ts b/e2e/chat-management.spec.ts index db94a0bc..492fa655 100644 --- a/e2e/chat-management.spec.ts +++ b/e2e/chat-management.spec.ts @@ -219,17 +219,9 @@ test("delete chat: sidebar delete action removes the chat from history", async ( await page.goto("/assistant"); const textarea = page.getByPlaceholder("How can I help?"); await expect(textarea).toBeVisible({ timeout: 10_000 }); - // ── Step 2: create the chat, riding out transient gateway 502s ─────────────── + // ── Step 2: create the chat ────────────────────────────────────────────────── // ChatInput.handleSubmit creates the chat (saveChat → POST /chat/create) then - // navigates to /assistant/chat/. The local Supabase/Kong gateway - // intermittently returns 502 on that POST under load; saveChat then returns null - // (createChat throws → caught in ChatHistoryContext.saveChat), so no chat is - // created and no navigation happens — the page stays on /assistant with the - // textarea intact, so re-submitting recovers the transient failure. This mirrors - // the bounded create-with-retry the project-assistant test (Step 7-8 below) uses - // for the same flaky POST. A genuinely broken create/navigation never reaches - // /assistant/chat/ on any attempt, so the final waitForURL still fails and - // the regression this test guards is preserved. + // navigates to /assistant/chat/. const newChatUrl = /\/assistant\/chat\/.+/; // Sending the first message kicks off auto title-generation @@ -246,22 +238,11 @@ test("delete chat: sidebar delete action removes the chat from history", async ( ) .catch(() => null); - const CREATE_ATTEMPTS = 4; - for (let attempt = 0; attempt < CREATE_ATTEMPTS; attempt++) { - if (newChatUrl.test(page.url())) break; - // Re-assert the demo model as the active (available) model in case a - // remount reset it to the default Gemini (which the ApiKeyMissingModal - // would block on), then re-fill and re-submit. - await selectDemoModel(page); - await textarea.fill(message); - await textarea.press("Enter"); - try { - await page.waitForURL(newChatUrl, { timeout: 20_000 }); - break; - } catch { - // transient gateway 5xx on POST /chat/create — still on /assistant; retry - } - } + // Pick the keyless demo model so the submit isn't blocked by the + // ApiKeyMissingModal, then send the first message. + await selectDemoModel(page); + await textarea.fill(message); + await textarea.press("Enter"); await page.waitForURL(newChatUrl, { timeout: 20_000 }); // Let auto title-generation land before renaming, so it cannot clobber the @@ -296,22 +277,13 @@ test("delete chat: sidebar delete action removes the chat from history", async ( .locator("div.group.relative.h-8.rounded-md") .filter({ has: targetTitle }); - // ── Step 5-7: delete that specific chat, riding out flaky Supabase 500s ────── - // deleteChatFn (ChatHistoryContext.tsx:157-168) optimistically removes the row - // then, on API error, refetches via loadChats() which RESTORES it — so a - // transient 500 on DELETE /chat/ re-adds the row. Retry the open→Delete - // interaction until the uniquely-titled row is gone. A genuinely broken delete - // never removes it on any attempt, so the final assertion still fails. - // SidebarChatItem.tsx:132-144: the "Delete" DropdownMenuItem calls + // ── Step 5-7: delete that specific chat ────────────────────────────────────── + // deleteChatFn (ChatHistoryContext.tsx:157-168) optimistically removes the + // row. SidebarChatItem.tsx:132-144: the "Delete" DropdownMenuItem calls // deleteChat(chat.id) directly — no confirmation dialog. - const DELETE_ATTEMPTS = 4; - for (let attempt = 0; attempt < DELETE_ATTEMPTS; attempt++) { - if (!(await targetTitle.isVisible().catch(() => false))) break; - await targetRow.hover(); - await targetRow.locator("button").last().click(); - await page.getByRole("menuitem", { name: "Delete" }).click(); - await expect(targetTitle).toBeHidden({ timeout: 10_000 }).catch(() => {}); - } + await targetRow.hover(); + await targetRow.locator("button").last().click(); + await page.getByRole("menuitem", { name: "Delete" }).click(); await expect(targetTitle).toBeHidden({ timeout: 10_000 }); }); @@ -325,13 +297,10 @@ test("project assistant: create a new chat and submit a question", async ({ page // removing that router.push: "+ Create New" then no longer navigates and the // Step 8 waitForURL below fails.) - // This test creates a project then a chat (two sequential write round-trips plus - // a route compile/render each). Under a loaded dev server those round-trips can - // each take tens of seconds, so give the whole test — and the two create waits — - // generous headroom. Reliability matters more than speed here. The bounded - // create-with-retry on both the project and the chat (each up to a few - // re-submits under load) needs headroom beyond the create round-trips. - test.setTimeout(180_000); + // This test creates a project then a chat (two sequential write round-trips + // plus a navigation each) and ends with an LLM-backed submit, so give it + // headroom beyond the 30s default. + test.setTimeout(120_000); // ── Step 1: navigate to projects ───────────────────────────────────────────── await page.goto("/projects"); @@ -354,105 +323,41 @@ test("project assistant: create a new chat and submit a question", async ({ page // "Create project" submit button only exists on the second step. await page.getByRole("button", { name: "Next", exact: true }).click(); - // ── Step 4: submit the form (resilient to transient gateway 502s) ──────────── + // ── Step 4: submit the form ────────────────────────────────────────────────── // NewProjectModal.handleSubmit does NOT navigate itself — it calls onCreated() // then onClose(). ProjectsOverview.onCreated (ProjectsOverview.tsx:475-478) // inserts the row AND router.push(`/projects/${p.id}`), so the app navigates // straight to the project detail page; the name never re-appears in a list to // click. Wait for that navigation instead of asserting a list row. - // - // The local Supabase/Kong gateway intermittently returns 502 on POST /projects - // under load. On a failed create, handleSubmit catches the error, leaves the - // modal open with the name retained, and re-enables the submit button — so a - // transient failure is recovered by re-submitting. A genuine broken - // create/navigation never reaches /projects/ on any attempt, so the final - // assertion below still fails and that regression is preserved. const submitBtn = page.getByRole("button", { name: /create project|creating/i, }); const projectUrl = /\/projects\/[^/]+$/; - const CREATE_ATTEMPTS = 4; - for (let attempt = 0; attempt < CREATE_ATTEMPTS; attempt++) { - if (projectUrl.test(page.url())) break; - // toBeEnabled rides out a slow in-flight "Creating…" from a prior attempt. - await expect(submitBtn).toBeEnabled({ timeout: 10_000 }); - await submitBtn.click(); - try { - await page.waitForURL(projectUrl, { timeout: 20_000 }); - break; - } catch { - // transient gateway 5xx — modal stays open, name retained; retry - } - } + await expect(submitBtn).toBeEnabled({ timeout: 10_000 }); + await submitBtn.click(); await page.waitForURL(projectUrl, { timeout: 20_000 }); - // ── Step 6: open the assistant tab and reach the empty-state "+ Create New" ── + // ── Step 6: open the assistant tab and reach the empty-state "Create" ──────── // The project assistant is now a nested route (/projects/[id]/assistant), not a // ?tab= query on the detail page. Navigating straight there avoids ambiguity // with the sidebar "Assistant" nav item. - // - // The project workspace fetches getProject(id) on mount with no client-side - // refetch. Under load that GET can transiently 502, leaving project=null so the - // page renders "Project not found" and the assistant section never mounts. The - // project row genuinely exists (Step 4 navigated to its id), so a reload - // refetches and recovers. Bounded-retry the load until the empty-state - // "+ Create New" button (ProjectAssistantTable.tsx:110-115, shown when - // chats.length === 0) is visible, reloading past any transient "Project not - // found". A genuinely broken assistant tab never shows the button on any - // attempt, so the final assertion still fails. const assistantUrl = page.url() + "/assistant"; // The olp UI replaced the old "+ Create New" text link with a PillButton - // reading "Create" in the ProjectAssistantTable empty state. + // reading "Create" in the ProjectAssistantTable empty state + // (ProjectAssistantTable.tsx:110-115, shown when chats.length === 0). const createNewBtn = page.getByRole("button", { name: "Create", exact: true }); - const projectNotFound = page.getByText("Project not found"); - const TAB_ATTEMPTS = 4; - for (let attempt = 0; attempt < TAB_ATTEMPTS; attempt++) { - await page.goto(assistantUrl); - // Race the empty-state button against the transient "Project not found". - const outcome = await Promise.race([ - createNewBtn - .waitFor({ state: "visible", timeout: 20_000 }) - .then(() => "ready") - .catch(() => "retry"), - projectNotFound - .waitFor({ state: "visible", timeout: 20_000 }) - .then(() => "notfound") - .catch(() => "retry"), - ]); - if (outcome === "ready") break; - // "notfound" (transient getProject 502) or a timeout — reload to refetch. - } + await page.goto(assistantUrl); await expect(createNewBtn).toBeVisible({ timeout: 20_000 }); - // ── Step 7-8: click "+ Create New" and wait for the project chat URL ───────── + // ── Step 7-8: click "Create" and wait for the project chat URL ─────────────── // handleNewChat (ProjectPage.tsx:515-519) calls saveChat(projectId) then - // router.push(`/projects/${projectId}/assistant/chat/${id}`). On a transient - // gateway 502, saveChat returns null (createChat throws → caught in - // ChatHistoryContext.saveChat), so no chat is created and no navigation - // happens; the empty-state button stays mounted (chats still empty), so - // re-clicking recovers the transient failure. + // router.push(`/projects/${projectId}/assistant/chat/${id}`). // // REGRESSION (the target of this test): if handleNewChat's router.push is - // removed, saveChat still succeeds and adds the chat to state, so the - // empty-state "+ Create New" button is replaced by the chat list and no - // navigation occurs — the loop's visibility guard then stops retrying and the - // final waitForURL fails. If saveChat itself is broken, navigation never - // happens either. So a genuine break fails on every attempt and is preserved. + // removed — or saveChat itself is broken — no navigation happens and the + // waitForURL below fails. const chatUrl = /\/projects\/.+\/assistant\/chat\/.+/; - const CHAT_ATTEMPTS = 4; - for (let attempt = 0; attempt < CHAT_ATTEMPTS; attempt++) { - if (chatUrl.test(page.url())) break; - // Empty-state button gone without navigation ⇒ a chat was created but - // never navigated (router.push regression): stop so it surfaces below. - if (!(await createNewBtn.isVisible().catch(() => false))) break; - await createNewBtn.click(); - try { - await page.waitForURL(chatUrl, { timeout: 20_000 }); - break; - } catch { - // transient gateway 5xx — empty-state remains; retry - } - } + await createNewBtn.click(); await page.waitForURL(chatUrl, { timeout: 20_000 }); // ── Step 9: assert the ChatInput textarea is visible ───────────────────────── diff --git a/e2e/critical-path.spec.ts b/e2e/critical-path.spec.ts index c72abfb9..a453067b 100644 --- a/e2e/critical-path.spec.ts +++ b/e2e/critical-path.spec.ts @@ -55,11 +55,11 @@ test("create project, upload PDF, ask a question and receive a response", async page, }) => { test.skip(!hasLlmKey, LLM_SKIP_REASON); - /* This end-to-end flow (create + upload + navigate + chat) is throttled by - the local Supabase stack and needs far more than the 30s default. The - per-test `{ timeout }` option that test() accepts is silently ignored by - Playwright (that object only takes tag/annotation), so set it here. */ - test.setTimeout(180_000); + /* This end-to-end flow (create + upload + navigate + chat) needs more than + the 30s default. The per-test `{ timeout }` option that test() accepts is + silently ignored by Playwright (that object only takes tag/annotation), + so set it here. */ + test.setTimeout(120_000); /* ── Step 1: navigate to projects ─────────────────────────────────────── */ await page.goto("/projects"); @@ -96,71 +96,29 @@ test("create project, upload PDF, ask a question and receive a response", async ).toBeVisible({ timeout: 5_000 }); /* ── Step 5: submit the form ──────────────────────────────────────────── */ - /* The modal's FileDirectory (useDirectoryData) fires a burst of Supabase - requests when the modal opens — a getProject() per existing project. - Submitting mid-burst makes the local Supabase gateway (Kong) return a 502 - ("An invalid response was received from the upstream server"), surfaced - by the modal inline as text-red-500. Let those requests settle first so - the create POST goes through cleanly. */ - await page - .waitForLoadState("networkidle", { timeout: 45_000 }) - .catch(() => {}); - /* The PDF upload runs inside NewProjectModal.handleSubmit (await Promise.all([uploadProjectDocument(...)])) BEFORE onCreated fires, so the "Creating…" button state can persist for many seconds while the file uploads. ProjectsOverview.onCreated then router.push()es to the new project page, so wait for that navigation (generously, to cover the - upload). Race it against the inline error so a residual transient 502 is - detected immediately and retried by re-submitting (safe — the failed - request created nothing). */ - const inlineError = page.locator("form p.text-red-500"); - for (let attempt = 1; attempt <= 5; attempt++) { - await page.click('button[type="submit"]'); - const outcome = await Promise.race([ - page - .waitForURL(/\/projects\/[^/]+$/, { timeout: 30_000 }) - .then(() => "nav" as const) - .catch(() => "timeout" as const), - inlineError - .waitFor({ state: "visible", timeout: 30_000 }) - .then(() => "error" as const) - .catch(() => "timeout" as const), - ]); - if (outcome === "nav") break; - if (attempt === 5) { - throw new Error( - `create project: never navigated (last outcome: ${outcome})`, - ); - } - await inlineError - .waitFor({ state: "hidden", timeout: 2_000 }) - .catch(() => {}); - } + upload). */ + await page.click('button[type="submit"]'); + await page.waitForURL(/\/projects\/[^/]+$/, { timeout: 30_000 }); /* ── Step 6: open the project assistant ───────────────────────────────── */ /* We're already on /projects/[id] (Documents tab by default). The project assistant is now a nested route, /projects/[id]/assistant. Navigate there directly rather than clicking through the tab bar to avoid ambiguity with - the "Assistant" item in the sidebar nav. The workspace fetches - getProject() on mount and does NOT retry, so under the local-Supabase load - the page can land on a permanent "Project not found" or a slow skeleton; - re-navigate until the assistant tab's empty-state "Create" affordance - renders. (The olp UI replaced the old "+ Create New" text link with a - PillButton reading "Create" — ProjectAssistantTable empty state.) */ + the "Assistant" item in the sidebar nav. (The olp UI replaced the old + "+ Create New" text link with a PillButton reading "Create" — + ProjectAssistantTable empty state.) */ const projectUrl = page.url().split("?")[0]; const createNew = page.getByRole("button", { name: "Create", exact: true }); - for (let attempt = 1; attempt <= 6; attempt++) { - await page.goto(`${projectUrl}/assistant`); - await page - .waitForLoadState("networkidle", { timeout: 20_000 }) - .catch(() => {}); - if (await createNew.isVisible().catch(() => false)) break; - } + await page.goto(`${projectUrl}/assistant`); - /* The assistant tab shows a chat list. Click "+ Create New" to open the - chat interface where the text input appears. */ - await expect(createNew).toBeVisible({ timeout: 10_000 }); + /* The assistant tab shows a chat list. Click "Create" to open the chat + interface where the text input appears. */ + await expect(createNew).toBeVisible({ timeout: 20_000 }); await createNew.click(); /* Navigates to /projects/{id}/assistant (new chat UI) */ await page.waitForURL(/\/projects\/.+\/assistant/, { timeout: 10_000 }); diff --git a/e2e/project-management.spec.ts b/e2e/project-management.spec.ts index cac7bd71..2a096ed3 100644 --- a/e2e/project-management.spec.ts +++ b/e2e/project-management.spec.ts @@ -33,18 +33,11 @@ async function createProject( projectName: string, filePath?: string, ) { - /* These tests are throttled by the local Supabase stack, which the app - hammers on every modal open (see the settle note below). The per-test - `{ timeout }` option passed to test() is silently ignored by Playwright - (that object only accepts tag/annotation), so the tests would otherwise - run at the 30s default — too tight for the directory storm. Raise it - here, where the slow work happens, so every caller gets the budget. - - The storm scales with the number of projects the test user has - accumulated, and the modal now mounts FileDirectory on its second - ("Add Documents") step, so creation alone can consume most of a 120s - budget and starve whatever the caller does afterwards. */ - test.setTimeout(180_000); + /* Creation is a navigation + modal wizard + (optionally) a file upload; the + per-test `{ timeout }` option passed to test() is silently ignored by + Playwright (that object only accepts tag/annotation), so raise the budget + here, where the slow work happens, for every caller. */ + test.setTimeout(60_000); await page.goto("/projects"); await expect(page).toHaveURL(/\/projects/, { timeout: 10_000 }); @@ -74,113 +67,48 @@ async function createProject( ).toBeVisible({ timeout: 5_000 }); } - /* The modal mounts a FileDirectory whose useDirectoryData hook fires several - Supabase-backed requests (listProjects + listStandaloneDocuments, plus a - getProject per project) the moment the modal opens. Submitting while those - are still in flight makes the local Supabase gateway (Kong) buckle under - the concurrent load and return a 502 — "An invalid response was received - from the upstream server" — which the API surfaces as a 500 and the modal - shows inline (text-red-500). Letting the modal's requests settle first - makes creation reliable (measured 6/6 vs 3/6 without this wait). - - useDirectoryData fires getProject() for EVERY existing project, so this - storm grows with the project count and can take ~20s once many test - projects have accumulated. Bound the wait so we never hang on it; if it - doesn't fully settle, the submit retry below absorbs any residual 502. */ - await page - .waitForLoadState("networkidle", { timeout: 45_000 }) - .catch(() => {}); - /* Submit — NewProjectModal's onCreated calls router.push(`/projects/${id}`). The PDF upload runs (awaited) inside handleSubmit before onCreated fires, so allow extra time for navigation when a file is attached. - Even with the settle above, a transient upstream 502 can still slip - through occasionally. NewProjectModal surfaces it inline (text-red-500) - while keeping the modal open and the form state intact, so re-clicking - submit retries safely (the failed request created nothing). Race the - navigation against that inline error so a failure is detected immediately - and retried, rather than burning the full navigation timeout each time. */ + (The modal's FileDirectory used to fan out a getProject() request per + existing project on open, which could overwhelm the local Supabase + gateway and required settle-waits plus a submit-retry loop here. The + directory now loads via one batched listProjects?include=documents + request, so a single submit is reliable.) + + The documents step's primary action submits the form (its label flips + to "Creating…" while in flight, so match on the submit type instead). */ const navTimeout = filePath ? 30_000 : 15_000; - const inlineError = page.locator("form p.text-red-500"); - for (let attempt = 1; attempt <= 5; attempt++) { - /* The documents step's primary action submits the form (its label flips - to "Creating…" while in flight, so match on the submit role instead). */ - await page.locator('button[type="submit"]').click(); - const outcome = await Promise.race([ - page - .waitForURL(/\/projects\/.+/, { timeout: navTimeout }) - .then(() => "nav" as const) - .catch(() => "timeout" as const), - inlineError - .waitFor({ state: "visible", timeout: navTimeout }) - .then(() => "error" as const) - .catch(() => "timeout" as const), - ]); - if (outcome === "nav") return; - if (attempt === 5) { - throw new Error( - `createProject: never navigated to /projects/ (last outcome: ${outcome})`, - ); - } - // transient upstream error (or stall) → wait for the inline message to - // clear so the next iteration's race can't latch onto the stale one, - // then resubmit. - await inlineError - .waitFor({ state: "hidden", timeout: 2_000 }) - .catch(() => {}); - } + await page.locator('button[type="submit"]').click(); + await page.waitForURL(/\/projects\/.+/, { timeout: navTimeout }); } /** - * Navigate to the projects list and return the table row for `projectName`, - * re-navigating (which refetches) if the list doesn't render it in time. - * ProjectsOverview gates its table on listProjects(); under the local-Supabase - * load that call can be slow or transiently 502 (the page then shows "Could not - * load projects."), so a single goto isn't reliable. Rows are
; - * the sidebar "Recent Projects" renders the same name as a