Review test coverage across the project

[Variour]

Context

Flagged during adversarial review of #452. That PR adds ~250 lines of new frontend state-machine logic (renderDiscoveredPeers, setupDiscoveredDevice, the wifi/update stall-timeout machinery in data/index.html) plus new mock-server simulation endpoints (simulateDiscoveredWifiConnect, simulateDiscoveredGraduate in server/index.js), and ships with zero corresponding test changes — server/index.test.js is untouched, so the PR's "36/36 passing" is the same count as before and verifies none of the new behavior.

This isn't specific to #452 — it's an example of a broader pattern worth checking project-wide.

Ask

Do a general pass over test coverage:

• Where are the biggest gaps between shipped features and what server/index.test.js / other test files actually exercise?
• Are there conventions we should adopt (e.g. "new endpoint or new frontend state machine needs a test") to stop this from recurring?
• Is coverage tracked at all today, or only judged ad hoc per PR?

Not asking for 100% coverage — asking for a deliberate look at where the risk currently sits.

[Variour]

Where the gaps are

server/index.js (1214 lines, mock server) — the biggest gap. server/index.test.js (344 lines) only exercises 10 of roughly 70 routes: /api/config (get/post), /api/wifi (get/add), /api/lights (get/add/update), /api/sounds (get/add/update), and /api/storage (get/upload/delete) — mostly validation and collision-detection logic. Completely untested:

• Peers: setgroup, setaudiogroup, setvolume, setscenesync, pushconfig, triggerupdate, checkupdate — including the async setTimeout-based simulation machinery (simulateDiscoveredWifiConnect, simulateDiscoveredGraduate, simulatePeerUpdateCycle, simulatePeerCheckCycle, otaPeerError) that mesh,web: add frozen Hello discovery layer for cross-firmware onboarding #452 added to.
• Scenes (create/save/delete/sync/resolve), playlists, audio groups, buttons, automations, groups, mesh (search/wifipolicy/wifiretry), and all of /api/update/* (status/check/apply/trigger/prs).
• The WebSocket connection handler (wss.on('connection', ...)) itself.

data/index.html (7010 lines, frontend) — zero automated coverage. There is no frontend test file. The only verification path is the /run and /validate-ui agent skills, which launch Playwright ad hoc per PR ("do the lightest useful check... 1–2 happy-path interactions") and leave nothing behind — no persisted spec, no CI step runs it. The state machines #452 added (renderDiscoveredPeers, setupDiscoveredDevice, _armStallTimeout) fall entirely in this untested surface.

src/ (20,921 lines, firmware C++) — zero unit tests. No test/ directory and no PlatformIO native/Unity test environment in platformio.ini. The clang-format CI job checks formatting only. Mesh crypto, config migration, scene sync, and pattern rendering are verified only by the fact that full builds compile.

server/auth.js (227 lines) is the one well-covered area — auth.test.js (142 lines) covers login redirect, the OAuth callback for both allowed and rejected users, token issuance, session cookie tampering, and logout.

Is coverage tracked?

No. There is no coverage tool (c8/nyc/istanbul) anywhere in package.json or CI. ci-lint.yml's test job runs npm test (node --test server/*.test.js) as pass/fail only — no percentage or threshold is surfaced on the PR. Coverage is judged ad hoc per PR, which is exactly how #452 could cite "36/36 passing" as if it meant something, when it was the same count as before and covered none of the new behavior.

Conventions

.claude/skills/feature-checklist/SKILL.md already mandates API/schema/WebSocket-event parity between src/web/WebServer.h and server/index.js before a PR — but it says nothing about tests. That is the actual hole: a new mock endpoint or frontend state machine can satisfy the whole parity checklist and still ship with no corresponding test.

---

Generated by Claude Code