Every browser-driven check in this repo launched Chromium and nothing else.
That is fine for most of the web client, whose behavior is React and Mantine.
It is not fine for the MCP Apps sandbox, which is built out of the primitives
that genuinely diverge between engines: a CSP <meta> injected as the first
<head> child of a srcdoc document, a nested sandboxed iframe, a
Permissions-Policy allow attribute, and postMessage origin discipline across
those two frames. Nothing else covers that surface — sandbox-csp.test.ts
asserts which policy STRING is built (environment-independent by
construction), and no Storybook story reaches the sandbox at all, since all
three App stories use a data: placeholder iframe and a mock bridge.
The engine is now a parameter:
SMOKE_BROWSER=webkit npm run smoke:web:app
SMOKE_BROWSER=firefox npm run smoke:web:engine # all three smokes
Unset means chromium, so `npm run ci` and `npm run smoke` are unchanged. An
unrecognized value is an error rather than a fallback — falling back would
report a green Chromium run under a job labelled "webkit", claiming coverage
that never ran — and a launch failure names the engine that failed and its own
`playwright install` invocation.
Selection, launching, and the fatal-vs-benign page-diagnostics split now live
in one shared scripts/lib/headless-browser.mjs. The three smokes had each
hand-rolled the diagnostics split and two had their own copy of loadChromium;
pack:verify goes through the same helper, pinned to Chromium explicitly since
it is a packaging check.
The npm scripts drop their `cd clients/web && npx playwright install chromium`
prefix for scripts/install-smoke-browser.mjs. The engine is a variable now, and
a ${VAR:-default} expansion does not expand under Windows' cmd.exe; npx is also
a .cmd shim there that a shell-free spawn cannot start, so this resolves the
Playwright CLI through resolveNodeBin — the #1939 problem solved the #1939 way.
CI gains a `Sandbox smokes (<engine>)` matrix job rather than matrixing `build`,
which also runs validate, the coverage gate, two verify gates and Storybook —
all engine-independent. Both publish jobs now need it.
WebKit is supported by the tooling but deliberately NOT in the CI matrix yet:
pointing the smokes at it immediately found #2132, a pre-existing Safari
incompatibility in the web client's SSE transport that strands the last message
on /api/mcp/events, so an MCP App never leaves "loading". That is a product bug
affecting real Safari users, with its own diagnosis and a verified fix, and it
is not a matrix change. Adding `webkit` here is a one-word diff once it lands.
An engine is either green in CI or absent from it; a continue-on-error job would
report coverage nobody is held to.
Signed-off-by: cliffhall <cliff@futurescale.com>
148 lines
5 KiB
JavaScript
148 lines
5 KiB
JavaScript
/**
|
|
* Unit tests for the headless-browser engine selection (#2086).
|
|
*
|
|
* The smokes cannot check this themselves: each one resolves exactly one engine
|
|
* per process and then spends its whole run inside the happy path, so the branch
|
|
* that matters most — an unrecognized `SMOKE_BROWSER` — is dead code from their
|
|
* point of view. It is also the branch whose failure is silent rather than loud:
|
|
* a fallback to Chromium there would report a green run under a job labelled
|
|
* "webkit", claiming coverage that never ran.
|
|
*
|
|
* `loadBrowser`'s own launch path is deliberately not covered here — it takes a
|
|
* real browser binary, which is what the smokes are for. Only its argument
|
|
* validation, which fails before any of that, is.
|
|
*/
|
|
|
|
import assert from "node:assert/strict";
|
|
import { describe, it } from "node:test";
|
|
import {
|
|
BROWSER_ENV_VAR,
|
|
DEFAULT_BROWSER,
|
|
SUPPORTED_BROWSERS,
|
|
attachPageDiagnostics,
|
|
loadBrowser,
|
|
resolveBrowserName,
|
|
} from "./headless-browser.mjs";
|
|
|
|
describe("resolveBrowserName", () => {
|
|
it("defaults to chromium when the variable is unset", () => {
|
|
assert.equal(resolveBrowserName({}), DEFAULT_BROWSER);
|
|
});
|
|
|
|
it("treats an empty or whitespace-only value as unset", () => {
|
|
// A CI expression that resolves to nothing (`SMOKE_BROWSER: ${{ … }}` with
|
|
// an undefined matrix key) sets the variable to "" rather than removing it.
|
|
for (const raw of ["", " "]) {
|
|
assert.equal(
|
|
resolveBrowserName({ [BROWSER_ENV_VAR]: raw }),
|
|
DEFAULT_BROWSER,
|
|
);
|
|
}
|
|
});
|
|
|
|
it("accepts every supported engine", () => {
|
|
for (const name of SUPPORTED_BROWSERS) {
|
|
assert.equal(resolveBrowserName({ [BROWSER_ENV_VAR]: name }), name);
|
|
}
|
|
});
|
|
|
|
it("normalizes surrounding whitespace and case", () => {
|
|
assert.equal(
|
|
resolveBrowserName({ [BROWSER_ENV_VAR]: " WebKit " }),
|
|
"webkit",
|
|
);
|
|
});
|
|
|
|
it("rejects an unrecognized engine rather than falling back", () => {
|
|
assert.throws(
|
|
() => resolveBrowserName({ [BROWSER_ENV_VAR]: "safari" }),
|
|
// The message must quote what was asked for AND list what is accepted —
|
|
// "safari" is the most likely typo here and its fix is not guessable.
|
|
(err) =>
|
|
/safari/.test(err.message) &&
|
|
SUPPORTED_BROWSERS.every((name) => err.message.includes(name)),
|
|
);
|
|
});
|
|
});
|
|
|
|
describe("loadBrowser", () => {
|
|
it("rejects an unsupported engine before touching Playwright", async () => {
|
|
await assert.rejects(
|
|
// A repo root that does not exist: reaching the `createRequire` would
|
|
// throw a different (resolution) error, so this also pins the ORDER —
|
|
// validation first, so the message names the real mistake.
|
|
() => loadBrowser("/nonexistent-repo-root", "safari"),
|
|
/unsupported browser "safari"/,
|
|
);
|
|
});
|
|
});
|
|
|
|
describe("attachPageDiagnostics", () => {
|
|
/** Minimal Playwright `page` stand-in: records handlers, replays events. */
|
|
function fakePage() {
|
|
const handlers = {};
|
|
return {
|
|
on: (event, fn) => {
|
|
(handlers[event] ??= []).push(fn);
|
|
},
|
|
emit: (event, arg) => handlers[event]?.forEach((fn) => fn(arg)),
|
|
};
|
|
}
|
|
|
|
const consoleMessage = (type, text) => ({
|
|
type: () => type,
|
|
text: () => text,
|
|
});
|
|
|
|
it("splits fatal console errors from benign noise", () => {
|
|
const page = fakePage();
|
|
const diagnostics = attachPageDiagnostics(page);
|
|
|
|
page.emit("console", consoleMessage("error", "Uncaught (in promise) boom"));
|
|
page.emit(
|
|
"console",
|
|
consoleMessage("error", "Failed to fetch dynamically imported module x"),
|
|
);
|
|
// The two shapes that used to flake CI, and must stay diagnostics.
|
|
page.emit(
|
|
"console",
|
|
consoleMessage("error", "Failed to load resource: net::ERR_FAILED"),
|
|
);
|
|
page.emit(
|
|
"console",
|
|
consoleMessage("error", "Warning: each child needs a key"),
|
|
);
|
|
// Non-error console output is ignored entirely.
|
|
page.emit("console", consoleMessage("log", "Uncaught looking but a log"));
|
|
|
|
assert.deepEqual(diagnostics.fatalConsole(), [
|
|
"Uncaught (in promise) boom",
|
|
"Failed to fetch dynamically imported module x",
|
|
]);
|
|
assert.deepEqual(diagnostics.benignConsole(), [
|
|
"Failed to load resource: net::ERR_FAILED",
|
|
"Warning: each child needs a key",
|
|
]);
|
|
});
|
|
|
|
it("counts every pageerror as fatal, whatever it was thrown as", () => {
|
|
const page = fakePage();
|
|
const diagnostics = attachPageDiagnostics(page);
|
|
|
|
page.emit("pageerror", new Error("sync boom"));
|
|
page.emit("pageerror", "thrown as a string");
|
|
page.emit("console", consoleMessage("error", "benign"));
|
|
|
|
assert.deepEqual(diagnostics.pageErrors, [
|
|
"sync boom",
|
|
"thrown as a string",
|
|
]);
|
|
assert.deepEqual(diagnostics.fatal(), ["sync boom", "thrown as a string"]);
|
|
});
|
|
|
|
it("reports nothing on a clean page", () => {
|
|
const diagnostics = attachPageDiagnostics(fakePage());
|
|
assert.deepEqual(diagnostics.fatal(), []);
|
|
assert.deepEqual(diagnostics.benignConsole(), []);
|
|
});
|
|
});
|