Deflake

.claude/skills/e2e/SKILL.md

@@ -81,8 +81,8 @@ Playwright is configured with `trace: "retain-on-failure"`. After a failure, che
 pnpm exec playwright show-trace test-results/<test-folder>/trace.zip
 ```
 
-### Cross-worker flakiness
-All workers' servers share one websocket (skalop) server, so another worker's actions can revalidate your page mid-interaction — a click on a React Aria button can register the press start but never complete it. The helpers (`waitForPOSTResponse`, `UserCard.open`) retry for this; a spec failing this way under full-suite load but passing alone usually needs its flow routed through such a retrying helper, not a sleep.
+### Re-render races
+Skalop (websocket) is fully disconnected in e2e — the build has an empty `VITE_SKALOP_WS_URL` and worker servers get empty `SKALOP_SYSTEM_MESSAGE_URL`/`SKALOP_TOKEN` (see `e2e/global-setup.ts`), so cross-worker websocket crosstalk cannot cause flakes. Google Fonts are also blocked at the context level so font swaps never reflow the page mid-test. Re-renders from the test's own action revalidations can still swallow a React Aria press (press start registers, press end never fires — no POST); `waitForPOSTResponse` retries for this, so route flows through it rather than adding sleeps. When e2e tests for chat/websocket features are added, skalop needs a per-worker instance or stub with a runtime-derived WS URL.
 
 ## Test pattern reference
 

app/root.tsx

@@ -19,6 +19,7 @@ import {
 	Scripts,
 	ScrollRestoration,
 	type ShouldRevalidateFunction,
+	useFetchers,
 	useHref,
 	useLoaderData,
 	useMatches,
@@ -433,10 +434,24 @@ export const ErrorBoundary = () => {
 
 function HydrationTestIndicator() {
 	const isHydrated = useHydrated();
+	const navigation = useNavigation();
+	const revalidator = useRevalidator();
+	const fetchers = useFetchers();
 
 	if (!isHydrated) return null;
 
-	return <div style={{ display: "none" }} data-testid="hydrated" />;
+	const routerIdle =
+		navigation.state === "idle" &&
+		revalidator.state === "idle" &&
+		fetchers.every((fetcher) => fetcher.state === "idle");
+
+	return (
+		<div
+			style={{ display: "none" }}
+			data-testid="hydrated"
+			data-router-idle={routerIdle ? "true" : undefined}
+		/>
+	);
 }
 
 function Fonts() {

e2e/comp-analyzer.spec.ts

@@ -29,7 +29,7 @@ test.describe("Composition Analyzer", () => {
 
 		await expect(compAnalyzer.locators.categorizationToggle).not.toBeVisible();
 
-		expect(page.url()).toContain("weapons=");
+		await expect(page).toHaveURL(/weapons=/);
 
 		// Weapons should still be selected after reload
 		await page.reload();
@@ -63,7 +63,7 @@ test.describe("Composition Analyzer", () => {
 
 		// Switch categorization and test URL persistence
 		await compAnalyzer.selectCategorization("sub");
-		expect(page.url()).toContain("categorization=sub");
+		await expect(page).toHaveURL(/categorization=sub/);
 
 		await page.reload();
 

e2e/helpers/playwright.ts

@@ -32,6 +32,17 @@ type TestFixtures = {
 };
 
 export const test = base.extend<TestFixtures, WorkerFixtures>({
+	context: async ({ context }, use) => {
+		// Google Fonts load with display=swap and every test context re-fetches
+		// them, so the swap reflows the page mid-test (e.g. re-collapsing the
+		// tournament nav between a visibility check and a click). Block them so
+		// layout settles at first paint and stays put.
+		await context.route(
+			/^https:\/\/fonts\.(googleapis|gstatic)\.com\//,
+			(route) => route.abort(),
+		);
+		await use(context);
+	},
 	workerPort: [
 		// biome-ignore lint/correctness/noEmptyPattern: Playwright requires object destructuring
 		async ({}, use, workerInfo) => {
@@ -201,80 +212,13 @@ async function retryPost(
 }
 
 export async function submit(page: Page, testId?: string) {
-	// Started before the click because the data GET can land before awaiting the
-	// POST response hands control back to us, and `page.waitForResponse` only
-	// sees responses arriving after it is called.
-	const dataGet = watchForDataGetAfterPOST(page);
-
-	try {
-		const postRes = await waitForPOSTResponse(page, async () => {
-			await page.getByTestId(testId ?? "submit-button").click();
-		});
-
-		// Remix returns 202 from action endpoints when the action threw/returned a
-		// redirect. The fetcher then drives a client-side navigation and, once
-		// that completes, fires a GET against the new route's data. If we return
-		// before that GET fires, a subsequent Link click can be aborted mid-flight
-		// by the queued navigation (ERR_ABORTED on the new route's .data fetch),
-		// leaving the test on the old page.
-		if (postRes.status() !== 202) return;
-
-		await dataGet.fired;
-		// Toast flash params are stripped right after via a replace navigation
-		// (without revalidation); wait for it so it can't abort a later click.
-		await expect(page).not.toHaveURL(/__(?:success|error)=/);
-	} finally {
-		dataGet.stop();
-	}
-}
-
-/**
- * Resolves once a route data GET follows the POST, without missing one that
- * arrives while the caller is still awaiting the POST response.
- */
-function watchForDataGetAfterPOST(page: Page) {
-	const TIMEOUT = 15_000;
-
-	let postSeen = false;
-	let resolveFired: () => void = () => {};
-	let rejectFired: (error: Error) => void = () => {};
-
-	const fired = new Promise<void>((resolve, reject) => {
-		resolveFired = resolve;
-		rejectFired = reject;
+	await waitForPOSTResponse(page, async () => {
+		await page.getByTestId(testId ?? "submit-button").click();
 	});
 
-	const onResponse = (res: Response) => {
-		if (!postSeen) {
-			postSeen = res.request().method() === "POST";
-			return;
-		}
-
-		if (res.request().method() === "GET" && res.url().includes(".data")) {
-			resolveFired();
-		}
-	};
-	page.on("response", onResponse);
-
-	const timeout = setTimeout(
-		() =>
-			rejectFired(
-				new Error(
-					`submit: no route data GET followed the redirecting POST within ${TIMEOUT}ms`,
-				),
-			),
-		TIMEOUT,
-	);
-
-	return {
-		fired,
-		stop: () => {
-			clearTimeout(timeout);
-			page.off("response", onResponse);
-			// Nothing awaits `fired` when the POST wasn't a redirect
-			resolveFired();
-		},
-	};
+	// Toast flash params are stripped right after via a replace navigation
+	// (without revalidation); wait for it so it can't abort a later click.
+	await expect(page).not.toHaveURL(/__(?:success|error)=/);
 }
 
 export async function waitForPOSTResponse(page: Page, cb: () => Promise<void>) {
@@ -288,6 +232,7 @@ export async function waitForPOSTResponse(page: Page, cb: () => Promise<void>) {
 	// completes into a submit, so no POST fires — e.g. when a re-render lands
 	// mid-press. Re-issue the action when the expected POST doesn't arrive
 	// within the per-attempt window.
+	let response: Response | undefined;
 	for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) {
 		const responsePromise = page.waitForResponse(
 			(res) => res.request().method() === "POST",
@@ -295,13 +240,30 @@ export async function waitForPOSTResponse(page: Page, cb: () => Promise<void>) {
 		);
 		await cb();
 		try {
-			return await responsePromise;
+			response = await responsePromise;
+			break;
 		} catch (error) {
 			if (attempt === MAX_ATTEMPTS) throw error;
 		}
 	}
 
-	throw new Error("waitForPOSTResponse: unreachable");
+	// The POST's revalidation (and any redirect it drives) is still in flight;
+	// an interaction landing mid-flight aborts it, and routes that opt out of
+	// revalidation on navigation (e.g. to.$id) then keep the stale data.
+	await expectRouterIdle(page);
+
+	return response!;
+}
+
+/** Waits until no navigation, revalidation or fetcher is in flight. */
+async function expectRouterIdle(page: Page) {
+	// A submit's redirect plus the target page's loaders can exceed the default
+	// expect timeout when the full suite is loading all workers.
+	await expect(page.getByTestId("hydrated")).toHaveAttribute(
+		"data-router-idle",
+		"true",
+		{ timeout: 15_000 },
+	);
 }
 
 export function isNotVisible(locator: Locator) {
@@ -314,15 +276,12 @@ export function modalClickConfirmButton(page: Page) {
 
 /**
  * Clicks a tournament nav tab by its testId, opening the overflow ("More") menu
- * first when the tab has collapsed into it on the current viewport. Retried as a
- * whole because the nav can re-collapse between the visibility check and the click.
+ * first when the tab has collapsed into it on the current viewport.
  */
 export async function clickNavTab(page: Page, testId: string) {
 	const visibleTab = page.locator(`[data-testid="${testId}"]:visible`);
-	await expect(async () => {
-		if ((await visibleTab.count()) === 0) {
-			await page.getByRole("button", { name: "More" }).click();
-		}
-		await visibleTab.click({ timeout: 2_000 });
-	}).toPass({ timeout: 15_000 });
+	if ((await visibleTab.count()) === 0) {
+		await page.getByRole("button", { name: "More" }).click();
+	}
+	await visibleTab.click();
 }

e2e/params.spec.ts

@@ -27,11 +27,11 @@ test.describe("Weapon parameters", () => {
 
 		await expect(showAllWeaponsButton).toBeVisible();
 		await expect(weaponHeaders).toHaveCount(initialColumnCount - 1);
-		expect(page.url()).toMatch(/hidden=\d/);
+		await expect(page).toHaveURL(/hidden=\d/);
 
 		// Refresh keeps the hidden selection
 		await page.reload();
-		expect(page.url()).toMatch(/hidden=\d/);
+		await expect(page).toHaveURL(/hidden=\d/);
 		await expect(showAllWeaponsButton).toBeVisible();
 		await expect(weaponHeaders).toHaveCount(initialColumnCount - 1);
 
@@ -39,7 +39,7 @@ test.describe("Weapon parameters", () => {
 		await weaponParams.showAllWeapons();
 		await expect(showAllWeaponsButton).not.toBeVisible();
 		await expect(weaponHeaders).toHaveCount(initialColumnCount);
-		expect(page.url()).not.toMatch(/hidden=\d/);
+		await expect(page).not.toHaveURL(/hidden=\d/);
 
 		// Comparison bar graph
 		await weaponParams.openParamComparison();
@@ -81,11 +81,11 @@ test.describe("Weapon parameters", () => {
 			weaponParams.locators;
 
 		await weaponParams.openPatchHistoryTab();
-		expect(page.url()).toContain("tab=patches");
+		await expect(page).toHaveURL(/tab=patches/);
 
 		// Refresh keeps the selected tab
 		await page.reload();
-		expect(page.url()).toContain("tab=patches");
+		await expect(page).toHaveURL(/tab=patches/);
 		await expect(patchHistoryTab).toHaveAttribute("aria-selected", "true");
 
 		// Either patch columns are shown or the empty state
@@ -94,10 +94,10 @@ test.describe("Weapon parameters", () => {
 		// Toggle "Show sub & special changes" and verify it persists on refresh
 		await expect(subAndSpecialChangesSwitch).toBeChecked();
 		await weaponParams.toggleSubAndSpecialChanges();
-		expect(page.url()).toContain("kitExtras=false");
+		await expect(page).toHaveURL(/kitExtras=false/);
 
 		await page.reload();
-		expect(page.url()).toContain("kitExtras=false");
+		await expect(page).toHaveURL(/kitExtras=false/);
 		await expect(subAndSpecialChangesSwitch).not.toBeChecked();
 	});
 });