Skip to content

test: run test-all.sh in CI with a summary (#174 follow-up) - #187

Merged
dborup merged 14 commits into
masterfrom
codex/issue-174-test-all-ci
Oct 3, 2026
Merged

dborup merged 14 commits into
masterfrom
codex/issue-174-test-all-ci

Conversation

@adminopenclaw8-sketch

Copy link
Copy Markdown
Collaborator

Relates to #174, #173, #177, #178

test-all.sh now runs every file and prints a summary. CI runs it as the JS unit step, and a self-test makes sure that no root test file can go unrun again. It also covers the review nits from #173, #177 and #178, and an inventory of the root tests that no runner registered.

No production code changed (git diff origin/master...HEAD -- public/ is empty), and no npm dependencies were added. Fork guards are unchanged: 9 × github.repository == 'Kpa-clawbot/CoreScope' on both base and head. No guard lines, triggers, permissions or concurrency were touched.

Base: cbd562cd, with origin/master (bedbe1f1, #167) merged in through a normal merge commit (535d6e88). The only conflict was the one-line test-issue-147-packets-url-detail-params.js registration in test-all.sh.

1. test-all.sh: every file runs, with a summary at the end

  • set -e is gone. Each file runs through run <file> (POSIX sh). The script counts passes and failures, lists the failed files at the end and exits 1 if any file failed.
  • test-live-anims.js is registered. It was repaired in test: issue 174 part A — emoji gate, live anim, naive banner, observers headings #177 but never registered.
  • test-test-all.js is the self-test. It runs the real test-all.sh under /bin/sh and dash (when present) in a temp dir, with its file list swapped for fixture files. It checks that:
    • a failing file does not stop the run;
    • a thrown error, a non-zero exit and a missing file are all listed;
    • the exit code is non-zero, and an all-green run exits 0;
    • every registered file exists and is registered only once;
    • no registered file loads playwright, launches Chromium or reads BASE_URL.
  • test-privacy-page.js asserted the literal set -e line and node test-privacy-page.js. It now asserts the run line and the exit-1 path, which is the property it was guarding.

Demo in a copy, with two deliberately failing files (throw at the top of test-aging.js, and process.exitCode = 1 in test-hash-color.js): all 200 files ran, and the run ended with exit 1:

═══════════════════════════════════════
  198 passed, 2 failed (200 files)
  Failed:
    test-aging.js
    test-hash-color.js
═══════════════════════════════════════

2. CI: one list, sh test-all.sh replaces the JS unit lines

The "Run JS unit tests" step in Go Build & Test listed 88 files. 44 of them were also in test-all.sh, and 44 were only in deploy.yml. The 64 other test-all.sh files never ran in CI. I moved the 44 deploy-only files into test-all.sh and replaced the step body with node --version and sh test-all.sh. The step fails if any file fails: sh exits 1, and the job shell is bash -e.

Timings with Node 22.22.1, run locally, 3 runs each:

Variant Files Time
old step (deploy.yml lines) 88 ~7.9 s
test-all.sh before this PR 108 ~8.9 s
chosen: one test-all.sh step (after moving the deploy-only files) 152 ~12.2 s
alternative: keep the old lines and add a test-all.sh step 88 + 108 ~16.8 s, 44 files twice
final test-all.sh (with the inventory below) 200 ~14.7 s

On master, CI took about 5 s for the old step (run 37105041262, timestamps at second resolution). The chosen variant has no double runs and leaves one list to maintain. The CI unit job has no npm ci, so I also ran test-all.sh without node_modules: 200/200.

3. Inventory: root tests that no runner ran

On cbd562cd, 70 root test-*.js files were run by neither test-all.sh nor deploy.yml, and 8 of them have "e2e" in the name. A 71st, test-rx-coverage-mobile-nav-e2e.js, appears in deploy.yml only in a comment. The non-E2E files were run with Node 22, without node_modules (as in the CI unit job), with a 120 s limit.

test-test-all.js now also fails when a root test-*.js runs in neither test-all.sh (run <file>) nor deploy.yml (node <file>), unless it is listed in KNOWN_UNREGISTERED with a reason. An entry that is deleted, or that gets registered, has to be removed, so the list can only shrink.

Registered: 49 unit files and 4 E2E files

The 45 green unit files

test-1108-region-hide-nodes.js, test-a11y-1715-dark-role-swatches.js, test-analytics-distance-view-path.js, test-analytics-wardriving-tab.js, test-anl1-tooltip-render.js, test-channel-color-picker.js, test-channel-decrypt-ecb.js, test-channel-decrypt-m345.js, test-channel-live-decrypt-userprefix.js, test-channel-live-decrypt.js, test-color-picker-ux.js, test-compare-flood-filter.js, test-compare-overlap.js, test-embed-mode-1369.js, test-geofilter-draft.js, test-hash-color.js, test-issue-1166-first-seen-column.js, test-issue-1189-composed-cell.js, test-issue-1189-live-iata-badge.js, test-issue-1415-packets-layout.js, test-issue-1488-marker-stroke-vars.js, test-issue-1496-reset-all-complete.js, test-issue-1563-aggregate-and-inflight.js, test-issue-1574-live-map-max-nodes.js, test-issue-1606-pagination.js, test-issue-1644-redesign.js, test-issue-1648-followup-phosphor-leaks.js, test-issue-1648-m5-emoji-scan.js, test-issue-1668-m2-contrast.js, test-issue-1825-observer-node-cross-links.js, test-issue-1836-crossnav-case-normalization.js, test-issue-1843-node-qr-quiet-zone.js, test-live-dt-cap-1524.js, test-live-legend-helper.js, test-node-reach-coverage-debounce.js, test-observer-detail-view-path.js, test-observer-naive-clock-1478.js, test-path-inspector.js, test-payload-labels-content.js, test-payload-labels-namespace.js, test-perf-anomaly.js, test-perf-render-1258.js, test-pull-to-reconnect.js, test-rx-coverage-config-race.js, test-slideover-1056-rowsel-strict.js

Red unit files: not fixed here, proposed for a follow-up issue

The causes were checked against git history. Where the table says "test passes at X^ and fails at X", that was run.

File First failure Likely cause
test-channel-colors.js getRowStyle … assigned GRP_TXT channel: should have left border (19/2) 68a4628e (Kpa-clawbot#675) changed the row style from a 4px border plus tint to a 3px border only. Intentional; stale test.
test-channel-ux-followup.js new copy points at the ✕ button for individual key removal (28/1) b812a98a (Kpa-clawbot#1648 M3) reworded the hint to "Use the close button…". Stale.
test-channel-ux-round2.js iconBtn('ch-share-btn', ...) is called with '📤 Share' (13/1) b812a98a (Kpa-clawbot#1648 M3): the label is now the #ph-share-network sprite plus " Share". Stale.
test-drag-manager.js Escape during drag reverts to corner position (18/1) 2b45f787 (Kpa-clawbot#1567) switched to removeAttribute('data-dragged'). The test's mock does not update dataset; a real DOM does. Mock gap.
test-fluid-scaffolding.js spacing token --space-xs declared with clamp() (2/13) f0addfda (Kpa-clawbot#1668) added a palette :root block first, and the test only reads the first :root. The tokens still exist.
test-hop-resolver-affinity.js Should pick NodeA (geo-closest) — got: NodeB (16/1) 2b9f3056 (Kpa-clawbot#874), haversine centroid. The test passes at 2b9f3056^ and fails at 2b9f3056. With haversine, NodeB really is closer (70.88 vs 71.06 km), so the fixture geometry is wrong.
test-issue-1470-card-bg-contrast.js [data-theme="dark"] block sets --card-bg…: not found (9/1) e2212f50 (Kpa-clawbot#1627) added a style.css comment containing [data-theme="dark"], and indexOf lands on the comment. Passes at e2212f50^.
test-issue-1646-compare-polish.js .compare-vs font-size < .compare-select font-size: could not parse (13/1) d954ea74 (Kpa-clawbot#1668 M5) added a comment with font-size:10px inside .compare-vs, and the parser reads the comment.
test-packets.js getDetailPreview handles CHAN type (115/13) 30627454 (Kpa-clawbot#1648 M2): 13 emoji assertions; the code now renders #ph-* sprites. Mechanical, but 13 edits.
test-perf-disk-io-1120.js WAL >100 MB fires ⚠️ flag (10/5) 30627454: ⚠️ is now #ph-warning. The "no ⚠️" negatives pass vacuously. The backfill cases target the old single-snapshot detector, which a26a412c (Kpa-clawbot#1593) replaced. Needs rework.
test-table-sort.js Cannot find module 'jsdom' jsdom is not a dependency. Port to vm or delete.
test-marker-outline-weight.js ERR_MODULE_NOT_FOUND for the Playwright Test runner package A Playwright Test spec (ESM, page.goto). The runner package is not a dependency, and the spec also needs a server.
test-touch-targets.js .nav-btn: rendered 44.0x44.0 … expected >=48x48 (5 failures, run with node_modules and Chromium, no server) The test is from eaf14a61 (Kpa-clawbot#1060, 48px targets), and the current style.css sets 44px minimums. Whether 48 or 44 is the intended target needs a decision. Not classified as stale or as a regression.

E2E files that stay unregistered

All were run against a local server built with CI's fixture recipe: freshen-fixture.sh, the Kpa-clawbot#1486/Kpa-clawbot#1791 seed block from deploy.yml, corescope-migrate, and seed-2073-route-adverts.sql.

File Result Should it go in the Playwright step?
test-channel-modal-e2e.js 12/2: "Add Channel button is visible" (button text is now "+ Add"), and "sidebar renders three sections" times out Yes, after updating it for the chat-app sidebar (Kpa-clawbot#1376)
test-node-reach-e2e.js TimeoutError waiting for #nqMap .leaflet-container Unclear. test-node-reach-coverage-e2e.js already runs in CI, so check for overlap first. Cause not investigated.
test-issue-1522-trace-url-sync-e2e.js needs the Playwright Test runner package (not a dependency) Only if ported to plain playwright
test-path-inspector-e2e.js same Only if ported (test-path-inspector-coverage-e2e.js already runs)
test-rx-coverage-mobile-nav-e2e.js SKIP: clientRxCoverage is disabled in the fixture config No; the existing deploy.yml comment already says so

4. Review nits

Change Test that catches it
a test-pr-1490-live-map-gpu-animations-e2e.js: drain bound 5000 → 2500 ms. The fade-pane step now counts Leaflet renderer canvases per pane before and after the burst: exactly one on the animations pane, and none added elsewhere. Before, the contrail alone satisfied the check. <vp>: the animations drain to 0, <vp>: the fading polylines render on the animations pane only
b test-rx-coverage-escape.js case 5: mounts the real page, runs createMap's deferred drawCoverage(), answers /api/rx-coverage with hostile node names and an unnamed node with a hostile prefix, and checks the exact escaped labels in the hex tooltip (coverageNodeRow). The cases share a mountPage harness. public/rx-coverage.js is unchanged. 5. hex tooltip: hostile node names and prefixes are escaped
c test-packet-path-map.js: all four role icons and the first-to-hear #ph-flag are aria-hidden sprite icons at the start of their tooltips, and no tooltip icon lacks aria-hidden="true" role icons and the first-to-hear flag render as aria-hidden sprite icons
d test-issue-1407-cb-preset-propagation.js §H: document.body = null, getComputedStyle(null) throws (as in browsers), and the value exists only on documentElement document.body null: ROLE_COLORS.repeater resolves on documentElement
e test-issue-1648-m3-emoji-scan.js: 🏓 and 🤖 moved to ALLOW_EXACT['channels.js']. Each must occur exactly once; only that occurrence is cut out, and the rest of the line is still scanned. The 🤖 entry is now the full avatar element. channels.js clean … / app.js clean …

Note for (a): the commit message of b4c96772 gives 665–682 ms for "10/10 runs against the instrumented frontend". Those 10 runs actually hit a server that still served plain public/, because my local start script stopped the wrong process. I reran with the instrumented build: the served live.js contains __coverage__. All 10 runs were green, at 668–684 ms per viewport. The bound (2500 ms) is unaffected.

Mutants

Each mutant ran in a copy of the tree (rsync, never in the worktree). The E2E mutants were served from a copy of public/ on a second server.

# Mutant Old test New test Caught by
a1 live.js ~4339: pane: removed from the main fade line only green red (desktop + mobile) the fading polylines render on the animations pane only ("added a renderer canvas to overlay")
a2 pane: removed from the contrail only green red same
a3 animation 660 → 3000 ms (drain ~3 s) green (5 s bound) red the animations drain to 0
b1 coverageNodeRow: name not escaped – red case 5
b2 prefix not escaped – red case 5
b3 label always the prefix – red case 5
b4 escapeHtml without & – red cases 1, 2, 5
c1 phIcon without aria-hidden – red role icons / first-to-hear case
c2 first-to-hear flag removed – red same
c3 flag swapped for another glyph – red same
c4 flag inlined without aria-hidden – red same
c5 sensor icon dropped / client icon dropped – red / red same
d1 getComputedStyle(document.body) only – red §H
d2 documentElement only (pre-#178) – red §G
e1 extra ✅ on the pong line green red channels.js clean
e2 <div>🤖</div> in app.js green red app.js clean
e3 📡 after the 🤖 avatar on the same line green red channels.js clean
e4 second avatar in channels.js / pong text in app.js / 🏓 removed (stale exception) – red each channels.js clean / app.js clean
t1 test-all.sh: old semantics (bare node under set -e) – red failures do not stop the run (sh + dash)
t2 exit 1 removed / failure counted as a pass / failed file not recorded – red each same
t3 duplicate / missing / E2E file registered – red each no file is registered twice / every registered file exists / no registered file needs a browser or a server
t4 new unregistered root test / registered file dropped / E2E line dropped / allowlisted file registered or deleted – red each every root test-*.js runs … / KNOWN_UNREGISTERED has no stale entries
t5 set -e added in front of the new run() – green Equivalent mutant: node runs inside an if condition, which set -e ignores
p test-all.sh without the exit-1 path / old node registration line – red test-privacy-page.js is registered exactly once in test-all.sh
m room count off by one / letter swapped / pill without letter – red test-map-clustering.js pill case
pc occupied corners not skipped / feed default bl → tl / cycle reversed – red test-panel-corner.js
cv server home always replaced / config dropped / not deep-copied – red test-customizer-v2.js

Possible production bug found, not fixed (public/packets.js is on the do-not-touch list)

When 30627454 (Kpa-clawbot#1648 M2) migrated ▶/▼ to sprites, the collapsed packet-group toggle became #ph-caret-up (public/packets.js:2359), while the expanded one is #ph-caret-down. Collapsed toggles elsewhere use ph-caret-right, which is in the sprite. Collapsed group rows probably show an up-caret. Found while diagnosing test-packets.js, checked in the source and the diff, not checked in a browser.

Other notes

  • The emoji regex in the 1648 scans covers U+1F300–1FAFF and U+2600–27BF only, so ⭐ (U+2B50) and other U+2B00 symbols are not caught. Noticed during the e4 mutants; not changed here.
  • test-issue-1697-mqtt-mobile-e2e.js has "e2e" in its name but is a vm unit test (it was already in the unit step). It moved into test-all.sh, so the E2E check looks at content (playwright, chromium.launch, BASE_URL), not names.

Verification (local, macOS, Node 22.22.1)

Run Result
sh test-all.sh (/bin/sh and dash), with and without node_modules 200/200, exit 0
sh test-all.sh with two deliberately failing files (copy) exit 1, both files listed, 200 run
test-pr-1490-…-e2e.js, plain public/ 14/14 (11 runs)
test-pr-1490-…-e2e.js, public-instrumented/ 14/14, 10/10 runs, 668–684 ms
4 newly registered E2E files green, 3/3 each
Fork guards 9 → 9

Not verified

  • The packets.js caret: source only, no browser.
  • The other E2E files in the Playwright step were not run locally; CI runs them.
  • test-touch-targets.js: whether 44px or 48px is the intended target.
  • test-node-reach-e2e.js: the cause of the timeout was not investigated.
  • Linux /bin/sh (dash) locally: only macOS /bin/dash was tested. CI's ubuntu runner covers it.

🤖 Generated with Claude Code

dborup and others added 14 commits October 3, 2026 10:10
test-all.sh used `set -e`, so it stopped at the first failing file and
the rest never ran. Each file now runs through run(): failures are
counted, the summary lists every failed file, and the script exits 1 if
any file failed.

- register test-live-anims.js (fixed in #177, never registered)
- test-test-all.js: runs the real test-all.sh under /bin/sh and dash
  with fixture files (a failure does not stop the run, is listed, exit
  is non-zero; all green exits 0) and checks the file list (exists, no
  duplicates, no Playwright/BASE_URL files)
- test-privacy-page.js asserted the literal `set -e`; it now asserts the
  `run` registration line and the exit-1 path

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The "Run JS unit tests" step in the Go Build & Test job listed 88 files,
44 of which test-all.sh also listed, while the 64 other test-all.sh
files never ran in CI. The 44 files that were only in deploy.yml move
into test-all.sh, and the step now runs `sh test-all.sh`: one list,
every file runs once, and the step fails if any file fails.

Node 22, local, 3 runs: old step (88 files) ~7.9 s, test-all.sh after
this change (152 files) ~12.2 s. Keeping the old lines and adding a
test-all.sh step would take ~16.8 s and run 44 files twice.

Fork guards unchanged (9).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
70 root test-*.js files were run by neither test-all.sh nor deploy.yml.
Of the 62 without "e2e" in the name, these 45 pass on Node 22 without
node_modules or a server (3 consecutive full test-all.sh runs green).
test-live-anims.js was registered in the first commit. The 16 red ones
are listed in the PR and not changed here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Of the eight root *e2e*.js files that no runner registered, these four
pass locally (3/3 runs each, Node 22, Chromium 1208):

- test-a11y-1705-subpath-hop-prefix-e2e.js
- test-a11y-1719-contrast-root-causes-e2e.js
- test-nodes-favorite-rerender-e2e.js
- test-packet-trace-alignment-e2e.js

None uses the fixture server. The other four stay out (listed in the PR).

Fork guards unchanged (9).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test-test-all.js now lists every root test-*.js and requires each one
to run in test-all.sh (`run <file>`) or deploy.yml (`node <file>`). The
21 files that cannot run yet are in KNOWN_UNREGISTERED with a reason;
an entry that is deleted or gets registered must be removed, so the
list can only shrink.

Mutants (all red): a new unregistered root test; an allowlisted file
registered; a test-all.sh line dropped; an E2E line dropped from
deploy.yml; an allowlisted file deleted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…174)

Review nit from #177: the tests covered the repeater and room icons
only. The new case checks all four role icons and the first-to-hear
#ph-flag, each as an aria-hidden sprite icon at the start of its
tooltip, and that no tooltip icon lacks aria-hidden="true".

Mutants (all red): phIcon without aria-hidden; flag removed; flag
swapped for another glyph; flag inlined without aria-hidden; sensor or
client icon dropped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#174)

Review nit from #178: section G covered the <body> read, but not the
`document.body || document.documentElement` fallback. roles.js loads in
<head>, where body is still null. Section H nulls body, makes
getComputedStyle throw on null (as browsers do) and serves a non-default
value on documentElement only.

Mutants: "getComputedStyle(document.body)" only is red in H;
"documentElement only" (the pre-#178 code) is red in G.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…js-only (#174)

Review nit from #178: the 🏓 and 🤖 exceptions were line substrings in
the global allowlist, so any emoji on the pong or avatar line passed,
and the same text in another M3 file passed too. They are now in
ALLOW_EXACT['channels.js']: each must occur exactly once, only that
occurrence is cut out, and the rest of the line is still scanned.

Mutants, old test → new test: extra ✅ on the pong line green → red;
`<div>🤖</div>` in app.js green → red; 📡 after the avatar green → red.
Also red: a second avatar in channels.js, the pong text in app.js, the
🏓 removed (stale exception).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…174)

Follow-up from #173: coverageNodeRow (the RX coverage hex tooltip)
escapes node names and prefixes, but no test covered it. Case 5 mounts
the real page, lets createMap's deferred drawCoverage() run, answers
/api/rx-coverage with one hex of hostile node names plus an unnamed node
with a hostile prefix, and checks the exact escaped labels in the
recorded tooltip. public/rx-coverage.js is unchanged; the vm harness is
shared with cases 1-4 (mountPage).

Mutants (all red in case 5): name unescaped; prefix unescaped; label
always the prefix; escapeHtml without & escaping (also red in 1 and 2).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…vas (#174)

Review nits from #173:

- Drain bound 5000 → 2500 ms. The burst drains in ~0.7 s locally and in
  CI. 10/10 runs against the instrumented frontend: 665–682 ms per
  viewport.
- The fade-pane step only required a renderer canvas on the animations
  pane, which the contrail alone creates, so removing `pane:` from the
  main fade line (live.js ~4339) stayed green. The step now counts
  Leaflet renderer canvases per pane before and after the burst: exactly
  one on the animations pane, and no new one on any other pane.

Mutants, old test → new test (served from a copy of public/): main line
without `pane:` green → red; contrail without `pane:` green → red;
animation 660 → 3000 ms green → red.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…le (#174)

The pill assertion expected bare counts (>3<). Since 40aa02b (Kpa-clawbot#1360,
"cluster pill shows letter+count") the pills read R3 / C2 / M1. The
other 4 cases already passed. Registered in test-all.sh and removed from
KNOWN_UNREGISTERED.

Mutants (red): room count off by one; repeater letter swapped; pill
without the letter (pre-Kpa-clawbot#1360).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he file (#174)

Since b74a64c (Kpa-clawbot#1799) live.js throws at load when window.PayloadLabels
is missing, so all 14 cases failed in the sandbox. index.html loads
payload-labels.js first; the sandbox now does the same, like the
analytics tests. Registered in test-all.sh and removed from
KNOWN_UNREGISTERED.

Mutants (red): occupied corners not skipped; feed default bl → tl;
corner cycle reversed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
"returns server defaults when overrides is {}" passed a config without
`home`; since 54fab05 (Kpa-clawbot#525) computeEffective fills in DEFAULT_HOME
then, so the deep-equal failed (59/60). The fixture now carries a
server `home`, as the real config does. Registered in test-all.sh and
removed from KNOWN_UNREGISTERED.

Mutants (red): server home always replaced by DEFAULT_HOME (red in this
case); server config dropped; server config not deep-copied.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dborup

dborup commented Oct 3, 2026

Copy link
Copy Markdown
Owner

Review — CS-MacBook review PR#187 test-all i CI — head b23d8d9

Dom: APPROVE med nits (approve with nits)

Independent, read-only review of bedbe1f1..b23d8d9f (14 commits including the master merge 535d6e88, 12 files). I worked on git archive copies, changed nothing in the repo, pushed nothing and touched no shared environment. The head was b23d8d9fd5a03ae99c28350d030a22ec05482716 before and after (checked with git ls-remote).

Tags: [F] browser run, [T] test or CI run, [A] analysis of code, [K] not checked.

No blocking findings. Everything the description claims held up. The nits below are small and none needs to block the merge.

Findings

Prio Where Finding
P3 test-test-all.js:157 The registry check finds deploy.yml references with /\bnode (test-…\.js)\b/, which also matches a commented-out line. A new root test whose only "registration" is # node test-x.js in deploy.yml passes the guard (mutant G3). Suggestion: skip lines that start with #. [T]
P3 test-test-all.js:131 KNOWN_UNREGISTERED only gets a "no stale entries" check (:167). A new file can be added to the list and the guard stays green (mutant G4). "The list can only shrink" holds by convention and comment, not by test. A pinned length, for example assert(Object.keys(KNOWN_UNREGISTERED).length <= 18), would enforce it. [T]
P3 .github/workflows/deploy.yml:137 The unit step runs in the Go job, and actions/setup-node appears only in the E2E job (:243, Node 22). The unit step therefore uses the runner's preinstalled Node. The CI log shows v22.23.3 today, but it is not pinned, so a runner image update can change it. Adding setup-node@v5 with node-version: '22' to the Go job would pin it. [T][A]
P3 .github/workflows/deploy.yml:524-525 test-a11y-1705-subpath-hop-prefix-e2e.js and test-a11y-1719-contrast-root-causes-e2e.js use no browser and no server: both exit 0 with PLAYWRIGHT_BROWSERS_PATH pointing nowhere. CHROMIUM_REQUIRE=1 is a no-op for them (they never read it, and neither does test-packet-trace-alignment-e2e.js on :527). They could go in test-all.sh and fail in about 12 s in the Go job instead of after the E2E job starts. The guard would accept them there. [T][A]
P3 test-all.sh:13-22 No per-file timeout, and a file that calls process.exit(0) early counts as passed. Both existed before this PR (each node X line had the same property), so this is not a regression. For a hang, the Go job's 40 minute limit is the only stop. [T][A]

Answers to points 1-6

1. CI step.

  • All 88 removed node test-*.js lines are in test-all.sh: 88 unique removed lines, 0 missing, no duplicates. The old test-all.sh files (106) are all still there too. 44 files were in both lists and are now registered once. [A]
  • The new step is node --version then sh test-all.sh. The Go job's default shell is bash -e {0} (visible in the log), so the step's status is sh's exit status. test-all.sh ends with exit 1 when any file failed, and exits 0 otherwise. No pipe is involved, so pipefail is irrelevant for this step. [T][A]
  • Node: CI ran the step on v22.23.3 (see the P3 on pinning). node_modules is not present when the step runs, and 200/200 pass on a clean git archive without it. [T]
  • The suite is order independent: the file list reversed gave 200/200 and exit 0. A full run leaves no stray files in the tree. [T]
  • The four new Playwright lines (:524-527): test-nodes-favorite-rerender-e2e.js starts its own server and test-packet-trace-alignment-e2e.js serves its own page. The two contrast gates need neither a server nor a browser, so no BASE_URL is needed. The Playwright job uses shell: bash, which in Actions is bash -eo pipefail, so a failing file fails the step through the | tee. All four pass locally and in CI (PASS lines for 1705 and 1719, all favorite-rerender checks, 17 checks for the trace alignment). [F][T]
  • Fork guards: 9 × github.repository == 'Kpa-clawbot/CoreScope' on both base and head. [A]

2. test-all.sh.

  • POSIX: the script ran identically under macOS sh (bash in sh mode) and under dash, both 200/200 and exit 0. It only uses $(( )), a function and if/for. [T]
  • Every file runs after a failure. Copy with two failing files (one throw, one process.exitCode = 1): 198 passed, 2 failed (200 files), both files listed under Failed:, exit 1. [T]
  • No file runs twice or not at all: 200 unique run lines, all files exist. 338 root test-*.js = 200 unit + 120 in deploy.yml + 18 KNOWN_UNREGISTERED, and no overlap between the unit and deploy lists. [A]
  • A file that exits 0 early passes (confirmed with process.exit(0) at the top of test-aging.js: 200/200). There is no per-file timeout. See the last P3.

3. test-test-all.js.

  • It fails on a new empty root test (9 passed, 1 failed, message names the file) and on a file removed from test-all.sh (mutants G1, G2). [T]
  • It does not catch a commented-out deploy.yml reference (G3) and does not stop the exception list from growing (G4). See the P3 findings.
  • I ran all 18 KNOWN_UNREGISTERED files. The 10 "red" unit tests exit 1 and the first failing assertion matches the stated reason in each case. The jsdom and @playwright/test entries fail with module-not-found, and package.json has playwright but not @playwright/test or jsdom. [T]
  • Not verified: the reasons for the five E2E entries and test-touch-targets.js ("red in Chromium", "skips while clientRxCoverage is off"), because they need a running app and its config. [K]

4. The 45 newly registered tests and the three repaired ones. The block is 48 files: 45 new plus the three repaired files. Each repair follows an intended change in the production code:

  • test-map-clustering.js: role pills read letter + count since regression(map): #1357 cluster role pills lost the count number — show letter+count not letter-only Kpa-clawbot/CoreScope#1360. public/map.js:2960-2968 builds letter + n. [A]
  • test-panel-corner.js: index.html:177 loads payload-labels.js before live.js at :227, and live.js throws without it. [A]
  • test-customizer-v2.js: computeEffective fills in DEFAULT_HOME when the server config has no home, so the fixture now carries one. [A]
  • None of them weakened: the assertions are the same or stricter (>R3< instead of >3<). Mutants U1, U2, U3 are caught by the repaired files themselves. [T]

5. Nits 4a-4e. Each described mutant is caught, and the test that catches it is named in the table below.

  • 4a: the canvas-per-pane check catches a fade line that leaves the animations pane even though the contrail stays (E1a). The old count >= 1 check would have passed that. The 2500 ms bound catches an engine that drains in about 4 s (E2), which the old 5000 ms bound would have passed. [F]
  • Stability of 2500 ms against public-instrumented/: 10 of 10 local runs pass, drain 667-680 ms on both viewports. 6 more runs with 20 busy loops on a 10-core machine: 663-691 ms. In CI the drain was 742 ms (desktop) and 675 ms (mobile). That is a margin of about 3.4x. The server was confirmed by port, process and working directory, and the served roles.js was instrumented. [F][T]
  • 4b, 4c, 4d, 4e: see U4-U10.

6. Merge. 535d6e88 merged master bedbe1f1 cleanly apart from the one registration line. Master has since moved to ae595682 (a Go-only change that touches no file in this PR). git merge-tree of the head against it exits 0 with no conflicts, and no new root test-*.js came in, so the guard still holds. [A]

Test counts

  • sh test-all.sh on head, Node v26.0.0 (Node 22 is not installed locally): 200 passed, 0 failed, exit 0 (also under dash, and in reversed order). [T]
  • Same suite in CI, Node v22.23.3: 200 passed, 0 failed (200 files), about 12 s. test-test-all.js: 8 passed in CI, 10 locally (it also runs under dash). [T]
  • Four new E2E lines: 4 of 4 pass locally and in CI. test-pr-1490-live-map-gpu-animations-e2e.js: 10 of 10 (14 checks each), plus 6 of 6 under load, and 14 of 14 in CI. [F][T]

Mutants

# Mutant Caught by
U1 map.js pill drops the role letter test-map-clustering.js, test-issue-1360-pill-letter-count.js
U2 live.js corner cycle skips a corner test-panel-corner.js
U3 computeEffective drops nodeColors test-customizer-v2.js
U4 hex tooltip node name not escaped test-rx-coverage-escape.js (case 5)
U5 hex tooltip prefix not escaped test-rx-coverage-escape.js (case 5)
U6 roles.js has no documentElement fallback when <body> is null test-issue-1407-cb-preset-propagation.js (H)
U7 first-to-hear icon changed from flag test-packet-path-map.js
U8 tooltip icon loses aria-hidden test-packet-path-map.js
U9 second emoji added on the allow-listed pong line test-issue-1648-m3-emoji-scan.js
U10 allow-listed pong text copied into another file test-issue-1648-m3-emoji-scan.js
U11 test-all.sh exits 0 on failure test-test-all.js, test-privacy-page.js
U12 set -e added to test-all.sh survives, equivalent: the run function calls node inside if, so set -e changes nothing
U13 failure not counted test-test-all.js
U14 failed file not listed in the summary test-test-all.js
U15 a file registered twice test-test-all.js
U16 an E2E file added to the unit list test-test-all.js
E1a fade line leaves the animations pane (contrail stays) test-pr-1490-…-e2e.js "animations pane only"
E1b both fade lines leave the pane same test
E2 animation engine drains in about 4 s same file, drain step at 2500 ms
G1 new empty root test test-test-all.js registry check
G2 file dropped from test-all.sh test-test-all.js registry check
G3 commented-out deploy.yml reference not caught (see P3)
G4 new file added to KNOWN_UNREGISTERED not caught (see P3)

CI on head (run 37109895646)

  • Go Build & Test: pass.
  • Playwright E2E Tests: pass.
  • Build & Publish Docker Image: pass.
  • Release Artifacts, Deploy Staging, Publish Badges & Summary: skipped, as expected for a PR.

Not verified

  • Node 22 locally (v26 only). The CI result on v22.23.3 stands in. [K]
  • Whether every one of the 120 deploy.yml root tests sits in an unconditional step. [K]
  • The reasons for the five E2E KNOWN_UNREGISTERED entries and test-touch-targets.js. [K]
  • CI timing on slower runners than the one used here, beyond the single CI run above. [K]

@dborup
dborup marked this pull request as ready for review October 3, 2026 09:25
@dborup
dborup merged commit c94a006 into master Oct 3, 2026
6 checks passed
dborup added a commit that referenced this pull request Oct 4, 2026
test: fix or retire the orphan tests left after #187 (#189)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants