fix(smoke): route the Chromium tier through ENGINE_SMOKES too

I claimed ENGINE_SMOKES was the single home for "which smokes are
engine-sensitive". It was not: npm run smoke still hard-coded the same
three, so a fourth entry would have reached the Firefox and on-demand runs
and silently skipped the DEFAULT Chromium run -- which is the one GitHub
CI executes. The exact drift the list was introduced to remove, left in
the one tier that matters most.

npm run smoke now ends in smoke:web:chromium, which goes through the
runner like the other two tiers.

Two things fall out of that:

- The Chromium tier passes its engine as a literal argument, so an
  ambient SMOKE_BROWSER can no longer redirect it. Without that,
  SMOKE_BROWSER=firefox npm run ci would have run Firefox twice and never
  exercised Chromium at all -- a gate silently testing something other
  than what it claims.

- run-engine-smokes.test.mjs now asserts that no tier names an individual
  smoke, that both gated tiers name their engine explicitly, and that the
  on-demand entry point is the one that follows the environment. This is
  the durable half: a tier running fewer smokes is invisible at runtime,
  because it still passes. Mutation-verified -- restoring the old hard-
  coded chain fails the test.

Docs updated to state the wider claim rather than the one that was true
of only two tiers.

Addresses the suppressed comment on Copilot review 5026921914 of #2133.

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-26 08:55:51 -04:00
commit 1cebe71761
4 changed files with 59 additions and 7 deletions

View file

@ -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.

View file

@ -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",

View file

@ -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`).

View file

@ -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$/);
});
});