Agent Manager commited on
Commit
cf4bd43
Β·
1 Parent(s): d735b67

Discover test suites instead of listing them by hand

Browse files

Both package.json files carried the suite list on one line β€” `node a.test.mjs &&
node b.test.mjs && …` β€” so every PR that added a test edited the same line and
any two of them conflicted by construction; six times in the last few days. The
damage is not the conflict, it is the resolution: taking one side drops the
other PR's suite from the run, CI stays green, and the missing coverage is
invisible. `npm test` in each package now discovers what to run.

The rule is `test/*.test.mjs`, then `*.test.mjs` at the package root,
alphabetically within each. A suite that must stay out of the default set says
so in its own header β€” a line containing `am-test: manual` and the reason β€” and
every run prints what it skipped and why, so nothing goes quiet again. That
covers the five that were already excluded by omission: the four server suites
needing Chromium and a full web build (terminal-ui, screenshot-input,
reader-info, mobile) and web's statusMark.render, which keep their own scripts.

Sequential, deliberately. `node --test` would discover the same files but runs
them in parallel, and suites here bind fixed ports (migration 7893, resize 7895,
trace-download 7898) and drive Chromium. Those ports are distinct today only
because whoever added each picked a free number; nothing enforces it, and the
first suite that copies an existing PORT constant would produce a flake that
reads as a product bug. The runner spawns one child at a time and stops at the
first failure with its exit code, which is what the `&&` chain did.

Two server suites start running that never had: test/backup.test.mjs and
test/backup-health.test.mjs, added with the bucket-backup work (#26, #34) and
never referenced by any script. They are node:test files, they pass, they take
45ms between them, and they touch no ports β€” 33 assertions that were being
carried but not run.

scripts/run-suites.mjs ADDED
@@ -0,0 +1,94 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ // Runs a package's test suites: every `*.test.mjs` in `test/`, then every one at
2
+ // the package root, in that order, one at a time.
3
+ //
4
+ // WHY THIS EXISTS. Both package.json files used to carry the suite list by hand
5
+ // β€” `node a.test.mjs && node b.test.mjs && …` on one line. Every PR that added
6
+ // a test edited that line, so any two such PRs conflicted by construction (six
7
+ // times in the last few days), and the natural resolution β€” take one side β€”
8
+ // silently drops the other PR's suite from the run. It stays green and nobody
9
+ // notices the coverage is gone. Discovery removes the shared line: adding
10
+ // `test/foo.test.mjs` is enough to make it run.
11
+ //
12
+ // SEQUENTIAL, DELIBERATELY. `node --test` would also discover these files, but
13
+ // it runs them in PARALLEL by default, and several suites here start a real
14
+ // server on a fixed port (migration on 7893, resize on 7895, trace-download on
15
+ // 7898) or drive Chromium. Those ports do not collide today, but only because
16
+ // whoever added each one picked a free number β€” nothing enforces it, and the
17
+ // first suite that copies an existing PORT constant produces a flake that reads
18
+ // as a product bug. This fleet has already lost time to exactly that symptom.
19
+ // One at a time costs wall-clock and nothing else.
20
+ //
21
+ // EXIT SEMANTICS match the `&&` chain it replaces: the first failing suite stops
22
+ // the run and its exit code is this process's exit code. That holds for both
23
+ // kinds of file here β€” the standalone scripts that print their own results and
24
+ // call process.exit, and the two `node:test` suites, which node exits non-zero
25
+ // for when a test fails (verified, not assumed).
26
+ //
27
+ // OPTING OUT. A suite that must not run in the default set says so in its own
28
+ // header, on a line containing `am-test: manual` plus the reason. It is declared
29
+ // where a reader will see it rather than by absence from a list somewhere else,
30
+ // and every run prints what it skipped and why, so coverage cannot go quiet.
31
+ //
32
+ // Usage: node ../scripts/run-suites.mjs [substring …]
33
+ // (a substring filters to matching suites β€” for running one by hand)
34
+ import fs from 'node:fs';
35
+ import path from 'node:path';
36
+ import { spawnSync } from 'node:child_process';
37
+
38
+ const MANUAL = /am-test:\s*manual\s*[β€”:-]?\s*(.*)/;
39
+ const HEAD_BYTES = 4096; // the marker belongs in the header, not line 900
40
+
41
+ const listDir = (dir) => {
42
+ let names = [];
43
+ try { names = fs.readdirSync(dir); } catch { return []; }
44
+ return names
45
+ .filter((n) => n.endsWith('.test.mjs'))
46
+ .sort((a, b) => a.localeCompare(b, 'en'))
47
+ .map((n) => path.join(dir, n))
48
+ .filter((p) => fs.statSync(p).isFile());
49
+ };
50
+
51
+ // `test/` first, then the package root: the root files are the older ones, and
52
+ // keeping them behind the directory keeps the common case (a new suite in
53
+ // test/) at the front of the run.
54
+ const found = [...listDir('test'), ...listDir('.')];
55
+
56
+ const filters = process.argv.slice(2);
57
+ const suites = [];
58
+ const skipped = [];
59
+ for (const file of found) {
60
+ let head = '';
61
+ try {
62
+ const fd = fs.openSync(file, 'r');
63
+ const buf = Buffer.alloc(HEAD_BYTES);
64
+ head = buf.subarray(0, fs.readSync(fd, buf, 0, HEAD_BYTES, 0)).toString('utf8');
65
+ fs.closeSync(fd);
66
+ } catch { /* unreadable: let node report it */ }
67
+ const manual = head.match(MANUAL);
68
+ if (manual) { skipped.push({ file, why: manual[1].trim() }); continue; }
69
+ if (filters.length && !filters.some((f) => file.includes(f))) continue;
70
+ suites.push(file);
71
+ }
72
+
73
+ const pkg = path.basename(process.cwd());
74
+ if (!suites.length) {
75
+ console.error(`no suites found in ${pkg}/${filters.length ? ` matching ${filters.join(', ')}` : ''}`);
76
+ process.exit(1);
77
+ }
78
+ const plural = (n) => `${n} suite${n === 1 ? '' : 's'}`;
79
+ console.log(`${pkg}: ${plural(suites.length)}\n`);
80
+
81
+ for (const [i, file] of suites.entries()) {
82
+ console.log(`── [${i + 1}/${suites.length}] ${file}`);
83
+ const r = spawnSync(process.execPath, [file], { stdio: 'inherit' });
84
+ const code = r.status === null ? 1 : r.status;
85
+ if (code !== 0) {
86
+ console.error(`\n${file} FAILED (${r.signal ? `signal ${r.signal}` : `exit ${code}`})`);
87
+ console.error(`${plural(i)} had passed before it; the rest were not started.`);
88
+ process.exit(code);
89
+ }
90
+ console.log('');
91
+ }
92
+
93
+ console.log(`${pkg}: ${plural(suites.length)} passed`);
94
+ for (const { file, why } of skipped) console.log(` skipped ${file} β€” ${why || 'marked manual'}`);
server/mobile.test.mjs CHANGED
@@ -5,6 +5,7 @@
5
  // visual viewport is replaced with a controllable EventTarget so the keyboard
6
  // test covers both viewport height and iOS's non-zero offsetTop.
7
  //
 
8
  // npm run test:mobile
9
  import fs from 'node:fs';
10
  import os from 'node:os';
 
5
  // visual viewport is replaced with a controllable EventTarget so the keyboard
6
  // test covers both viewport height and iOS's non-zero offsetTop.
7
  //
8
+ // am-test: manual β€” Chromium, a full web build and port 7896; `npm run test:mobile`.
9
  // npm run test:mobile
10
  import fs from 'node:fs';
11
  import os from 'node:os';
server/package.json CHANGED
@@ -16,7 +16,7 @@
16
  "test:ui": "node terminal-ui.test.mjs && node screenshot-input.test.mjs && node reader-info.test.mjs",
17
  "test:screenshots": "node screenshot-input.test.mjs",
18
  "test:mobile": "node mobile.test.mjs",
19
- "test": "node test/archive.test.mjs && node test/trace-download.test.mjs && node test/attachments.test.mjs && node state-checkpoint.test.mjs && node test/usage.test.mjs && node test/operations.test.mjs && node test/hidden.test.mjs && node test/slowfs.test.mjs && node test/spawn-group.test.mjs && node test/revive.test.mjs && node test/repin.test.mjs && node test/codex-repin.test.mjs && node test/opencode-resume.test.mjs && node test/input-required.test.mjs && node test/terminal-modes.test.mjs && node test/trace-tail.test.mjs && node test/trace-window.test.mjs && node migration.test.mjs && node resize.test.mjs"
20
  },
21
  "engines": {
22
  "node": ">=20.19"
 
16
  "test:ui": "node terminal-ui.test.mjs && node screenshot-input.test.mjs && node reader-info.test.mjs",
17
  "test:screenshots": "node screenshot-input.test.mjs",
18
  "test:mobile": "node mobile.test.mjs",
19
+ "test": "node ../scripts/run-suites.mjs"
20
  },
21
  "engines": {
22
  "node": ">=20.19"
server/reader-info.test.mjs CHANGED
@@ -11,6 +11,7 @@
11
  *
12
  * Set READER_INFO_PUBLIC_DIR to a prebuilt web/dist to skip the build, and
13
  * READER_INFO_PORT to move off the default when suites run in parallel.
 
14
  */
15
  import fs from 'node:fs';
16
  import os from 'node:os';
 
11
  *
12
  * Set READER_INFO_PUBLIC_DIR to a prebuilt web/dist to skip the build, and
13
  * READER_INFO_PORT to move off the default when suites run in parallel.
14
+ * am-test: manual β€” Chromium, a full web build and READER_INFO_PORT; `npm run test:ui`.
15
  */
16
  import fs from 'node:fs';
17
  import os from 'node:os';
server/screenshot-input.test.mjs CHANGED
@@ -12,6 +12,7 @@
12
  * - the creation dialog has no redundant file picker.
13
  *
14
  * Set SCREENSHOT_PUBLIC_DIR to a prebuilt web/dist to skip the build.
 
15
  */
16
  import fs from 'node:fs';
17
  import os from 'node:os';
 
12
  * - the creation dialog has no redundant file picker.
13
  *
14
  * Set SCREENSHOT_PUBLIC_DIR to a prebuilt web/dist to skip the build.
15
+ * am-test: manual β€” Chromium, a full web build and SCREENSHOT_PORT; `npm run test:ui`.
16
  */
17
  import fs from 'node:fs';
18
  import os from 'node:os';
server/terminal-ui.test.mjs CHANGED
@@ -7,6 +7,7 @@
7
  * - ...without letting that lingering selection shadow Ctrl+C's SIGINT
8
  *
9
  * Set TERMUI_PUBLIC_DIR to a prebuilt web/dist to skip the build.
 
10
  */
11
  import fs from 'node:fs';
12
  import os from 'node:os';
 
7
  * - ...without letting that lingering selection shadow Ctrl+C's SIGINT
8
  *
9
  * Set TERMUI_PUBLIC_DIR to a prebuilt web/dist to skip the build.
10
+ * am-test: manual β€” Chromium, a full web build and port 7897; `npm run test:ui`.
11
  */
12
  import fs from 'node:fs';
13
  import os from 'node:os';
web/package.json CHANGED
@@ -13,7 +13,7 @@
13
  "dev": "vite",
14
  "build": "tsc --noEmit && vite build",
15
  "typecheck": "tsc --noEmit",
16
- "test": "node test/stepMarkdown.test.mjs && node test/statusMark.test.mjs && node test/mobileBack.test.mjs && node test/pendingExchange.test.mjs && node test/composerAlign.test.mjs && node test/fileWrapToggle.test.mjs && node test/exchanges.test.mjs && node test/sessionTitle.test.mjs && node test/overviewSort.test.mjs && node test/drafts.test.mjs && node test/settingsMobile.test.mjs && node test/traceWindows.test.mjs && node test/sidebar-dnd.test.mjs",
17
  "test:render": "node test/statusMark.render.test.mjs",
18
  "preview": "vite preview"
19
  },
 
13
  "dev": "vite",
14
  "build": "tsc --noEmit && vite build",
15
  "typecheck": "tsc --noEmit",
16
+ "test": "node ../scripts/run-suites.mjs",
17
  "test:render": "node test/statusMark.render.test.mjs",
18
  "preview": "vite preview"
19
  },
web/test/statusMark.render.test.mjs CHANGED
@@ -6,8 +6,10 @@
6
  // mark, and a state pseudo-element never paints over the inline provider/CLI
7
  // colours carried by bare `.status` dots.
8
  //
9
- // Needs Chromium; run with: node test/statusMark.render.test.mjs
10
- // (not in `npm test`, which stays browser-free β€” see package.json's test:render)
 
 
11
  import assert from 'node:assert/strict';
12
  import fs from 'node:fs';
13
  import path from 'node:path';
 
6
  // mark, and a state pseudo-element never paints over the inline provider/CLI
7
  // colours carried by bare `.status` dots.
8
  //
9
+ // am-test: manual β€” needs Chromium; run with `npm run test:render`.
10
+ // Kept out of the default suite as its own script, unchanged by the discovery
11
+ // change. (The note that used to sit here said `npm test` stays browser-free;
12
+ // that stopped being true when traceWindows.test.mjs joined it.)
13
  import assert from 'node:assert/strict';
14
  import fs from 'node:fs';
15
  import path from 'node:path';