Stabilize E2E tests

.claude/skills/e2e/SKILL.md

@@ -85,6 +85,13 @@ When a failure needs full DOM snapshots to understand, re-run just that test wit
 ### Re-render races
 Live events run on an in-process event bus per worker server (SSE, see `app/features/events`), so cross-worker 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.
 
+Other known sources, each handled once in the helpers or page objects:
+- YouTube is stubbed at the context level (a fake `iframe_api`, everything else aborted): the real player loads from the internet at its own pace and, arriving mid-test, closed an open select on the VoD form
+- dnd-kit stops every click in the document for 50ms after a drop; drag helpers end with `waitForDropToSettle` so the next click lands
+- A tab or link clicked before hydration (after a raw `page.reload()`, say) loads the target as a new document and the click after it is lost; reload through the page object's `reload()` which waits for hydration
+- Popovers close on navigation in a passive effect, so for a frame the old panel and the new page both show the same names; scope locators to `main` where a name can appear in both
+- React Router drops a fetcher's redirect when a navigation (a revalidation included) started after the submission; broadcast revalidations are jittered up to 1.5s after a live event, so one could land mid-submission and the action silently did nothing. `holdRevalidationsDuring` (used by `useActionSubmit` and `SendouForm`) defers them until the submission settles
+
 ## Test pattern reference
 
 Every test builds its own data with factories and drives the UI through page objects:
@@ -112,7 +119,7 @@ Key rules:
 - Use `navigate()` instead of `page.goto()` — it waits for hydration (page objects' `goto()` methods wrap it)
 - Use `submit()` instead of clicking submit buttons directly — it waits for the POST response
 - Use `impersonate(page, userId?)` to authenticate. Default is admin (ADMIN_ID); prefer N-ZAP (`NZAP_TEST_ID`) when the flow doesn't need admin rights
-- Avoid `page.waitForTimeout` — use assertions or `waitFor` patterns instead
+- Avoid `page.waitForTimeout` — use assertions or `waitFor` patterns instead (the one exception is `waitForDropToSettle`, dnd-kit's post-drop window has nothing observable to wait on)
 - Import `test` from `./helpers/playwright` (not from `@playwright/test`) — it includes worker port fixtures and the database reset
 - Factory writes must be followed by a helper that talks to the server (`navigate`, `impersonate`, `submit`) or the test fails with "writes the server never saw"
 

app/features/chat/revalidation-scope.test.ts

@@ -1,6 +1,7 @@
 import type { ShouldRevalidateFunctionArgs } from "react-router";
 import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
 import {
+	holdRevalidationsDuring,
 	isMatchResultsScopedRevalidation,
 	revalidateWithScope,
 	scheduleBroadcastRevalidation,
@@ -175,3 +176,84 @@ describe("scheduleBroadcastRevalidation", () => {
 		expect(isMatchResultsScopedRevalidation(revalidationArgs())).toBe(false);
 	});
 });
+
+describe("holdRevalidationsDuring", () => {
+	test("a revalidation requested during a submission runs once it has settled", async () => {
+		const submission = deferred();
+		const revalidate = vi.fn(() => Promise.resolve());
+
+		const held = holdRevalidationsDuring(() => submission.promise);
+		revalidateWithScope(revalidate, undefined);
+		expect(revalidate).not.toHaveBeenCalled();
+
+		submission.resolve();
+		await held;
+		expect(revalidate).toHaveBeenCalledTimes(1);
+	});
+
+	test("revalidations requested during a submission collapse into one", async () => {
+		const submission = deferred();
+		const revalidate = vi.fn(() => Promise.resolve());
+
+		const held = holdRevalidationsDuring(() => submission.promise);
+		revalidateWithScope(revalidate, "MATCH_RESULTS");
+		revalidateWithScope(revalidate, "MATCH_RESULTS");
+
+		submission.resolve();
+		await held;
+		expect(revalidate).toHaveBeenCalledTimes(1);
+	});
+
+	test("a deferred revalidation of another scope widens to unscoped", async () => {
+		const submission = deferred();
+		const revalidation = deferred();
+
+		const held = holdRevalidationsDuring(() => submission.promise);
+		revalidateWithScope(() => revalidation.promise, "MATCH_RESULTS");
+		revalidateWithScope(() => revalidation.promise, undefined);
+
+		submission.resolve();
+		await held;
+		expect(isMatchResultsScopedRevalidation(revalidationArgs())).toBe(false);
+
+		revalidation.resolve();
+		await flushMicrotasks();
+	});
+
+	test("the hold lasts until every overlapping submission has settled", async () => {
+		const first = deferred();
+		const second = deferred();
+		const revalidate = vi.fn(() => Promise.resolve());
+
+		const firstHeld = holdRevalidationsDuring(() => first.promise);
+		const secondHeld = holdRevalidationsDuring(() => second.promise);
+		revalidateWithScope(revalidate, undefined);
+
+		first.resolve();
+		await firstHeld;
+		expect(revalidate).not.toHaveBeenCalled();
+
+		second.resolve();
+		await secondHeld;
+		expect(revalidate).toHaveBeenCalledTimes(1);
+	});
+
+	test("a failing submission releases the hold", async () => {
+		const revalidate = vi.fn(() => Promise.resolve());
+
+		const held = holdRevalidationsDuring(() =>
+			Promise.reject(new Error("network")),
+		);
+		revalidateWithScope(revalidate, undefined);
+
+		await expect(held).rejects.toThrow("network");
+		expect(revalidate).toHaveBeenCalledTimes(1);
+	});
+
+	test("nothing is held outside a submission", () => {
+		const revalidate = vi.fn(() => Promise.resolve());
+
+		revalidateWithScope(revalidate, undefined);
+		expect(revalidate).toHaveBeenCalledTimes(1);
+	});
+});

app/features/chat/revalidation-scope.ts

@@ -11,12 +11,49 @@ let pendingRevalidations = 0;
 let oldestPendingStartedAt: number | null = null;
 let revalidationGeneration = 0;
 let scheduledBroadcast: { scope: RevalidateScope | null } | null = null;
+let heldSubmissions = 0;
+let deferredRevalidation: {
+	revalidate: () => Promise<void>;
+	scope: RevalidateScope | null;
+} | null = null;
+
+/**
+ * Runs a fetcher submission, holding broadcast revalidations back until it has settled (its
+ * redirect followed). React Router drops a fetcher's redirect when a navigation started after
+ * the submission, and a revalidation is one: a broadcast landing mid-flight would leave the
+ * user on the page with nothing happening. The held revalidation runs once, afterwards.
+ */
+export async function holdRevalidationsDuring(submission: () => Promise<void>) {
+	heldSubmissions++;
+	try {
+		await submission();
+	} finally {
+		heldSubmissions--;
+		if (heldSubmissions === 0 && deferredRevalidation) {
+			const { revalidate, scope } = deferredRevalidation;
+			deferredRevalidation = null;
+			revalidateWithScope(revalidate, scope ?? undefined);
+		}
+	}
+}
 
 /** Runs a broadcast triggered revalidation, remembering its scope while in flight so `shouldRevalidate` can skip loaders the broadcast cannot have changed. */
 export function revalidateWithScope(
 	revalidate: () => Promise<void>,
 	scope: RevalidateScope | undefined,
 ) {
+	if (heldSubmissions > 0) {
+		// like an absorbed broadcast, a deferred revalidation of a different scope widens to unscoped
+		const widens =
+			deferredRevalidation !== null &&
+			deferredRevalidation.scope !== (scope ?? null);
+		deferredRevalidation = {
+			revalidate,
+			scope: widens ? null : (scope ?? null),
+		};
+		return;
+	}
+
 	forgetStalePendingRevalidations();
 
 	if (!scope) {

app/form/SendouForm.tsx

@@ -9,6 +9,7 @@ import * as v from "valibot";
 import type { SendouButtonProps } from "~/components/elements/Button";
 import { FormMessage } from "~/components/FormMessage";
 import { SubmitButton } from "~/components/SubmitButton";
+import { holdRevalidationsDuring } from "~/features/chat/revalidation-scope";
 import { FormField as FormFieldComponent } from "./FormField";
 import { getFormFieldMetadata } from "./fields";
 import styles from "./SendouForm.module.css";
@@ -505,11 +506,13 @@ function createFormActions({
 		const submitted = revalidateRoot
 			? { ...values, revalidateRoot: true }
 			: values;
-		fetcher.submit(submitted as Record<string, string>, {
-			method: "post",
-			action,
-			encType: "application/json",
-		});
+		void holdRevalidationsDuring(() =>
+			fetcher.submit(submitted as Record<string, string>, {
+				method: "post",
+				action,
+				encType: "application/json",
+			}),
+		);
 	};
 
 	const setClientError = (name: string, error: string | undefined) => {

app/hooks/useActionSubmit.ts

@@ -1,4 +1,5 @@
 import { type FetcherWithComponents, useFetcher } from "react-router";
+import { holdRevalidationsDuring } from "~/features/chat/revalidation-scope";
 import {
 	type ActionsOf,
 	type FieldsOf,
@@ -36,9 +37,13 @@ export function useActionSubmit<TSchema extends AnySchema>(
 		const fields = (rest[0] ?? {}) as Record<string, unknown>;
 
 		if (opts?.encType === "application/json") {
-			fetcher.submit(
-				{ _action: action, ...fields } as Parameters<typeof fetcher.submit>[0],
-				{ method: "post", action: opts?.action, encType: "application/json" },
+			void holdRevalidationsDuring(() =>
+				fetcher.submit(
+					{ _action: action, ...fields } as Parameters<
+						typeof fetcher.submit
+					>[0],
+					{ method: "post", action: opts?.action, encType: "application/json" },
+				),
 			);
 			return;
 		}
@@ -48,7 +53,9 @@ export function useActionSubmit<TSchema extends AnySchema>(
 			if (value === undefined || value === null) continue;
 			payload[name] = serializeFieldValue(value);
 		}
-		fetcher.submit(payload, { method: "post", action: opts?.action });
+		void holdRevalidationsDuring(() =>
+			fetcher.submit(payload, { method: "post", action: opts?.action }),
+		);
 	};
 
 	return { submit, fetcher, state: fetcher.state };

e2e/helpers/playwright.ts

@@ -32,6 +32,17 @@ declare global {
 	}
 }
 
+/** `YT.Player` that never readies, so a VoD form behaves as it does before the real one loads. */
+const YOUTUBE_IFRAME_API_STUB = `
+window.YT = {
+	Player: class {
+		getCurrentTime() { return 0; }
+		destroy() {}
+	},
+};
+window.onYouTubeIframeAPIReady?.();
+`;
+
 export const MOBILE_VIEWPORT = { width: 375, height: 667 };
 export const TABLET_VIEWPORT = { width: 768, height: 1024 };
 
@@ -79,6 +90,18 @@ export const test = base.extend<TestFixtures, WorkerFixtures>({
 			/^https:\/\/fonts\.(googleapis|gstatic)\.com\//,
 			(route) => route.abort(),
 		);
+		// The VoD pages embed a YouTube player, which loads from the internet
+		// (player, ads, telemetry) at a pace of its own. Under load it landed
+		// mid-test, and the frame arriving closed the select being filled in.
+		// A stub player API keeps the pages off the network.
+		await context.route(/^https:\/\/www\.youtube\.com\//, (route) =>
+			new URL(route.request().url()).pathname === "/iframe_api"
+				? route.fulfill({
+						contentType: "text/javascript",
+						body: YOUTUBE_IFRAME_API_STUB,
+					})
+				: route.abort(),
+		);
 		await use(context);
 	},
 	workerPort: [
@@ -470,6 +493,14 @@ async function expectRouterIdle(page: Page) {
 	}
 }
 
+/** dnd-kit stops every click in the document for this long after a drop (`PointerSensor.detach`). */
+const DND_KIT_CLICK_SUPPRESSION_MS = 50;
+
+/** Waits out dnd-kit's post-drop click suppression, which nothing observable marks the end of. Call after the `mouse.up()` of a drag. */
+export async function waitForDropToSettle(page: Page) {
+	await page.waitForTimeout(2 * DND_KIT_CLICK_SUPPRESSION_MS);
+}
+
 /** Asserts the page rendered rather than the error boundary catching something. */
 export async function expectNoErrorPage(page: Page) {
 	await expect(page.getByTestId("error-page")).toHaveCount(0);

e2e/pages/friends/friends-page.ts

@@ -52,8 +52,9 @@ export class FriendsPage {
 		);
 	}
 
+	/** Scoped to the page content: the mobile friends panel still shows the same name for a frame after navigating here from it. */
 	friendButton(name: string) {
-		return this.page.getByRole("button", { name });
+		return this.page.getByRole("main").getByRole("button", { name });
 	}
 
 	friend(name: string) {

e2e/pages/layout/mobile-nav.ts

@@ -72,7 +72,10 @@ export class MobileNav {
 	private async settleAnimations() {
 		await this.page.evaluate(() =>
 			Promise.all(
-				document.getAnimations().map((animation) => animation.finished),
+				document.getAnimations().map((animation) =>
+					// a cancelled animation (its element gone, or another taking its place) is as settled as a finished one
+					animation.finished.catch(() => {}),
+				),
 			),
 		);
 	}

e2e/pages/tier-list-maker/tier-list-maker-page.ts

@@ -1,7 +1,12 @@
 import type { Page } from "@playwright/test";
 import invariant from "~/utils/invariant";
 import { TIER_LIST_MAKER_URL } from "~/utils/urls";
-import { expect, expectIsHydrated, navigate } from "../../helpers/playwright";
+import {
+	expect,
+	expectIsHydrated,
+	navigate,
+	waitForDropToSettle,
+} from "../../helpers/playwright";
 
 type ItemType =
 	| "main-weapon"
@@ -64,7 +69,9 @@ export class TierListMakerPage {
 	}
 
 	async openTab(type: ItemType) {
-		await this.page.getByRole("tab", { name: TAB_NAMES[type] }).click();
+		const tab = this.page.getByRole("tab", { name: TAB_NAMES[type] });
+		await tab.click();
+		await expect(tab).toHaveAttribute("aria-selected", "true");
 	}
 
 	async setPlacementMode(mode: "drag" | "click") {
@@ -94,6 +101,7 @@ export class TierListMakerPage {
 		await this.page.mouse.up();
 
 		await expect(emptyTiers).toHaveCount(emptyCountBefore - 1);
+		await waitForDropToSettle(this.page);
 	}
 
 	async clickFirstItem(type: ItemType) {

e2e/pages/tournament/tournament-brackets-page.ts

@@ -2,6 +2,7 @@ import type { Page } from "@playwright/test";
 import { tournamentBracketsPage } from "~/features/tournament-bracket/tournament-bracket-urls";
 import {
 	expect,
+	expectIsHydrated,
 	modalClickConfirmButton,
 	navigate,
 	submit,
@@ -49,6 +50,11 @@ export class TournamentBracketsPage {
 		});
 	}
 
+	async reload() {
+		await this.page.reload();
+		await expectIsHydrated(this.page);
+	}
+
 	teamName(name: string) {
 		return this.page.getByText(name);
 	}
@@ -152,6 +158,8 @@ export class TournamentBracketsPage {
 				this.page.getByTestId("back-to-bracket-button"),
 			).toBeVisible();
 		}).toPass();
+		// a click that beat hydration loads the match page as a new document
+		await expectIsHydrated(this.page);
 		return new TournamentMatchPage(this.page);
 	}
 

e2e/pages/tournament/tournament-seeds-page.ts

@@ -1,6 +1,10 @@
 import type { Page } from "@playwright/test";
 import { tournamentAdminPage } from "~/utils/urls";
-import { navigate, submit } from "../../helpers/playwright";
+import {
+	navigate,
+	submit,
+	waitForDropToSettle,
+} from "../../helpers/playwright";
 
 const DRAG_TARGET_Y = 500;
 
@@ -34,6 +38,7 @@ export class TournamentSeedsPage {
 		// the drag & drop library only registers the drop when moved in steps
 		await this.page.mouse.move(0, DRAG_TARGET_Y, { steps: 10 });
 		await this.page.mouse.up();
+		await waitForDropToSettle(this.page);
 	}
 
 	save() {

e2e/tournament-bracket.spec.ts

@@ -182,7 +182,7 @@ test.describe("Tournament bracket", () => {
 
 		// past the 26min limit of a Bo3
 		await page.clock.fastForward("30:00");
-		await page.reload();
+		await brackets.reload();
 
 		match = await brackets.openMatch(matchId);