The job was trialled and did not earn its place. It was genuinely cheap — ~2 minutes, running in parallel with the 15-minute build job, so zero added wall-clock — but across a dozen runs Firefox never once disagreed with Chromium. It was paying runner time and carrying a real flake surface (playwright install --with-deps runs apt-get update, which fails whenever a third-party repo in the runner image breaks; that is what the one red run was) to re-confirm a result already in hand. Everything that made cross-engine checking possible stays: SMOKE_BROWSER, the shared headless-browser helper, the installer, and the tests. So "SMOKE_BROWSER=firefox npm run smoke:web:engine" is there for the moment it is worth running — before touching the sandbox, the CSP builder or the proxy page — without taxing every push to re-derive the same answer. The workflow is now byte-identical to v2/main again; the publish jobs go back to needing only build. Docs say what is true rather than what was intended, and AGENTS.md records the reasoning so the job is not re-added on the general argument that cross-engine coverage is good — that argument was already accepted, and the on-demand path is what serves it. Re-add it on evidence: a real cross-engine regression a gate would have caught. Also swept the now-stale "matrix" language out of the smokes and the resolver, including the empty-SMOKE_BROWSER rationale, which no longer leans on a matrix key that does not exist. Signed-off-by: cliffhall <cliff@futurescale.com>
237 lines
8.7 KiB
JavaScript
237 lines
8.7 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 to a caller who asked
|
|
* for another engine — coverage claimed that never ran.
|
|
*
|
|
* `loadBrowser`'s failure branches are covered here too, through its injectable
|
|
* `loadPlaywright`. The smokes cannot reach them by construction —
|
|
* `install-smoke-browser` fetches the binary before the smoke runs, so a passing
|
|
* smoke says nothing about what a failing one would print. And that message is
|
|
* the deliverable: #2086's acceptance criterion is that a missing browser fails
|
|
* naming *that engine* and its own `playwright install` command, which is
|
|
* exactly the kind of string that rots into naming the wrong one.
|
|
*
|
|
* Only the successful launch is left to the smokes, since it needs a real binary.
|
|
*/
|
|
|
|
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("rejects a present-but-empty value instead of defaulting", () => {
|
|
// This is the case worth being strict about. `SMOKE_BROWSER="$ENGINE"` with
|
|
// ENGINE unset — or a CI expression naming a key that does not exist — sets
|
|
// the variable to "" rather than removing it, so defaulting here would
|
|
// silently run Chromium for a caller who asked for something else. That is
|
|
// the same false-coverage outcome the unrecognized-value branch exists to
|
|
// stop. Only an ABSENT variable may select the default.
|
|
for (const raw of ["", " ", "\t\n"]) {
|
|
assert.throws(
|
|
() => resolveBrowserName({ [BROWSER_ENV_VAR]: raw }),
|
|
(err) => {
|
|
assert.match(err.message, /set but empty/);
|
|
// Names the actual remedy — "unset it", which is different from
|
|
// "set it to nothing" and is the whole point here.
|
|
assert.match(err.message, /did not resolve/);
|
|
assert.match(err.message, /Unset it/);
|
|
return true;
|
|
},
|
|
`expected ${JSON.stringify(raw)} to be rejected`,
|
|
);
|
|
}
|
|
});
|
|
|
|
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", () => {
|
|
/** A Playwright stand-in whose every engine rejects on launch. */
|
|
const launchAlwaysFails = (message) => () => ({
|
|
chromium: { launch: () => Promise.reject(new Error(message)) },
|
|
firefox: { launch: () => Promise.reject(new Error(message)) },
|
|
webkit: { launch: () => Promise.reject(new Error(message)) },
|
|
});
|
|
|
|
it("rejects an unsupported engine before touching Playwright", async () => {
|
|
let loaded = false;
|
|
await assert.rejects(
|
|
() =>
|
|
loadBrowser("/nonexistent-repo-root", "safari", {
|
|
loadPlaywright: () => {
|
|
loaded = true;
|
|
return {};
|
|
},
|
|
}),
|
|
/unsupported browser "safari"/,
|
|
);
|
|
// Pins the ORDER, not just the message: validating first is what makes the
|
|
// error name the real mistake instead of a downstream resolution failure.
|
|
assert.equal(loaded, false);
|
|
});
|
|
|
|
it("names the engine that failed, and its own install command", async () => {
|
|
// The whole point of #2086's acceptance criterion: a reader whose WebKit
|
|
// binary is missing must not be sent to install chromium.
|
|
for (const name of SUPPORTED_BROWSERS) {
|
|
await assert.rejects(
|
|
() =>
|
|
loadBrowser("/repo", name, {
|
|
loadPlaywright: launchAlwaysFails("Executable doesn't exist"),
|
|
}),
|
|
(err) => {
|
|
assert.match(err.message, new RegExp(`^${name} failed to launch`));
|
|
assert.match(
|
|
err.message,
|
|
new RegExp(
|
|
`npx playwright install --with-deps ${name}\\\`(?![\\s\\S]*--with-deps (?!${name}))`,
|
|
),
|
|
);
|
|
// The underlying cause survives, so the reader can tell a missing
|
|
// binary from a sandbox/permissions problem.
|
|
assert.match(err.message, /Executable doesn't exist/);
|
|
// And no OTHER engine is named anywhere in the message.
|
|
for (const other of SUPPORTED_BROWSERS.filter((b) => b !== name)) {
|
|
assert.ok(
|
|
!err.message.includes(other),
|
|
`message for ${name} must not mention ${other}: ${err.message}`,
|
|
);
|
|
}
|
|
return true;
|
|
},
|
|
);
|
|
}
|
|
});
|
|
|
|
it("sends an unresolvable Playwright to `npm install`, not `playwright install`", async () => {
|
|
// Two different failures with two different remedies. Suggesting
|
|
// `playwright install` here would be actively wrong — it fetches browser
|
|
// binaries and cannot install the missing npm package.
|
|
await assert.rejects(
|
|
() =>
|
|
loadBrowser("/repo", "firefox", {
|
|
loadPlaywright: () => {
|
|
throw new Error("Cannot find module 'playwright'");
|
|
},
|
|
}),
|
|
(err) => {
|
|
assert.match(err.message, /could not resolve the Playwright package/);
|
|
assert.match(err.message, /npm install/);
|
|
assert.ok(
|
|
!/playwright install/.test(err.message),
|
|
`must not suggest \`playwright install\` for a missing package: ${err.message}`,
|
|
);
|
|
assert.match(err.message, /Cannot find module 'playwright'/);
|
|
return true;
|
|
},
|
|
);
|
|
});
|
|
});
|
|
|
|
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(), []);
|
|
});
|
|
});
|