test: fix or retire the remaining orphan tests (#189) - #197
Merged
Merged
Conversation
…189) 68a4628 (Kpa-clawbot#675) intentionally changed getRowStyle to a 3px left border with no background tint (comment in channel-colors.js cites Kpa-clawbot#674). The test still expected the old 4px border + 10% tint. Assert the exact string for GRP_TXT and the CHAN alias. Mutant: 3px -> 4px in channel-colors.js turns both cases red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
…tests; register (#189) b812a98 (Kpa-clawbot#1648 M3) replaced the ✕ and 📤 glyphs with Phosphor sprites: the privacy hint now says "Use the close button to remove individual channels", and the share button glyph is the #ph-share-network sprite + " Share". Both were deliberate; the tests still asserted the emoji. - test-channel-ux-followup.js: assert the new copy, and that the remove button it refers to renders the #ph-x close icon. - test-channel-ux-round2.js: assert the share glyph argument is the aria-hidden #ph-share-network sprite + " Share". Mutants (channels.js): hint reworded; remove glyph #ph-x -> #ph-trash; " Share" dropped; icon dropped. Each turns its test red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
…utes; register (#189) 2b45f78 (Kpa-clawbot#1567) moved the Escape revert to clearPanel(), which calls removeAttribute('data-dragged'). In a browser that also clears dataset.dragged; the mock kept dataset and attributes in two separate objects, so "Escape during drag reverts to corner position" saw a stale dataset.dragged === 'true'. The code is right, the mock was not. dataset is now a Proxy over the same attribute map, like a DOMStringMap. Mutants (drag-manager.js clearPanel): remove a different attribute instead of data-dragged; set dataset.dragged = 'false' instead of removing it. Both turn the Escape test red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
…189) f0addfd (Kpa-clawbot#1668 M2) added the palette :root block above the fluid scaffolding block. The test read only the first :root, so all 13 token checks failed although the tokens are still declared with clamp(). rootVar() now strips comments, scans every unconditional top-level :root block (blocks nested in @media are skipped) and keeps the last declaration, as the cascade does. Mutants (style.css): --space-xs -> 4px; --gutter removed; a later :root block overriding --fs-md with 14px. Each turns the test red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
…losest; register (#189) 2b9f305 (Kpa-clawbot#874) switched the geo fallback to haversine. NodeA (37,-122) and NodeB (38,-123) were mirror images around NodeC in degrees, and by haversine NodeB is the closer one (70.88 vs 71.06 km). The test passes at 2b9f305^ and fails at 2b9f305: the resolver is right, the fixture was wrong. NodeA moves to (37.2,-122.2), ~43 km from NodeC vs ~71 km for NodeB, and the test now asserts that geometry with HopResolver.haversineKm. This also makes Tests 1 and 5 meaningful: with the old fixture they still passed when affinity scoring was disabled, because NodeB won on distance anyway. Mutants (hop-resolver.js): geo sort reversed -> Test 2 (and the lat/lon=0 cases) red; graph strategy disabled -> Tests 1 and 5 red (green with the old fixture). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
…46 tests; register (#189) Both tests parse style.css with indexOf/regex and were tripped by comments added later, not by a CSS change: - test-issue-1470-card-bg-contrast.js: e2212f5 (Kpa-clawbot#1627) added a comment mentioning [data-theme="dark"] above the real block, so indexOf landed on the comment and --card-bg was "not found". The dark block still sets var(--surface-2). - test-issue-1646-compare-polish.js: d954ea7 (Kpa-clawbot#1668 M5) added a comment containing "font-size:10px+color:..." inside .compare-vs, and the font-size regex read the comment. Comments are blanked with their newlines kept, so the line-anchored @media helper is unaffected. Mutants (style.css): [data-theme="dark"] --card-bg -> var(--surface-1); .compare-vs font-size 10px -> 14px. Each turns its test red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
….js; keep the caret check red (#189) 3062745 (Kpa-clawbot#1648 M2) replaced the row emoji in getDetailPreview and buildGroupRowHtml with Phosphor sprites, one to one. 12 of the 13 red assertions only checked the old emoji; they now check the aria-hidden sprite (phIcon helper): chat-circle, broadcast, house-line, thermometer, radio, lock, envelope, shuffle, eye. The 13th is not stale. Before 3062745 a collapsed group showed ▶; the migration mapped it to #ph-caret-up, so collapsed groups now show an up caret. The assertion now expects #ph-caret-right (and not the expanded #ph-caret-down) and stays red: public/packets.js is not touched here while #185 edits it. test-packets.js stays on KNOWN_UNREGISTERED with that reason until the caret is fixed. Mutants (packets.js): TXT_MSG envelope -> chat-circle; eye icon dropped; room icon -> broadcast. Each adds a failure. With the caret changed to #ph-caret-right in a copy, the file is 128/128 green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
…the rolling detector; register (#189) Two things had moved under this test: - 3062745 (Kpa-clawbot#1648 M2) replaced the⚠️ flag with the #ph-warning sprite. The four "fires" checks failed, and the four "does NOT fire" checks passed vacuously because they looked for an emoji that is never rendered. All eight now match the sprite right after the value (flagged(html, value)); the negative checks also assert the value is rendered. - a26a412 (Kpa-clawbot#1593) replaced the single-snapshot tx-ratio detector with detectPerfAnomalies (per-source 5-min rolling baseline, 30 s minimum history). The two backfill cases drove the old detector (window._perfWriteSourcesPrev, tx>=100 gate). The detector maths is covered by test-perf-anomaly.js, so these two cases now test the page wiring instead: three refreshes build history on window, the spiking source's row shows rate/baseline and the flag while a steady source's row does not, and a 10 s history suppresses the flag. Mutants (perf.js): WAL > -> >=; cache < -> <=; WAL threshold 100 -> 1000; row flag never rendered; history cleared every refresh; minHistorySec 30 -> 0; any flag rendered on every row. Each turns the test red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
…189) jsdom was never a dependency, so this test could not run anywhere and public/table-sort.js had no unit test (only the observers-sort E2E). The file now loads table-sort.js with vm and a small DOM shim defined in the test: attributes, classList, style, child lists with real moves on appendChild, remove(), listeners + click(), cells, textContent, and a selector engine for the few selectors table-sort.js uses. All 22 cases are kept. The two arrow cases expected ▲/▼; Kpa-clawbot#1648 M2 switched the arrow to the #ph-caret-up / #ph-caret-down sprite, so they check that now. Mutants (table-sort.js): direction ignored; up/down carets swapped; destroy() keeps aria-sort; rows not re-appended; custom comparators ignored; NaN sorts first. Each turns the test red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
…ster in CI (#189) test-marker-outline-weight.js was an @playwright/test ESM spec, and that runner is not a dependency, so it never ran. Kpa-clawbot#1521 rewrote it to check that the canvas pulse highlight ring keeps a >= 2px stroke while visible; nothing else covers that (test-live-dt-cap-1524.js only checks stepPulse's dt cap). Renamed to test-live-pulse-ring-weight-e2e.js and ported to the plain playwright pattern of test-pr-1490-live-map-gpu-animations-e2e.js (CHROMIUM_REQUIRE / SKIP, rAF awaited in the page). It follows the one pulse it queues until the engine drops it, and requires frames in both the 3px (hl_op > 0.4) and the 2px phase. The old spec also passed when no pulse was queued (min over no frames = Infinity >= 2); the port fails then. Registered in the Playwright step of deploy.yml against the fixture server. Fork guards unchanged (9). Mutants (live.js, served from a copy): thin phase 2 -> 1px; initial weight 3 -> 1; pulseNode returns before queueing. Each turns it red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
… and Kpa-clawbot#1111; register in CI (#189) Two of its 14 steps were stale, not regressions: - 7085524 (Kpa-clawbot#1227, Kpa-clawbot#1224 mobile UX) shortened the sidebar button to a "+ Add" chip; the accessible name is still "Add channel". The step now checks visibility, the chip text and the aria-label. - 12d96a9 (Kpa-clawbot#1111) hides the My Channels section while this browser holds no key, so waiting for all three sections on a fresh page timed out. The step now expects Network + Encrypted and no My Channels, and a new last step expects My Channels with the channel the test added. Kept (not deleted) because two steps are not covered elsewhere in CI: clicking the close button hides the modal, and a valid PSK is persisted under corescope_channel_keys. Registered in the Playwright step against the fixture server. Fork guards unchanged (9). Mutants (channels.js, served from a copy): My Channels rendered while empty; close button ignored; aria-label changed. Each turns it red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
… in CI (#189) The Reach page E2E timed out waiting for '#nqMap .leaflet-container'. The map does render: L.map('nqMap') adds .leaflet-container to #nqMap itself, so a descendant selector can never match. The test has been red since it was added in e2212f5 (Kpa-clawbot#1627); it was never registered. It now waits for '#nqMap.leaflet-container' (visible) and an attached map pane, and only when the node itself has a location, which is when node-reach.js builds the map. Overlap with test-node-reach-coverage-e2e.js: none in practice. That file tests the coverage layer only and skips in CI because the fixture has clientRxCoverage off. This file is the only CI test of the Reach endpoint shape, the toggles' row counts and the map. Mutants (node-reach.js, served from a copy): map never built; incoming and outgoing filters swapped. Each turns it red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
…ster in CI (#189) test-path-inspector-e2e.js was an @playwright/test spec (not a dependency), so it never ran. Its fixed prefixes 2c,a1 also have no candidates in the CI fixture, so the Show on Map cases could not pass there anyway. The port keeps what nothing else in CI covers: #mapSidePane collapsed on load and expanded by the toggle; the pane's prefix search renders one row per API candidate; Show on Map draws the route (.mc-rt-edge) and enters route view; the Tools landing links to the Path Inspector and to Trace. The prefix pair is found through /api/paths/inspect (bounded at 400 probes) instead of being hard-coded. Dropped, covered elsewhere: the standalone deep link (test-path-inspector-coverage-e2e.js, "deep link ?prefixes=2c …") and the #/traces redirect (test-issue-1883-redirect-history.js). "Switching candidate clears prior polyline" asserted nothing. The coverage file's header, which still called this spec unwired, is updated. Registered in the Playwright step. Fork guards unchanged (9). Mutants (served from a copy): Show on Map draws nothing (map.js); pane results always "No candidates" (map.js); pane rendered expanded (map.js); Tools landing link renamed (app.js). Each turns it red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
…playwright; register in CI (#189) test-issue-1522-trace-url-sync-e2e.js was an @playwright/test spec (not a dependency), so the Kpa-clawbot#1523 fix (doTrace() writes #/tools/trace/<hash> with replaceState) had no running test. test-e2e-playwright.js only checks that the Trace page loads and returns results. Ported to the plain playwright pattern. Both cases are kept and made stricter: clicking Trace must issue GET /api/traces/<hash>, put the hash in the URL and not grow history (replaceState, not pushState); a deep link must pre-fill the input, actually run the trace and keep the URL. Registered in the Playwright step. Fork guards unchanged (9). Mutants (traces.js, served from a copy): replaceState removed; replaceState -> pushState; deep-link auto-trace removed. Each turns it red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
…189) The 48 vs 44 px decision for .nav-btn and .ch-icon-btn is left to the maintainer: #85 kept 44 (WCAG 2.5.5), and upstream later closed 2052 with 48 (Kpa-clawbot#2078). The harness is also stale independently of that decision (no viewport meta, so mobile-only controls render 0x0; it still lists .compare-btn, removed in Kpa-clawbot#1646). Reason text only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv
Collaborator
Author
Rapport — CS-pve-agent2 PR#197 #189 orphan tests — head c9c085bStatus: Draft, CI green. 15 of the 18 orphan entries are resolved (10 unit files registered, 5 E2E files registered), and 3 remain on Evidence tags: [T] run here (local test or CI), [A] analysis of source/history, [K] taken from the issue or another PR's text and not re-run. Files
Proposal: 48 vs 44 px for
|
| File | plain public/ |
public-instrumented/ |
|---|---|---|
test-live-pulse-ring-weight-e2e.js |
3/3 (4/4 steps) | 3/3 |
test-channel-modal-e2e.js |
3/3 (15/15) | 3/3 |
test-node-reach-e2e.js |
3/3 | 3/3 |
test-path-inspector-e2e.js |
3/3 (6/6) | 3/3 |
test-issue-1522-trace-url-sync-e2e.js |
3/3 (3/3) | 3/3 |
Mutants for the E2E files ran on a second server that served a mutated copy of public/. Each run first confirmed that the mutated file was actually being served [T].
CI (run 37132401311, head c9c085b) [T]
| Job | Result |
|---|---|
| ✅ Go Build & Test | success. Run JS unit tests (test-all.sh) on Node v22.23.3: 211 passed, 0 failed (211 files) |
| 🎭 Playwright E2E Tests | success. All 5 new files ran on the instrumented fixture server, none skipped: pulse ring 4/4 (34 frames, 19 thick + 15 thin), channel modal 15/15, node-reach OK, path inspector 6/6 (pair 04,0a), trace URL sync 3/3 |
| 🏗️ Build & Publish Docker Image | success |
| 📦 Release Artifacts / 🚀 Deploy Staging / 📝 Publish Badges & Summary | skipped (PR run) |
Known remaining items
- The caret bug and
test-touch-targets.js, both above. test-packets.jsstays out of CI until the caret is fixed: 127 green assertions are not running there. An alternative is to move the caret assertion into its own red file and registertest-packets.jsnow. I did not do that because it would start the caret work, which was out of scope.- Observation, not a test failure [T]: on
#/map, once a Path Inspector route is shown, the route view (body.mc-route-active) moves#leaflet-mapover the still-expanded Path Inspector pane. Choosing another candidate means closing the route view first. This looks like the intended full-page route mode, so I did not change it. - The P3 follow-ups in the issue comment were not addressed here: a commented
deploy.ymlline counting as registered, the list size not pinned, Node not pinned in the unit job, the 1705/1719 gates, and no per-file timeout.
Collaborator
Author
Review — CS-pve-agent1 PR#197 orphan tests — head c9c085bDom: APPROVE med nits This review is read-only and independent. I tested Evidence tags:
Findings
None of these block the PR. 1. No weakened testsEvery changed expectation follows a deliberate code change, and I confirmed each commit in git history [A]:
My own mutants, run in a copy of the tree. Each one is mine and differs from the author's.
2. Ports
3. Registration
4. Runs
E2E mutants ran on a second server that served a mutated copy of 5. Production code
Not verified
|
dborup
marked this pull request as ready for review
October 4, 2026 05:57
adminopenclaw8-sketch
pushed a commit
that referenced
this pull request
Oct 4, 2026
Resolve test-test-all.js: keep master's KNOWN_UNREGISTERED (#197) and drop the test-touch-targets.js entry, which this branch registers in the Playwright step of deploy.yml. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #189
This PR works through the orphan root tests that #187 left on
KNOWN_UNREGISTEREDintest-test-all.js. Each one is repaired and registered, ported, or kept on the list with a reason. The causes were traced in git history before any expectation was changed. Every repaired file was checked with at least one mutant of the production code, run in a copy of the tree and never committed, and each mutant turned the test red.git diff origin/master...HEAD -- public/is empty.github.repository == 'Kpa-clawbot/CoreScope'indeploy.ymland 1 inrelease-fast-path.yml.deploy.yml: the only change is five lines appended to the Playwright E2E list.Result
6f7a5e7c)test-all.shKNOWN_UNREGISTEREDentriesThe three entries that remain:
test-packets.js: one deliberately red assertion for the collapsed-caret bug;test-touch-targets.js: waits on the 48 vs 44 px decision;test-rx-coverage-mobile-nav-e2e.js: unchanged, it skips on purpose.Unit files
test-channel-colors.js68a4628e(Kpa-clawbot#675) deliberately switched the row style to a 3px border with no tintborder-left:3px solid …;for GRP_TXT and CHANtest-channel-ux-followup.jsb812a98a(Kpa-clawbot#1648 M3) reworded the hint to "Use the close button…" and changed ✕ to#ph-x#ph-x#ph-trashtest-channel-ux-round2.jsb812a98a: 📤 is now the#ph-share-networksprite + " Share"test-drag-manager.js2b45f787(Kpa-clawbot#1567) usesremoveAttribute('data-dragged'), and the mock'sdatasetwas a separate objectdatasetis now a Proxy over the attribute map, like aDOMStringMapdataset.dragged='false'test-fluid-scaffolding.jsf0addfda(Kpa-clawbot#1668) added a palette:rootfirst, and the test read only the first block:rootblock with comments stripped; the last declaration wins--space-xs: 4px;--gutterremoved; a later:rootoverrides--fs-mdtest-hop-resolver-affinity.js2b9f3056(Kpa-clawbot#874), haversine: NodeB really was closer (70.88 vs 71.06 km). Green at2b9f3056^, red at2b9f3056test-issue-1470-card-bg-contrast.jse2212f50(Kpa-clawbot#1627) added a comment containing[data-theme="dark"]style.csswith comments removed--card-bg→surface-1test-issue-1646-compare-polish.jsd954ea74(Kpa-clawbot#1668 M5) added a comment containingfont-size:10pxinside.compare-vs.compare-vs10px → 14pxtest-perf-disk-io-1120.js30627454:#ph-warning, and the four "noa26a412c(Kpa-clawbot#1593) replaced the backfill detectordetectPerfAnomalies(history across refreshes, flag only on the spiking row, guard against less than 30 s of history). The detector itself is covered bytest-perf-anomaly.js>→>=; cache<→<=; WAL threshold ×10; row flag never rendered; history cleared each refresh;minHistorySec0; flag on every rowtest-table-sort.jsjsdomis not a dependency, sotable-sort.jshad no unit test#ph-caret-up/downsprite (Kpa-clawbot#1648 M2)destroy()keepsaria-sort; rows not re-appended; custom comparator ignored; NaN sorts firsttest-packets.js30627454(Kpa-clawbot#1648 M2), emoji → sprites. The 13th failure is a real bug: a collapsed group shows#ph-caret-up#ph-caret-rightand stays red;public/packets.jsis left alone while #185 edits it#ph-caret-rightin a copy, the file is 128/128 greentest-touch-targets.jsE2E files
test-marker-outline-weight.js→test-live-pulse-ring-weight-e2e.jsplaywright, registered@playwright/testESM spec; nothing else covers the ≥2px pulse ring (Kpa-clawbot#1521)pulseNodereturns earlytest-channel-modal-e2e.js70855249(Kpa-clawbot#1227) changed the label to "+ Add";12d96a9d(Kpa-clawbot#1111) hides My Channels while it is emptyaria-label; expects no My Channels on a fresh page, then My Channels after the PSK add. Kept because ✕-close and PSK persistence are not covered elsewherearia-labelchangedtest-node-reach-e2e.jsL.map('nqMap')puts.leaflet-containeron#nqMapitself, so the descendant selector never matched. It has been red since it was added ine2212f50. No real overlap:test-node-reach-coverage-e2e.jsskips in CI (clientRxCoverageoff)#nqMap.leaflet-container, and only when the node has a locationtest-path-inspector-e2e.js@playwright/testspec. Its prefixes2c,a1have no candidates in the CI fixture.mc-rt-edge) and the Tools landing links. Finds a prefix pair through/api/paths/inspect(at most 400 probes)test-issue-1522-trace-url-sync-e2e.js@playwright/testspec; the Kpa-clawbot#1523 fix had no running testhistory.lengthis unchanged (replaceState), and the deep link really tracesreplaceStateremoved; →pushState; deep-link auto-trace removedRemoved cases from the path-inspector spec, and the tests that cover them:
test-path-inspector-coverage-e2e.js("deep link ?prefixes=2c …")#/traces/<hash>redirecttest-issue-1883-redirect-history.jsNot done here
public/packets.js~2359,#ph-caret-upwhere ▶ used to be). Untouched because feat(packets): show hop ambiguity, resolved per observer (port of upstream 2099) #185 is editingpackets.js.test-packets.jsnow has the red assertion for it, and it goes green once the caret is#ph-caret-right.test-touch-targets.js: a 48 vs 44 px proposal is posted as a PR comment for the maintainer to decide. Apart from that decision, the harness has no viewport meta, so the page lays out at 980 px and the mobile-only.filter-toggle-btnand.ch-back-btnrender 0×0. It also still lists.compare-btn, which was removed in polish: observer compare follow-ups from #1645 visual review Kpa-clawbot/CoreScope#1646.Verification (local, Linux, Node v22.23.3, Chromium 1208)
sh test-all.shbefore / afterdash test-all.shtest-all.shin a copy withoutnode_modules(like the CI unit job)node test-test-all.jstest-packets.js: exit 1, only the caret assertion)corescope-migrate,seed-2073), plainpublic/public-instrumented/(scripts/instrument-frontend.sh)scripts/check-xss-sinks.sh --diff origin/masterpublic/changes🤖 Generated with Claude Code
https://claude.ai/code/session_01YY43YSM5M2JfDKYYxociMv