A suite run in which no test passed exits non-zero and names its skips (issue 693)

The runner exits 0 when every test skipped, so a bed that stopped before its
subject and skipped its only test as NOT REACHED was recorded as passed, "0
passed, 0 failed". The suite now collects each skip and its reason through a
reporter of its own, prints NOT PASSED and a skipped line per skip after its
recorded line, keeps them in the receipt, and exits 1 when no test passed or a
test was not reached. A skip for a declared reason beside passes stays passed.

src/lastrun.ts

@@ -24,6 +24,7 @@ import { mkdirSync, readFileSync, writeFileSync } from "node:fs";
 import { homedir } from "node:os";
 import { dirname, join } from "node:path";
 
+import type { Outcome, Skip } from "./outcomes.ts";
 import { repositories } from "./repos.ts";
 
 /** What a run was taken against, per repository. */
@@ -38,6 +39,11 @@ export interface Receipt {
   against: Against;
   /** The test files this run was pointed at. See {@link endToEnd}. */
   ran: string[];
+  /** How many tests skipped, and each with its reason (novox/hq issue 693). Absent on older receipts. */
+  skipped?: number;
+  skips?: Skip[];
+  /** Why the run did not pass although nothing failed — no test passed, or one was not reached. */
+  notPassed?: string;
 }
 
 /**
@@ -104,6 +110,7 @@ export function record(
   ran: string[],
   env = process.env,
   builtAgainst?: Against,
+  outcome?: Outcome,
 ): Receipt {
   const receipt: Receipt = {
     at: new Date().toISOString(),
@@ -115,6 +122,8 @@ export function record(
     // never in the binaries. A verdict is only worth something attributed to one exact state.
     against: builtAgainst ?? whatWasTested(env),
     ran,
+    ...(outcome ? { skipped: outcome.skipped, skips: outcome.skips } : {}),
+    ...(outcome?.notPassed ? { notPassed: outcome.notPassed } : {}),
   };
   const path = receiptPath();
   mkdirSync(dirname(path), { recursive: true });
@@ -166,9 +175,14 @@ export function judge(
 
   const days = (now.getTime() - Date.parse(receipt.at)) / 86_400_000;
   const lines: string[] = [];
+  // A run that proved nothing did not pass, though nothing failed (novox/hq issue 693): no test passed,
+  // or one skipped as not reached. A receipt from before the field is judged by its count alone.
+  const notPassed = receipt.failed > 0 ? "" : receipt.notPassed ?? (receipt.passed === 0 ? "no test passed" : "");
   const outcome = receipt.failed > 0
     ? `last ran ${ago(days)} and ${receipt.failed} test(s) failed`
-    : `last passed ${ago(days)}, ${receipt.passed} test(s)`;
+    : notPassed
+      ? `last ran ${ago(days)} and did not pass: ${notPassed}`
+      : `last passed ${ago(days)}, ${receipt.passed} test(s)`;
   lines.push(`the end-to-end suite ${outcome}`);
 
   let moved = false;
@@ -192,7 +206,9 @@ export function judge(
     lines.push(`  That run did not include ${endToEnd}, so it raised no machines.`);
   }
 
-  if (receipt.failed > 0) {
+  for (const s of receipt.skips ?? []) lines.push(`  skipped: ${s.test} — ${s.reason || "no reason given"}`);
+
+  if (receipt.failed > 0 || notPassed) {
     lines.push("  Nothing has been proven end to end since.");
   } else if (moved) {
     lines.push("  What it proved was proven about code that has since changed.");
@@ -201,7 +217,7 @@ export function judge(
   }
 
   return {
-    current: endToEndRan && receipt.failed === 0 && !moved && days <= staleAfterDays,
+    current: endToEndRan && receipt.failed === 0 && !notPassed && !moved && days <= staleAfterDays,
     lines,
   };
 }

src/outcomes.ts

@@ -0,0 +1,113 @@
+/**
+ * What a suite's tests did, beyond the runner's totals: which were skipped and why, and whether the run
+ * proved anything (novox/hq issue 693).
+ *
+ * **A run in which no test passed has proven nothing, whatever its exit code.** The runner exits 0 when
+ * every test skipped, and the lab recorded such a run as passed: the genesis bed stopped before its
+ * subject, skipped its only test as "NOT REACHED", and the run read "0 passed, 0 failed", passed.
+ * Failure must be loud, and a green run that verified nothing is the quietest failure there is.
+ *
+ * The default export is a reporter for node's test runner (`--test-reporter`): it writes one JSON line per
+ * skipped test, with its file, its name and the reason it gave, word for word. The reasons come from the
+ * runner's events rather than from the spec reporter's text, because a reason runs over several lines and a
+ * later test's output lands between them.
+ */
+
+import { readFileSync } from "node:fs";
+
+/** One skipped test and the reason it gave ("" when it gave none). */
+export interface Skip {
+  file: string;
+  test: string;
+  reason: string;
+}
+
+/** What a receipt keeps beside the totals: the skips, and why the run did not pass when it did not. */
+export interface Outcome {
+  skipped: number;
+  skips: Skip[];
+  notPassed?: string;
+}
+
+/**
+ * The words a bed's test skips with when the run stopped before its subject: the subject was not reached,
+ * so nothing about it was proven (the beds say "<what> NOT REACHED: <where it stopped>").
+ */
+export const NOT_REACHED = "NOT REACHED";
+
+/**
+ * Whether a run passed, and if not, why — the design limit of issue 693, in one place:
+ *
+ *   - a test failed: not passed (the runner already says so);
+ *   - **no test passed**: not passed, whatever was skipped — a run that skipped everything, or ran
+ *     nothing, verified nothing;
+ *   - **a test skipped as NOT REACHED**: not passed, even beside tests that passed — its subject was not
+ *     reached, which is the bed failing to get where it was sent, not a skip anyone declared expected;
+ *   - otherwise passed, its other skips listed with their reasons: a test skipped for a reason its file
+ *     declares (no hypervisor, an optional bed not asked for) beside tests that passed is an expected
+ *     skip, and the run still proved what passed.
+ *
+ * Counts the runner did not report (null) decide nothing here: that is reportOn's "no receipt" case.
+ */
+export function judgeRun(
+  counts: { passed: number | null; failed: number | null },
+  skips: Skip[],
+): { passed: boolean; why: string } {
+  if (counts.failed !== null && counts.failed > 0) return { passed: false, why: `${counts.failed} test(s) failed` };
+  const unreached = skips.filter((s) => s.reason.includes(NOT_REACHED));
+  if (counts.passed === 0) {
+    return {
+      passed: false,
+      why: skips.length > 0
+        ? `no test passed: ${skips.length} skipped${unreached.length > 0 ? `, ${unreached.length} of them as ${NOT_REACHED}` : ""}`
+        : "no test passed: none ran",
+    };
+  }
+  if (unreached.length > 0) {
+    return { passed: false, why: `${unreached.length} test(s) skipped as ${NOT_REACHED}: their subject was not reached` };
+  }
+  return { passed: true, why: "" };
+}
+
+/** A reason on one line, for a log line and a record: its whitespace folded, and at most 500 characters. */
+export function oneLine(reason: string): string {
+  const flat = reason.replace(/\s+/g, " ").trim();
+  return flat.length > 500 ? `${flat.slice(0, 499)}…` : flat;
+}
+
+/** The skips the reporter wrote to a file; none when it wrote nothing or the file cannot be read. */
+export function readSkips(path: string): Skip[] {
+  let text: string;
+  try {
+    text = readFileSync(path, "utf8");
+  } catch {
+    return [];
+  }
+  const skips: Skip[] = [];
+  for (const line of text.split("\n")) {
+    if (!line.trim()) continue;
+    try {
+      const s = JSON.parse(line) as Partial<Skip>;
+      skips.push({ file: String(s.file ?? ""), test: String(s.test ?? ""), reason: String(s.reason ?? "") });
+    } catch {
+      // A torn last line, when the runner was killed mid-write: the lines before it still stand.
+    }
+  }
+  return skips;
+}
+
+interface Event {
+  type: string;
+  data: { name?: string; file?: string; skip?: string | boolean; details?: { type?: string } };
+}
+
+/** The reporter: one JSON line per skipped test (not per skipped suite: its tests are reported each). */
+export default async function* skipsReporter(source: AsyncIterable<Event>): AsyncGenerator<string> {
+  for await (const event of source) {
+    if (event.type !== "test:pass" && event.type !== "test:fail") continue;
+    const { skip, details } = event.data;
+    if (skip === undefined || skip === false || details?.type === "suite") continue;
+    const said: Skip = { file: event.data.file ?? "", test: event.data.name ?? "", reason: typeof skip === "string" ? skip : "" };
+    yield `${JSON.stringify(said)}\n`;
+  }
+}

src/suite.ts

@@ -10,11 +10,16 @@
  */
 
 import { spawn } from "node:child_process";
+import { mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { isAbsolute, join, relative } from "node:path";
+import { fileURLToPath } from "node:url";
 import { endToEnd, record, whatWasTested } from "./lastrun.ts";
+import { judgeRun, oneLine, readSkips, type Outcome, type Skip } from "./outcomes.ts";
 import { rebuild } from "./rebuild.ts";
 
 /** counted is what the runner said, or nulls when it said nothing recognisable. */
-export function counted(output: string): { passed: number | null; failed: number | null } {
+export function counted(output: string): { passed: number | null; failed: number | null; skipped: number | null } {
   // The runner's own summary lines, each on a line of its own. Anchored, so a test *named*
   // "pass 3" cannot be mistaken for the total — which is not a hypothetical worry in a suite whose
   // tests are named in sentences.
@@ -30,6 +35,7 @@ export function counted(output: string): { passed: number | null; failed: number
   return {
     passed: total(/^\s*(?:\u2139|#)\s*pass\s+(\d+)\s*$/m),
     failed: total(/^\s*(?:\u2139|#)\s*fail\s+(\d+)\s*$/m),
+    skipped: total(/^\s*(?:\u2139|#)\s*skipped\s+(\d+)\s*$/m),
   };
 }
 
@@ -57,16 +63,38 @@ export async function runSuite(args: string[]): Promise<number> {
   // `test/integration/canary.test.ts` is still there and still runs when it is named; it is no
   // longer raised on the way to everything else.
 
-  const { code, seen } = await runFiles(files);
-  console.log("\n" + reportOn(counted(seen), (p, f) => record(p, f, files, process.env, against)));
-  return code;
+  const { code, seen, skips } = await runFiles(files);
+  const counts = counted(seen);
+  console.log("\n" + reportOn(counts, skips, (p, f, outcome) => record(p, f, files, process.env, against, outcome)));
+  return exitOf(code, counts, skips);
 }
 
-/** Run some test files, passing their output through as it arrives. */
-async function runFiles(files: string[]): Promise<{ code: number; seen: string }> {
+/**
+ * The suite's exit: the runner's, unless it says 0 for a run that did not pass (novox/hq issue 693). The
+ * runner exits 0 when every test skipped; the lab takes this exit as the run's verdict, so a run that
+ * proved nothing must not end in 0.
+ */
+export function exitOf(code: number, counts: { passed: number | null; failed: number | null }, skips: Skip[]): number {
+  if (code !== 0) return code;
+  if (counts.passed === null || counts.failed === null) return code;
+  return judgeRun(counts, skips).passed ? 0 : 1;
+}
+
+/** The reporter that writes each skipped test and its reason (outcomes.ts), by its path. */
+const skipsReporter = fileURLToPath(new URL("./outcomes.ts", import.meta.url));
+
+/**
+ * Run some test files, passing their output through as it arrives, and collect what was skipped and why:
+ * the spec reporter to stdout as before, and the skips reporter beside it into a file of the run's own.
+ */
+async function runFiles(files: string[]): Promise<{ code: number; seen: string; skips: Skip[] }> {
+  const scratch = mkdtempSync(join(tmpdir(), "mesh-lab-suite-"));
+  const skipsFile = join(scratch, "skips.jsonl");
   const running = spawn(
     process.execPath,
     ["--test", "--test-concurrency=1", "--experimental-strip-types",
+      "--test-reporter=spec", "--test-reporter-destination=stdout",
+      `--test-reporter=${skipsReporter}`, `--test-reporter-destination=${skipsFile}`,
       ...files],
     { stdio: ["inherit", "pipe", "inherit"] },
   );
@@ -81,7 +109,14 @@ async function runFiles(files: string[]): Promise<{ code: number; seen: string }
   const code: number = await new Promise((resolve) => {
     running.on("close", (c) => resolve(c ?? 1));
   });
-  return { code, seen };
+  // Named as the files were given, relative to where the suite runs, rather than as the runner's absolute paths.
+  const named = (file: string) => {
+    const rel = isAbsolute(file) ? relative(process.cwd(), file) : file;
+    return rel.startsWith("..") ? file : rel;
+  };
+  const skips = readSkips(skipsFile).map((s) => ({ ...s, file: named(s.file) }));
+  rmSync(scratch, { recursive: true, force: true });
+  return { code, seen, skips };
 }
 
 /**
@@ -92,16 +127,31 @@ async function runFiles(files: string[]): Promise<{ code: number; seen: string }
  * could check it — which is 04-ISSUES/005 in miniature, inside the fix for it.
  */
 export function reportOn(
-  counts: { passed: number | null; failed: number | null },
-  write: (passed: number, failed: number) => { against: Record<string, string>; ran: string[] },
+  counts: { passed: number | null; failed: number | null; skipped?: number | null },
+  skips: Skip[],
+  write: (passed: number, failed: number, outcome: Outcome) => { against: Record<string, string>; ran: string[] },
 ): string {
   if (counts.passed === null || counts.failed === null) {
     // A run whose result could not be read is a run nobody can say anything about. Writing
     // "0 failed" because nothing said otherwise is how a green record comes to mean nothing.
     return "could not read what the runner reported; no receipt written";
   }
-  const receipt = write(counts.passed, counts.failed);
+  // **A skip is said, with its reason, and a run that proved nothing says NOT PASSED** (novox/hq issue
+  // 693): "0 passed, 0 failed" read as a clean run. The lab module reads these lines into its record.
+  const skipped = counts.skipped ?? skips.length;
+  const judged = judgeRun({ passed: counts.passed, failed: counts.failed }, skips);
+  const outcome: Outcome = {
+    skipped,
+    skips: skips.map((s) => ({ ...s, reason: oneLine(s.reason) })),
+    ...(judged.passed ? {} : { notPassed: judged.why }),
+  };
+  const receipt = write(counts.passed, counts.failed, outcome);
   const against = Object.entries(receipt.against).map(([n, c]) => `${n} ${c}`).join(", ");
-  return `recorded: ${receipt.ran.join(", ")} — ${counts.passed} passed, ` +
-    `${counts.failed} failed, against ${against || "nothing in git"}`;
+  const lines = [`recorded: ${receipt.ran.join(", ")} — ${counts.passed} passed, ` +
+    `${counts.failed} failed, ${skipped} skipped, against ${against || "nothing in git"}`];
+  if (!judged.passed) lines.push(`NOT PASSED: ${judged.why}`);
+  // As the runner's own spec line has it, "<test> (<file>) # <reason>": a test's name may hold any
+  // punctuation, so the file in brackets is what the lab module reads the line apart by.
+  for (const s of outcome.skips) lines.push(`skipped: ${s.test} (${s.file}) # ${s.reason || "no reason given"}`);
+  return lines.join("\n");
 }