Parallelize E2E (slot-scope all daemon ports, 4 workers); stabilize visual smoke - #37
Merged
Conversation
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
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.
Summary
Makes the E2E scenarios suite run in parallel (was serial) and resolves the perpetually-red
E2E visual smokejob.Goal 1 — Parallelize E2E
Obsolete-pin removal
The daemon fixture used to PIN the broker MQTT ports to
1883/9001because the web UI was believed to hardcodews://<host>:9001/mqtt. The single-origin refactor (#23-26) made that pin obsolete: the browser now connects to a same-origin/mqttWebSocket bridged onto the daemon's own web port (broker_ws_same_origin: truevia/api/capabilities; resolved bymqtt-store.svelte.jsresolveBrokerUrl/sameOriginBrokerUrl). The browser never touches the broker WS port.What changed
e2e/fixtures/daemon.ts: ALL four daemon ports are now slot-scoped (portsForSlot):mcp=9930+N*10,web=9931+N*10,broker-tcp=9932+N*10,broker-ws=9933+N*10. The broker still binds its TCP/WS listeners (configbroker.port/broker.ws_port, confirmed inconfig.py/broker.py) for the daemon's own + non-web clients, now on per-slot ports. Each daemon also gets its own$HOME(mkdtemp) + data dir, so isolation is total.playwright.config.js:workers: CI ? 4,retries: CI ? 1(the real flakes, LWT + rename races, were fixed in CI green: fix 3.10 version test, WS-bridge LWT, and settings rename regression #36, so 2 retries was overkill).e2e/fixtures/browser.ts: fixed the benign-console-noise filter. The prior WIP regex(CLOSING|CLOSED) statedid NOT match Chromium's actual verbatim messageWebSocket is already in CLOSING or CLOSED state., so the teardown-race noise leaked and failed scenarios under worker contention. Corrected to match the whole phrase. All scenarios collect console errors through the singleappPagefixture, so this one fix covers every spec.Before / after (local, full suite, 246 tests)
--workers=1)--workers=4)~2x faster.
Isolation evidence (multiple runs)
--workers=4x3: 1, 2, 2 failures, ALL purely the benignWebSocket is already in CLOSING or CLOSED state.teardown message (243-244/246 passing).--workers=4x3: rc=0, 245 passed, 0 failed every run (117s / 115s / 115s).already in use, zeroDaemon failed to start, zerostate_unsafe_mutation, no state bleed. The only contention-surfaced issue was the cosmetic console message, now filtered. Per-worker isolation is proven.Goal 2 — Visual smoke resolution
Decision: on-demand, with a CI-matching regeneration path (no docker available locally to render like the runner, which the preferred fix needs).
test-e2e-visualjob no longer runs on push/PR (that auto run was perpetually red, never blocking but red, because baselines were generated off-CI and flake ~1-4% vs GitHub's font render). It is nowworkflow_dispatch-gated.workflow_dispatchinputupdate_baselines: whentrue, the job runs--update-snapshotsON theubuntu-latestrunner and commits the regenerated*-linux.pngbaselines back to the branch, so baselines can be made to match CI exactly (gh workflow run ci.yml -f update_baselines=true). Whenfalse, it compares + uploads diffs for review.maxDiffPixelRatio: 0.02(config +screenshot.ts). Note: Playwright takesMath.minofmaxDiffPixelsandmaxDiffPixelRatio, so the old per-callmaxDiffPixels: 500would have neutered any ratio; the helper now defaults to the ratio and only forwards an absolute count when a caller explicitly tightens a snapshot.web/e2e/README.md(also refreshed the stale pinned-port content).Net: the pipeline shows no perpetual red for visuals, on-demand visual checks remain, and there's a one-click path to CI-matching baselines.
Gates
pytest -q: 1469 passedpnpm test(vitest): 1195 passed (89 files)pnpm build: compiles--workers=4: 3/3 runs green, 245 passed each🤖 Generated with Claude Code