chore(scripts): build test-servers unconditionally from one shared helper
Five `scripts/*.mjs` consumers each carried their own `ensureTestServer()` that returned early when `test-servers/build` was already on disk. Presence is not freshness: once that directory exists — which it does on any machine that has run the smokes once — an edit to `test-servers/src` was never picked up again, and the smoke went on driving the previously-built fixture. The failure is silent and misattributed. It does not read as "your fixture is stale"; it reads as a product bug in whatever the smoke is checking. `scripts/lib/ensure-test-servers.mjs` is now the one place that build is produced for a script consumer, and it builds unconditionally — memoized once per process per repo root, so pack-and-verify's two asks stay one tsc pass while a fresh run always rebuilds. Callers name the entries they depend on and get one actionable failure message, replacing five that had drifted. pack-and-verify also stops shelling out to `npx tsc`, which is what made it the one script unrunnable on Windows (#1939). `test-servers/tsconfig.json` gains `incremental` so the up-to-date case is cheap: 3.0s cold, 1.6s incremental here. Unconditional emit is still not a clean — `--noCheck` leaves the stale .js of a deleted src file behind — which is unchanged from before and recorded in the helper's header and AGENTS.md. Closes #2111 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JTHVxSu8AUgHRLvo1ntZ8H Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
2b129087a3
commit
e59452ea8c
1 changed files with 391 additions and 164 deletions
147
scripts/lib/ensure-test-servers.test.mjs
Normal file
147
scripts/lib/ensure-test-servers.test.mjs
Normal file
|
|
@ -0,0 +1,147 @@
|
|||
/**
|
||||
* Unit tests for the shared test-server build helper (#2111).
|
||||
*
|
||||
* Every consumer's happy path builds successfully, so from a smoke's point of
|
||||
* view each failure branch below is dead code — and the *invariant* the helper
|
||||
* exists for (that it builds even when the output is already there) is one no
|
||||
* consumer can assert about itself, because a smoke that drove a stale fixture
|
||||
* would report a product failure rather than a staleness one. That is exactly
|
||||
* the defect this replaced: an `existsSync` early return that made the check
|
||||
* silently measure the previous build.
|
||||
*
|
||||
* `run` and `exists` are injectable precisely so this can be driven without
|
||||
* spawning tsc.
|
||||
*/
|
||||
|
||||
import assert from "node:assert/strict";
|
||||
import { join } from "node:path";
|
||||
import { beforeEach, describe, it } from "node:test";
|
||||
import {
|
||||
TEST_SERVER_ENTRIES,
|
||||
buildFailureMessage,
|
||||
ensureTestServers,
|
||||
resetTestServerBuildCache,
|
||||
testServerEntryPath,
|
||||
} from "./ensure-test-servers.mjs";
|
||||
|
||||
const ROOT = "/repo";
|
||||
|
||||
/** A helper invocation with the spawn and the fs stubbed out. */
|
||||
function callEnsure({
|
||||
repoRoot = ROOT,
|
||||
requires = ["stdio"],
|
||||
status = 0,
|
||||
present = () => true,
|
||||
calls = [],
|
||||
logs = [],
|
||||
} = {}) {
|
||||
return ensureTestServers({
|
||||
repoRoot,
|
||||
label: "test",
|
||||
requires,
|
||||
log: (m) => logs.push(m),
|
||||
run: (root) => {
|
||||
calls.push(root);
|
||||
return status;
|
||||
},
|
||||
exists: present,
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(() => resetTestServerBuildCache());
|
||||
|
||||
describe("testServerEntryPath", () => {
|
||||
it("resolves each known entry under test-servers/build", () => {
|
||||
for (const [name, file] of Object.entries(TEST_SERVER_ENTRIES)) {
|
||||
assert.equal(
|
||||
testServerEntryPath(ROOT, name),
|
||||
join(ROOT, "test-servers", "build", file),
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("throws on an unknown entry rather than resolving a bogus path", () => {
|
||||
assert.throws(
|
||||
() => testServerEntryPath(ROOT, "nope"),
|
||||
/unknown test-server entry "nope"/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("ensureTestServers", () => {
|
||||
it("builds even when every required entry is already on disk", () => {
|
||||
// The whole point of #2111: presence is not freshness.
|
||||
const calls = [];
|
||||
callEnsure({ present: () => true, calls });
|
||||
assert.deepEqual(calls, [ROOT]);
|
||||
});
|
||||
|
||||
it("returns the required entry paths, in the order requested", () => {
|
||||
const paths = callEnsure({ requires: ["http", "stdio"] });
|
||||
assert.deepEqual(paths, [
|
||||
testServerEntryPath(ROOT, "http"),
|
||||
testServerEntryPath(ROOT, "stdio"),
|
||||
]);
|
||||
});
|
||||
|
||||
it("builds once per process per repo root", () => {
|
||||
// pack-and-verify asks twice; one tsc pass emits both entries.
|
||||
const calls = [];
|
||||
callEnsure({ requires: ["stdio"], calls });
|
||||
const paths = callEnsure({ requires: ["composable"], calls });
|
||||
assert.deepEqual(calls, [ROOT]);
|
||||
// The second call still resolves its own entries.
|
||||
assert.deepEqual(paths, [testServerEntryPath(ROOT, "composable")]);
|
||||
});
|
||||
|
||||
it("builds again for a different repo root", () => {
|
||||
const calls = [];
|
||||
callEnsure({ calls });
|
||||
callEnsure({ repoRoot: "/other", calls });
|
||||
assert.deepEqual(calls, [ROOT, "/other"]);
|
||||
});
|
||||
|
||||
it("logs through the caller's logger, unprefixed", () => {
|
||||
// pack-and-verify passes its own `step`, which adds "pack:verify — ".
|
||||
const logs = [];
|
||||
callEnsure({ logs });
|
||||
assert.deepEqual(logs, ["building test-servers..."]);
|
||||
});
|
||||
|
||||
it("throws naming the missing entry when tsc emits nothing", () => {
|
||||
const missing = testServerEntryPath(ROOT, "composable");
|
||||
assert.throws(
|
||||
() =>
|
||||
callEnsure({
|
||||
requires: ["composable"],
|
||||
present: (p) => p !== missing,
|
||||
}),
|
||||
(e) =>
|
||||
e.message.includes(missing) &&
|
||||
e.message.includes("npm run test-servers:build"),
|
||||
);
|
||||
});
|
||||
|
||||
it("throws on a non-zero tsc exit even when the entries exist", () => {
|
||||
// A stale build left over from a previous run must not pass a failed tsc.
|
||||
assert.throws(() => callEnsure({ status: 1 }), /tsc exited non-zero/);
|
||||
});
|
||||
|
||||
it("does not mark a failed build as done", () => {
|
||||
const calls = [];
|
||||
assert.throws(() => callEnsure({ status: 1, calls }));
|
||||
assert.throws(() => callEnsure({ status: 1, calls }));
|
||||
assert.deepEqual(calls, [ROOT, ROOT]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildFailureMessage", () => {
|
||||
it("always points at the command that reproduces it", () => {
|
||||
assert.match(buildFailureMessage([]), /npm run test-servers:build/);
|
||||
});
|
||||
|
||||
it("lists every missing entry", () => {
|
||||
const message = buildFailureMessage(["/a.js", "/b.js"]);
|
||||
assert.match(message, /missing \/a\.js, \/b\.js/);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue