Repository navigation
Description
Summary
PR #187 (the #174 follow-up) wires test-all.sh into CI. It registers 45 green orphan root tests and repairs 3 more. 13 orphan unit tests and 4 orphan E2E files are still red. They stay on test-all.sh's known-unregistered list until they are fixed or deleted. #187's PR description has the full table with the first failure and the traced cause. This issue tracks fixing them.
It also covers one small UI bug that the same work found.
Red unit tests (state on master, causes traced in #187)
| File | First failure | Likely cause | Suggested action |
|---|---|---|---|
test-channel-colors.js |
GRP_TXT row should have left border (19/2) | 68a4628e (#675) changed row style to a 3px border only |
Update expectation |
test-channel-ux-followup.js |
hint points at ✕ (28/1) | b812a98a (Kpa-clawbot#1648 M3) reworded to "Use the close button…" |
Update expectation |
test-channel-ux-round2.js |
'📤 Share' label (13/1) |
b812a98a: now #ph-share-network sprite + " Share" |
Update expectation |
test-drag-manager.js |
Escape during drag reverts corner (18/1) | 2b45f787 (Kpa-clawbot#1567) uses removeAttribute('data-dragged'); the mock does not update dataset |
Fix the mock |
test-fluid-scaffolding.js |
--space-xs declared with clamp() (2/13) |
f0addfda (Kpa-clawbot#1668) added an earlier :root block; the test reads only the first |
Parse all :root blocks |
test-hop-resolver-affinity.js |
expected NodeA (geo-closest), got NodeB (16/1) | 2b9f3056 (Kpa-clawbot#874) haversine: NodeB really is closer (70.88 vs 71.06 km) |
Fix the fixture coordinates |
test-issue-1470-card-bg-contrast.js |
dark block sets --card-bg: not found (9/1) |
e2212f50 (Kpa-clawbot#1627) added a CSS comment containing [data-theme="dark"] |
Make the parser skip comments |
test-issue-1646-compare-polish.js |
.compare-vs font-size could not parse (13/1) |
d954ea74 (Kpa-clawbot#1668 M5) added a comment with font-size:10px |
Make the parser skip comments |
test-packets.js |
getDetailPreview handles CHAN type (115/13) |
30627454 (Kpa-clawbot#1648 M2): 13 emoji assertions; the code renders #ph-* sprites now |
Update the 13 expectations |
test-perf-disk-io-1120.js |
WAL >100 MB fires |
30627454 (#ph-warning); the backfill cases target the old detector that a26a412c (Kpa-clawbot#1593) replaced |
Rework |
test-table-sort.js |
Cannot find module 'jsdom' |
jsdom is not a dependency |
Port to vm or delete |
test-marker-outline-weight.js |
Playwright Test runner not installed (ESM spec) | Needs @playwright/test and a server |
Port to plain playwright or delete |
test-touch-targets.js |
.nav-btn 44×44, expected ≥48×48 (5 failures) |
Test from eaf14a61 (Kpa-clawbot#1060, 48px); current CSS is 44px. The house rule (Kpa-clawbot#2052) says 48 |
Decide 48 vs 44, then fix CSS or test |
Red or unregistered E2E files
| File | Problem | Suggested action |
|---|---|---|
test-channel-modal-e2e.js |
12/2: button text is now "+ Add"; "three sections" times out (Kpa-clawbot#1376 chat-app sidebar) | Update and register |
test-node-reach-e2e.js |
Timeout waiting for #nqMap .leaflet-container |
Check overlap with test-node-reach-coverage-e2e.js (already in CI); fix or delete |
test-issue-1522-trace-url-sync-e2e.js |
Needs the Playwright Test runner | Port to plain playwright or delete |
test-path-inspector-e2e.js |
Same | test-path-inspector-coverage-e2e.js already runs; likely delete |
test-rx-coverage-mobile-nav-e2e.js is skipped on purpose: the fixture has clientRxCoverage off, as documented in deploy.yml.
UI bug: collapsed packet groups show an up-caret
public/packets.js ~2359 (master bedbe1f1): a collapsed group header renders #ph-caret-up (▲), and an expanded one renders #ph-caret-down. Before 30627454 (Kpa-clawbot#1648 M2, the emoji → Phosphor migration), a collapsed row showed ▶. The collapsed state should use a right-pointing caret (ph-caret-right) or whatever matches the expand affordance elsewhere. Seen in the source only; not checked in a browser.
Acceptance
- For every file: fix it (with a mutant that turns it red), or delete it and name the test that covers the same thing. When fixed, remove it from
KNOWN_UNREGISTEREDintest-test-all.jsand register it intest-all.shor the Playwright step. - The
test-touch-targets.jsdecision (48 vs 44 px) is recorded. - The caret fix has a test. Do not touch
public/packets.jswhile another open PR edits it. - No new npm dependencies. Fork guards unchanged.
Activity
dborup commentedon Oct 3, 2026
Additions from the review of #187 (merged as
c94a0063), all P3:test-test-all.js~157 accepts# node test-x.jsindeploy.ymlas a registration. Only uncommented lines should count.KNOWN_UNREGISTEREDcan grow. There is only a stale-entry check, so "can only shrink" is a convention. Consider pinning the list size, or failing when an entry is added without a linked issue.sh test-all.shruns in the Go job and uses the runner's Node (v22.23.3 today).setup-nodeonly exists in the E2E job. Pin Node 22 there too.1705and1719pass without one, soCHROMIUM_REQUIRE=1does nothing for them. They could move intotest-all.shand fail earlier.test-all.sh. A file that callsprocess.exit(0)early counts as passed, and a hanging file is only stopped by the 40-minute job limit. This was already the case before test: run test-all.sh in CI with a summary (#174 follow-up) #187.Merge pull request #197 from dborup/codex/issue-189-orphan-tests
dborup commentedon Oct 4, 2026
Status after #197 (merged as
33b0dfe5): 15 of the 18 orphan entries are resolved. 10 unit files and 5 E2E files are now registered.Still on
KNOWN_UNREGISTERED, so this issue stays open:test-touch-targets.js: the decision is 48 px, through a separate port of upstream PR2078. That port is in progress on branchcodex/port-upstream-2078-touch-targets-48. It should register the file and reword the reason intest-test-all.js.test-packets.js: 12 of 13 assertions are fixed. The 13th is the collapsed-group caret bug inpublic/packets.js, which waits because feat(packets): show hop ambiguity, resolved per observer (port of upstream 2099) #185 (parked) touches the same file.Low-priority nits from the #197 review, for a later pass:
test-channel-modal-e2e.jsis overstated, because PSK persistence is covered elsewhere;test-table-sort.jshas no test for sort persistence and restore, a gap it inherited from the jsdom original;test-node-reach-e2e.jsdepend on fixture data. They run with the current fixture.dborup commentedon Oct 4, 2026
Status after #201 (merged as
8bafcdf2): touch targets are now 48 px, through a port of upstream PR2078.test-touch-targets.jsis registered and removed fromKNOWN_UNREGISTERED.The only remaining entry from this issue is the collapsed-group caret in
test-packets.js/public/packets.js. It waits for #185, which is parked and touches the same file, plus the deliberately skipped entry.Merge pull request #229 from dborup/codex/issue-189-orphan-tests-r2
dborup commentedon Oct 4, 2026
Partly addressed by #229 (merged as
370185e5).test-packets.js, which now has 127 plain assertions plus the caret case marked asknownBug('#189')..fav-starwas raised from 44 to 48 px. The remaining 44 px controls outside the touch-target list are tracked separately.Still open here: the collapsed packet group shows
#ph-caret-up. The fix waits until #211 is merged, because #211 editspublic/packets.js. When the fix lands, theknownBug('#189')case intest-packets.jsbecomes a plain assertion.Note from the #229 review:
nodes.jsuses caret-up for "Collapse" and caret-down for "Expand / Show all". The fix should pick one disclosure convention and apply it consistently, e.g. caret-right when collapsed and caret-down when expanded.Merge pull request #248 from dborup/codex/issue-189-group-caret
dborup commentedon Oct 5, 2026
Fixed by #248 (merged as
c12aa60b).