diff --git a/AGENTS.md b/AGENTS.md index 6c47e2c1..d8f2b3f7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -946,7 +946,7 @@ SMOKE_BROWSER=webkit npm run smoke:web:engine # any engine, on demand SMOKE_BROWSER=firefox npm run smoke:web:app # one smoke, one engine ``` -**Firefox runs in the local pre-push gate (`npm run ci`) and NOT in GitHub CI.** That split is the whole design — see the gate bullet below. `ENGINE_SMOKES` in `scripts/run-engine-smokes.mjs` is the single list of which smokes are engine-sensitive; add a fourth there and every engine picks it up. `smoke:web:firefox` passes the engine as an argument rather than relying on the ambient `SMOKE_BROWSER`, so the gate cannot be silently redirected to another engine by a stray variable — verified, an explicit argument beats it. +**Firefox runs in the local pre-push gate (`npm run ci`) and NOT in GitHub CI.** That split is the whole design — see the gate bullet below. `ENGINE_SMOKES` in `scripts/run-engine-smokes.mjs` is the single list of which smokes are engine-sensitive, and **every tier reads it** — the default Chromium run inside `npm run smoke`, the Firefox pass in the gate, and the on-demand command. Add a fourth there and all three pick it up. A tier that enumerated the smokes itself would silently run fewer and still pass, so `run-engine-smokes.test.mjs` asserts that none of them does. `smoke:web:firefox` passes the engine as an argument rather than relying on the ambient `SMOKE_BROWSER`, so the gate cannot be silently redirected to another engine by a stray variable — verified, an explicit argument beats it. Everything about the selection lives in **`scripts/lib/headless-browser.mjs`** — `resolveBrowserName`, `loadBrowser`, and the `attachPageDiagnostics` / `FATAL_CONSOLE` split the smokes had each hand-rolled. Reach for it rather than launching Playwright in a new script. diff --git a/package.json b/package.json index eba1ca4c..82771596 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,7 @@ "coverage:tui": "cd clients/tui && npm run test:coverage", "coverage:web": "cd clients/web && npm run test:coverage", "coverage:launcher": "cd clients/launcher && npm run test:coverage", - "smoke": "npm run smoke:launcher && npm run smoke:cli && npm run smoke:tui && npm run smoke:web && npm run smoke:web:browser && npm run smoke:web:app && npm run smoke:web:elicit", + "smoke": "npm run smoke:launcher && npm run smoke:cli && npm run smoke:tui && npm run smoke:web && npm run smoke:web:chromium", "smoke:cli": "node scripts/smoke-cli.mjs", "smoke:tui": "node scripts/smoke-tui.mjs", "smoke:web": "node scripts/smoke-web.mjs", @@ -74,6 +74,7 @@ "smoke:web:app": "node scripts/install-smoke-browser.mjs && node scripts/smoke-web-app.mjs", "smoke:web:elicit": "node scripts/install-smoke-browser.mjs && node scripts/smoke-web-elicitation.mjs", "smoke:web:engine": "node scripts/run-engine-smokes.mjs", + "smoke:web:chromium": "node scripts/run-engine-smokes.mjs chromium", "smoke:web:firefox": "node scripts/run-engine-smokes.mjs firefox", "smoke:launcher": "node scripts/smoke-launcher.mjs", "pack:verify": "node scripts/install-smoke-browser.mjs chromium && node scripts/pack-and-verify.mjs", diff --git a/scripts/run-engine-smokes.mjs b/scripts/run-engine-smokes.mjs index 29c041a7..71883b54 100644 --- a/scripts/run-engine-smokes.mjs +++ b/scripts/run-engine-smokes.mjs @@ -8,10 +8,13 @@ * from Node does, on every platform — the same reason `install-smoke-browser` * exists rather than an inline shell expansion. * - * It also gives the smoke list ONE home. It used to be an `&&` chain in - * package.json alongside a second chain in the workflow; adding a fourth smoke - * meant remembering both. Now `ENGINE_SMOKES` is the list, and both the - * on-demand command and the pre-push gate run through it. + * It also gives the smoke list ONE home. `ENGINE_SMOKES` is that list, and + * EVERY tier reads it — the default Chromium run inside `npm run smoke` + * (`smoke:web:chromium`), the Firefox pass in the pre-push gate + * (`smoke:web:firefox`), and the on-demand `smoke:web:engine`. Getting that + * wrong is silent: a tier that enumerated the smokes itself would simply run + * fewer of them and still pass, so `run-engine-smokes.test.mjs` asserts that no + * tier names an individual smoke. * * Usage: `node scripts/run-engine-smokes.mjs [engine]`. With no argument the * engine comes from `SMOKE_BROWSER` (default `chromium`). diff --git a/scripts/run-engine-smokes.test.mjs b/scripts/run-engine-smokes.test.mjs index ca8516c6..47197d17 100644 --- a/scripts/run-engine-smokes.test.mjs +++ b/scripts/run-engine-smokes.test.mjs @@ -12,12 +12,15 @@ */ import assert from "node:assert/strict"; -import { existsSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; import { describe, it } from "node:test"; import { ENGINE_SMOKES } from "./run-engine-smokes.mjs"; const scriptDir = import.meta.dirname; +const scripts = JSON.parse( + readFileSync(join(scriptDir, "..", "package.json"), "utf8"), +).scripts; describe("ENGINE_SMOKES", () => { it("names every browser-driven smoke, and each one exists", () => { @@ -43,3 +46,48 @@ describe("ENGINE_SMOKES", () => { assert.equal(ENGINE_SMOKES[0], "smoke-web-browser.mjs"); }); }); + +describe("every engine tier consumes ENGINE_SMOKES", () => { + // The point of the list is defeated if any tier enumerates the smokes itself. + // `npm run smoke` used to do exactly that (Copilot, #2133): a fourth entry in + // ENGINE_SMOKES would have reached the Firefox and on-demand runs and silently + // skipped the DEFAULT Chromium run — which is the one GitHub CI executes. That + // is invisible at runtime, because a tier running fewer smokes still passes. + + it("the Chromium tier goes through the runner, not its own list", () => { + assert.match(scripts["smoke:web:chromium"], /run-engine-smokes\.mjs/); + assert.match(scripts.smoke, /smoke:web:chromium/); + for (const individual of [ + "smoke:web:browser", + "smoke:web:app", + "smoke:web:elicit", + ]) { + assert.ok( + !scripts.smoke.includes(individual), + `\`smoke\` names ${individual} directly — it must route through ` + + "`smoke:web:chromium` so every tier reads one list", + ); + } + }); + + it("the Firefox tier goes through the runner and is in the pre-push gate", () => { + assert.match(scripts["smoke:web:firefox"], /run-engine-smokes\.mjs/); + assert.match(scripts.ci, /smoke:web:firefox/); + }); + + it("each gated tier names its engine explicitly rather than reading the env", () => { + // An ambient SMOKE_BROWSER must not be able to redirect a gate: without the + // literal engine, `SMOKE_BROWSER=firefox npm run ci` would run Firefox twice + // and never exercise Chromium at all. + assert.match( + scripts["smoke:web:chromium"], + /run-engine-smokes\.mjs chromium$/, + ); + assert.match( + scripts["smoke:web:firefox"], + /run-engine-smokes\.mjs firefox$/, + ); + // The on-demand entry point is the one that SHOULD follow the environment. + assert.match(scripts["smoke:web:engine"], /run-engine-smokes\.mjs$/); + }); +});