4 Commits
Author SHA1 Message Date
efitenandClaude Opus 5.5 ef12535364 test(map): wait for the new route sidebar before measuring it (#2109)
Step (9) of `tests/e2e/test-path-inspector-e2e.js` fails intermittently
on master with `(3-9) fixture route coverage: 0 !== 700`: 3 of the 18
runs that executed it since 2026-09-30, including master runs
[37120155356](https://github.com/Kpa-clawbot/CoreScope/actions/runs/37120155356)
and
[37156362836](https://github.com/Kpa-clawbot/CoreScope/actions/runs/37156362836).
A red master run skips the image build, so `:edge` is not rebuilt.

## Cause

The step clicks a candidate and then waits for any `.mc-rt-sidebar`. The
previous route's sidebar survives the hash-only `goto` to `#/map`, and
`drawPacketRoute` (`public/map.js:1080`) replaces it only after
`/api/resolve-hops` answers. So the wait matched the old sidebar, and
the checks that follow measured either the old sidebar or, if the
replacement landed between the locator resolving and the evaluation, a
detached element, whose `getBoundingClientRect()` width is 0.

This is a test defect. The app keeps the previous route on screen while
the next one loads, which is intended.

## Fix

Remember the sidebar before the click and wait until a different one is
in the DOM. Test-only, one file.

## Evidence

Locally against the fixture, with `/api/resolve-hops` delayed through
`page.route` in a throwaway copy of the test, measuring which sidebar
the step-9 assertion sees:

| Delay | Before | After |
|---|---|---|
| 0 ms | new sidebar, pass | new sidebar, pass |
| 80 to 100 ms | old sidebar, `isConnected: false`, width 0 (3 of 5
runs) | new sidebar, pass (5 of 5) |
| 1500 ms | old sidebar, pass: the step checked the wrong page | new
sidebar, pass |

The unmodified fixed test then passed 3 of 3 runs.

## Not done

- I did not look for the same wait pattern in other suites.
- The delay probe is not committed. It needs a timing window to be
useful, and that window would be a fixed sleep in CI.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 12:27:02 +02:00
n30nex 093e320c2b fix(map): keep Path Inspector candidates accessible and cover route replacement (#2082)
## Fix

Fixes #2060. Fixes #2081.

Path Inspector route drawing and candidate replacement now have
deterministic browser coverage. An idempotent SQL seed adds four
synthetic repeaters and four edges after fixture migration, without
changing packet ordering. The test uses the real API, normal clicks,
visible Leaflet geometry, and removal of every previous route object.

Restoring coverage exposed a desktop layout bug: drawing the first route
moved the map over the next candidate button. The route sidebar now
participates in the existing flex layout, including resizing and
collapse, while preserving the mobile bottom sheet.

The historical zero-candidate result did not reproduce on the current
baseline. No search thresholds or API behavior changed. No new
dependencies or customizer settings.

## Validation

- Local assertion-red history: `042db2b` requires seeded candidates;
`ca1d22a` exposes the blocked second click. Subsequent commits repair
layout and narrow-window behavior.
- All 183 standalone suites passed. Unchanged-base Go failures: #2083
readiness race (fixed in #2084) and Windows symlink privilege.
- Real Chromium: 11 Path Inspector/layout checks, 33 related map checks,
and core E2E (131 passed, 3 existing fixture skips).
- CSS-variable checks, XSS diff check, inventory, whitespace and PII
checks passed. Seed remains valid after periodic graph refresh.

Local browser validation used Chromium with a 60-second navigation
budget; the OpenClaw profile and external preflight script were
unavailable. Repository checks ran directly. [CI run
36343343569](https://github.com/Kpa-clawbot/CoreScope/actions/runs/36343343569)
records the initial assertion-red commit. Final CI must pass before
merge.
2026-09-30 11:40:49 +02:00
efitenandClaude Opus 5 25f8426d32 test: make every suite in tests/e2e run, and tell the truth when it does (#2053)
Closes #2037.

Step 1 (#2045) wired in the nine suites that already passed. This is
steps 2 and 3: the four that ran and failed, and the five that could not
run at all. After this, the count of suites in `tests/e2e` invoked by
nothing goes from 18 to 0.

## Step 2 — the four that ran and failed

Triaged against a fixture server in CI with full output kept, not the
four-line tail the first probe saved.

| suite | verdict |
|---|---|
| `test-packets-scope-column.js` | passes on master today. It failed on
2026-09-18, so something between the two fixed it. Wired in unchanged
rather than investigated. |
| `test-node-reach-e2e.js` | test wrong. It waited for the reach map
whenever any *link* had GPS; `public/node-reach.js` builds the map only
when the *node* has coordinates. Guaranteed 10s timeout on a node with
positioned neighbours and no position of its own. |
| `test-channel-modal-e2e.js` | both failures test-side. The Add
button's visible label was shortened to "+ Add" with the accessible name
moved to `aria-label` (`public/channels.js:746`); and
`.ch-section-mychannels` is conditional on the visitor having added a
channel, which has not happened at that point in the suite. |
| `test-touch-targets.js` | three test-side, two a product finding. |

The three test-side touch-target failures were the harness measuring
controls that are not shown: `.compare-btn` (the CTA was removed in
#1646, and `style.css` says so), `.ch-back-btn` (`display:none` outside
the mobile channels layout), and `.filter-toggle-btn` (`display:none` on
mobile since #1461; the control shown is the navbar mirror, which
`mobile-page-actions.js:70` builds as a `.nav-btn`, so it was already
measured). All three are dropped from the table with the reason recorded
in the file.

The remaining two are **not** a test problem: `.nav-btn` and
`.ch-icon-btn` are each declared twice in `public/style.css`, 48px in
the touch-target block and 44px in their own component rule, and the
later one wins. Rather than lower the blanket or hide the failures, the
suite now has `DEFAULT_MIN = 48` plus a `MIN_OVERRIDES` table holding
those two at their effective 44, so a third selector dropping to 44
still fails the build. The contradiction is **#2052**, with both ways
out costed; the override entries should go when it is settled.

## Step 3 — the five that could not run

None is deleted. I checked each selector and seam against the product
before deciding, and every one still targets a surface that exists and
that nothing else covers.

Four were written against `@playwright/test`, a runner the project
neither installs nor uses anywhere else. Adopting a second runner for
twelve tests costs more than porting them, and the precedent is already
set: `test-path-inspector-coverage-e2e.js` exists, as its own header
says, because `test-path-inspector-e2e.js` could not run. So they are
ported to the plain-node Chromium pattern the other 109 suites use.

- **`test-issue-1522-trace-url-sync-e2e.js`** — the trace hash in the
URL, both directions. `test-e2e-playwright.js` covers that the page
loads and searches; it never looks at the URL, which is the whole of
#1522.
- **`test-marker-outline-weight.js`** — the canvas pulse ring never
thins below 2px. There is no CSS rule to read and axe cannot see inside
a canvas, so sampling the seam is the only way. Added a guard that the
ring was actually visible, so the weight check cannot pass vacuously on
a pulse that never rendered.
- **`test-pr-1490-live-map-gpu-animations-e2e.js`** — the queue drains,
the engine sleeps again, the fading trails stay under the cap of 5, and
the canvas sits on `animationsPane` rather than under the markers.
- **`test-path-inspector-e2e.js`** — reduced to what nothing else
covers: the map side pane, the `/#/traces/<hash>` redirect, the tools
landing. Its standalone-page test duplicated the wired coverage suite
and is dropped. Its "switching candidate clears prior polyline" case
ended after the click with a comment and no assertion, which is the same
green-but-empty problem this issue is about; it now compares path
counts, and skips loudly when the fixture yields too few candidates.

The fifth, **`test-table-sort.js`**, needed `jsdom`, which was declared
nowhere. It is a unit test of `public/table-sort.js` filed under
`tests/e2e`, so: `jsdom` is a devDependency (lockfile updated, `npm ci`
stays consistent), the file moved to `tests/unit/`, and the
`domIntegration` group in `scripts/non-unit-tests.json` is gone with its
only member. It runs 22 tests. 20 passed immediately; 2 had rotted,
because #1648 M2 replaced the up/down glyphs with Phosphor sprites and
the direction moved out of `textContent` into the `<use href>`. Those
two now read the sprite ref and the `aria-sort` value, so they also
guard the accessible announcement.

## Verification

`tests/unit/test-table-sort.js` 22/22 and `test-test-inventory.js` pass
locally; the E2E suites need a fixture server, which I cannot build here
(no cgo toolchain since #1992), so CI is their first run as committed.
The triage above was measured in CI, not assumed.

## Not done

The per-assertion skips named in the second comment on #2037 are
untouched: the two flaky packet-detail cases, the fixture-data ones, and
the two `clientRxCoverage` suites that skip wholesale while reporting
success. Those need a fixture deployment with coverage enabled, which is
its own change. I have not opened it.

Option 2 from the issue, making `test-test-inventory.js` require a
`deploy.yml` line for every `tests/e2e` file, is also not here. It is
the right guard and it is now enforceable, since the list is finally at
zero, but it belongs in its own change where a red build means what it
says.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-22 11:45:11 +02:00
Alex B 893773338e chore(tests): move root test-*.js into tests/unit and tests/e2e (#2036)
Moves 290 root test-*.js into tests/unit (177, listed in test-all.sh) and tests/e2e (113, classified in scripts/non-unit-tests.json), per #1981 and PR-D of #1385. Root goes from 348 entries to 48. test-all.sh and test-fixtures/ stay put. The inventory guard now fails if a test reappears in the root or sits in the wrong folder.

Verified independently of the diff: the invoked sets are unchanged (test-all.sh 177 before and after, deploy.yml 96 before and after, both identical as sets), and a full local run of test-all.sh on master and on the branch produced 4702 output lines each whose only differences are absolute paths, stack-trace line numbers shifted by the REPO_ROOT line, the inventory wording and two perf ratios. The guard was mutation-checked: a test back in the root, a unit suite in tests/e2e, and a suite dropped from test-all.sh each make it exit 1. CI run 35246304316 ran 97 suites from tests/e2e and is green.

Follow-up 9335c51d finished the instruction files: no bare root test command is left in AGENTS.md, the squad charters, .github or docs, and every tests/ path they name resolves.

Merged by the interim maintainer without a second human reviewer: CI and the local runs above are the independent checks.

Known and deliberately out of scope: 18 of the 113 files in tests/e2e are invoked by no runner at all, and one of them cannot run anywhere because it requires jsdom, which is not a declared dependency. Tracked separately.
2026-09-17 19:06:07 +02:00