Skip to content

test: unify frontend test runs and prevent inventory drift - #1965

Open
n30nex wants to merge 2 commits into
Kpa-clawbot:masterfrom
n30nex:codex/fix-test-runner-drift
Open

test: unify frontend test runs and prevent inventory drift#1965
n30nex wants to merge 2 commits into
Kpa-clawbot:masterfrom
n30nex:codex/fix-test-runner-drift

Conversation

@n30nex

@n30nex n30nex commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

npm test and CI maintained different frontend test lists, leaving stale assertions and working regression gates outside CI. Make test-all.sh the authoritative standalone list for npm and CI, restore 19 stale test/harness files to the current SVG, CSS and application behavior, and add an inventory guard for every root test-*.js file.

Fixes #1858.

The runner now executes 163 standalone suites, including all 65 previously selected by the CI unit step. The inventory accounts for 275 root suites and rejects missing, duplicate, stale or undocumented classifications. Browser suites retain their existing E2E setup and selection. The E2E workflow section and all later jobs are unchanged.

Validation:

  • The baseline runner reproduces the CSS-preset failure; the red test commit reports unclassified test files.
  • npm test and the standalone runner pass all 163 suites, independently rerun after implementation.
  • New inventory self-checks prove omissions, duplicates, stale paths and missing prerequisites fail.
  • No production/frontend application files or dependencies changed. Tests now exercise current output or supply missing browser behavior in VM fixtures; status, value and security assertions remain.

Remaining coverage limits are documented in docs/frontend-testing.md: 107 scripts require browser setup, four require the currently undeclared Playwright test runner, and one requires undeclared jsdom. Eighteen existing non-unit suites remain outside CI. Inventory membership records prerequisites; it is not a claim that every browser suite is validated. The npm coverage report currently measures url-state.js, not total frontend coverage.

Following #1922, this test-infrastructure repair is separate from the runtime and release fixes. It overlaps PR #1964 in test-all.sh and the CI unit-test step: merge #1964 first, then retain test-issue-1956-release-routing.js in this authoritative runner when rebasing this PR. Review current Go and E2E job results separately from workflow-approval or staging-runner state.

@efiten

efiten commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Reviewed and I want this in. It needs a rebase first, on exactly the overlap you predicted.

#1964 is merged (2288e28d), so master now carries node test-issue-1956-release-routing.js in test-all.sh and the corresponding entry in the CI unit-test step. Your branch rewrites both, so there is one conflict block in each:

test-all.sh                  1 conflict block
.github/workflows/deploy.yml 1 conflict block  (the "Run JS unit tests (packet-filter)" step)

Both are additive on master's side, so the resolution should be: keep your new structure, make sure test-issue-1956-release-routing.js ends up inside the inventory rather than dropped. That test is the only proof #1956 stays fixed, and it would disappear silently, which is precisely the class of drift this PR exists to stop.

What I checked while it was still applying cleanly:

#1858 is genuinely still open. #1895 unstuck npm test, but the two lists staying out of sync was never fixed, so this is not a duplicate of work already merged.

The runtime cost is acceptable, measured rather than assumed. Running 163 suites instead of 65 puts Go Build & Test at 910s on your run against 823s on a comparable PR from the same day. About 10% for roughly two and a half times the suites, and Playwright was actually shorter on your run. Nobody has to argue about whether this slows CI down.

The part I value most is the inventory self-check. A test list that can silently omit a suite is worse than no list, because it reports green for tests nobody ran. Making omissions, duplicates, stale paths and missing prerequisites fail the run is the thing that keeps this fixed after everyone forgets why.

Push the rebase and I will merge it. If the resolution turns out to be more than mechanical, say so here rather than forcing it.

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.

test: npm test is red on master — test-all.sh and the CI test list have diverged

2 participants