chore: state the smoke-helper and CI wall-clock budgets (#2333)
The five rows #2323 left as "assess", each either stated with its reasoning and measurement, or shared with a constant that already has one. - F4/F5: `driveAppFlow` and `connectViaDeepLink` default to `BROWSER_TIMEOUTS.ui` / `.roundTrip` instead of restating 30_000 / 45_000 by hand; values unchanged, now one place to raise. A test in each helper pins the defaults. - F1: `announced-child.mjs` states `DEFAULTS` ({ timeoutMs: 30_000, pollMs: 250 }) with the measurement — boot-to-announce of the composable test server is 135ms median at load average 15, and a passing start pays one poll interval, not the budget. - F7: `pty.mjs` names `SCRIPT_PROBE_TIMEOUT_MS` (5s; the probe spawns in 1.3ms median, 3.6ms max under load) and stops folding ETIMEDOUT into "no script(1)": a starved probe now fails `smoke:tui` loudly instead of turning it into an exit-0 skip. - G1: every job in `.github/workflows/main.yml` declares `timeout-minutes` as a hung-job guard — roughly twice the slowest run observed over the last 40 push runs (and the last three releases for the publish jobs), rounded up to the next five minutes, with the range stated per job. AGENTS.md's timeout section now points at where these live instead of at this issue. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsmUAZ6aLUAFqenjJKSSRp Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall
parent
4a522ebc8c
commit
3f7ba20ae6
11 changed files with 229 additions and 28 deletions
23
.github/workflows/main.yml
vendored
23
.github/workflows/main.yml
vendored
|
|
@ -17,9 +17,22 @@ on:
|
|||
permissions:
|
||||
contents: read
|
||||
|
||||
# Every job below declares `timeout-minutes` (#2333). It is a HUNG-JOB GUARD,
|
||||
# not a flake remedy: GitHub runners are not the contended machine the local
|
||||
# gate runs on, and nothing measured implicates them — so no value here is
|
||||
# sized for load. The rule, for every job in this file, is roughly TWICE the
|
||||
# slowest run observed, rounded up to the next five minutes: real headroom for
|
||||
# a slow runner, and a hang cut off in minutes instead of at GitHub's
|
||||
# 360-minute default. Each job states the range it was read from; when one
|
||||
# legitimately outgrows its number, raise it here with the new range.
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
# Observed 7.6–11.8 min across the 40 push runs before this landed
|
||||
# (2026-09-11 → 09-13), all since the coverage split (#2159). A release run
|
||||
# before that split reached 15.6 min, with the coverage gate still inside
|
||||
# this job — that shape no longer exists.
|
||||
timeout-minutes: 25
|
||||
|
||||
steps:
|
||||
- name: Checkout code
|
||||
|
|
@ -150,6 +163,8 @@ jobs:
|
|||
# time tests out at the 5s default. Separate jobs get separate runners.
|
||||
coverage:
|
||||
runs-on: ubuntu-latest
|
||||
# Observed 5.9–8.5 min across the same 40 push runs as `build`.
|
||||
timeout-minutes: 20
|
||||
|
||||
steps:
|
||||
- name: Checkout code
|
||||
|
|
@ -195,6 +210,11 @@ jobs:
|
|||
concurrency:
|
||||
group: publish-npm
|
||||
cancel-in-progress: false
|
||||
# Observed 2.3–2.8 min across the last three releases (2.4.0–2.6.0), of
|
||||
# which `pack:verify` is about half and the only step that reaches the
|
||||
# registry. The publish itself is atomic on npm's side, so a cut-off here
|
||||
# leaves nothing half-published.
|
||||
timeout-minutes: 10
|
||||
permissions:
|
||||
contents: read
|
||||
# Required for npm provenance (`--provenance` mints a signed attestation
|
||||
|
|
@ -283,6 +303,9 @@ jobs:
|
|||
if: github.event_name == 'release'
|
||||
environment: release
|
||||
needs: [build, coverage]
|
||||
# Observed 14.5–15.4 min across the last three releases (2.4.0–2.6.0);
|
||||
# the linux/arm64 half of the build runs under QEMU and is most of it.
|
||||
timeout-minutes: 35
|
||||
permissions:
|
||||
contents: read
|
||||
packages: write
|
||||
|
|
|
|||
15
AGENTS.md
15
AGENTS.md
|
|
@ -472,8 +472,19 @@ free to run a full `local:gate`). Raising a budget nobody chose hides no race.
|
|||
per-project cap without re-running that measurement.
|
||||
- **The Playwright locator budgets the web smokes use are named constants in
|
||||
`scripts/lib/browser-timeouts.mjs`** — raise one there, not at a call site.
|
||||
The *remaining* smoke-helper budgets and CI's missing `timeout-minutes` are
|
||||
tracked on #2333 rather than settled here.
|
||||
The shared flow helpers (`deep-link-connect.mjs`, `mcp-app-flow.mjs`)
|
||||
default to the same constants, so a budget that moves there moves for every
|
||||
smoke at once. The process-level budgets are stated the same way in their
|
||||
own modules — `DEFAULTS` in `announced-child.mjs` and `render-smoke.mjs`,
|
||||
`SCRIPT_PROBE_TIMEOUT_MS` in `pty.mjs` — each with the measurement that
|
||||
sized it (#2333).
|
||||
- **Every CI job declares `timeout-minutes`, sized from observed runs.** It is
|
||||
a hung-job guard, not a flake remedy — GitHub runners are not the contended
|
||||
machine — so the rule is roughly twice the slowest run observed, rounded up
|
||||
to the next five minutes, stated per job in `.github/workflows/main.yml`
|
||||
with the range it was read from. A job that starts taking longer gets its
|
||||
number raised there, with the new range; the 360-minute default it replaces
|
||||
is how a hang blocks a runner for six hours.
|
||||
- **Do not scale a fixed sleep.** A `setTimeout(r, N)` with no condition is not
|
||||
a timeout: it always waits the full window, so raising it slows every passing
|
||||
run and still races on a loaded one. Replace one with a condition wait when it
|
||||
|
|
|
|||
|
|
@ -23,6 +23,34 @@
|
|||
import { spawn } from "node:child_process";
|
||||
import { setTimeout as delay } from "node:timers/promises";
|
||||
|
||||
/**
|
||||
* The readiness budget and poll interval, stated once (#2333) — the same shape
|
||||
* as `render-smoke.mjs`'s `DEFAULTS`, and for the same reason: a budget that
|
||||
* lives in a parameter default is one nobody revisits.
|
||||
*
|
||||
* `timeoutMs` is a ceiling on a poll, not a sleep: the wait returns on the
|
||||
* first poll that sees the announcement, so a passing start pays only the
|
||||
* child's real boot time plus, at most, one `pollMs`. Every smoke that needs a
|
||||
* test server goes through this, which makes it the widest of the smoke
|
||||
* budgets — and the one with the most headroom. Measured on the composable
|
||||
* test server (`server-composable.js --config mcp-app-http`) through this
|
||||
* helper with a 25ms poll, ten starts on a machine at load average 15: boot to
|
||||
* announce was 135ms median, 137ms max. 30s is more than two hundred times
|
||||
* that; what it has to absorb is not a slow server but a machine several
|
||||
* gates deep, where a Node process can take seconds to reach its first line.
|
||||
*
|
||||
* `pollMs` is the passing path's cost. The same measurement at the default
|
||||
* interval landed at 251–254ms every run, which is one interval exactly: the
|
||||
* child announces well inside the first poll, so a start pays ~250ms of which
|
||||
* ~115ms is waiting to look. That is under a second across a whole gate, so
|
||||
* it is stated rather than tuned — lowering it buys nothing anyone would
|
||||
* notice and costs a tighter scan loop on the child's output.
|
||||
*/
|
||||
export const DEFAULTS = Object.freeze({
|
||||
timeoutMs: 30_000,
|
||||
pollMs: 250,
|
||||
});
|
||||
|
||||
/**
|
||||
* @param {object} opts
|
||||
* @param {string} opts.command Executable to spawn.
|
||||
|
|
@ -34,8 +62,8 @@ import { setTimeout as delay } from "node:timers/promises";
|
|||
* Called synchronously with the child immediately after spawn, before any
|
||||
* waiting. This is the caller's teardown handle.
|
||||
* @param {string} opts.what Noun used in error messages ("MCP test server").
|
||||
* @param {number} [opts.timeoutMs] Readiness budget (default 30s).
|
||||
* @param {number} [opts.pollMs] Poll interval (default 250ms).
|
||||
* @param {number} [opts.timeoutMs] Readiness budget (default `DEFAULTS.timeoutMs`).
|
||||
* @param {number} [opts.pollMs] Poll interval (default `DEFAULTS.pollMs`).
|
||||
* @returns {Promise<{ child: import("node:child_process").ChildProcess, match: RegExpMatchArray }>}
|
||||
*/
|
||||
export async function startAnnouncedChild({
|
||||
|
|
@ -45,8 +73,8 @@ export async function startAnnouncedChild({
|
|||
pattern,
|
||||
onSpawn,
|
||||
what,
|
||||
timeoutMs = 30_000,
|
||||
pollMs = 250,
|
||||
timeoutMs = DEFAULTS.timeoutMs,
|
||||
pollMs = DEFAULTS.pollMs,
|
||||
}) {
|
||||
const child = spawn(command, args, {
|
||||
cwd,
|
||||
|
|
|
|||
|
|
@ -8,7 +8,9 @@
|
|||
* same three or four decisions repeated, so they are named once here and the
|
||||
* scripts import them — the same shape `render-smoke.mjs`'s `DEFAULTS` already
|
||||
* has, and for the same reason: a budget nobody can find is a budget nobody
|
||||
* revisits.
|
||||
* revisits. The shared flow helpers (`deep-link-connect.mjs`,
|
||||
* `mcp-app-flow.mjs`) default their budgets to these too (#2333); they used to
|
||||
* carry `ui`/`roundTrip` as literals of their own.
|
||||
*
|
||||
* Three of the four are ceilings on a poll rather than sleeps — `ui`,
|
||||
* `roundTrip` and `nested` each return the instant their locator condition
|
||||
|
|
|
|||
|
|
@ -25,8 +25,17 @@
|
|||
* Takes only the slice of Playwright's `page` it uses (`goto`, `locator` →
|
||||
* `waitFor`/`getAttribute`), which is what lets its failure branches be
|
||||
* unit-tested against a stand-in — a smoke only ever drives its happy path.
|
||||
*
|
||||
* The two budgets default to `BROWSER_TIMEOUTS` rather than restating them
|
||||
* (#2333): `goto` and the status element attaching are the page loading and
|
||||
* painting (`ui`), and `data-status="connected"` only appears after a round
|
||||
* trip to the MCP server (`roundTrip`). `driveAppFlow` in `mcp-app-flow.mjs`
|
||||
* passes the same two through, so the two helpers cannot drift apart — which
|
||||
* is the point: they used to carry the same literals by hand.
|
||||
*/
|
||||
|
||||
import { BROWSER_TIMEOUTS } from "./browser-timeouts.mjs";
|
||||
|
||||
/**
|
||||
* The deep link that connects to `mcpUrl`.
|
||||
*
|
||||
|
|
@ -57,8 +66,8 @@ export async function connectViaDeepLink({
|
|||
page,
|
||||
url,
|
||||
expectDeepLink = true,
|
||||
gotoTimeoutMs = 30_000,
|
||||
connectTimeoutMs = 45_000,
|
||||
gotoTimeoutMs = BROWSER_TIMEOUTS.ui,
|
||||
connectTimeoutMs = BROWSER_TIMEOUTS.roundTrip,
|
||||
}) {
|
||||
const response = await page.goto(url, {
|
||||
waitUntil: "domcontentloaded",
|
||||
|
|
|
|||
|
|
@ -17,24 +17,35 @@
|
|||
|
||||
import assert from "node:assert/strict";
|
||||
import { describe, it } from "node:test";
|
||||
import { BROWSER_TIMEOUTS } from "./browser-timeouts.mjs";
|
||||
import {
|
||||
buildConnectDeepLink,
|
||||
connectViaDeepLink,
|
||||
} from "./deep-link-connect.mjs";
|
||||
|
||||
/** Minimal Playwright `page` stand-in. `plan` maps a selector to its behavior. */
|
||||
/**
|
||||
* Minimal Playwright `page` stand-in. `plan` maps a selector to its behavior.
|
||||
* `gotoTimeouts` and `waits` record the budget each call was given, so a test
|
||||
* can pin the defaults without a real browser.
|
||||
*/
|
||||
function fakePage(plan, { gotoStatus = 200 } = {}) {
|
||||
const gotos = [];
|
||||
const gotoTimeouts = [];
|
||||
const waits = {};
|
||||
return {
|
||||
gotos,
|
||||
goto: async (url) => {
|
||||
gotoTimeouts,
|
||||
waits,
|
||||
goto: async (url, opts) => {
|
||||
gotos.push(url);
|
||||
gotoTimeouts.push(opts?.timeout);
|
||||
return { ok: () => gotoStatus === 200, status: () => gotoStatus };
|
||||
},
|
||||
locator: (selector) => {
|
||||
const entry = plan[selector] ?? {};
|
||||
return {
|
||||
waitFor: async () => {
|
||||
waitFor: async (opts) => {
|
||||
waits[selector] = opts?.timeout;
|
||||
if (entry.waitFails) throw new Error(`timeout: ${selector}`);
|
||||
},
|
||||
getAttribute: async (name) => entry.attrs?.[name] ?? null,
|
||||
|
|
@ -96,6 +107,18 @@ describe("connectViaDeepLink", () => {
|
|||
assert.deepEqual(page.gotos, ["http://x/"]);
|
||||
});
|
||||
|
||||
it("defaults each budget to the shared BROWSER_TIMEOUTS entry (#2333)", async () => {
|
||||
// These used to be 30_000 / 45_000 literals of the helper's own — the same
|
||||
// values `browser-timeouts.mjs` names `ui` and `roundTrip`, kept in step by
|
||||
// hand. Pinning the defaults to the constants is what makes a raise there
|
||||
// reach this helper and `driveAppFlow` at once.
|
||||
const page = fakePage(happyPlan());
|
||||
await connectViaDeepLink({ page, url: "http://x/" });
|
||||
assert.deepEqual(page.gotoTimeouts, [BROWSER_TIMEOUTS.ui]);
|
||||
assert.equal(page.waits[STATUS], BROWSER_TIMEOUTS.ui);
|
||||
assert.equal(page.waits[CONNECTED], BROWSER_TIMEOUTS.roundTrip);
|
||||
});
|
||||
|
||||
it("reports a non-200 document instead of waiting on selectors", async () => {
|
||||
await assert.rejects(
|
||||
connectViaDeepLink({
|
||||
|
|
|
|||
|
|
@ -30,6 +30,7 @@
|
|||
|
||||
import { join } from "node:path";
|
||||
import { startAnnouncedChild } from "./announced-child.mjs";
|
||||
import { BROWSER_TIMEOUTS } from "./browser-timeouts.mjs";
|
||||
import {
|
||||
buildConnectDeepLink,
|
||||
connectViaDeepLink,
|
||||
|
|
@ -167,6 +168,13 @@ export function buildAppDeepLink({
|
|||
* Takes only the small slice of Playwright's `page` it uses (`goto`, `locator`
|
||||
* → `waitFor`/`getAttribute`/`count`), which is what lets its failure paths be
|
||||
* unit-tested against a stand-in — a smoke only ever exercises its happy path.
|
||||
*
|
||||
* The three budgets are `BROWSER_TIMEOUTS` entries, not restated literals
|
||||
* (#2333). The first two are handed straight to `connectViaDeepLink`, so the
|
||||
* two helpers share one pair by construction. `ready` is a `roundTrip` too:
|
||||
* the widget has to load inside the sandbox and answer through the bridge
|
||||
* before the attribute can flip, which is the same shape as the connect wait,
|
||||
* not a render of something already on screen (`nested`).
|
||||
*/
|
||||
export async function driveAppFlow({
|
||||
page,
|
||||
|
|
@ -174,9 +182,9 @@ export async function driveAppFlow({
|
|||
expectDeepLink = true,
|
||||
what = "app",
|
||||
extraDiagnostics = null,
|
||||
gotoTimeoutMs = 30_000,
|
||||
connectTimeoutMs = 45_000,
|
||||
readyTimeoutMs = 45_000,
|
||||
gotoTimeoutMs = BROWSER_TIMEOUTS.ui,
|
||||
connectTimeoutMs = BROWSER_TIMEOUTS.roundTrip,
|
||||
readyTimeoutMs = BROWSER_TIMEOUTS.roundTrip,
|
||||
}) {
|
||||
// 1-2. Navigated, the deep link accepted by the token gate, and connected.
|
||||
await connectViaDeepLink({
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@
|
|||
|
||||
import assert from "node:assert/strict";
|
||||
import { describe, it } from "node:test";
|
||||
import { BROWSER_TIMEOUTS } from "./browser-timeouts.mjs";
|
||||
import {
|
||||
buildAppDeepLink,
|
||||
driveAppFlow,
|
||||
|
|
@ -23,9 +24,15 @@ import {
|
|||
sandboxProxyPageFor,
|
||||
} from "./mcp-app-flow.mjs";
|
||||
|
||||
/** Minimal Playwright `page` stand-in. `plan` maps a selector to its behavior. */
|
||||
/**
|
||||
* Minimal Playwright `page` stand-in. `plan` maps a selector to its behavior.
|
||||
* `waits` records the budget each `waitFor` was given, so a test can pin the
|
||||
* defaults without a real browser.
|
||||
*/
|
||||
function fakePage(plan, { gotoStatus = 200 } = {}) {
|
||||
const waits = {};
|
||||
return {
|
||||
waits,
|
||||
goto: async () => ({
|
||||
ok: () => gotoStatus === 200,
|
||||
status: () => gotoStatus,
|
||||
|
|
@ -33,7 +40,8 @@ function fakePage(plan, { gotoStatus = 200 } = {}) {
|
|||
locator: (selector) => {
|
||||
const entry = plan[selector] ?? {};
|
||||
return {
|
||||
waitFor: async () => {
|
||||
waitFor: async (opts) => {
|
||||
waits[selector] = opts?.timeout;
|
||||
if (entry.waitFails) throw new Error(`timeout: ${selector}`);
|
||||
},
|
||||
getAttribute: async (name) => entry.attrs?.[name] ?? null,
|
||||
|
|
@ -110,6 +118,17 @@ describe("driveAppFlow", () => {
|
|||
await driveAppFlow({ page: fakePage(happyPlan()), url: "http://x/" });
|
||||
});
|
||||
|
||||
it("shares its connect budgets with connectViaDeepLink and waits a roundTrip for ready (#2333)", async () => {
|
||||
// The connect pair is handed straight through, so `ui`/`roundTrip` here
|
||||
// are the same two `connectViaDeepLink` defaults to; `ready` is a round
|
||||
// trip of its own (sandbox load + bridge handshake), not a `nested` render.
|
||||
const page = fakePage(happyPlan());
|
||||
await driveAppFlow({ page, url: "http://x/" });
|
||||
assert.equal(page.waits[STATUS], BROWSER_TIMEOUTS.ui);
|
||||
assert.equal(page.waits[CONNECTED], BROWSER_TIMEOUTS.roundTrip);
|
||||
assert.equal(page.waits[READY], BROWSER_TIMEOUTS.roundTrip);
|
||||
});
|
||||
|
||||
it("reports a non-200 document instead of waiting on selectors", async () => {
|
||||
await assert.rejects(
|
||||
driveAppFlow({
|
||||
|
|
|
|||
|
|
@ -108,6 +108,26 @@ export function ptyCommand({ command, args = [], flavor }) {
|
|||
throw new Error(`unknown script(1) flavor: ${flavor}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* How long `script --version` gets to exit (#2333).
|
||||
*
|
||||
* A ceiling on a synchronous spawn, not a sleep: the probe returns the moment
|
||||
* the process exits, and `script --version` exits at once on every flavor —
|
||||
* BSD `script` with a usage error, util-linux and busybox with a version line.
|
||||
* Measured here on darwin, twenty spawns with the machine at load average 4
|
||||
* (recent peak 19): 1.3ms median, 3.6ms max. Five seconds is over a thousand
|
||||
* times that, and what it has to absorb is process creation on a machine
|
||||
* several gates deep, not the command.
|
||||
*
|
||||
* The number matters less than what its expiry means. `spawnSync` reports a
|
||||
* timeout in the same `error` slot as ENOENT, and `probeScriptVersion` used to
|
||||
* read both as "no `script(1)`" — so a starved probe turned `smoke:tui` into a
|
||||
* documented **skip**, exit 0, with a message blaming the machine's PATH. A
|
||||
* false skip on the one pre-push gate is worse than a false failure, which is
|
||||
* why the timeout is now told apart and thrown instead (see below).
|
||||
*/
|
||||
export const SCRIPT_PROBE_TIMEOUT_MS = 5_000;
|
||||
|
||||
/**
|
||||
* Probe the local `script(1)`: is it there, and what does it say about itself?
|
||||
*
|
||||
|
|
@ -124,23 +144,42 @@ export function ptyCommand({ command, args = [], flavor }) {
|
|||
* answers `illegal option -- -` plus its usage on stderr — which is exactly the
|
||||
* evidence `scriptFlavorFor` reads. Only `error` means "could not run it".
|
||||
*
|
||||
* @param {(cmd: string, args: string[]) => { stdout?: string, stderr?: string, error?: Error }} [runner]
|
||||
* A **timeout** is not unavailability either, and it is the one `error` that
|
||||
* must not be folded into "not available": `script(1)` was found and started,
|
||||
* and the machine did not let it finish. Reporting that as "no `script(1)` on
|
||||
* PATH" would make `smoke:tui` skip — a green exit on a run that tested
|
||||
* nothing — so it throws, naming the budget, and the caller fails loudly.
|
||||
*
|
||||
* @param {(cmd: string, args: string[]) => { stdout?: string, stderr?: string, error?: Error & { code?: string } }} [runner]
|
||||
* Injected for tests; defaults to a real `spawnSync`.
|
||||
* @returns {{ available: boolean, output: string }}
|
||||
* @throws {Error} when the probe timed out (`error.code === "ETIMEDOUT"`).
|
||||
*/
|
||||
export function probeScriptVersion(
|
||||
runner = (cmd, args) =>
|
||||
spawnSync(cmd, args, { encoding: "utf8", timeout: 5000 }),
|
||||
spawnSync(cmd, args, {
|
||||
encoding: "utf8",
|
||||
timeout: SCRIPT_PROBE_TIMEOUT_MS,
|
||||
}),
|
||||
) {
|
||||
let r;
|
||||
try {
|
||||
const r = runner("script", ["--version"]);
|
||||
// `error` covers ENOENT, EACCES and the timeout; a missing result at all
|
||||
// means the runner told us nothing, which is not evidence of a working one.
|
||||
if (!r || r.error) return { available: false, output: "" };
|
||||
return { available: true, output: `${r.stdout ?? ""}${r.stderr ?? ""}` };
|
||||
r = runner("script", ["--version"]);
|
||||
} catch {
|
||||
// A runner that throws outright (injected, or a future spawn shape).
|
||||
return { available: false, output: "" };
|
||||
}
|
||||
if (r?.error?.code === "ETIMEDOUT") {
|
||||
throw new Error(
|
||||
`\`script --version\` did not exit within ${SCRIPT_PROBE_TIMEOUT_MS}ms — ` +
|
||||
"a starved or hung probe, not a missing `script(1)`, so smoke:tui " +
|
||||
"must not skip on it",
|
||||
);
|
||||
}
|
||||
// `error` covers ENOENT and EACCES; a missing result at all means the runner
|
||||
// told us nothing, which is not evidence of a working one.
|
||||
if (!r || r.error) return { available: false, output: "" };
|
||||
return { available: true, output: `${r.stdout ?? ""}${r.stderr ?? ""}` };
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -151,11 +190,16 @@ export function probeScriptVersion(
|
|||
* invocation" send the reader somewhere different — so the failure carries a
|
||||
* `reason` rather than being a bare `null` the caller has to narrate.
|
||||
*
|
||||
* A probe that **timed out** is neither — it throws through here untouched,
|
||||
* because "the machine would not run `script`" is a failure to report, not a
|
||||
* reason to skip (see `probeScriptVersion`).
|
||||
*
|
||||
* @param {object} [opts]
|
||||
* @param {string} [opts.platform]
|
||||
* @param {() => { available: boolean, output: string }} [opts.probe]
|
||||
* @returns {{ ok: true, flavor: string, wrap: (spec: { command: string, args?: string[] }) => { command: string, args: string[] } }
|
||||
* | { ok: false, reason: string }}
|
||||
* @throws {Error} when the probe timed out.
|
||||
*/
|
||||
export function resolvePtyWrapper({
|
||||
platform = process.platform,
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import test from "node:test";
|
|||
import {
|
||||
ptyCommand,
|
||||
probeScriptVersion,
|
||||
SCRIPT_PROBE_TIMEOUT_MS,
|
||||
resolvePtyWrapper,
|
||||
scriptFlavorFor,
|
||||
shellQuote,
|
||||
|
|
@ -120,7 +121,7 @@ test("shellQuote survives an embedded single quote", () => {
|
|||
assert.equal(shellQuote("plain"), "'plain'");
|
||||
});
|
||||
|
||||
test("probeScriptVersion combines both streams and never throws", () => {
|
||||
test("probeScriptVersion combines both streams and reads a runner failure as unavailable", () => {
|
||||
assert.deepEqual(
|
||||
probeScriptVersion(() => ({ stdout: "out ", stderr: "err" })),
|
||||
{ available: true, output: "out err" },
|
||||
|
|
@ -168,13 +169,37 @@ test("probeScriptVersion treats result.error as unavailable, not as empty output
|
|||
probeScriptVersion(() => ({ error: enoent, stdout: "", stderr: "" })),
|
||||
{ available: false, output: "" },
|
||||
);
|
||||
// The timeout shape reports the same way.
|
||||
// An `error` with no recognisable code is still "could not run it".
|
||||
assert.equal(
|
||||
probeScriptVersion(() => ({ error: new Error("ETIMEDOUT") })).available,
|
||||
probeScriptVersion(() => ({ error: new Error("EACCES") })).available,
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
// The one `error` that must NOT read as unavailable (#2333). `script(1)` was
|
||||
// found and started; the machine did not let it finish. Folded into
|
||||
// `available: false` it becomes a `smoke:tui` SKIP — exit 0, blaming PATH — on
|
||||
// exactly the loaded machine the gate is supposed to be believable on.
|
||||
test("probeScriptVersion throws on a timed-out probe instead of reporting no script(1)", () => {
|
||||
const timedOut = Object.assign(new Error("spawnSync script ETIMEDOUT"), {
|
||||
code: "ETIMEDOUT",
|
||||
});
|
||||
assert.throws(
|
||||
() =>
|
||||
probeScriptVersion(() => ({ error: timedOut, stdout: "", stderr: "" })),
|
||||
new RegExp(`did not exit within ${SCRIPT_PROBE_TIMEOUT_MS}ms`),
|
||||
);
|
||||
// And it reaches the caller: resolvePtyWrapper must not turn it into a skip.
|
||||
assert.throws(
|
||||
() =>
|
||||
resolvePtyWrapper({
|
||||
platform: "darwin",
|
||||
probe: () => probeScriptVersion(() => ({ error: timedOut })),
|
||||
}),
|
||||
/did not exit within/,
|
||||
);
|
||||
});
|
||||
|
||||
test("resolvePtyWrapper reports a missing script(1) rather than guessing", () => {
|
||||
// linux would otherwise be classified util-linux off the empty output.
|
||||
const r = resolvePtyWrapper({
|
||||
|
|
|
|||
|
|
@ -93,7 +93,16 @@ if (!existsSync(launcher)) {
|
|||
// No `script(1)` (Windows). Running without a PTY is not a weaker check, it is
|
||||
// a guaranteed failure — so say why rather than report a crash as a defect.
|
||||
// `node-pty` is the portable answer if this ever needs to run there.
|
||||
const pty = resolvePtyWrapper();
|
||||
let pty;
|
||||
try {
|
||||
pty = resolvePtyWrapper();
|
||||
} catch (err) {
|
||||
// The probe timed out: `script(1)` exists and the machine did not let it
|
||||
// finish. That is a starved gate, not a platform without a PTY — reported as
|
||||
// a failure so a load spike cannot quietly turn this smoke into a skip
|
||||
// (#2333). The budget and the measurement are on `SCRIPT_PROBE_TIMEOUT_MS`.
|
||||
fail(err instanceof Error ? err.message : String(err));
|
||||
}
|
||||
if (!pty.ok) {
|
||||
skip(
|
||||
`${pty.reason} — the Ink TUI needs raw mode, which requires a real ` +
|
||||
|
|
|
|||
Loading…
Reference in a new issue