test(e2e): multi-board output isolation — verify each board's actual hardware output#1386
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Docs review
Verdict: looks-good — the only docs change is the new multi-board-output.spec.ts row in web/tests/COVERAGE.md, and every claim in it checks out against the spec:
- Ports 7000/7001 match
ensureTwoBoardsWithConnections. - Manual + scheduled send, cross-board leak checks, per-board pause isolation, schedule-list consistency, board-deletion consistency, and board-selector UI clarity each map to a real test.
- The two "known gaps as fixme" (board 2 driving #1241, client reinit on board mutations) match the two
test.fixmeblocks.
Voice matches the terse table style, no marketing language, no PII, and FiestaBoard/Vestaboard usage is correct.
🤖 Auto-review by claude-docs-review.yml.
d536359 to
328910a
Compare
There was a problem hiding this comment.
Docs review
Verdict: looks-good — the sole doc change is one added row in web/tests/COVERAGE.md, and it is accurate.
The new multi-board-output.spec.ts row matches the spec faithfully: ports 7000/7001, the manual/scheduled-send + leak-check + pause-isolation + schedule-list + board-deletion + UI-clarity scenarios, and both test.fixme known gaps (board-2 driving epic #1241, client reinit on board mutations) all line up with the actual test file. Voice, table formatting, branding, and privacy all check out.
One minor, pre-existing note (outside this diff, so no inline suggestion): the section header on line 16 reads ### Playwright Spec Files (29 files), but the tree already carried 33 .spec.ts files before this PR — and this PR adds a 34th. That counter was already stale; worth bumping to (34 files) while you are in this file, but it is not introduced by this change.
🤖 Auto-review by claude-docs-review.yml.
…layer New multi-board-output.spec.ts drives two boards with real mock Local-API connections (ports 7000/7001) and asserts what each physical board actually received — a layer no existing spec covered: - manual send and scheduled send land on board 1's hardware only - a board-2-scoped schedule never leaks onto board 1 (content + history) - per-board pause blocks output without disturbing the other board - per-board schedule lists stay disjoint/complete; default page and enabled flag are independent - deleting a board leaves no schedules that activate on the survivor - UI: schedule board selector names the selected board, fully swaps the schedule view, and the per-board enabled toggle follows the selection Two verified product gaps are pinned as test.fixme so they activate the moment the behavior lands: - board 2 never receives its scheduled content (display engine drives boards[0] only — epic #1241) - /settings/board mutations never reinitialize the board client, so after deleting board 1 the promoted board's content is still delivered to the REMOVED board's hardware until restart Support changes: - helpers.ts: worker-aware MOCK_BOARD_URL_2, getMockBoardState2(), gridToText() decoder, ensureTwoBoardsWithConnections(); fix getMockBoardState(port) to stop sending ?port= (the mock keys state by internal listening port, so host-mapped ports 404'd — why the port-2 constants were dead imports) - ci.yml / integration-tests.yml: mock-board containers now run with PORTS=7000,7001 so the second board exists in CI Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
328910a to
12955c8
Compare
There was a problem hiding this comment.
a11y review: no a11y concerns
This PR touches only test infrastructure and CI config — no web UI source (web/src/**/*.tsx) and no message-catalog (web/messages/**) changes — so there's no accessibility surface to review here.
Changed files:
.github/workflows/ci.yml,.github/workflows/integration-tests.yml— mock-boardPORTS=7000,7001env wiringweb/tests/COVERAGE.md— doc table rowweb/tests/helpers.ts— multi-board test helpersweb/tests/multi-board-output.spec.ts— new E2E spec
Positive note (not a finding): the new spec queries the UI through its accessibility layer rather than brittle CSS/text hooks — getByRole("combobox", { name: "Select board to manage" }), getByRole("option", { name: "Office" }), getByRole("heading", { name: "Schedule" }), and toHaveAttribute("aria-checked", …). These lock in accessible names and roles on the board selector and schedule toggle, so any future regression that strips those a11y attributes would break these tests. That's the pattern we want.
No WCAG 2.2 AA issues to flag. Reviewing only the a11y dimension — general correctness of the test logic is out of scope for this pass.
There was a problem hiding this comment.
Docs review
Verdict: looks-good — the only docs change is one table row in web/tests/COVERAGE.md for multi-board-output.spec.ts, and every claim in it (ports 7000/7001, manual + scheduled delivery, cross-board leak checks, per-board pause isolation, schedule-list consistency, board-deletion consistency, board-selector UI clarity, and the two test.fixme gaps for epic #1241 / client reinit) matches the spec added in the same diff. Voice, format, and privacy all check out.
🤖 Auto-review by claude-docs-review.yml.
The schedule-lists test assumed board 1's default_page_id was null on a container shared with earlier tests, which intermittently left a default page behind (failed in two CI runs). Set both boards' defaults explicitly and assert independence instead, and verify clearing one board leaves the other untouched. Also give the board-selector visibility check the same 15s budget as the page heading. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Docs review
Verdict: looks-good (one small pre-existing note).
Only one doc file changed: web/tests/COVERAGE.md. The added multi-board-output.spec.ts row is accurate against the new spec — ports 7000/7001, manual + scheduled hardware-layer assertions, cross-board leak checks, per-board pause isolation, schedule-list consistency, board-deletion consistency, selector UI clarity, and the two test.fixme gaps (board 2 driving / epic #1241, client reinit on board mutations) all match the code. Voice and branding check out (Vestaboard hardware vs. FiestaBoard software used correctly).
Note (outside this diff, non-blocking): the table header on line 16 reads: Playwright Spec Files (30 files), but there are now 35 spec.ts files (34 at base + this one). The count was already stale before this PR; since this change adds to that exact table, worth bumping to (35 files) while you are here.
🤖 Auto-review by claude-docs-review.yml.
…ribute The theme toggle test asserted the html element's raw class attribute round-trips exactly. But "no classes" is null before the first toggle (attribute absent) and "" after toggling dark on and off (the theme lib removes its class but leaves the empty attribute), so the assertion failed deterministically on every CI run while the toggle itself works correctly. Compare normalized class sets instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Docs review
Verdict: looks-good — the one docs change (a new row in web/tests/COVERAGE.md) is accurate and well-formed.
The added row for multi-board-output.spec.ts matches the spec at PR head: manual + scheduled hardware-layer send checks, cross-board leak checks, per-board pause isolation, per-board schedule-list consistency, board-deletion consistency, and board-selector UI clarity — plus both test.fixme gaps (board 2 driving / epic #1241, client reinit on board mutations). Voice, formatting, privacy, and Vestaboard/FiestaBoard usage all check out.
Optional (out of diff): the section header on line 16 still reads ### Playwright Spec Files (30 files), but the table lists more than that (35 *.spec.ts files exist at head). This count was already stale before this PR and the line is not in the diff, so no inline suggestion — worth a one-line bump if you are touching the file anyway.
🤖 Auto-review by claude-docs-review.yml.
Adopt the exact null-vs-empty-class normalization that already landed on main (PR #1380), making navigation.spec.ts byte-identical to main so the next main<->next merge auto-resolves instead of conflicting on two equivalent fixes for the same brittle assertion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Docs review
Verdict: looks-good — the only docs change is one added row in web/tests/COVERAGE.md for multi-board-output.spec.ts.
The row is accurate against the spec it documents: mock ports 7000/7001, epic #1241 (board-2 driving), the client-reinit gap, and every described scenario (manual + scheduled send to the right board's hardware, cross-board leak checks, per-board pause isolation, schedule-list/board-deletion consistency, board-selector UI clarity) all match the actual test file. Branding, voice, and privacy all check out; no relative links introduced.
🤖 Auto-review by claude-docs-review.yml.
1 participant
Summary
Our multi-board e2e coverage stopped at the API/UI-filtering layer: no test ever sent distinct content to two boards and verified what each physical board actually received. The two-port mock emulator (7000/7001) and the port-2 helper constants existed but were dead code — and the port-2 state helper was subtly broken (
?port=used host-mapped ports which the mock 404s), and CI's mock containers only bound a single port, so such tests couldn't even run in CI.This PR adds
web/tests/multi-board-output.spec.ts, which configures two boards with real mock Local-API connections and asserts at the hardware layer:board_id=*; per-board default page and enabled flag are independent; deleting a board leaves no schedules that activate on the survivorVerified product gaps, pinned as
test.fixmeBoth were reproduced manually against a fresh production-image container built from this branch:
boards[0]only (src/main.py). Board 2 with an active, enabled schedule receives zero messages. This is the known per-board driving gap (epic Epic: Per-board scheduling, driving & sidebar board selector #1241)./settings/boardadd/remove/update never callreinitialize_board_client()(only legacy/config/boarddoes). After deleting board 1, the promoted board's scheduled content is delivered to the removed board's connection until restart. Reproduced: after deletion, mock port 7000 (removed board) received the message; port 7001 (surviving board) got nothing.The fixme tests carry full assertions, so they can be activated the moment the fixes land.
Support changes
helpers.ts: worker-awareMOCK_BOARD_URL_2(derives:7001from each worker's mock URL in CI),getMockBoardState2(),gridToText()decoder,ensureTwoBoardsWithConnections(); fixedgetMockBoardState(port)to stop passing?port=ci.yml/integration-tests.yml: mock-board containers now run withPORTS=7000,7001COVERAGE.md: new spec rowTesting
multi-board,multi-board-schedule,note-multiboard-extended,mock-board,note-pages,integration— 84 passedprettier --checkandeslintclean on changed files🤖 Generated with Claude Code