Skip to content

test: 13 red orphan unit tests + 4 orphan E2E files left after #187; collapsed packet groups show an up-caret #189

Closed

Description

@dborup

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.

Relates to #174, #187.

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 ⚠️ flag (10/5) 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_UNREGISTERED in test-test-all.js and register it in test-all.sh or the Playwright step.
  • The test-touch-targets.js decision (48 vs 44 px) is recorded.
  • The caret fix has a test. Do not touch public/packets.js while another open PR edits it.
  • No new npm dependencies. Fork guards unchanged.

Activity

  1. dborup commented on Oct 3, 2026

    @dborup
    OwnerAuthor

    Additions from the review of #187 (merged as c94a0063), all P3:

    1. Guard counts a commented-out line as registered. test-test-all.js ~157 accepts # node test-x.js in deploy.yml as a registration. Only uncommented lines should count.
    2. KNOWN_UNREGISTERED can 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.
    3. Node is not pinned for the unit step. sh test-all.sh runs in the Go job and uses the runner's Node (v22.23.3 today). setup-node only exists in the E2E job. Pin Node 22 there too.
    4. Two contrast gates do not need a browser. 1705 and 1719 pass without one, so CHROMIUM_REQUIRE=1 does nothing for them. They could move into test-all.sh and fail earlier.
    5. No per-file timeout in test-all.sh. A file that calls process.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.
  2. added a commit that references this issue on Oct 4, 2026
  3. dborup commented on Oct 4, 2026

    @dborup
    OwnerAuthor

    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 PR 2078. That port is in progress on branch codex/port-upstream-2078-touch-targets-48. It should register the file and reword the reason in test-test-all.js.
    • test-packets.js: 12 of 13 assertions are fixed. The 13th is the collapsed-group caret bug in public/packets.js, which waits because feat(packets): show hop ambiguity, resolved per observer (port of upstream 2099) #185 (parked) touches the same file.
    • The deliberately skipped entry, unchanged.

    Low-priority nits from the #197 review, for a later pass:

    • the stated reason for keeping test-channel-modal-e2e.js is overstated, because PSK persistence is covered elsewhere;
    • test-table-sort.js has no test for sort persistence and restore, a gap it inherited from the jsdom original;
    • parts of test-node-reach-e2e.js depend on fixture data. They run with the current fixture.
  4. dborup commented on Oct 4, 2026

    @dborup
    OwnerAuthor

    Status after #201 (merged as 8bafcdf2): touch targets are now 48 px, through a port of upstream PR 2078. test-touch-targets.js is registered and removed from KNOWN_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.

  5. added a commit that references this issue on Oct 4, 2026
  6. dborup commented on Oct 4, 2026

    @dborup
    OwnerAuthor

    Partly addressed by #229 (merged as 370185e5).

    Still open here: the collapsed packet group shows #ph-caret-up. The fix waits until #211 is merged, because #211 edits public/packets.js. When the fix lands, the knownBug('#189') case in test-packets.js becomes a plain assertion.

    Note from the #229 review: nodes.js uses 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.

  7. added a commit that references this issue on Oct 5, 2026
  8. dborup commented on Oct 5, 2026

    @dborup
    OwnerAuthor

    Fixed by #248 (merged as c12aa60b).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions