Merge pull request 'lab: a run whose suite proved nothing reads as failed, with its skips (issue 693)' (#287) from fix/693-a-run-that-proved-nothing-is-not-passed into main

modules/lab/test/runs.test.ts

@@ -5,7 +5,7 @@ import { spawnSync } from "node:child_process";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 
-import { installPlan, installStep, NPM_REPOSITORIES, REPOSITORIES, script } from "../tools/runs.ts";
+import { installPlan, installStep, NPM_REPOSITORIES, outcomeOf, readStatus, REPOSITORIES, script } from "../tools/runs.ts";
 
 // Defends novox/hq issue 624: lab_run ran `npm ci` at the root of every checkout it installs, and
 // mesh-tools keeps its package in node-tools/ with no lock file at its root, so every run failed at its
@@ -381,3 +381,108 @@ test("a forge setting that is not http(s) trusts no origin of its own", async ()
     packument("ftp://novox.internal/api/packages/novox/npm/x.tgz")).get, root);
   assert.match(viaRoot.problem ?? "", /neither/);
 });
+
+// Defends novox/hq issue 693: a run whose only test skipped as NOT REACHED read "0 passed, 0 failed", exit 0,
+// and lab_status said passed. The log's tail, as run-2026-10-14T00-21-44-014Z on shanks wrote it.
+const oldLog = [
+  "STOPPED at step 16 of 24, base:          \t\t\t\tMake sure you have all the referenced content and try again.",
+  "\ufe63 genesis adopts its bus as the nats module in place: one bus, its streams kept, the node connected (588152.016874ms) # ADR 0333 §5 NOT REACHED: the first run stopped at step 16 of 24, base, before the broker step:",
+  "         \t\t\t\tMake sure you have all the referenced content and try again.",
+  "NotFound: content digest sha256:e2de96513ba9: not found",
+  "ℹ tests 1", "ℹ suites 0", "ℹ pass 0", "ℹ fail 0", "ℹ cancelled 0", "ℹ skipped 1", "ℹ todo 0",
+  "ℹ duration_ms 590254.163993",
+  "",
+  "recorded: test/integration/genesis-keeps-its-bus.test.ts — 0 passed, 0 failed, against mesh-lab 7ed7da1",
+].join("\n");
+
+// The same run, as the lab's suite says it once it names its skips (mesh-lab, issue 693).
+const newLog = [
+  "ℹ tests 1", "ℹ pass 0", "ℹ fail 0", "ℹ skipped 1",
+  "",
+  "recorded: test/integration/genesis-keeps-its-bus.test.ts — 0 passed, 0 failed, 1 skipped, against mesh-lab 7ed7da1",
+  "NOT PASSED: no test passed: 1 skipped, 1 of them as NOT REACHED",
+  "skipped: genesis adopts its bus — one bus (test/integration/genesis-keeps-its-bus.test.ts) # ADR 0333 §5 NOT REACHED: the first run stopped at step 16 of 24, base: NotFound: content digest",
+].join("\n");
+
+function anEndedRun(log: string, state: "passed" | "failed", exit: number): { work: string; id: string } {
+  const work = mkdtempSync(join(tmpdir(), "lab-runs-"));
+  const id = "run-2026-10-14T00-21-44-014Z";
+  mkdirSync(join(work, id));
+  writeFileSync(join(work, id, "run.log"), log);
+  writeFileSync(join(work, id, "status.json"), JSON.stringify({
+    id, state, exit, started: "2026-10-14T00:21:44.014Z", ended: "2026-10-14T00:33:11.179Z",
+    tests: ["test/integration/genesis-keeps-its-bus.test.ts"], refs: {},
+  }));
+  return { work, id };
+}
+
+test("a run whose only test skipped as NOT REACHED, from a lab of before, reads as failed with the reason", () => {
+  const { work, id } = anEndedRun(oldLog, "passed", 0);
+  const s = readStatus(work, id);
+  assert.equal(s?.state, "failed", JSON.stringify(s));
+  assert.equal(s?.exit, 0);
+  assert.equal(s?.outcome?.passed, 0);
+  assert.equal(s?.outcome?.skipped, 1);
+  assert.equal(s?.outcome?.notPassed, "no test passed: 1 skipped, 1 of them as NOT REACHED");
+  assert.match(s?.outcome?.skips[0]?.reason ?? "", /^ADR 0333 §5 NOT REACHED: the first run stopped at step 16 of 24, base, before the broker step:$/);
+  // Kept in the run's record, not only answered.
+  const kept = JSON.parse(readFileSync(join(work, id, "status.json"), "utf8"));
+  assert.equal(kept.state, "failed");
+  assert.equal(kept.outcome.notPassed, "no test passed: 1 skipped, 1 of them as NOT REACHED");
+  rmSync(work, { recursive: true, force: true });
+});
+
+test("the lab's own NOT PASSED and skipped lines are read, a test name's punctuation and all", () => {
+  const { work, id } = anEndedRun(newLog, "failed", 1);
+  const s = readStatus(work, id);
+  assert.equal(s?.state, "failed");
+  assert.equal(s?.outcome?.notPassed, "no test passed: 1 skipped, 1 of them as NOT REACHED");
+  assert.deepEqual(s?.outcome?.skips, [{
+    file: "test/integration/genesis-keeps-its-bus.test.ts",
+    test: "genesis adopts its bus — one bus",
+    reason: "ADR 0333 §5 NOT REACHED: the first run stopped at step 16 of 24, base: NotFound: content digest",
+  }]);
+  rmSync(work, { recursive: true, force: true });
+});
+
+test("a run that passed tests and skipped one for a declared reason stays passed, its skip listed", () => {
+  const log = [
+    "✔ a module lands (12ms)",
+    "﹣ a builder builds (0.1ms) # set MESH_LAB_BUILDER to a built mesh-builder",
+    "ℹ tests 2", "ℹ pass 1", "ℹ fail 0", "ℹ skipped 1",
+  ].join("\n");
+  const { work, id } = anEndedRun(log, "passed", 0);
+  const s = readStatus(work, id);
+  assert.equal(s?.state, "passed");
+  assert.equal(s?.outcome?.notPassed, undefined);
+  assert.equal(s?.outcome?.skips[0]?.reason, "set MESH_LAB_BUILDER to a built mesh-builder");
+  rmSync(work, { recursive: true, force: true });
+});
+
+test("a run that ran no test at all is not passed", () => {
+  assert.equal(outcomeOf("ℹ tests 0\nℹ pass 0\nℹ fail 0\nℹ skipped 0\n").notPassed, "no test passed: none ran");
+});
+
+// Review 254 on issue 693: a test's own output, printed before the suite reports, is not the suite's verdict.
+test("a line a test printed that looks like the suite's is not read as the suite's", () => {
+  const log = [
+    "NOT PASSED: a test printed this",
+    "skipped: a forged skip (test/integration/x.test.ts) # FORGED NOT REACHED",
+    "✔ a module lands (12ms)",
+    "ℹ tests 1", "ℹ pass 1", "ℹ fail 0", "ℹ skipped 0",
+    "",
+    "recorded: test/integration/x.test.ts — 1 passed, 0 failed, 0 skipped, against mesh-lab 7ed7da1",
+  ].join("\n");
+  const o = outcomeOf(log);
+  assert.equal(o.notPassed, undefined, JSON.stringify(o));
+  assert.deepEqual(o.skips, []);
+});
+
+test("not reached is matched in any case", () => {
+  const log = [
+    "✔ a module lands (12ms)",
+    "﹣ the broker step (0.1ms) # the broker step was not reached",
+    "ℹ tests 2", "ℹ pass 1", "ℹ fail 0", "ℹ skipped 1",
+  ].join("\n");
+  assert.equal(outcomeOf(log).notPassed, "1 test(s) skipped as NOT REACHED: their subject was not reached");
+});

modules/lab/tools/runs.ts

@@ -286,6 +286,117 @@ export interface RunStatus {
   commits?: Record<string, string>;
   exit?: number;
   pid?: number;
+  /** What the suite said of its tests, read from the run's log once it ended (novox/hq issue 693). */
+  outcome?: Outcome;
+}
+
+/** One skipped test, its file and the reason it gave. */
+export interface Skip {
+  file: string;
+  test: string;
+  reason: string;
+}
+
+/**
+ * What a run's suite said of its tests: its totals, each skip with its reason, and — when the run did not
+ * pass although its exit said 0 — why (`notPassed`). Totals the runner did not print are absent.
+ */
+export interface Outcome {
+  passed?: number;
+  failed?: number;
+  skipped?: number;
+  skips: Skip[];
+  notPassed?: string;
+}
+
+/**
+ * outcomeOf reads what a run's log says of its tests (novox/hq issue 693).
+ *
+ * **A run in which no test passed has proven nothing**, and the runner exits 0 when every test skipped:
+ * the genesis bed skipped its only test as "NOT REACHED", the suite printed "0 passed, 0 failed", and the
+ * lab recorded the run as passed. The lab's suite now says it itself — a "NOT PASSED: <why>" line and a
+ * "skipped: <test> (<file>) # <reason>" line per skip after its "recorded:" line, and an exit that is not
+ * 0 — and this reads those lines. For a run of a lab from before (a branch, or an old log), the same rule is
+ * read from the runner's own output: its summary totals, and its skip lines' first line of reason.
+ *
+ * The rule, the lab suite's (mesh-lab src/outcomes.ts): **not passed when no test passed**, or when a test
+ * skipped as NOT REACHED (its subject was not reached); a skip for another reason beside tests that passed
+ * leaves the run passed, its reason listed.
+ */
+export function outcomeOf(log: string): Outcome {
+  const plain = log.replace(/\u001b\[[0-9;]*m/g, "");
+  const total = (word: string) => {
+    // The last summary in the log: the runner's own, once per run.
+    const all = [...plain.matchAll(new RegExp(`^\\s*(?:\u2139|#)\\s*${word}\\s+(\\d+)\\s*$`, "gm"))];
+    const last = all[all.length - 1];
+    return last ? Number(last[1]) : undefined;
+  };
+  const outcome: Outcome = { skips: [] };
+  const passed = total("pass");
+  const failed = total("fail");
+  const skipped = total("skipped");
+  if (passed !== undefined) outcome.passed = passed;
+  if (failed !== undefined) outcome.failed = failed;
+  if (skipped !== undefined) outcome.skipped = skipped;
+
+  // **The suite's own lines are read only after its last "recorded:" line** (novox/hq issue 693, review
+  // 254): a test prints what it likes before the suite reports, and a line of its that looked like the
+  // suite's — "NOT PASSED: …", "skipped: … # NOT REACHED" — would otherwise stand for the suite's verdict.
+  const recorded = [...plain.matchAll(/^recorded: .*$/gm)];
+  const lastRecorded = recorded[recorded.length - 1];
+  const suiteSaid = lastRecorded ? plain.slice((lastRecorded.index ?? 0) + lastRecorded[0].length) : "";
+
+  // The suite's own line, "skipped: <test> (<file>) # <reason>": the file in brackets, a .ts, parts it.
+  for (const m of suiteSaid.matchAll(/^skipped: (.*?) \(([^()]+\.ts)\) # (.*)$/gm)) {
+    outcome.skips.push({ file: m[2] ?? "", test: m[1] ?? "", reason: m[3] === "no reason given" ? "" : m[3] ?? "" });
+  }
+  if (outcome.skips.length === 0) {
+    // A lab from before names no skip itself; its runner's spec line does: "﹣ <test> (<ms>ms) # <reason>".
+    for (const m of plain.matchAll(/^\s*\ufe63 (.*?) \([\d.]+m?s\) # (.*)$/gm)) {
+      const reason = (m[2] ?? "").trim();
+      outcome.skips.push({ file: "", test: m[1] ?? "", reason: reason === "SKIP" ? "" : reason });
+    }
+  }
+
+  const said = suiteSaid.match(/^NOT PASSED: (.*)$/m);
+  if (said) {
+    outcome.notPassed = (said[1] ?? "").trim();
+  } else if ((failed ?? 0) === 0) {
+    // In any case: "not reached" says the same as "NOT REACHED".
+    const unreached = outcome.skips.filter((s) => s.reason.toUpperCase().includes("NOT REACHED")).length;
+    if (passed === 0) {
+      outcome.notPassed = outcome.skips.length > 0 || (skipped ?? 0) > 0
+        ? `no test passed: ${skipped ?? outcome.skips.length} skipped${unreached > 0 ? `, ${unreached} of them as NOT REACHED` : ""}`
+        : "no test passed: none ran";
+    } else if (unreached > 0) {
+      outcome.notPassed = `${unreached} test(s) skipped as NOT REACHED: their subject was not reached`;
+    }
+  }
+  return outcome;
+}
+
+/**
+ * settled is a run that ended, with its outcome read from its log: a run whose exit said 0 and whose suite
+ * proved nothing reads as failed, with why (novox/hq issue 693). Read once and kept in its status, so the
+ * record says it too; a run still going, stopped or lost is left as it is.
+ */
+export function settled(work: string, s: RunStatus): RunStatus {
+  if (s.outcome || !s.ended || (s.state !== "passed" && s.state !== "failed")) return s;
+  let log: string;
+  try {
+    log = readFileSync(join(runDir(work, s.id), "run.log"), "utf8");
+  } catch {
+    return s;
+  }
+  const outcome = outcomeOf(log);
+  const next: RunStatus = { ...s, outcome };
+  if (next.state === "passed" && outcome.notPassed) next.state = "failed";
+  try {
+    writeFileSync(statusPath(work, s.id), JSON.stringify(next, null, 2));
+  } catch {
+    // Read again next time: what lab_status answers is the same either way.
+  }
+  return next;
 }
 
 export function runDir(work: string, id: string): string {
@@ -304,7 +415,7 @@ export function readStatus(work: string, id: string): RunStatus | undefined {
     if (!["passed", "failed", "stopped", "lost"].includes(s.state) && s.pid && !alive(s.pid)) {
       s.state = "lost";
     }
-    return s;
+    return settled(work, s);
   } catch {
     return undefined;
   }