commit 9081189448cd7045f6eeddc5839f901e280bd799
parent edd9aff9329c955cd58db651b192498f97a91abc
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Thu, 30 Jul 2026 04:36:43 -0400
Fix all 22 red e2e specs, and make the sharded route actually run
The editor suite had 22 permanently-red tests and a "fast" containerised route
whose numbers nobody could compare to anything. Both are fixed. Serial dev-mode
goes 370/392 -> 390/392; the sharded route goes from not starting at all to
389-391/392 in 7-11 min against 23.8 min serial.
Nearly every red was ASSERTION DRIFT, not the locator collisions they looked
like. Each was diagnosed by running it, which changed four conclusions:
- deploy-page:8 and site-scope:50 were not strict-mode violations at all. They
failed with "element(s) not found": "Build multiple sites" exists nowhere (the
heading is "Build all sites") and the cockpit redesign 310a9af deleted the
role=group stat tiles.
- The "queues a job" reds were not the pre-hydration lost click. Each of those
tests first asserts an inline job pill that only renders once a jobId exists,
and that assertion passes — the POST happens. They asserted a raw job kind
against /jobs, which renders jobKindLabel(kind). The natural experiment is in
channels-actions.spec itself: its check-availability test passes only because
that kind has no entry in JOB_KINDS and falls back to the raw string.
Fixed once at the source: data-kind on the row + jobRowByKind() in helpers.
- sync-break-on-existing was never an order-dependent flake. It expects "sync"
where the row renders "Sync" — deterministic, every run.
- Two specs were green for the wrong reason. actionable's "hides the card"
asserted toHaveCount(0) on a label nothing carries, so it could not fail.
actionable:114 asserted a row /jobs hides by default (DEFAULT_HIDDEN_KINDS =
["refresh-report"]) and passed only by beating its own hydration, since
JobsTable renders every job while filters is still null.
Two defects found outside the red list:
- The sharded route was broken end to end. Every shard died with "Timed out
waiting 120000ms from config.webServer" because the export server 500s on a
missing export/public/summaries/manifest.json — generated, and deliberately
excluded by .dockerignore. Host runs only worked because that file happens to
exist in the working copy. Dockerfile.test now seeds an empty-site manifest.
The README's "2.5x on 4 shards" described a route that could not start.
- deploy-page:64 was an optimistic-write race, not hydration: BuildModeToggle
flips aria-pressed via setMode() before startTransition runs the server
action, so the test reloaded before settings.json had been written. A 15s
retry did not fix it; polling the persisted file does.
cut-release.spec no longer touches tracked files: EDITOR_CHANGELOG_FILE (with
the default unchanged) routes all four changelog readers at one path, dev:test
and start:test point it at a gitignored editor/test-changelog.md, and the spec
unchecks "Commit changelog" — no test ever asserted anything about git, and that
checkbox defaulting to on is what produced stray release commits.
run-sharded-e2e.mjs now defaults to --retries=0 so its failures are comparable
to a serial run (CI=true stays, because reuseExistingServer needs it, but it no
longer implies retries: 2), forwards remaining args to every shard so a subset
can be sharded, and prints a combined pass/fail count instead of exit codes.
playwright.config.ts records why workers stays at 1, naming the three pieces of
shared state with file:line — the four globalThis singletons being the one that
per-worker directories cannot fix.
resetData() now passes maxRetries to fs.rm: a job left running by the previous
spec can write into the tree mid-delete, which surfaced as ENOTEMPTY.
README and plans/FACTS.md carry both baselines with mode, retries and TEST
COUNTS — omitting the count is how the old table stayed wrong by ~14x while the
suite grew from 90 tests to 392.
Verified: common 386/386, export playwright 150/150, tsc clean in
common/editor/export, eslint clean. A rotating 1-3 test tail remains, different
members almost every run, measured while an unrelated project ran its own
Playwright suite on the same box at load 15-27.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat:
25 files changed, 572 insertions(+), 91 deletions(-)
diff --git a/.gitignore b/.gitignore
@@ -91,6 +91,9 @@ yarn-error.log*
# editor e2e fixtures and ephemeral state
/editor/test-transcripts/
/editor/test-settings.json
+# Disposable changelog for the cut-release e2e (EDITOR_CHANGELOG_FILE in
+# dev:test/start:test). Keeps that spec off the repo's tracked CHANGELOG.md.
+/editor/test-changelog.md
/editor/playwright-report/
/editor/test-results/
/editor/blob-report/
diff --git a/Dockerfile.test b/Dockerfile.test
@@ -18,6 +18,21 @@ RUN pnpm install --frozen-lockfile
COPY . .
+# The editor's playwright.config starts an export dev server (it serves the
+# server-rendered footer/branding during editor e2e). That server reads
+# export/public/summaries/manifest.json on render — a GENERATED artifact, and one
+# .dockerignore deliberately excludes, so it is never in this image. Without it
+# every shard's export server 500s on its readiness URL and playwright dies with
+# "Timed out waiting 120000ms from config.webServer", i.e. the whole sharded
+# route fails before running a single test. Host runs only work because that file
+# happens to exist in the working copy from a previous build.
+#
+# Seed a valid empty-site manifest so the container is self-sufficient. Tests
+# that need real summaries stub them per-request with page.route.
+RUN mkdir -p export/public/summaries && \
+ printf '%s\n' '{"version":3,"totalCount":0,"pageSize":1000,"pageCount":0,"generatedAt":"2026-01-01T00:00:00.000Z","channels":[],"groups":[],"defaultGroupId":"default","siteId":"testsite"}' \
+ > export/public/summaries/manifest.json
+
RUN pnpm --filter editor exec next build
WORKDIR /repo/editor
diff --git a/README.md b/README.md
@@ -76,17 +76,52 @@ Playwright covers the editor UI from `editor/e2e/`. Two ways to run it:
Shard count defaults to `min(max(2, cpus/2), 8)`. Override with `SHARDS=N pnpm e2e:sharded` or `node scripts/run-sharded-e2e.mjs --shards N`.
+ **Retries default to 0**, so a sharded run reports the same failures a sequential one does.
+ (The containers set `CI=true` because `playwright.config`'s
+ `reuseExistingServer: !process.env.CI` needs it — each shard must start its own servers — but
+ that would otherwise also switch on `retries: 2` and quietly hide flaky tests. The runner
+ passes an explicit `--retries=0` to override it.) Raise it deliberately with
+ `--retries N` when you actually want to measure flakiness.
+
+ **Anything else on the command line is forwarded to `playwright test` in every shard**, so a
+ subset can be sharded too:
+
+ ```bash
+ pnpm e2e:sharded -- --grep "digest"
+ node scripts/run-sharded-e2e.mjs --shards 4 e2e/deploy-page.spec.ts
+ ```
+
+ After merging the blob reports the runner prints a combined `N passed / M failed`, not just
+ per-shard exit codes.
+
+### Which route to use when
+
+Use **`pnpm e2e`** while iterating — it reuses a running dev server, picks up source edits
+without a rebuild, and gives you a single readable log. Use **`pnpm e2e:sharded`** to verify a
+branch: it is several times faster over the whole suite, and it runs against `next start`
+(a real build) rather than `next dev`.
+
### Wall-time comparison
-Measured on this machine (8 CPUs, 90 tests, 4 shards):
+**Always quote the test count with the time.** The previous version of this table was measured
+at 90 tests and stayed in the README unchanged while the suite grew to 392 — which made its
+"104 s sequential" wrong by more than an order of magnitude, invisibly.
+
+Measured 2026-07-30 on this machine (8 CPUs, **392 tests**, `--retries=0`):
| Mode | Wall time | Notes |
| --- | --- | --- |
-| `pnpm e2e` (sequential) | **104 s** | one worker, `next dev` |
-| `pnpm e2e:sharded` (image cached) | **42 s** | 4 shards × `next start`, ~2.5× speedup |
-| `pnpm e2e:sharded` (cold image build) | ~117 s | one-off ~75 s `docker build` + ~42 s test run |
+| `pnpm e2e` (sequential) | **23.8 min** (1428 s) | one worker, `next dev` |
+| `pnpm e2e:sharded --shards 4` (image cached) | **7.1 min** (424 s) | 4 shards × `next start`, **~3.4× speedup** |
+| `docker build` (cached layers) | ~40–110 s | one-off, paid on dependency or source changes |
+
+Caveat on precision: the box was **not** quiet during these runs (an unrelated project was
+running its own Playwright suite at load ~15–23). A second sequential run under heavier load
+took 29.7 min for the same result, so treat these as an envelope rather than a constant.
-The cold build is only paid the first time or after dependency changes; the steady-state run is the second row.
+4 shards were used above to bound memory, since each shard runs its own editor + export
+servers. One spec legitimately disagrees between the two routes: `widget.spec`'s "no buttons"
+is red under `next dev` (which injects a Dev Tools button) and green under `next start`.
## CLI shims
diff --git a/common/lib/paths.ts b/common/lib/paths.ts
@@ -48,6 +48,15 @@ export type Paths = {
// re-launch with one button. Survives restarts, unlike the in-memory job
// registry. See common/jobs/bookmarks.ts.
bookmarksFile: string;
+ // The two CHANGELOG.md files the "cut release" flow reads and rewrites. They
+ // are TRACKED source files, and cutting a release optionally makes a real git
+ // commit — so e2e must be able to point them somewhere disposable. Overridable
+ // via EDITOR_CHANGELOG_FILE / EXPORT_CHANGELOG_FILE; the editor's dev:test and
+ // start:test scripts set the first to editor/test-changelog.md (gitignored).
+ // Without this the changelog spec rewrites the repo's own changelog and can
+ // leave a stray "Release editor <version>" commit behind.
+ editorChangelogFile: string;
+ exportChangelogFile: string;
lmdbPath: string;
exportDir: string;
// The dir Next.js serves at "/" (also holds checked-in static assets). For
@@ -184,6 +193,12 @@ export function getPaths(): Paths {
path.join(path.dirname(exportPublicDir), ".export-builds"),
settingsFile:
process.env.SETTINGS_FILE ?? path.join(monorepoRoot, "settings.json"),
+ editorChangelogFile:
+ process.env.EDITOR_CHANGELOG_FILE ??
+ path.join(monorepoRoot, "editor", "CHANGELOG.md"),
+ exportChangelogFile:
+ process.env.EXPORT_CHANGELOG_FILE ??
+ path.join(exportDir, "CHANGELOG.md"),
chartsConfigFile:
process.env.CHARTS_CONFIG_FILE ??
path.join(monorepoRoot, "chart-templates.json"),
diff --git a/editor/app/changelog/page.tsx b/editor/app/changelog/page.tsx
@@ -1,6 +1,6 @@
import { readFileSync } from "node:fs";
-import path from "node:path";
import type { Metadata } from "next";
+import { getPaths } from "yt-dlp-transcript-common/lib/paths";
import { Changelog } from "yt-dlp-transcript-common/components/Changelog";
import {
extractUnreleasedSection,
@@ -15,10 +15,9 @@ export const dynamic = "force-dynamic";
export const metadata: Metadata = { title: "Changelog" };
function loadChangelog(): string {
- return readFileSync(
- path.join(process.cwd(), "CHANGELOG.md"),
- "utf8",
- );
+ // Must be the same file cutReleaseAction rewrites, or the page renders one
+ // changelog while the form edits another.
+ return readFileSync(getPaths().editorChangelogFile, "utf8");
}
export default function ChangelogPage() {
diff --git a/editor/app/deploy/components/BuildSitesPanel.tsx b/editor/app/deploy/components/BuildSitesPanel.tsx
@@ -58,7 +58,15 @@ export function BuildSitesPanel({
sites.some((s) => selected.has(s.siteId) && !s.cloudflareProject);
return (
- <div className="flex flex-col gap-4">
+ // Named group: this panel's "Deploy after build" checkbox is a twin of the
+ // one in BuildAllSitesButton, and both sit in the same <section> on /deploy.
+ // Without a name to scope by, getByLabel("Deploy after build") resolves to
+ // two elements and every assertion on it dies with a strict-mode violation.
+ <div
+ role="group"
+ aria-label="Build specific sites"
+ className="flex flex-col gap-4"
+ >
<div className="flex flex-col gap-2 rounded-md border border-border p-3">
<div className="flex flex-wrap gap-x-6 gap-y-2">
{sites.map((s) => (
diff --git a/editor/app/deploy/cutReleaseAction.ts b/editor/app/deploy/cutReleaseAction.ts
@@ -16,8 +16,9 @@ export type CutReleaseState =
function changelogPathFor(workspace: "editor" | "export"): string {
const paths = getPaths();
- if (workspace === "export") return path.join(paths.exportDir, "CHANGELOG.md");
- return path.join(paths.monorepoRoot, "editor", "CHANGELOG.md");
+ return workspace === "export"
+ ? paths.exportChangelogFile
+ : paths.editorChangelogFile;
}
function todayISO(): string {
diff --git a/editor/app/jobs/components/JobsTable.tsx b/editor/app/jobs/components/JobsTable.tsx
@@ -213,6 +213,11 @@ export function JobsTable({ jobs }: { jobs: JobListEntry[] }) {
return (
<tr
key={j.id}
+ // The kind cell renders a human label (jobKindLabel), so the
+ // machine kind is not matchable from the row text. Expose it
+ // here as well as on the cell's title: asserting on the label
+ // text is what rotted 9 e2e tests when kinds gained labels.
+ data-kind={j.kind ?? undefined}
className="border-t border-border"
>
<td className="px-3 py-2 font-mono text-xs">
diff --git a/editor/app/layout.tsx b/editor/app/layout.tsx
@@ -1,5 +1,4 @@
import { readFileSync } from "node:fs";
-import path from "node:path";
import type { Metadata } from "next";
import { Suspense } from "react";
import Link from "next/link";
@@ -39,10 +38,7 @@ export async function generateMetadata(): Promise<Metadata> {
function readChangelogLatestDate(): string | null {
try {
- const source = readFileSync(
- path.join(process.cwd(), "CHANGELOG.md"),
- "utf8",
- );
+ const source = readFileSync(getPaths().editorChangelogFile, "utf8");
return getLatestChangelogDate(source);
} catch {
return null;
diff --git a/editor/app/page.tsx b/editor/app/page.tsx
@@ -1,5 +1,4 @@
import { readFileSync } from "node:fs";
-import path from "node:path";
import type { Metadata } from "next";
import { getPaths } from "yt-dlp-transcript-common/lib/paths";
import { getSettings } from "yt-dlp-transcript-common/lib/settings";
@@ -35,7 +34,7 @@ export async function generateMetadata(): Promise<Metadata> {
function loadChangelog(): string | null {
try {
- return readFileSync(path.join(process.cwd(), "CHANGELOG.md"), "utf8");
+ return readFileSync(getPaths().editorChangelogFile, "utf8");
} catch {
return null;
}
diff --git a/editor/e2e/actionable.spec.ts b/editor/e2e/actionable.spec.ts
@@ -1,7 +1,7 @@
import { mkdir, writeFile } from "node:fs/promises";
import { dirname } from "node:path";
import { test, expect } from "@playwright/test";
-import { resetData, resolvePath, readJson } from "./helpers";
+import { resetData, resolvePath, readJson, jobRowByKind } from "./helpers";
const YT_SNAPSHOT_REL =
"test-transcripts/channels/test-youtube/snapshot.json";
@@ -122,9 +122,25 @@ test("'Update all reports' queues a refresh-report job per channel", async ({
);
await page.goto("/jobs");
- await expect(
- page.getByRole("row").filter({ hasText: "refresh-report" }).first(),
- ).toBeVisible({ timeout: 15_000 });
+ // refresh-report is the ONE kind /jobs hides by default
+ // (DEFAULT_HIDDEN_KINDS in app/jobs/jobsFilterStorage.ts), so un-hide it
+ // before asserting. Without this the test passes only by RACING ITS OWN
+ // HYDRATION: JobsTable renders every job while `filters` is still null
+ // (`if (!filters) return jobs`) and drops refresh-report as soon as
+ // localStorage loads. That race is won under `next dev` and lost under
+ // `next start`, which is why this failed only in the sharded route.
+ // The chip is always present: `kinds` is derived from ALL jobs, not the
+ // visible ones. Retried because a click before hydration fires nothing.
+ const kindChip = page.getByRole("button", {
+ name: "refresh-report",
+ exact: true,
+ });
+ await expect(async () => {
+ await kindChip.click();
+ await expect(jobRowByKind(page, "refresh-report").first()).toBeVisible({
+ timeout: 2_000,
+ });
+ }).toPass({ timeout: 20_000 });
});
test("syncing a channel auto-regenerates its report", async ({ page }) => {
@@ -173,9 +189,9 @@ test("inline 'Download missing' queues a download-missing job", async ({
).toBeVisible({ timeout: 10_000 });
await page.goto("/jobs");
- await expect(
- page.getByRole("row").filter({ hasText: "download-missing" }).first(),
- ).toBeVisible({ timeout: 10_000 });
+ await expect(jobRowByKind(page, "download-missing").first()).toBeVisible({
+ timeout: 10_000,
+ });
});
test("inline 'Transcribe pending' queues a whisper-all job", async ({
@@ -192,28 +208,38 @@ test("inline 'Transcribe pending' queues a whisper-all job", async ({
).toBeVisible({ timeout: 10_000 });
await page.goto("/jobs");
- await expect(
- page.getByRole("row").filter({ hasText: "whisper-all" }).first(),
- ).toBeVisible({ timeout: 10_000 });
+ await expect(jobRowByKind(page, "whisper-all").first()).toBeVisible({
+ timeout: 10_000,
+ });
});
-test("dashboard surfaces a 'Needs attention' card when work is pending", async ({
+// The cockpit redesign (c63f5cb) renamed this card from "Needs attention" to
+// "Needs work" and changed its shape: it is now a <section> that always renders
+// — listing channels when there is work and "Everything's handled." when there
+// isn't — rather than a single link that disappeared when idle. The old
+// "hides the card" assertion passed VACUOUSLY once the label changed (a count
+// of an element nobody labels is always 0), so it is rewritten to assert the
+// empty state it actually means.
+test("dashboard surfaces the 'Needs work' card when work is pending", async ({
page,
}) => {
await resetData("youtube-with-playlist");
await page.goto("/channels/test-youtube"); // auto-generate snapshot
await page.goto("/");
- await expect(page.getByLabel("needs attention")).toBeVisible();
- await expect(page.getByLabel("needs attention")).toHaveAttribute(
- "href",
- "/actionable",
- );
+ // getByRole("region"), not getByLabel: the label match is a case-insensitive
+ // substring, so getByLabel("Needs work") also picks up each
+ // <li aria-label="needs work <slug>"> inside the card.
+ const card = page.getByRole("region", { name: "Needs work" });
+ await expect(card).toBeVisible();
+ await expect(card.getByLabel("needs work test-youtube")).toBeVisible();
+ // The card's channel-count link is what navigates to the full list.
+ await expect(card.locator('a[href="/actionable"]')).toBeVisible();
});
-test("dashboard hides the 'Needs attention' card when nothing is pending", async ({
+test("dashboard's 'Needs work' card reads as empty when nothing is pending", async ({
page,
}) => {
await resetData("empty");
await page.goto("/");
- await expect(page.getByLabel("needs attention")).toHaveCount(0);
+ await expect(page.getByLabel("needs work empty")).toBeVisible();
});
diff --git a/editor/e2e/bulk-actions.spec.ts b/editor/e2e/bulk-actions.spec.ts
@@ -55,8 +55,12 @@ test("bulk transcribe submits a single batch job on the transcription queue", as
await page.goto("/jobs");
const newest = page.getByRole("row").nth(1); // row 0 is the header
- // One batch job, not three transcribe-one jobs.
- await expect(newest).toContainText("whisper-bucket-downloaded-no-transcript");
+ // One batch job, not three transcribe-one jobs. Matched on the machine kind:
+ // the cell renders the label "Transcribe downloaded audio".
+ await expect(newest).toHaveAttribute(
+ "data-kind",
+ "whisper-bucket-downloaded-no-transcript",
+ );
await expect(newest).toContainText("test-transcribe");
await expect(newest).toContainText("transcription");
});
@@ -81,7 +85,10 @@ test("bulk transcribe honors a custom queue", async ({ page }) => {
await page.goto("/jobs");
const newest = page.getByRole("row").nth(1);
- await expect(newest).toContainText("whisper-bucket-downloaded-no-transcript");
+ await expect(newest).toHaveAttribute(
+ "data-kind",
+ "whisper-bucket-downloaded-no-transcript",
+ );
await expect(newest).toContainText("qBulk");
});
@@ -100,7 +107,7 @@ test("bulk retry download submits a single retry-bucket job on the platform queu
const newest = page.getByRole("row").nth(1);
// One retry-bucket job, not three download-one-pipeline jobs. The channel URL
// is odysee.com, so the default queue is platform:odysee.
- await expect(newest).toContainText("retry-bucket");
+ await expect(newest).toHaveAttribute("data-kind", "retry-bucket");
await expect(newest).toContainText("test-transcribe");
await expect(newest).toContainText("platform:odysee");
});
diff --git a/editor/e2e/channels-actions.spec.ts b/editor/e2e/channels-actions.spec.ts
@@ -53,5 +53,6 @@ test("Transcribe missing defaults to the system-wide 'transcription' queue", asy
await page.goto("/jobs");
const firstRow = page.getByRole("row").nth(1);
await expect(firstRow).toContainText("transcription");
- await expect(firstRow).toContainText("whisper-all");
+ // Match the machine kind, not the rendered label ("Transcribe all").
+ await expect(firstRow).toHaveAttribute("data-kind", "whisper-all");
});
diff --git a/editor/e2e/cleanup-actionable.spec.ts b/editor/e2e/cleanup-actionable.spec.ts
@@ -1,7 +1,7 @@
import { mkdir, writeFile } from "node:fs/promises";
import { dirname } from "node:path";
import { test, expect } from "@playwright/test";
-import { resetData, resolvePath } from "./helpers";
+import { resetData, resolvePath, jobRowByKind } from "./helpers";
import { baseUrl } from "./baseUrl";
const SLUG = "test-transcribe";
@@ -90,7 +90,7 @@ test("inline 'Clean audio' queues a clean-audio-transcribed job when confirmed",
await page.goto("/jobs");
await expect(
- page.getByRole("row").filter({ hasText: "clean-audio-transcribed" }).first(),
+ jobRowByKind(page, "clean-audio-transcribed").first(),
).toBeVisible({ timeout: 10_000 });
});
diff --git a/editor/e2e/cut-release.spec.ts b/editor/e2e/cut-release.spec.ts
@@ -2,7 +2,19 @@ import { readFile, writeFile } from "node:fs/promises";
import { test, expect } from "@playwright/test";
import { resolvePath } from "./helpers";
-const CHANGELOG_PATH = resolvePath("CHANGELOG.md");
+// This spec drives a flow that REWRITES a changelog file and can make a real
+// git commit. It used to point at the repo's own tracked editor/CHANGELOG.md,
+// which meant: a failed run left the working tree dirty, a successful run left
+// a stray "Release editor <version>" commit on the current branch, and the run
+// failed outright whenever the tree was already dirty (the action refuses to
+// commit over unrelated changes).
+//
+// It now targets editor/test-changelog.md — disposable, gitignored, and pointed
+// at by EDITOR_CHANGELOG_FILE in the dev:test/start:test scripts, which is the
+// same file the /changelog page reads (common/lib/paths.ts editorChangelogFile).
+// "Commit changelog" is unchecked in every test below: none of them assert
+// anything about git, and leaving it on would try to `git add` an ignored file.
+const CHANGELOG_PATH = resolvePath("test-changelog.md");
const WITH_UNRELEASED =
"# Changelog\n" +
@@ -27,16 +39,17 @@ function todayISO(): string {
return `${y}-${m}-${d}`;
}
-let original: string;
-
-test.beforeEach(async () => {
- original = await readFile(CHANGELOG_PATH, "utf8");
-});
-
+// Leave a valid changelog behind: /changelog reads this file unconditionally.
test.afterEach(async () => {
- await writeFile(CHANGELOG_PATH, original);
+ await writeFile(CHANGELOG_PATH, WITHOUT_UNRELEASED);
});
+// The form defaults "Commit changelog" to checked; these tests only care about
+// the file rewrite and the form state, so turn it off.
+async function uncheckCommit(page: import("@playwright/test").Page) {
+ await page.getByRole("checkbox", { name: "Commit changelog" }).uncheck();
+}
+
test("cutting a release removes the Unreleased heading from the file", async ({
page,
}) => {
@@ -44,6 +57,7 @@ test("cutting a release removes the Unreleased heading from the file", async ({
await page.goto("/changelog");
await page.locator("#version-editor").fill("9.9.10");
+ await uncheckCommit(page);
await page.getByRole("button", { name: /^cut release$/i }).click();
await expect(
@@ -62,6 +76,7 @@ test("form disables after a successful cut on reload", async ({ page }) => {
await page.goto("/changelog");
await page.locator("#version-editor").fill("9.9.10");
+ await uncheckCommit(page);
await page.getByRole("button", { name: /^cut release$/i }).click();
await expect(
page.getByRole("status").filter({ hasText: "Cut release 9.9.10." }),
diff --git a/editor/e2e/deploy-page.spec.ts b/editor/e2e/deploy-page.spec.ts
@@ -1,5 +1,12 @@
import { test, expect } from "@playwright/test";
-import { resetData, writeSite } from "./helpers";
+import type { Page } from "@playwright/test";
+import { resetData, writeSite, readJson } from "./helpers";
+
+// The "pick specific sites" panel (BuildSitesPanel). /deploy renders two
+// controls named "Deploy after build" — one here, one in BuildAllSitesButton —
+// so anything targeting this panel's copy must be scoped to it by name.
+const batchPanel = (page: Page) =>
+ page.getByRole("group", { name: "Build specific sites" });
test.beforeEach(async () => {
await resetData("empty");
@@ -13,7 +20,7 @@ test("deploy page reads as a lifecycle: release → build & deploy → batch", a
const release = page.getByRole("heading", { name: "Release notes" });
const buildDeploy = page.getByRole("heading", { name: "Build & deploy" });
- const batch = page.getByRole("heading", { name: "Build multiple sites" });
+ const batch = page.getByRole("heading", { name: "Build all sites" });
await expect(release).toBeVisible();
await expect(buildDeploy).toBeVisible();
await expect(batch).toBeVisible();
@@ -35,13 +42,19 @@ test("Build & deploy is enabled only when the active site has a Cloudflare proje
// With a Cloudflare project → enabled.
await page.goto("/deploy?site=with-proj");
await expect(
- page.getByRole("button", { name: "Build & deploy" }),
+ // exact: true — getByRole's name match is a case-insensitive SUBSTRING by
+ // default, so a bare "Build & deploy" also matches "Build & deploy all
+ // sites" in the batch section below.
+ page.getByRole("button", { name: "Build & deploy", exact: true }),
).toBeEnabled();
// Without one → disabled, with an explanatory notice.
await page.goto("/deploy?site=no-proj");
await expect(
- page.getByRole("button", { name: "Build & deploy" }),
+ // exact: true — getByRole's name match is a case-insensitive SUBSTRING by
+ // default, so a bare "Build & deploy" also matches "Build & deploy all
+ // sites" in the batch section below.
+ page.getByRole("button", { name: "Build & deploy", exact: true }),
).toBeDisabled();
await expect(
page.getByText(/has no Cloudflare Pages project configured/i),
@@ -60,13 +73,40 @@ test("build-mode toggle persists the choice and shows the Docker follow-up note"
"true",
);
- await group.getByRole("button", { name: "Docker" }).click();
- await expect(group.getByRole("button", { name: "Docker" })).toHaveAttribute(
- "aria-pressed",
- "true",
- );
+ // Retry the click until the toggle actually flips. A click dispatched before
+ // React hydrates fires NOTHING — no handler, no request, no error — so a bare
+ // click().then(assert) is a coin flip whenever hydration lags, which is what
+ // happens under the sharded route's concurrent containers (seen as
+ // aria-pressed still "false" 5s after the click). Same idiom as
+ // channel-site-membership.spec's setSiteChecked.
+ const docker = group.getByRole("button", { name: "Docker" });
+ await expect(async () => {
+ await docker.click();
+ await expect(docker).toHaveAttribute("aria-pressed", "true", {
+ timeout: 1_000,
+ });
+ }).toPass({ timeout: 15_000 });
await expect(page.getByText(/Docker builds are a follow-up/i)).toBeVisible();
+ // Wait for the choice to actually reach settings.json before reloading.
+ // BuildModeToggle updates its own state OPTIMISTICALLY — setMode(next) flips
+ // aria-pressed immediately and only then does startTransition run
+ // setBuildModeAction — so the assertions above prove nothing about
+ // persistence. Reloading straight after them races the write: on a fast host
+ // the write wins, in a loaded container it does not, which is why this test
+ // passed under `pnpm e2e` and failed under `pnpm e2e:sharded`.
+ await expect
+ .poll(
+ async () =>
+ (
+ await readJson<{ buildPipeline?: { mode?: string } }>(
+ "test-settings.json",
+ )
+ ).buildPipeline?.mode,
+ { timeout: 10_000 },
+ )
+ .toBe("docker");
+
// Persisted: reload and the toggle is still on Docker.
await page.goto("/deploy?site=testsite");
await expect(
@@ -94,8 +134,10 @@ test("batch panel: selecting sites enables the launch button and reflects deploy
await expect(launch).toBeEnabled();
await expect(page.getByText("2 selected")).toBeVisible();
- // Opting into deploy renames the launch button.
- await page.getByLabel("Deploy after build").check();
+ // Opting into deploy renames the launch button. Scoped to the batch panel:
+ // BuildAllSitesButton has an identically-labelled checkbox in the same
+ // section, so an unscoped getByLabel resolves to two elements.
+ await batchPanel(page).getByLabel("Deploy after build").check();
await expect(
page.getByRole("button", { name: "Build & deploy selected" }),
).toBeVisible();
diff --git a/editor/e2e/helpers.ts b/editor/e2e/helpers.ts
@@ -31,7 +31,22 @@ async function fileExists(p: string): Promise<boolean> {
}
export async function resetData(fixtureName: string | null = null) {
- await rm(testTranscriptsDir, { recursive: true, force: true });
+ // maxRetries is load-bearing, not defensive padding. A job left running by the
+ // PREVIOUS spec (the registry/runner singletons live in the one Next server,
+ // and are only cleared by the invalidate-cache call at the end of this
+ // function) can write a file back into this tree while the recursive walk is
+ // deleting it — the walk then empties a directory, the runner re-creates a
+ // file in it, and the rmdir fails with ENOTEMPTY. Node retries the whole
+ // operation with linear backoff on exactly that errno set (also EBUSY/EPERM),
+ // which is enough for a runner that is about to notice its job is gone.
+ // Observed as two unrelated-looking full-suite failures at
+ // actionable.spec:181 and pipeline.spec:106; both pass in isolation.
+ await rm(testTranscriptsDir, {
+ recursive: true,
+ force: true,
+ maxRetries: 10,
+ retryDelay: 100,
+ });
await rm(testSettingsFile, { force: true });
await cp(defaultTestSettingsFile, testSettingsFile);
if (fixtureName) {
@@ -110,6 +125,27 @@ export async function copyFixture(name: string) {
}
// ---------------------------------------------------------------------------
+// Jobs table
+// ---------------------------------------------------------------------------
+
+// A /jobs row selected by its MACHINE kind.
+//
+// The kind column renders jobKindLabel(kind) (common/jobs/jobKinds.ts), not the
+// kind itself, so `getByRole("row").filter({ hasText: "whisper-all" })` matches
+// nothing once a kind gains a label — silently, and only for kinds that have
+// one. That drift is exactly what made 9 specs permanently red: "whisper-all"
+// renders "Transcribe all", "sync" renders "Sync", "retry-bucket" renders
+// "Retry", while label-less kinds like "check-availability" fall back to the raw
+// string and kept passing. Match the data-kind attribute instead; it is the
+// kind, so it cannot drift with the copy.
+export function jobRowByKind(
+ page: import("@playwright/test").Page,
+ kind: string,
+): import("@playwright/test").Locator {
+ return page.locator(`tr[data-kind="${kind}"]`);
+}
+
+// ---------------------------------------------------------------------------
// Digest fixtures
// ---------------------------------------------------------------------------
diff --git a/editor/e2e/new-channel-onboarding.spec.ts b/editor/e2e/new-channel-onboarding.spec.ts
@@ -6,6 +6,13 @@
//
// The fake yt-dlp (e2e/fixtures/bin/fake-ytdlp.mjs) answers the probe with the
// deterministic name "Fake Probe Channel".
+//
+// getByLabel("URL") is always { exact: true } here. getByLabel matches by
+// SUBSTRING, and three other controls on this form wrap descriptions ending
+// "Requires a URL." inside their <label>, which puts "URL" in their accessible
+// names — so the bare locator resolves to four elements (the url textbox, the
+// fetchPlaylist and prioritizeDownload checkboxes, and the syncIntervalMinutes
+// select). Only the textbox has the exact accessible name "URL".
import { test, expect } from "@playwright/test";
import { resetData, readJson, pathExists } from "./helpers";
@@ -17,7 +24,7 @@ test("derives platform, handling, slug and queue from a YouTube URL offline", as
await page.goto("/channels/new");
await page
- .getByLabel("URL")
+ .getByLabel("URL", { exact: true })
.fill("https://www.youtube.com/@Veritasium/videos");
await expect(page.getByLabel("platform")).toHaveValue("youtube");
@@ -36,7 +43,7 @@ test("an unknown host is offered its own per-domain queue, marked (new)", async
await resetData("empty");
await page.goto("/channels/new");
- await page.getByLabel("URL").fill("https://vimeo.com/someuser");
+ await page.getByLabel("URL", { exact: true }).fill("https://vimeo.com/someuser");
// detectPlatform doesn't recognize vimeo → platform stays Auto, handling
// falls to transcribe, and the queue is the per-domain fallback marked new.
@@ -54,7 +61,7 @@ test("Fetch details fills the name from the yt-dlp probe", async ({ page }) => {
await page.goto("/channels/new");
await page
- .getByLabel("URL")
+ .getByLabel("URL", { exact: true })
.fill("https://www.youtube.com/@testchan/videos");
await page.getByRole("button", { name: "fetch details" }).click();
@@ -73,7 +80,7 @@ test("creating with 'Fetch playlist now' stores the playlist", async ({
await page.goto("/channels/new");
await page
- .getByLabel("URL")
+ .getByLabel("URL", { exact: true })
.fill("https://www.youtube.com/@testchan/videos");
await page.getByRole("button", { name: "fetch details" }).click();
await expect(page.locator('input[name="name"]')).toHaveValue(
@@ -104,7 +111,7 @@ test("'Add to top of auto-queue' prepends a channel leaf and enables the runner"
await page.goto("/channels/new");
await page
- .getByLabel("URL")
+ .getByLabel("URL", { exact: true })
.fill("https://www.youtube.com/@testchan/videos");
await page.getByRole("button", { name: "fetch details" }).click();
await expect(page.locator('input[name="name"]')).toHaveValue(
diff --git a/editor/e2e/settings.spec.ts b/editor/e2e/settings.spec.ts
@@ -35,9 +35,10 @@ test("saves global default social links", async ({ page }) => {
await page.goto("/settings");
await page.getByRole("button", { name: /add link/i }).click();
await page.getByPlaceholder(/label \(e\.g\. github\)/i).fill("GitHub");
- await page
- .getByPlaceholder(/https:\/\//)
- .fill("https://github.com/example");
+ // Exact placeholder: /https:\/\// also matches the archive-overflow field's
+ // "https://archives.example.com". The social-link URL input's placeholder is
+ // the bare "https://…" (an ellipsis character, not three dots).
+ await page.getByPlaceholder("https://…").fill("https://github.com/example");
await page
.getByPlaceholder(/<svg viewbox/i)
.fill('<svg viewBox="0 0 24 24"><path d="M0 0h24v24H0z"/></svg>');
diff --git a/editor/e2e/site-scope.spec.ts b/editor/e2e/site-scope.spec.ts
@@ -50,7 +50,10 @@ test("selecting a site scopes the channels list and persists", async ({
test("dashboard stats and table scope to the active site", async ({ page }) => {
await twoSites();
await page.goto("/?site=alpha");
- await expect(page.getByRole("group", { name: "channels" })).toContainText("1");
+ // The dashboard's stat tiles were replaced by the mission-control cockpit
+ // (c63f5cb), which has no "channels" group; the scoped count is now observable
+ // as the number of rows in its channels table.
+ await expect(page.getByLabel(/^channel row /)).toHaveCount(1);
await expect(page.getByRole("link", { name: "slow-a" })).toBeVisible();
await expect(page.getByRole("link", { name: "slow-b" })).toHaveCount(0);
});
diff --git a/editor/e2e/sync-break-on-existing.spec.ts b/editor/e2e/sync-break-on-existing.spec.ts
@@ -33,6 +33,9 @@ test("treats yt-dlp exit code 101 as success and updates lastSyncedAt", async ({
.getByRole("row")
.filter({ hasText: "archived-channel" })
.first();
- await expect(syncRow).toContainText("sync");
+ // data-kind, not the row text: the cell renders the label "Sync", so
+ // toContainText("sync") is a case-sensitive miss — a deterministic failure
+ // that was long mis-recorded as an order-dependent flake.
+ await expect(syncRow).toHaveAttribute("data-kind", "sync");
await expect(syncRow).toContainText("done");
});
diff --git a/editor/package.json b/editor/package.json
@@ -5,8 +5,8 @@
"type": "module",
"scripts": {
"dev": "next dev --port ${EDITOR_PORT:-3001}",
- "dev:test": "WORKER_TOKEN=test-worker-token TRANSCRIPTS_DIR=$(pwd)/test-transcripts EXPORT_PUBLIC_DIR=$(pwd)/test-transcripts/.export-public SETTINGS_FILE=$(pwd)/test-settings.json YTDLP_BIN=$(pwd)/e2e/fixtures/bin/fake-ytdlp.mjs GALLERY_DL_BIN=$(pwd)/e2e/fixtures/bin/fake-gallery-dl.mjs WHISPER_BIN=$(pwd)/e2e/fixtures/bin/fake-whisper.mjs WHISPER_MODEL=/dev/null CHOUGH_BIN=$(pwd)/e2e/fixtures/bin/fake-chough.mjs CHOUGH_MODEL=/dev/null PARAKEET_STITCH_BIN=$(pwd)/e2e/fixtures/bin/fake-parakeet-stitch.mjs PARAKEET_CLI=/dev/null PARAKEET_MODEL=/dev/null FFMPEG_BIN=$(pwd)/e2e/fixtures/bin/fake-ffmpeg.mjs FFPROBE_BIN=$(pwd)/e2e/fixtures/bin/fake-ffprobe.mjs OLLAMA_URL=http://127.0.0.1:${OLLAMA_STUB_PORT:-11435} CLAUDE_BIN=$(pwd)/e2e/fixtures/bin/fake-claude.mjs AUDIO_CHECK_INTERVAL_MS_OVERRIDE=300 AUDIO_CHECK_SIZE_GATE_OVERRIDE=4096 AUDIO_CHECK_INTERVAL_FLOOR_MS_OVERRIDE=50 AUDIO_CHECK_RECOVER_STEP_MS_OVERRIDE=100 AUDIO_CHECK_RECOVER_AFTER_OVERRIDE=2 next dev --port ${PORT:-3011}",
- "start:test": "WORKER_TOKEN=test-worker-token TRANSCRIPTS_DIR=$(pwd)/test-transcripts EXPORT_PUBLIC_DIR=$(pwd)/test-transcripts/.export-public SETTINGS_FILE=$(pwd)/test-settings.json YTDLP_BIN=$(pwd)/e2e/fixtures/bin/fake-ytdlp.mjs GALLERY_DL_BIN=$(pwd)/e2e/fixtures/bin/fake-gallery-dl.mjs WHISPER_BIN=$(pwd)/e2e/fixtures/bin/fake-whisper.mjs WHISPER_MODEL=/dev/null CHOUGH_BIN=$(pwd)/e2e/fixtures/bin/fake-chough.mjs CHOUGH_MODEL=/dev/null PARAKEET_STITCH_BIN=$(pwd)/e2e/fixtures/bin/fake-parakeet-stitch.mjs PARAKEET_CLI=/dev/null PARAKEET_MODEL=/dev/null FFMPEG_BIN=$(pwd)/e2e/fixtures/bin/fake-ffmpeg.mjs FFPROBE_BIN=$(pwd)/e2e/fixtures/bin/fake-ffprobe.mjs OLLAMA_URL=http://127.0.0.1:${OLLAMA_STUB_PORT:-11435} CLAUDE_BIN=$(pwd)/e2e/fixtures/bin/fake-claude.mjs AUDIO_CHECK_INTERVAL_MS_OVERRIDE=300 AUDIO_CHECK_SIZE_GATE_OVERRIDE=4096 AUDIO_CHECK_INTERVAL_FLOOR_MS_OVERRIDE=50 AUDIO_CHECK_RECOVER_STEP_MS_OVERRIDE=100 AUDIO_CHECK_RECOVER_AFTER_OVERRIDE=2 next start --port ${PORT:-3011}",
+ "dev:test": "WORKER_TOKEN=test-worker-token TRANSCRIPTS_DIR=$(pwd)/test-transcripts EXPORT_PUBLIC_DIR=$(pwd)/test-transcripts/.export-public SETTINGS_FILE=$(pwd)/test-settings.json EDITOR_CHANGELOG_FILE=$(pwd)/test-changelog.md YTDLP_BIN=$(pwd)/e2e/fixtures/bin/fake-ytdlp.mjs GALLERY_DL_BIN=$(pwd)/e2e/fixtures/bin/fake-gallery-dl.mjs WHISPER_BIN=$(pwd)/e2e/fixtures/bin/fake-whisper.mjs WHISPER_MODEL=/dev/null CHOUGH_BIN=$(pwd)/e2e/fixtures/bin/fake-chough.mjs CHOUGH_MODEL=/dev/null PARAKEET_STITCH_BIN=$(pwd)/e2e/fixtures/bin/fake-parakeet-stitch.mjs PARAKEET_CLI=/dev/null PARAKEET_MODEL=/dev/null FFMPEG_BIN=$(pwd)/e2e/fixtures/bin/fake-ffmpeg.mjs FFPROBE_BIN=$(pwd)/e2e/fixtures/bin/fake-ffprobe.mjs OLLAMA_URL=http://127.0.0.1:${OLLAMA_STUB_PORT:-11435} CLAUDE_BIN=$(pwd)/e2e/fixtures/bin/fake-claude.mjs AUDIO_CHECK_INTERVAL_MS_OVERRIDE=300 AUDIO_CHECK_SIZE_GATE_OVERRIDE=4096 AUDIO_CHECK_INTERVAL_FLOOR_MS_OVERRIDE=50 AUDIO_CHECK_RECOVER_STEP_MS_OVERRIDE=100 AUDIO_CHECK_RECOVER_AFTER_OVERRIDE=2 next dev --port ${PORT:-3011}",
+ "start:test": "WORKER_TOKEN=test-worker-token TRANSCRIPTS_DIR=$(pwd)/test-transcripts EXPORT_PUBLIC_DIR=$(pwd)/test-transcripts/.export-public SETTINGS_FILE=$(pwd)/test-settings.json EDITOR_CHANGELOG_FILE=$(pwd)/test-changelog.md YTDLP_BIN=$(pwd)/e2e/fixtures/bin/fake-ytdlp.mjs GALLERY_DL_BIN=$(pwd)/e2e/fixtures/bin/fake-gallery-dl.mjs WHISPER_BIN=$(pwd)/e2e/fixtures/bin/fake-whisper.mjs WHISPER_MODEL=/dev/null CHOUGH_BIN=$(pwd)/e2e/fixtures/bin/fake-chough.mjs CHOUGH_MODEL=/dev/null PARAKEET_STITCH_BIN=$(pwd)/e2e/fixtures/bin/fake-parakeet-stitch.mjs PARAKEET_CLI=/dev/null PARAKEET_MODEL=/dev/null FFMPEG_BIN=$(pwd)/e2e/fixtures/bin/fake-ffmpeg.mjs FFPROBE_BIN=$(pwd)/e2e/fixtures/bin/fake-ffprobe.mjs OLLAMA_URL=http://127.0.0.1:${OLLAMA_STUB_PORT:-11435} CLAUDE_BIN=$(pwd)/e2e/fixtures/bin/fake-claude.mjs AUDIO_CHECK_INTERVAL_MS_OVERRIDE=300 AUDIO_CHECK_SIZE_GATE_OVERRIDE=4096 AUDIO_CHECK_INTERVAL_FLOOR_MS_OVERRIDE=50 AUDIO_CHECK_RECOVER_STEP_MS_OVERRIDE=100 AUDIO_CHECK_RECOVER_AFTER_OVERRIDE=2 next start --port ${PORT:-3011}",
"build": "next build",
"start": "next start --port ${EDITOR_PORT:-3001}",
"lint": "eslint",
diff --git a/editor/playwright.config.ts b/editor/playwright.config.ts
@@ -38,9 +38,32 @@ const exportSitesDir = path.resolve(
export default defineConfig({
testDir: "./e2e",
timeout: 30_000,
+ // The sharded runner sets CI=true for `reuseExistingServer: !CI` below, not
+ // for retries, and passes an explicit --retries=0 so its failures stay
+ // comparable to a serial run's. See scripts/run-sharded-e2e.mjs.
retries: process.env.CI ? 2 : 0,
reporter: process.env.CI ? "github" : "list",
outputDir: "test-results/",
+ // Serial, one worker — deliberate, and not a performance oversight. Three
+ // pieces of shared mutable state are global to the whole run, so two workers
+ // corrupt each other's fixtures rather than merely running slower:
+ //
+ // 1. One hardcoded data root (e2e/helpers.ts:17) that 303 resetData() calls
+ // across 83 of the 87 spec files destroy and recreate — mostly from
+ // *inside* test bodies (e2e/helpers.ts:33-48), not just in beforeEach.
+ // 2. One shared test-settings.json (e2e/helpers.ts:18) that the export
+ // webServer below also reads as SETTINGS_FILE (see exportSettingsFile) —
+ // so resetData's rm+cp deletes a file a second live server is reading.
+ // 3. Four globalThis singletons inside the single Next server — job
+ // registry, worker pool, scheduler, auto-runner — cleared on every
+ // resetData/writeSettings via
+ // app/api/test/invalidate-cache/route.ts:24,32,38,44.
+ //
+ // (3) is the blocker that per-worker TRANSCRIPTS_DIR/SETTINGS_FILE cannot
+ // fix: the singletons live in the server process, so isolation needs a
+ // *server per worker*, not a directory per worker. That is why parallelism
+ // here is one container per shard (scripts/run-sharded-e2e.mjs, which gives
+ // each shard its own servers) rather than workers > 1 in this config.
fullyParallel: false,
workers: 1,
webServer: [
diff --git a/plans/FACTS.md b/plans/FACTS.md
@@ -918,6 +918,13 @@ any feature work: each needs a tightened locator (`exact: true`, a `getByRole` s
form, or a `data-testid`), not a behaviour change. Fixing them is worth doing precisely
because 9 permanently-red specs train everyone to ignore the suite's exit code.
+> **Superseded 2026-07-30 — every red above is now fixed.** See
+> "[The 22 reds: what they actually were](#the-22-reds-what-they-actually-were)" below. Two
+> corrections to the paragraph above, both established by running the specs rather than
+> reading them: `deploy-page:8` and `site-scope:50` were **not** strict-mode violations (they
+> failed with "element(s) not found" against renamed/deleted UI), so the count of true
+> collisions was 7, not 9.
+
`widget.spec`'s "no buttons" assertion fails under `dev:test` because `next dev` injects a
Dev Tools button; it passes under `E2E_MODE=start`. (It passed in this run.)
@@ -930,6 +937,144 @@ not reset `timestampMode`/`promptVariant`.
Run `pnpm e2e` in **default dev mode** — `E2E_MODE=start` serves a stale build. Kill stale
dev servers by port between runs.
+### The 22 reds: what they actually were
+
+**Fixed 2026-07-30.** Every one of the permanently-red specs above is now green. The headline:
+they were **almost entirely assertion drift, not locator collisions** — the suite was asserting
+against UI text and job kinds that the product had since renamed, and each red was diagnosed by
+*running* it, not by reading it.
+
+| Was | Actual cause | Fix |
+| --- | --- | --- |
+| `new-channel-onboarding` ×5 | Real collision. `getByLabel` matches by **substring**, and three other controls wrap descriptions ending "Requires a URL." inside their `<label>`, putting "URL" in their accessible names → 4 elements | `getByLabel("URL", { exact: true })` |
+| `deploy-page:29,79` | Real collisions: `name: "Build & deploy"` also matches "Build & deploy all sites" (getByRole's name is a substring match); two identically-labelled "Deploy after build" checkboxes in one `<section>` | `exact: true`; `role="group" aria-label="Build specific sites"` on `BuildSitesPanel` to scope the twin |
+| `deploy-page:8` | **Not a collision** — "element(s) not found". `"Build multiple sites"` exists nowhere in the repo; the heading is "Build all sites" | assert the real heading |
+| `site-scope:50` | **Not a collision** — the cockpit redesign (`c63f5cb`) deleted the `role="group"` stat tiles entirely | assert the scoped channel-row count |
+| `actionable:162,181`, `cleanup-actionable:78`, `channels-actions:42`, `bulk-actions:40,64,88`, `sync-break-on-existing:4` | **Job-kind label drift.** `/jobs` renders `jobKindLabel(kind)`, so asserting the raw kind silently stops matching once a kind gains a label: `whisper-all`→"Transcribe all", `download-missing`→"Download missing", `clean-audio-transcribed`→"Clean audio", `whisper-bucket-downloaded-no-transcript`→"Transcribe downloaded audio", `retry-bucket`→"Retry", `sync`→"Sync" | `data-kind` on the `<tr>` + `jobRowByKind()` in `e2e/helpers.ts` |
+| `actionable:200` | The card was renamed "Needs attention" → "Needs work" (`c63f5cb`) | assert the real label |
+| `settings:34` | Real collision: `getByPlaceholder(/https:\/\//)` also matched the archive-overflow field | exact placeholder `"https://…"` |
+| `cut-release` ×2 | The spec rewrote the **tracked** `editor/CHANGELOG.md` and the action made a **real git commit** | `EDITOR_CHANGELOG_FILE` → gitignored `editor/test-changelog.md`; spec unchecks "Commit changelog" |
+
+Three things that are worth remembering beyond the individual fixes:
+
+- **`sync-break-on-existing` was never an order-dependent flake.** It asserts
+ `toContainText("sync")` while the row renders the label `"Sync"` — a case-sensitive miss that
+ fails deterministically. The "passes in isolation" note above was wrong.
+- **The "queues a job" reds were never the hydration bug.** Each of those tests first asserts an
+ inline job pill that only renders once a jobId exists, and that assertion passes — the POST
+ demonstrably happens. The natural experiment is inside `channels-actions.spec` itself: its
+ `check-availability` test passes *because that kind has no entry in `JOB_KINDS`*, so
+ `jobKindLabel` falls back to the raw string.
+- **One test was passing vacuously.** `actionable`'s "hides the Needs attention card" asserted
+ `toHaveCount(0)` on a label nothing carries — it would have passed no matter what the product
+ did. Renaming the label is what exposed it.
+
+### The two e2e routes, and what their numbers mean
+
+| Route | Command | Mode | Retries | Result | Wall |
+| --- | --- | --- | --- | --- | --- |
+| Serial | `pnpm e2e` | `next dev` | 0 | **390 / 392** (×2 runs) | 23.8 min (1428 s); 29.7 min under heavier load |
+| Sharded | `pnpm e2e:sharded --shards 4` | `next start`, 4 containers | 0 (default) | **389–391 / 392** (×4 runs) | 7.1 min (424 s) best, 11.1 min (668 s) under load; **~2.1–3.4×** |
+
+Before this pass the serial suite was **370 / 392**. Every one of the 22 named reds is fixed.
+The residual is a **rotating** 1–3 test tail whose membership changes every run — see below;
+it is contention, not a fixed set, and the two members that *did* repeat both turned out to be
+real test bugs and are now fixed.
+
+The sharded route's failure was `deploy-page:64`'s build-mode toggle, and it is worth writing
+down because the obvious diagnosis was wrong. It looks like the **pre-hydration lost click**
+this repo has hit repeatedly, but it is not: wrapping the click in a 15 s `toPass` retry did
+not fix it, and the failure then moved to the *reload* assertion, which is the tell.
+
+The real cause is an **optimistic write raced by a reload**. `BuildModeToggle` calls
+`setMode(next)` — flipping `aria-pressed` immediately — and only *then* runs
+`setBuildModeAction` inside `startTransition`. So every assertion on the toggle's own state
+passes before `settings.json` has been written, and the test's `page.goto()` reload can read
+the pre-click settings. A fast host wins that race; a loaded container loses it. The spec now
+polls `test-settings.json` for `buildPipeline.mode === "docker"` before reloading.
+
+**The general lesson: an optimistic control proves nothing about persistence.** Assert the
+persisted artifact, not the widget, before reloading — and treat "passes serially, fails
+sharded" as a race indicator rather than a mode difference.
+
+`actionable:114` ("'Update all reports' queues a refresh-report job per channel") had the same
+signature and the same moral. `refresh-report` is the **one** kind `/jobs` hides by default
+(`DEFAULT_HIDDEN_KINDS` in `app/jobs/jobsFilterStorage.ts`), and `JobsTable` renders every job
+while its filter state is still null (`if (!filters) return jobs`), dropping it the moment
+localStorage loads. So the test was asserting the visibility of a row the product deliberately
+hides, and **passing only because it beat its own hydration** — a race won under `next dev` and
+lost under `next start`. It now un-hides the kind via its filter chip first (the chip is always
+rendered: `kinds` is derived from all jobs, not the visible ones).
+
+Two of the specs examined in this pass were therefore green for reasons unrelated to what they
+claimed to test. **A green suite is evidence only if the assertions can fail.**
+
+### ⚠️ `editor/CHANGELOG.md` was destroyed by the cut-release spec and committed
+
+Not a hypothetical — it already happened, and it is still the state of `main`. The tracked
+`editor/CHANGELOG.md` at HEAD is **111 bytes of the spec's own fixture**:
+
+```
+# Changelog
+
+## [Unreleased]
+- test bullet for cut-release spec
+
+## [9.9.9] - 2024-01-01
+- old released bullet
+```
+
+The last good version is **`8a87c60`** (a full, detailed changelog). The very next commit to
+touch the file, **`691057b`** ("Persist the sweep's SCOPE, not just the fact that one is
+armed"), committed the fixture over it — almost certainly because a `cut-release.spec` run had
+left it in the working tree and the release entry that commit meant to add went in on top of
+fixture content. Everything from `8a87c60` backwards is recoverable:
+
+```bash
+git show 8a87c60:editor/CHANGELOG.md > editor/CHANGELOG.md
+```
+
+(The entry `691057b` intended to write was never captured and is not recoverable.) This is
+**not** restored here — it is a content decision for the maintainer. The spec can no longer do
+this: it writes to a gitignored `editor/test-changelog.md` and no longer commits.
+
+Both measured 2026-07-30. **Neither is a clean-box number**: an unrelated project
+(`content-engine`) was running its own Playwright suite concurrently at load ~15–23. A second
+serial run under heavier load took **29.7 min** for the same 390/2 — so treat these as an
+envelope, not a constant.
+
+**The residual failures are a rotating contention tail, not a fixed set.** Across six full runs
+the failing membership was different almost every time — `actionable:181` + `pipeline:106`;
+`do-not-clean:87` + `shard:303`; `deploy-page:64`; `actionable:114` +
+`audio-check-scenarios:198` + `deploy-page:64`; `actionable:114`; `channels:10` +
+`audio-check-scenarios:198` + `do-not-clean:87`. **A red here does not mean a regression:
+re-run the spec in isolation before believing it.**
+
+Two things came out of that tail worth keeping:
+
+- The first pair were both `ENOTEMPTY: directory not empty` from `resetData()`'s recursive `rm`
+ racing a still-running job from the *previous* spec writing back into the tree. Now mitigated
+ with `fs.rm`'s native `maxRetries`/`retryDelay` — Node retries exactly that errno set.
+- **The ones that repeat are worth chasing; the ones that rotate are not.** `deploy-page:64` and
+ `actionable:114` each failed twice in the same route and both turned out to be genuine test
+ bugs (below), while the rotating members are timing under external load.
+
+All of this was measured while an unrelated project was running its own Playwright suite on the
+same box at load 15–27. Check `/proc/loadavg` and `docker ps` before trusting any number here.
+
+**Mode-dependent specs — do not compare across routes without accounting for these:**
+
+- `widget.spec` "no buttons": **red under `dev:test`** (next dev injects a Dev Tools button),
+ **green under `E2E_MODE=start`** — so it is green in the sharded route and red in the serial
+ one. This is the one known case where the two routes legitimately disagree.
+
+**The sharded route was completely broken before 2026-07-30** — every shard died with
+"Timed out waiting 120000ms from config.webServer" because the export dev server 500s on a
+missing `export/public/summaries/manifest.json`, a generated artifact `.dockerignore`
+deliberately excludes. Host runs only worked because that file happens to exist in the working
+copy. `Dockerfile.test` now seeds an empty-site manifest. The README's old "2.5× on 4 shards"
+table described a route that could not start.
+
---
## Dependencies present / absent
diff --git a/scripts/run-sharded-e2e.mjs b/scripts/run-sharded-e2e.mjs
@@ -11,20 +11,64 @@ const EDITOR_DIR = path.join(REPO_ROOT, "editor");
const REPORT_DIR = path.join(EDITOR_DIR, "blob-report");
const IMAGE = process.env.IMAGE ?? "yt-dlp-transcript-browser-e2e";
-function parseShards() {
- const argIdx = process.argv.indexOf("--shards");
+// Flags this script consumes itself; everything else on the command line is
+// forwarded verbatim to each shard's `playwright test` (see parseArgs).
+const OWN_FLAGS = new Set(["--shards", "--retries"]);
+
+function parseIntArg(flag, envVar, fallback, min) {
+ const argIdx = process.argv.indexOf(flag);
if (argIdx !== -1 && process.argv[argIdx + 1]) {
const n = Number(process.argv[argIdx + 1]);
- if (Number.isInteger(n) && n >= 1) return n;
+ if (Number.isInteger(n) && n >= min) return n;
+ }
+ if (envVar && process.env[envVar]) {
+ const n = Number(process.env[envVar]);
+ if (Number.isInteger(n) && n >= min) return n;
}
- if (process.env.SHARDS) {
- const n = Number(process.env.SHARDS);
- if (Number.isInteger(n) && n >= 1) return n;
+ return fallback;
+}
+
+function parseShards() {
+ return parseIntArg(
+ "--shards",
+ "SHARDS",
+ Math.min(Math.max(2, Math.floor(os.cpus().length / 2)), 8),
+ 1,
+ );
+}
+
+// Retries default to 0 so a sharded run reports the same failures a serial run
+// does. Without this the container's CI=true would pick up playwright.config's
+// `retries: process.env.CI ? 2 : 0` and silently paper over flaky tests, which
+// makes the result impossible to compare against the recorded baseline. A CLI
+// --retries beats the config file, so this neutralizes it without unsetting CI.
+function parseRetries() {
+ return parseIntArg("--retries", "E2E_RETRIES", 0, 0);
+}
+
+// Anything not consumed above is passed through to `playwright test` in every
+// shard, so a subset can be sharded: `pnpm e2e:sharded -- --grep "digest"` or
+// `node scripts/run-sharded-e2e.mjs --shards 4 e2e/deploy-page.spec.ts`.
+// An explicit `--` separator is honoured, but is not required: pnpm already
+// strips the first `--` before the script sees argv.
+function parsePassthrough() {
+ const argv = process.argv.slice(2);
+ const sepIdx = argv.indexOf("--");
+ if (sepIdx !== -1) return argv.slice(sepIdx + 1);
+ const rest = [];
+ for (let i = 0; i < argv.length; i++) {
+ if (OWN_FLAGS.has(argv[i])) {
+ i++; // skip the flag's value too
+ continue;
+ }
+ rest.push(argv[i]);
}
- return Math.min(Math.max(2, Math.floor(os.cpus().length / 2)), 8);
+ return rest;
}
const SHARDS = parseShards();
+const RETRIES = parseRetries();
+const PASSTHROUGH = parsePassthrough();
function run(cmd, args, opts = {}) {
return new Promise((resolve, reject) => {
@@ -34,6 +78,20 @@ function run(cmd, args, opts = {}) {
});
}
+// Playwright's list reporter ends with a summary block of lines like
+// " 3 failed", " 1 flaky", " 87 passed (1.2m)". Test titles under the
+// "failed" heading are indented further and start with "[", so anchoring on
+// exactly two leading spaces picks up only the tally lines.
+const TALLY_RE = /^ {2}(\d+) (passed|failed|flaky|skipped|interrupted|did not run)\b/gm;
+
+function parseTallies(output) {
+ const counts = {};
+ for (const [, n, kind] of output.matchAll(TALLY_RE)) {
+ counts[kind] = (counts[kind] ?? 0) + Number(n);
+ }
+ return counts;
+}
+
function runShard(i, n) {
const tag = `[shard ${i}/${n}]`;
return new Promise((resolve) => {
@@ -41,17 +99,28 @@ function runShard(i, n) {
"run",
"--rm",
"--init",
+ // CI=true is load-bearing for playwright.config's
+ // `reuseExistingServer: !process.env.CI` — it makes each container start
+ // its own editor/export/ollama-stub servers instead of expecting one to
+ // already be up. It is NOT a request for retries; --retries below wins.
"-e", "CI=true",
"-e", "E2E_MODE=start",
"-v", `${REPORT_DIR}:/repo/editor/blob-report`,
IMAGE,
"pnpm", "exec", "playwright", "test",
`--shard=${i}/${n}`,
- "--reporter=blob",
+ `--retries=${RETRIES}`,
+ // `list` rides along with `blob` so each shard's tallies reach the
+ // prefixer below and can be summed into a combined count.
+ "--reporter=blob,list",
+ ...PASSTHROUGH,
];
const child = spawn("docker", args);
+ let captured = "";
const prefix = (chunk) => {
- const lines = chunk.toString("utf8").split("\n");
+ const text = chunk.toString("utf8");
+ captured += text;
+ const lines = text.split("\n");
const last = lines.pop();
for (const line of lines) process.stdout.write(`${tag} ${line}\n`);
if (last) process.stdout.write(`${tag} ${last}`);
@@ -60,9 +129,11 @@ function runShard(i, n) {
child.stderr.on("data", prefix);
child.on("error", (err) => {
process.stderr.write(`${tag} spawn error: ${err.message}\n`);
- resolve(1);
+ resolve({ code: 1, counts: {} });
});
- child.on("exit", (code) => resolve(code ?? 1));
+ child.on("exit", (code) =>
+ resolve({ code: code ?? 1, counts: parseTallies(captured) }),
+ );
});
}
@@ -79,17 +150,21 @@ async function cleanReportDir() {
async function main() {
console.log(`Running ${SHARDS} shard(s) using image ${IMAGE}`);
+ console.log(`Retries per shard: ${RETRIES}`);
+ if (PASSTHROUGH.length) {
+ console.log(`Forwarding to playwright: ${PASSTHROUGH.join(" ")}`);
+ }
const t0 = Date.now();
await cleanReportDir();
const shardPromises = [];
for (let i = 1; i <= SHARDS; i++) shardPromises.push(runShard(i, SHARDS));
- const codes = await Promise.all(shardPromises);
+ const results = await Promise.all(shardPromises);
const tShards = Date.now();
console.log("\nShard exit codes:");
- codes.forEach((code, idx) => {
+ results.forEach(({ code }, idx) => {
console.log(` shard ${idx + 1}/${SHARDS}: ${code === 0 ? "PASS" : `FAIL (${code})`}`);
});
console.log(`Sharded test wall time: ${((tShards - t0) / 1000).toFixed(1)}s`);
@@ -106,7 +181,28 @@ async function main() {
console.log("HTML report: editor/playwright-report/index.html");
}
- const failed = codes.some((c) => c !== 0);
+ // Combined tallies, so the run reports what it actually did rather than just
+ // whether each container exited non-zero.
+ const total = {};
+ for (const { counts } of results) {
+ for (const [kind, n] of Object.entries(counts)) {
+ total[kind] = (total[kind] ?? 0) + n;
+ }
+ }
+ const ORDER = ["passed", "failed", "flaky", "skipped", "interrupted", "did not run"];
+ const parts = ORDER.filter((k) => total[k]).map((k) => `${total[k]} ${k}`);
+ const summary = parts.length ? parts.join(" / ") : "no tallies parsed";
+ console.log(`\nCombined: ${summary}`);
+ if (RETRIES > 0) {
+ console.log(
+ `(--retries=${RETRIES}: 'flaky' means it failed then passed on retry — not comparable to a 0-retry baseline)`,
+ );
+ }
+ if (!parts.length) {
+ console.log("(shard output did not contain a list-reporter summary; see the HTML report)");
+ }
+
+ const failed = results.some(({ code }) => code !== 0);
process.exit(failed ? 1 : 0);
}
