From ab9f07dd8aa8c9f668f291983dab2a5e17895e18 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 4 May 2026 20:10:26 +0800 Subject: [PATCH] fix(app): make e2e setup an auto fixture so first-of-spec tests get setup (#702) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(app): make e2e setup an auto fixture so it runs for the first test of every spec The fixtures.ts beforeEach was declared at the top level of a non-test fixture file. Playwright sometimes skipped it for the first test of a subsequent spec when multiple specs ran in the same worker — that test hit page.evaluate without the seed nonce or daemon registry in localStorage and failed (then passed on retry, masking the bug behind retries: 1). Reproduced locally by running sidebar-workspace then startup-loading: the first startup-loading test got no fixture setup and threw "Expected e2e seed nonce". Move the setup into an `auto: true` fixture, the canonical Playwright pattern for running setup for every test in a workspace. The afterEach console-attachment is folded into the same fixture's teardown. * refactor(server): simplify nullish handling in workspace-git-metadata Drop the explicit `null` arg in `deriveProjectSlug(input.cwd, null)` — the second parameter already defaults to `null`. Inline the `repoName && repoName.length > 0` check in `parseGitHubRepoNameFromRemote` since `pop()` on a non-empty array returns a non-empty string here, and `|| null` covers the empty-string case directly. --- packages/app/e2e/fixtures.ts | 184 +++++++++--------- .../src/server/workspace-git-metadata.ts | 5 +- 2 files changed, 92 insertions(+), 97 deletions(-) diff --git a/packages/app/e2e/fixtures.ts b/packages/app/e2e/fixtures.ts index 0433333b9..45b1344b9 100644 --- a/packages/app/e2e/fixtures.ts +++ b/packages/app/e2e/fixtures.ts @@ -1,8 +1,12 @@ import { test as base, expect, type Page } from "@playwright/test"; import { buildCreateAgentPreferences, buildSeededHost } from "./helpers/daemon-registry"; -// Extend base test to provide dynamic baseURL from global-setup -const test = base.extend({ +// Test setup is wired through an `auto: true` fixture rather than `test.beforeEach`. +// `test.beforeEach` declared at the top level of a non-test fixture file is unreliable +// across spec-file boundaries — Playwright sometimes skips it for the first test of a +// subsequent spec when multiple specs run in the same worker. Auto fixtures run +// reliably for every test that uses this `test` object. +const test = base.extend<{ paseoE2ESetup: void }>({ baseURL: async ({}, provide) => { const metroPort = process.env.E2E_METRO_PORT; if (!metroPort) { @@ -10,102 +14,94 @@ const test = base.extend({ } await provide(`http://localhost:${metroPort}`); }, -}); - -const consoleEntries = new WeakMap(); - -test.beforeEach(async ({ page }) => { - const daemonPort = process.env.E2E_DAEMON_PORT; - const metroPort = process.env.E2E_METRO_PORT; - if (!daemonPort) { - throw new Error( - "E2E_DAEMON_PORT is not set. Refusing to run e2e against the default daemon (e.g. localhost:6767). " + - "Ensure Playwright `globalSetup` starts the e2e daemon and exports E2E_DAEMON_PORT.", - ); - } - if (daemonPort === "6767") { - throw new Error( - "E2E_DAEMON_PORT is 6767. Refusing to run e2e against the default local daemon. " + - "Fix Playwright globalSetup to start an isolated test daemon and export its port.", - ); - } - if (!metroPort) { - throw new Error( - "E2E_METRO_PORT is not set. Ensure Playwright `globalSetup` starts Metro and exports E2E_METRO_PORT.", - ); - } - - // Hard guardrail: never allow tests to hit the developer's default daemon. - // This blocks both HTTP and WS attempts to :6767 (before any navigation). - await page.route(/:(6767)\b/, (route) => route.abort()); - await page.routeWebSocket(/:(6767)\b/, async (ws) => { - await ws.close({ code: 1008, reason: "Blocked connection to localhost:6767 during e2e." }); - }); - - const entries: string[] = []; - consoleEntries.set(page, entries); - - page.on("console", (message) => { - entries.push(`[console:${message.type()}] ${message.text()}`); - }); - - page.on("pageerror", (error) => { - entries.push(`[pageerror] ${error.message}`); - }); - - const nowIso = new Date().toISOString(); - const seedNonce = Math.random().toString(36).slice(2); - const serverId = process.env.E2E_SERVER_ID; - if (!serverId) { - throw new Error("E2E_SERVER_ID is not set - expected from Playwright globalSetup."); - } - const testDaemon = buildSeededHost({ - serverId, - endpoint: `127.0.0.1:${daemonPort}`, - nowIso, - }); - const createAgentPreferences = buildCreateAgentPreferences(testDaemon.serverId); - - await page.addInitScript( - ({ daemon, preferences, seedNonce: nonce }) => { - // `addInitScript` runs on every navigation (including reloads). Some tests intentionally - // override storage and reload; they can opt out of seeding for the *next* navigation by - // setting this flag before the reload. - const disableOnceKey = "@paseo:e2e-disable-default-seed-once"; - const disableValue = localStorage.getItem(disableOnceKey); - if (disableValue) { - localStorage.removeItem(disableOnceKey); - if (disableValue === nonce) { - return; - } + paseoE2ESetup: [ + async ({ page }, provide, testInfo) => { + const daemonPort = process.env.E2E_DAEMON_PORT; + const metroPort = process.env.E2E_METRO_PORT; + if (!daemonPort) { + throw new Error( + "E2E_DAEMON_PORT is not set. Refusing to run e2e against the default daemon (e.g. localhost:6767). " + + "Ensure Playwright `globalSetup` starts the e2e daemon and exports E2E_DAEMON_PORT.", + ); + } + if (daemonPort === "6767") { + throw new Error( + "E2E_DAEMON_PORT is 6767. Refusing to run e2e against the default local daemon. " + + "Fix Playwright globalSetup to start an isolated test daemon and export its port.", + ); + } + if (!metroPort) { + throw new Error( + "E2E_METRO_PORT is not set. Ensure Playwright `globalSetup` starts Metro and exports E2E_METRO_PORT.", + ); } - localStorage.setItem("@paseo:e2e", "1"); - localStorage.setItem("@paseo:e2e-seed-nonce", nonce); + // Hard guardrail: never allow tests to hit the developer's default daemon. + // This blocks both HTTP and WS attempts to :6767 (before any navigation). + await page.route(/:(6767)\b/, (route) => route.abort()); + await page.routeWebSocket(/:(6767)\b/, async (ws) => { + await ws.close({ code: 1008, reason: "Blocked connection to localhost:6767 during e2e." }); + }); - // Hard-reset anything that could point to a developer's real daemon. - localStorage.setItem("@paseo:daemon-registry", JSON.stringify([daemon])); - localStorage.removeItem("@paseo:settings"); - localStorage.setItem("@paseo:create-agent-preferences", JSON.stringify(preferences)); + const entries: string[] = []; + + page.on("console", (message) => { + entries.push(`[console:${message.type()}] ${message.text()}`); + }); + + page.on("pageerror", (error) => { + entries.push(`[pageerror] ${error.message}`); + }); + + const nowIso = new Date().toISOString(); + const seedNonce = Math.random().toString(36).slice(2); + const serverId = process.env.E2E_SERVER_ID; + if (!serverId) { + throw new Error("E2E_SERVER_ID is not set - expected from Playwright globalSetup."); + } + const testDaemon = buildSeededHost({ + serverId, + endpoint: `127.0.0.1:${daemonPort}`, + nowIso, + }); + const createAgentPreferences = buildCreateAgentPreferences(testDaemon.serverId); + + await page.addInitScript( + ({ daemon, preferences, seedNonce: nonce }) => { + // `addInitScript` runs on every navigation (including reloads). Some tests intentionally + // override storage and reload; they can opt out of seeding for the *next* navigation by + // setting this flag before the reload. + const disableOnceKey = "@paseo:e2e-disable-default-seed-once"; + const disableValue = localStorage.getItem(disableOnceKey); + if (disableValue) { + localStorage.removeItem(disableOnceKey); + if (disableValue === nonce) { + return; + } + } + + localStorage.setItem("@paseo:e2e", "1"); + localStorage.setItem("@paseo:e2e-seed-nonce", nonce); + + // Hard-reset anything that could point to a developer's real daemon. + localStorage.setItem("@paseo:daemon-registry", JSON.stringify([daemon])); + localStorage.removeItem("@paseo:settings"); + localStorage.setItem("@paseo:create-agent-preferences", JSON.stringify(preferences)); + }, + { daemon: testDaemon, preferences: createAgentPreferences, seedNonce }, + ); + + await provide(); + + if (entries.length > 0 && testInfo.status !== testInfo.expectedStatus) { + await testInfo.attach("browser-console", { + body: entries.join("\n"), + contentType: "text/plain", + }); + } }, - { daemon: testDaemon, preferences: createAgentPreferences, seedNonce }, - ); -}); - -test.afterEach(async ({ page }, testInfo) => { - const entries = consoleEntries.get(page); - if (!entries || entries.length === 0) { - return; - } - - if (testInfo.status === testInfo.expectedStatus) { - return; - } - - await testInfo.attach("browser-console", { - body: entries.join("\n"), - contentType: "text/plain", - }); + { auto: true }, + ], }); export { test, expect, type Page }; diff --git a/packages/server/src/server/workspace-git-metadata.ts b/packages/server/src/server/workspace-git-metadata.ts index 73acf93c7..a0a0fee01 100644 --- a/packages/server/src/server/workspace-git-metadata.ts +++ b/packages/server/src/server/workspace-git-metadata.ts @@ -25,8 +25,7 @@ export function parseGitHubRepoNameFromRemote(remoteUrl: string): string | null return null; } - const repoName = githubRepo.split("/").pop(); - return repoName && repoName.length > 0 ? repoName : null; + return githubRepo.split("/").pop() || null; } export function deriveProjectSlug(cwd: string, remoteUrl: string | null = null): string { @@ -51,7 +50,7 @@ export function buildWorkspaceGitMetadataFromSnapshot(input: { workspaceDisplayName: input.directoryName, gitRemote: null, isWorktree: false, - projectSlug: deriveProjectSlug(input.cwd, null), + projectSlug: deriveProjectSlug(input.cwd), repoRoot: null, currentBranch: null, remoteUrl: null,