ci(jtreg): run only the OpenJDK tests that cover java-web code no faster test covers

docs/development.md

@@ -71,6 +71,16 @@ HotSpot with default options (for example `vm.continuations` is true and
 `vm.debug` is false); a test that needs more is skipped.
 
 `test/jtreg/passing.txt` lists the tests CI runs, with `--only-passing`.
+It holds only tests that check java-web's own code. java-web runs OpenJDK's
+class library unmodified, so a test whose path through the VM a faster listed
+test already covers adds CI time but no check. `test/jtreg/redundant.txt`
+lists those, and a full run never adds them back; to run one again, move its
+line from `redundant.txt` to `passing.txt`. To redo the list, run
+`node scripts/jtreg/run.ts <jdk-source> --only-passing --coverage build/coverage --vm dist/node.js`
+after `pnpm build`. It runs both lists and writes the blocks of the VM each
+passing test ran. Then run `node scripts/jtreg/prune.ts build/coverage`. Tests in `locale-data.txt` and tests that fail under coverage
+always stay.
+
 A pull request runs the tests its changes need (`scripts/jtreg/select.ts`): a
 change to a file the tests load runs the test directories the table there maps
 it to plus every tenth listed test, while a change to any other loaded file or

scripts/jtreg/prune.ts

@@ -0,0 +1,72 @@
+// Splits the tests of passing.txt and redundant.txt between the two by what
+// they cover. Coverage comes from a run with --coverage, which reruns both:
+//
+//   node scripts/jtreg/run.ts <jdk> --only-passing --coverage build/coverage --vm dist/node.js
+//   node scripts/jtreg/prune.ts build/coverage
+//
+// java-web runs OpenJDK's class library unmodified, so a test only checks
+// java-web where it runs java-web's own code. Fastest first (times.tsv), a
+// test goes in passing.txt when it ran a block of the VM no test before it
+// there ran, and in redundant.txt otherwise. Tests in locale-data.txt stay in
+// passing.txt, and a test without a coverage file (it failed, timed out or did
+// not run) stays in the list it was in.
+
+import { existsSync, readFileSync, writeFileSync } from "node:fs";
+import { join } from "node:path";
+
+const root = join(import.meta.dirname, "..", "..");
+const dir = process.argv[2];
+if (!dir) {
+  console.error("usage: node scripts/jtreg/prune.ts <coverage dir>");
+  process.exit(2);
+}
+const file = (name: string) => join(root, "test", "jtreg", name);
+const list = (name: string) => existsSync(file(name))
+  ? readFileSync(file(name), "utf8").split("\n").map((l) => l.trim()).filter((l) => l && !l.startsWith("#")) : [];
+
+const passing = list("passing.txt"), wasRedundant = list("redundant.txt").filter((id) => !passing.includes(id));
+const read = (name: string) => {
+  try {
+    return readFileSync(file(name), "utf8");
+  } catch {
+    return "";
+  }
+};
+const times = new Map(read("times.tsv").split("\n").filter(Boolean).map((l) => {
+  const [ms, id] = l.split("\t");
+  return [id, Number(ms)];
+}));
+const median = [...times.values()].sort((a, b) => a - b)[times.size >> 1] ?? 0;
+const ms = (id: string) => times.get(id) ?? median;
+const locale = new Set(list("locale-data.txt"));
+const blocksOf = (id: string) => {
+  const f = join(dir, `${id.replace(/[#/]/g, "_")}.txt`);
+  return existsSync(f) ? readFileSync(f, "utf8").split("\n").filter(Boolean) : null;
+};
+
+const covered = new Set<string>();
+const keep = new Set<string>(), drop = new Set<string>();
+const candidates: [string, string[]][] = [];
+for (const id of [...passing, ...wasRedundant]) {
+  const blocks = blocksOf(id);
+  if (!blocks) (passing.includes(id) ? keep : drop).add(id);
+  else if (locale.has(id)) keep.add(id);
+  else {
+    candidates.push([id, blocks]);
+    continue;
+  }
+  for (const b of blocks ?? []) covered.add(b);
+}
+for (const [id, blocks] of candidates.sort((a, b) => ms(a[0]) - ms(b[0]))) {
+  if (blocks.some((b) => !covered.has(b))) {
+    keep.add(id);
+    for (const b of blocks) covered.add(b);
+  } else drop.add(id);
+}
+
+writeFileSync(file("passing.txt"), "# OpenJDK tests that pass on java-web; scripts/jtreg/run.ts --only-passing reruns them\n"
+  + [...keep].sort().join("\n") + "\n");
+writeFileSync(file("redundant.txt"), "# OpenJDK tests that pass on java-web but run no java-web code that a faster test in\n"
+  + "# passing.txt does not (scripts/jtreg/prune.ts). To run one again, move it to passing.txt.\n" + [...drop].sort().join("\n") + "\n");
+const min = (ids: Set<string>) => ([...ids].reduce((n, id) => n + ms(id), 0) / 60000).toFixed(0);
+console.log(`passing.txt: ${keep.size} tests (${min(keep)} test-minutes); redundant.txt: ${drop.size} (${min(drop)} test-minutes)`);

scripts/jtreg/run.ts

@@ -2,7 +2,7 @@
 //
 //   node scripts/jtreg/run.ts <jdk-source-checkout> [dir or file filters...]
 //        [--jobs 1] [--timeout 120] [--rerun] [--only-passing] [--shard 1/4] [--vm path/to/node.ts] [--cool 75]
-//        [--locale-data] [--library-dirs]
+//        [--locale-data] [--library-dirs] [--coverage dir]
 //
 // Each test is compiled the way jtreg does it, run on HotSpot first (only
 // tests that pass there count), then on java-web in a child process.
@@ -11,14 +11,17 @@
 // java-web alone, and fails if any of them regress. --shard k/n runs the kth
 // of n shards with about equal run time, for parallel CI jobs. Tests listed in
 // test/jtreg/locale-data.txt start java-web with --locale-data (every locale
-// costs start-up and run time, so the others run without it).
+// costs start-up and run time, so the others run without it). --coverage
+// writes, for each test that passes on java-web, the blocks of the VM's code it
+// ran to <dir>/<test>.txt, for scripts/jtreg/prune.ts; with --only-passing it
+// runs the tests in test/jtreg/redundant.txt too. Those stay out of passing.txt.
 
 import { createHash } from "node:crypto";
 import { spawn, spawnSync } from "node:child_process";
 import type { ChildProcess } from "node:child_process";
 import { appendFileSync, existsSync, mkdirSync, readFileSync, readdirSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs";
 import { release, totalmem } from "node:os";
-import { dirname, join, normalize, relative, resolve } from "node:path";
+import { basename, dirname, join, normalize, relative, resolve } from "node:path";
 import { hotspot } from "../hotspot.ts";
 
 const root = join(import.meta.dirname, "..", "..");
@@ -27,6 +30,8 @@ const resultsFile = join(out, "results.jsonl");
 const passingFile = join(root, "test", "jtreg", "passing.txt");
 // tests that need the locales other than English (java-web's --locale-data)
 const localeFile = join(root, "test", "jtreg", "locale-data.txt");
+// tests that pass but run no java-web code a faster listed test does not
+const redundantFile = join(root, "test", "jtreg", "redundant.txt");
 // how long each listed test takes on java-web, for splitting them into shards
 const timesFile = join(root, "test", "jtreg", "times.tsv");
 
@@ -49,6 +54,9 @@ const onlyPassing = has("--only-passing");
 // --locale-data runs every test with the locale data, to find the tests that need it
 const localeData = has("--locale-data");
 const printLibraryDirs = has("--library-dirs");
+const coverageDir = flag("--coverage", "");
+const coverage = coverageDir && resolve(coverageDir);
+if (coverage) mkdirSync(coverage, { recursive: true });
 const [shardK, shardN] = flag("--shard", "1/1").split("/").map(Number);
 if (!Number.isInteger(shardK) || !Number.isInteger(shardN) || shardK < 1 || shardK > shardN) {
   console.error("--shard takes k/n with 1 <= k <= n");
@@ -484,10 +492,25 @@ function clean(tc: TestCase, names: string[]) {
 
 interface RunResult { ok: boolean; ms: number; timedOut: boolean; tail: string }
 
+// The blocks of dist/node.js (or src/) a test's VMs ran, as "url:start-end"
+// lines in <coverage>/<test>.txt, from the V8 coverage all its actions wrote to raw.
+function keepCoverage(raw: string, test: string) {
+  const blocks = new Set<string>();
+  for (const f of readdirSync(raw)) {
+    for (const script of JSON.parse(readFileSync(join(raw, f), "utf8")).result) {
+      if (!script.url.startsWith("file:") || !script.url.includes(vm.endsWith(".js") ? vm : "/src/")) continue;
+      for (const fn of script.functions) for (const r of fn.ranges) if (r.count > 0) blocks.add(`${script.url}:${r.startOffset}-${r.endOffset}`);
+    }
+  }
+  writeFileSync(join(coverage, `${test}.txt`), [...blocks].join("\n"));
+}
+
 function exec(cmd: string, args: string[], cwd: string, limit: number): Promise<RunResult> {
   return new Promise((done) => {
     const start = Date.now();
-    const child = spawn(cmd, args, { cwd, stdio: ["ignore", "pipe", "pipe"], detached: true });
+    // java-web runs only, each into its test's directory (named as its work directory)
+    const raw = coverage && cmd === "node" ? join(coverage, "raw", basename(dirname(cwd))) : "";
+    const child = spawn(cmd, args, { cwd, stdio: ["ignore", "pipe", "pipe"], detached: true, env: raw ? { ...process.env, NODE_V8_COVERAGE: raw } : process.env });
     let text = "";
     const keep = (d: Buffer) => { text = (text + d.toString()).slice(-65536); };
     child.stdout.on("data", keep);
@@ -617,7 +640,8 @@ mkdirSync(out, { recursive: true });
 let selected: TestCase[];
 if (onlyPassing) {
   const under = (id: string) => !argv.length || argv.some((f) => id === f || id.startsWith(f + "/") || id.startsWith(f + "#"));
-  const ids = new Set(listed().filter(under));
+  // a coverage run also reruns the redundant tests, so prune.ts can reconsider them
+  const ids = new Set([...listed(), ...(coverage ? readList(redundantFile) : [])].filter(under));
   const files = [...new Set([...ids].map((id) => join(testRoot, id.split("#")[0])))];
   selected = files.flatMap(parseFile).filter((tc) => ids.has(tc.id));
 } else {
@@ -672,7 +696,14 @@ async function worker() {
     await coolDown();
     const tc = queue.shift();
     if (!tc) return;
+    const name = tc.id.replace(/[#/]/g, "_"), raw = join(coverage, "raw", name);
+    if (coverage) for (const f of [raw, join(coverage, `${name}.txt`)]) rmSync(f, { recursive: true, force: true });
     const r: Record<string, any> = { id: tc.id, ...(await runCase(tc)) };
+    if (coverage) {
+      // only a test that passed as a whole counts, with every action's coverage
+      if (r.status === "pass" && existsSync(raw)) keepCoverage(raw, name);
+      rmSync(raw, { recursive: true, force: true });
+    }
     counts[r.status as string] = (counts[r.status as string] ?? 0) + 1;
     finished++;
     if (onlyPassing) {
@@ -705,9 +736,10 @@ if (onlyPassing) {
   const all = [...latest.values()];
   // results cover only the tests run here, so the list keeps the others
   const passing = new Set(listed());
+  const redundant = new Set(readList(redundantFile));
   for (const r of all) {
-    if (r.status === "pass") passing.add(r.id);
-    else passing.delete(r.id);
+    if (r.status !== "pass") passing.delete(r.id);
+    else if (!redundant.has(r.id)) passing.add(r.id);
   }
   const total = (s: string) => all.filter((r) => r.status === s).length;
   writeFileSync(join(out, "summary.txt"),

scripts/jtreg/select.ts

@@ -10,7 +10,7 @@
 // all. A change to times.tsv alone runs none.
 
 import { execFileSync } from "node:child_process";
-import { readFileSync } from "node:fs";
+import { existsSync, readFileSync } from "node:fs";
 import { dirname, join } from "node:path";
 
 const root = join(import.meta.dirname, "..", "..");
@@ -71,7 +71,17 @@ const patterns = affects.map(([file, dirs]) =>
   [new RegExp(`^${file.replace(/[.+^${}()|[\]\\]/g, "\\$&").replaceAll("*", ".*")}$`), dirs] as const);
 const dirsFor = (file: string) => patterns.find(([re]) => re.test(file))?.[1];
 
-const shardsFor = (n: number) => Array.from({ length: Math.min(8, Math.ceil(n / 400)) }, (_, i) => i + 1);
+// about 10 test-minutes a shard, so a selection spreads over up to 8
+const times = new Map<string, number>();
+for (const l of (existsSync(join(root, timesFile)) ? readFileSync(join(root, timesFile), "utf8") : "").split("\n")) {
+  const [ms, id] = l.split("\t");
+  if (id && Number.isFinite(Number(ms))) times.set(id, Number(ms));
+}
+const median = [...times.values()].sort((a, b) => a - b)[times.size >> 1] ?? 0;
+const shardsFor = (ids: string[]) => {
+  const ms = ids.reduce((n, id) => n + (times.get(id) ?? median), 0);
+  return Array.from({ length: Math.min(8, Math.max(1, Math.ceil(ms / 600_000))) }, (_, i) => i + 1);
+};
 const listed = readFileSync(join(root, passingFile), "utf8").split("\n").map((l) => l.trim()).filter((l) => l && !l.startsWith("#"));
 const under = (id: string, f: string) => id === f || id.startsWith(f + "/") || id.startsWith(f + "#");
 
@@ -109,7 +119,8 @@ function select(base: string | undefined): string[] | "all" {
 
 const arg = process.argv[2];
 const picked = select(arg === "--full" ? undefined : arg);
-const count = picked === "all" ? listed.length : listed.filter((id) => picked.some((f) => under(id, f))).length;
+const chosen = picked === "all" ? listed : listed.filter((id) => picked.some((f) => under(id, f)));
+const count = chosen.length;
 console.log(`filters=${picked === "all" ? "" : picked.join(" ")}`);
-console.log(`shards=${JSON.stringify(count ? shardsFor(count) : [])}`);
+console.log(`shards=${JSON.stringify(count ? shardsFor(chosen) : [])}`);
 console.error(`${count} tests${picked === "all" ? " (all)" : picked.length ? ": " + picked.join(" ") : ""}`);