diff --git a/.gitignore b/.gitignore index 58b013ed..c3b7f2cc 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ /.moo/ /crates/alacritty_terminal/ /crates/browser/pkg +/e2e/.e2e-socket /e2e/playwright-report/ /e2e/test-results/ /result diff --git a/e2e/playwright.config.ts b/e2e/playwright.config.ts index 6429e6b8..626441dc 100644 --- a/e2e/playwright.config.ts +++ b/e2e/playwright.config.ts @@ -23,7 +23,16 @@ export default defineConfig({ projects: [ { name: "chromium", - use: { browserName: "chromium" }, + use: { + browserName: "chromium", + // Same escape hatch as playwright.dev.config.ts, so this exact suite + // can be reproduced outside CI: the npm-bundled browser does not start + // on NixOS (it cannot find libglib). Unset in CI, where the Nix + // playwright package brings its own working browser. + launchOptions: process.env.CHROMIUM_BIN + ? { executablePath: process.env.CHROMIUM_BIN } + : {}, + }, }, ], webServer: { diff --git a/e2e/start-servers.sh b/e2e/start-servers.sh index 943dfa52..c7b479f3 100755 --- a/e2e/start-servers.sh +++ b/e2e/start-servers.sh @@ -10,10 +10,20 @@ REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" TMPDIR_E2E="${BLIT_E2E_TMPDIR:-$(mktemp -d)}" export BLIT_SOCK="${TMPDIR_E2E}/blit-test.sock" +# Where a spec can find the server behind the gateway it is driving. Playwright +# starts this script as its own process tree, so an exported BLIT_SOCK reaches +# the gateway and nothing else — a spec that shells out to the CLI would +# otherwise resolve the *default* socket and quietly interrogate a different +# server. The file exists only while these servers do, so its absence +# correctly means "somebody else's gateway, use the CLI's own resolution". +SOCK_HANDOFF="${REPO_ROOT}/e2e/.e2e-socket" +printf '%s' "$BLIT_SOCK" >"$SOCK_HANDOFF" + cleanup() { # Kill child processes kill "$SERVER_PID" "$GATEWAY_PID" 2>/dev/null || true wait "$SERVER_PID" "$GATEWAY_PID" 2>/dev/null || true + rm -f "$SOCK_HANDOFF" rm -rf "$TMPDIR_E2E" } trap cleanup EXIT INT TERM diff --git a/e2e/tests/panel-subscriptions.spec.ts b/e2e/tests/panel-subscriptions.spec.ts index e54cf4d8..a9a96005 100644 --- a/e2e/tests/panel-subscriptions.spec.ts +++ b/e2e/tests/panel-subscriptions.spec.ts @@ -1,5 +1,6 @@ import { test, expect, type Page } from "@playwright/test"; import { execFileSync } from "child_process"; +import fs from "fs"; import path from "path"; /** @@ -19,8 +20,29 @@ import path from "path"; const BLIT = path.resolve(__dirname, "../../target/debug/blit"); +/** The env that points the CLI at the server the gateway under test proxies to. + * + * `start-servers.sh` puts its server on a private socket and publishes the + * path in `e2e/.e2e-socket`; the specs run in a different process tree, so + * that file is the only way the path reaches them. Without it — a developer + * reusing an already-running gateway — the CLI's own resolution is the right + * answer, because that gateway fronts the default server. + * + * Getting this wrong is silent: the CLI happily starts a second server on the + * default socket, and every assertion here reads that empty one instead. */ +function serverEnv(): Record { + const handoff = path.resolve(__dirname, "../.e2e-socket"); + if (!fs.existsSync(handoff)) return process.env; + const sock = fs.readFileSync(handoff, "utf8").trim(); + // A run killed hard enough to skip the script's cleanup leaves the file + // behind. Pointing the CLI at a socket with nothing on it is worse than not + // pointing it anywhere: it would start a server there and talk to that. + if (!sock || !fs.existsSync(sock)) return process.env; + return { ...process.env, BLIT_SOCK: sock }; +} + function blit(...args: string[]): string { - return execFileSync(BLIT, args, { encoding: "utf8", env: process.env }); + return execFileSync(BLIT, args, { encoding: "utf8", env: serverEnv() }); } /** Terminal subscriptions per client row, e.g. ["1:80x24", "2:?"]. @@ -37,7 +59,10 @@ function subscribedTerminals(): string[] { return terminals.sort(); } -/** Close every terminal, so leftovers from an earlier run cannot be counted. */ +/** Close every terminal, so leftovers cannot be counted — and, afterwards, so + * this spec's two `cat` sessions are not what the next spec finds focused. + * A later spec that types a command into a `cat` gets its own text back and + * reports the feature under test as broken. */ function closeAllTerminals() { for (const row of blit("terminal", "list").trim().split("\n").slice(1)) { const id = row.split("\t")[0]; @@ -66,6 +91,8 @@ async function open(page: Page, panels: string) { } test.describe("side panel subscriptions", () => { + test.afterAll(closeAllTerminals); + test("a parked terminal is unsubscribed while the preview panel is closed", async ({ page, }) => { diff --git a/e2e/tests/roots-entry.spec.ts b/e2e/tests/roots-entry.spec.ts index 4a56a14c..d02b418e 100644 --- a/e2e/tests/roots-entry.spec.ts +++ b/e2e/tests/roots-entry.spec.ts @@ -1,12 +1,17 @@ import { test, expect } from "@playwright/test"; /** - * Workspace roots is a Cmd+K entry, not a chord and not status bar chrome. + * Workspace roots is opened by the ⚙ beside the workspace-root selector, and + * by nothing else: not a chord, not status bar chrome, and not a Cmd+K entry. * - * The ⚙ beside the workspace-root selector in the left dock stays: it is - * contextual to the control it sits next to, not a second global affordance. + * The ⚙ is contextual to the control it sits next to, which is why it is the + * one that stays. The switcher deliberately carries only what has no other + * home, so an entry there would be a second global affordance for a dialog the + * left dock already opens in one click. */ -test("workspace roots lives in the Cmd+K menu", async ({ page }) => { +test("workspace roots opens from the dock, and from nowhere else", async ({ + page, +}) => { await page.goto("/"); await page.evaluate(() => localStorage.clear()); await page.goto("/#psk=test-secret"); @@ -29,11 +34,17 @@ test("workspace roots lives in the Cmd+K menu", async ({ page }) => { 0, ); - // It is an entry in the switcher, and it opens the roots overlay. + // Neither does the switcher, which lists only what the chrome cannot reach. await page.keyboard.press("ControlOrMeta+k"); - const entry = page.getByText("Workspace roots", { exact: true }).first(); - await expect(entry).toBeVisible({ timeout: 5_000 }); - await entry.click(); + await page.waitForTimeout(400); + await expect(page.getByText("Workspace roots", { exact: true })).toHaveCount( + 0, + ); + await page.keyboard.press("Escape"); + + // The ⚙ next to the workspace-root selector is the affordance that remains. + // Located by title: its accessible name is the glyph, which names nothing. + await page.getByTitle("Manage workspace roots").click(); await expect( page.getByText(/add a root|workspace roots/i).first(), ).toBeVisible({ timeout: 5_000 }); diff --git a/js/ui/src/createKeyboardShortcuts.ts b/js/ui/src/createKeyboardShortcuts.ts index 171cdbb3..b083577b 100644 --- a/js/ui/src/createKeyboardShortcuts.ts +++ b/js/ui/src/createKeyboardShortcuts.ts @@ -563,8 +563,9 @@ export function createKeyboardShortcuts(h: KeyboardShortcutHandlers): void { h.toggleOverlay("web"); return; } - // Workspace roots have no shortcut of their own: they are an entry in - // the Cmd+K switcher, alongside remotes, palette, and font. + // Workspace roots have no shortcut of their own: the ⚙ beside the + // workspace-root selector in the left dock opens them, which is also why + // the switcher deliberately carries no entry for them. if (mod && !e.shiftKey && e.key === "Enter") { if (h.overlay()) { // Let the overlay handle it.