mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site/e2e): fix login helper race condition causing navigation flake (#27107)
Fixes flake reported in [DEVEX-538](https://linear.app/codercom/issue/DEVEX-538/flake-create-user-with-password). ## Problem The `login()` e2e helper had a race condition causing intermittent navigation failures: ``` page.goto: Navigation to "/deployment/users" is interrupted by another navigation to "/" ``` After clicking Sign In, `LoginPage.tsx` does a hard navigation via `location.href = sanitizeRedirect(redirectTo)`. With no `?redirect=` param, `retrieveRedirect` defaults to `"/"`, so login navigates to `/`. The browser loads `/`, fires the `load` event, then React boots and the router does a client-side redirect from `/` to `/workspaces` (via `<Navigate to="/workspaces" replace />`). The old helper waited with `expectUrl(page).toHavePathName("/workspaces")`, which polls `page.url()` and resolves the moment the pathname matches. It has no awareness of page load state. So it resolved after the client-side redirect changed the URL, but before the `/workspaces` page components had mounted. When a test immediately called `page.goto()` afterward, pending React rendering could trigger a competing navigation. ## Fix Replace the URL polling with two Playwright-idiomatic waits: 1. `page.waitForURL(/\/workspaces/)` hooks into the browser's navigation lifecycle: it waits for the URL to match AND for the page to reach a load state (`"load"` by default), unlike `expectUrl` which is purely a string poll. 2. `await expect(page).toHaveTitle(/Workspaces/)` waits for the page title, which is set by the `WorkspacesPage` component. This proves React booted, auth resolved, and the page fully rendered, closing the window where pending React work could interfere with the next navigation. Also adds `{ waitUntil: "domcontentloaded" }` to `page.goto("/login")` for consistency with every other navigation helper in the file. > 🤖 Generated by Coder Agents on behalf of @jeremyruppel
This commit is contained in:
+10
-2
@@ -76,11 +76,19 @@ export async function login(page: Page, options: LoginOptions = users.owner) {
|
||||
// biome-ignore lint/suspicious/noExplicitAny: reset the current user
|
||||
(ctx as any)[Symbol.for("currentUser")] = undefined;
|
||||
await ctx.clearCookies();
|
||||
await page.goto("/login");
|
||||
await page.goto("/login", { waitUntil: "domcontentloaded" });
|
||||
await page.getByLabel("Email").fill(options.email);
|
||||
await page.getByLabel("Password").fill(options.password);
|
||||
await page.getByRole("button", { name: "Sign In" }).click();
|
||||
await expectUrl(page).toHavePathName("/workspaces");
|
||||
// Sign-in triggers a hard navigation to "/", then React Router
|
||||
// client-side redirects to "/workspaces" without firing a load event.
|
||||
// waitForURL alone resolves on the URL change, before WorkspacesPage
|
||||
// has mounted. The title check is the actual synchronization point:
|
||||
// it retries until the page component renders. Removing either wait
|
||||
// reintroduces a navigation race in tests that goto() right after
|
||||
// login. See https://github.com/coder/coder/pull/27107.
|
||||
await page.waitForURL((url) => url.pathname === "/workspaces");
|
||||
await expect(page).toHaveTitle("Workspaces - Coder");
|
||||
// biome-ignore lint/suspicious/noExplicitAny: update once logged in
|
||||
(ctx as any)[Symbol.for("currentUser")] = options;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user