Improve frontend test framework (#22824)

* add error allowlist file for error collector * add error collector for console + page + request errors * wire error collector into frigateApp fixture * add self-tests for error collector fixture * gate strict error mode on E2E_STRICT_ERRORS=1 * triage pre-existing errors and seed allowlist * add mockEmpty/mockError/mockDelay helpers for state-driven tests * add self-tests for mock override helpers * add mobile affordance helpers to BasePage * add lint script for banned spec patterns and @mobile rule * apply prettier fixes to new e2e files * rewrite export.spec.ts * clean up * move export spec rewrite and bugfix to separate branch
2026-07-21 03:09:02 +03:00 · 2026-04-09 14:42:36 -06:00
parent 98c2fe00c1
commit d113be5e19
9 changed files with 735 additions and 2 deletions
@@ -0,0 +1,116 @@
+/**
+ * Global allowlist of regex patterns that the error collector ignores.
+ *
+ * Each entry MUST include a comment explaining what it silences and why.
+ * The allowlist is filtered at collection time, so failure messages list
+ * only unfiltered errors.
+ *
+ * Per-spec additions go through the `expectedErrors` test fixture parameter
+ * (see error-collector.ts), not by editing this file. That keeps allowlist
+ * drift visible per-PR rather than buried in shared infrastructure.
+ *
+ * NOTE ON CONSOLE vs REQUEST ERRORS:
+ * When a network request returns a 5xx response, the browser emits two
+ * events that the error collector captures:
+ *   [request] "500 Internal Server Error <url>"  — from onResponse (URL included)
+ *   [console] "Failed to load resource: ..."     — from onConsole (URL NOT included)
+ *
+ * The request-level message includes the URL, so those patterns are specific.
+ * The console-level message text (from ConsoleMessage.text()) does NOT include
+ * the URL — the URL is stored separately in e.url. Therefore the console
+ * pattern for HTTP 500s cannot be URL-discriminated, and a single pattern
+ * covers all such browser echoes. This is safe because every such console
+ * error is already caught (and specifically matched) by its paired [request]
+ * entry below.
+ */
+
+export const GLOBAL_ALLOWLIST: RegExp[] = [
+  // -------------------------------------------------------------------------
+  // Browser echo of HTTP 5xx responses (console mirror of [request] events).
+  //
+  // Whenever the browser receives a 5xx response it emits a console error:
+  //   "Failed to load resource: the server responded with a status of 500
+  //    (Internal Server Error)"
+  // The URL is NOT part of ConsoleMessage.text() — it is stored separately.
+  // Every console error of this form is therefore paired with a specific
+  // [request] 500 entry below that names the exact endpoint. Allowlisting
+  // this pattern here silences the browser echo; the request-level entries
+  // enforce specificity.
+  // -------------------------------------------------------------------------
+  /Failed to load resource: the server responded with a status of 500/,
+
+  // -------------------------------------------------------------------------
+  // Mock infrastructure gaps — API endpoints not yet covered by ApiMocker.
+  //
+  // These produce 500s because Vite's preview server has no handler for them.
+  // Each is a TODO(real-bug): the mock should be extended so these endpoints
+  // return sensible fixture data in tests.
+  //
+  // Only [request] patterns are listed here; the paired [console] mirror is
+  // covered by the "Failed to load resource" entry above.
+  // -------------------------------------------------------------------------
+
+  // TODO(real-bug): ApiMocker registers "**/api/reviews**" (plural) but the
+  // app fetches /api/review (singular) for the review list and timeline.
+  // Affects: review.spec.ts, navigation.spec.ts, live.spec.ts, auth.spec.ts.
+  // Fix: add route handlers for /api/review and /api/review/** in api-mocker.ts.
+  /500 Internal Server Error.*\/api\/review(\?|\/|$)/,
+
+  // TODO(real-bug): /api/stats/history is not mocked; the system page fetches
+  // it for the detector/process history charts.
+  // Fix: add route handler for /api/stats/history in api-mocker.ts.
+  /500 Internal Server Error.*\/api\/stats\/history/,
+
+  // TODO(real-bug): /api/event_ids is not mocked; the explore/search page
+  // fetches it to resolve event IDs for display.
+  // Fix: add route handler for /api/event_ids in api-mocker.ts.
+  /500 Internal Server Error.*\/api\/event_ids/,
+
+  // TODO(real-bug): /api/sub_labels?split_joined=1 returns 500; the mock
+  // registers "**/api/sub_labels" which may not match when a query string is
+  // present, or route registration order causes the catch-all to win first.
+  // Fix: change the mock route to "**/api/sub_labels**" in api-mocker.ts.
+  /500 Internal Server Error.*\/api\/sub_labels/,
+
+  // TODO(real-bug): MediaMocker handles /api/*/latest.jpg but the app also
+  // requests /api/*/latest.webp (webp format) for camera snapshots.
+  // Affects: live.spec.ts, review.spec.ts, auth.spec.ts, navigation.spec.ts.
+  // Fix: add route handler for /api/*/latest.webp in MediaMocker.install().
+  /500 Internal Server Error.*\/api\/[^/]+\/latest\.webp/,
+  /failed: net::ERR_ABORTED.*\/api\/[^/]+\/latest\.webp/,
+
+  // -------------------------------------------------------------------------
+  // Mock infrastructure gap — WebSocket streams.
+  //
+  // Playwright's page.route() does not intercept WebSocket connections.
+  // The jsmpeg live-stream WS connections to /live/jsmpeg/* always fail
+  // with a 500 handshake error because the Vite preview server has no WS
+  // handler. TODO(real-bug): add WsMocker support for jsmpeg WebSocket
+  // connections, or suppress the connection attempt in the test environment.
+  // Affects: live.spec.ts (single camera view), auth.spec.ts.
+  // -------------------------------------------------------------------------
+  /WebSocket connection to '.*\/live\/jsmpeg\/.*' failed/,
+
+  // -------------------------------------------------------------------------
+  // Benign — lazy-loaded chunk aborts during navigation.
+  //
+  // When a test navigates away from a page while the browser is still
+  // fetching lazily-split JS/CSS asset chunks, the in-flight fetch is
+  // cancelled (net::ERR_ABORTED). This is normal browser behaviour on
+  // navigation and does not indicate a real error; the assets load fine
+  // on a stable connection.
+  // -------------------------------------------------------------------------
+  /failed: net::ERR_ABORTED.*\/assets\//,
+
+  // -------------------------------------------------------------------------
+  // Real app bug — Radix UI DialogContent missing accessible title.
+  //
+  // TODO(real-bug): A dialog somewhere in the app renders <DialogContent>
+  // without a <DialogTitle>, violating Radix UI's accessibility contract.
+  // The warning originates from the bundled main-*.js. Investigate which
+  // dialog component is missing the title and add a VisuallyHidden DialogTitle.
+  // Likely candidate: face-library or search-detail dialog in explore page.
+  // See: https://radix-ui.com/primitives/docs/components/dialog
+  // -------------------------------------------------------------------------
+  /`DialogContent` requires a `DialogTitle`/,
+];
@@ -0,0 +1,122 @@
+/**
+ * Collects console errors, page errors, and failed network requests
+ * during a Playwright test, with regex-based allowlist filtering.
+ *
+ * Usage:
+ *   const collector = installErrorCollector(page, [...GLOBAL_ALLOWLIST]);
+ *   // ... run test ...
+ *   collector.assertClean();   // throws if any non-allowlisted error
+ *
+ * The collector is wired into the `frigateApp` fixture so every test
+ * gets it for free. Tests that intentionally trigger an error pass
+ * additional regexes via the `expectedErrors` fixture parameter.
+ */
+
+import type { Page, Request, Response, ConsoleMessage } from "@playwright/test";
+
+export type CollectedError = {
+  kind: "console" | "pageerror" | "request";
+  message: string;
+  url?: string;
+  stack?: string;
+};
+
+export type ErrorCollector = {
+  errors: CollectedError[];
+  assertClean(): void;
+};
+
+function isAllowlisted(message: string, allowlist: RegExp[]): boolean {
+  return allowlist.some((pattern) => pattern.test(message));
+}
+
+function firstStackFrame(stack: string | undefined): string | undefined {
+  if (!stack) return undefined;
+  const lines = stack
+    .split("\n")
+    .map((l) => l.trim())
+    .filter(Boolean);
+  // Skip the error message line (line 0); return the first "at ..." frame
+  return lines.find((l) => l.startsWith("at "));
+}
+
+function isSameOrigin(url: string, baseURL: string | undefined): boolean {
+  if (!baseURL) return true;
+  try {
+    return new URL(url).origin === new URL(baseURL).origin;
+  } catch {
+    return false;
+  }
+}
+
+export function installErrorCollector(
+  page: Page,
+  allowlist: RegExp[],
+): ErrorCollector {
+  const errors: CollectedError[] = [];
+  const baseURL = (
+    page.context() as unknown as { _options?: { baseURL?: string } }
+  )._options?.baseURL;
+
+  const onConsole = (msg: ConsoleMessage) => {
+    if (msg.type() !== "error") return;
+    const text = msg.text();
+    if (isAllowlisted(text, allowlist)) return;
+    errors.push({
+      kind: "console",
+      message: text,
+      url: msg.location().url,
+    });
+  };
+
+  const onPageError = (err: Error) => {
+    const text = err.message;
+    if (isAllowlisted(text, allowlist)) return;
+    errors.push({
+      kind: "pageerror",
+      message: text,
+      stack: firstStackFrame(err.stack),
+    });
+  };
+
+  const onResponse = (response: Response) => {
+    const status = response.status();
+    if (status < 500) return;
+    const url = response.url();
+    if (!isSameOrigin(url, baseURL)) return;
+    const text = `${status} ${response.statusText()} ${url}`;
+    if (isAllowlisted(text, allowlist)) return;
+    errors.push({ kind: "request", message: text, url });
+  };
+
+  const onRequestFailed = (request: Request) => {
+    const url = request.url();
+    if (!isSameOrigin(url, baseURL)) return;
+    const failure = request.failure();
+    const text = `failed: ${failure?.errorText ?? "unknown"} ${url}`;
+    if (isAllowlisted(text, allowlist)) return;
+    errors.push({ kind: "request", message: text, url });
+  };
+
+  page.on("console", onConsole);
+  page.on("pageerror", onPageError);
+  page.on("response", onResponse);
+  page.on("requestfailed", onRequestFailed);
+
+  return {
+    errors,
+    assertClean() {
+      if (errors.length === 0) return;
+      const formatted = errors
+        .map((e, i) => {
+          const stack = e.stack ? `\n      ${e.stack}` : "";
+          const url = e.url && e.url !== e.message ? ` (${e.url})` : "";
+          return `  ${i + 1}. [${e.kind}] ${e.message}${url}${stack}`;
+        })
+        .join("\n");
+      throw new Error(
+        `Page emitted ${errors.length} unexpected error${errors.length === 1 ? "" : "s"}:\n${formatted}`,
+      );
+    },
+  };
+}
@@ -6,6 +6,11 @@
 * @playwright/test directly. The `frigateApp` fixture provides a
 * fully mocked Frigate frontend ready for interaction.
 *
+ * The fixture also installs the error collector (see error-collector.ts).
+ * Any console error, page error, or same-origin failed request that is
+ * not on the global allowlist or the test's `expectedErrors` list will
+ * fail the test in the fixture's teardown.
+ *
 * CRITICAL: All route/WS handlers are registered before page.goto()
 * to prevent AuthProvider from redirecting to login.html.
 */
@@ -17,6 +22,8 @@ import {
  type ApiMockOverrides,
 } from "../helpers/api-mocker";
 import { WsMocker } from "../helpers/ws-mocker";
+import { installErrorCollector, type ErrorCollector } from "./error-collector";
+import { GLOBAL_ALLOWLIST } from "./error-allowlist";

 export class FrigateApp {
  public api: ApiMocker;
@@ -67,10 +74,43 @@ export class FrigateApp {

 type FrigateFixtures = {
  frigateApp: FrigateApp;
+  /**
+   * Per-test additional allowlist regex patterns. Tests that intentionally
+   * trigger errors (e.g. error-state tests that hit a mocked 500) declare
+   * their expected errors here so the collector ignores them.
+   *
+   * Default is `[]` — most tests should not need this.
+   */
+  expectedErrors: RegExp[];
+  errorCollector: ErrorCollector;
 };

 export const test = base.extend<FrigateFixtures>({
-  frigateApp: async ({ page }, use, testInfo) => {
+  expectedErrors: [[], { option: true }],
+
+  errorCollector: async ({ page, expectedErrors }, use, testInfo) => {
+    const collector = installErrorCollector(page, [
+      ...GLOBAL_ALLOWLIST,
+      ...expectedErrors,
+    ]);
+    await use(collector);
+    if (process.env.E2E_STRICT_ERRORS === "1") {
+      collector.assertClean();
+    } else if (collector.errors.length > 0) {
+      // Soft mode: attach errors to the test report so they're visible
+      // without failing the run.
+      await testInfo.attach("collected-errors.txt", {
+        body: collector.errors
+          .map((e) => `[${e.kind}] ${e.message}${e.url ? ` (${e.url})` : ""}`)
+          .join("\n"),
+        contentType: "text/plain",
+      });
+    }
+  },
+
+  frigateApp: async ({ page, errorCollector }, use, testInfo) => {
+    // Reference the collector so its `use()` runs and teardown fires
+    void errorCollector;
    const app = new FrigateApp(page, testInfo.project.name);
    await app.installDefaults();
    await use(app);