fix(e2e): adapt Playwright fixture to mode-selector startup flow - #599
Closed
JavierRibaldelRio wants to merge 4 commits into
Closed
fix(e2e): adapt Playwright fixture to mode-selector startup flow#599JavierRibaldelRio wants to merge 4 commits into
JavierRibaldelRio wants to merge 4 commits into
Conversation
The app fixture assumed windows open automatically on launch (Backend Logs first, Control Station second), which stopped being true when "feat: selector mode" (143a00b) added a blocking "Select Mode" window that requires an explicit IPC event before any other window is created. Drive that IPC event directly, and capture the resulting Page objects from waitForEvent instead of re-indexing app.windows() later, since window creation order flipped (Control Station now opens before Backend Logs) and the selector window lingers open for a while after selection. Also add playwright as an explicit e2e devDependency alongside @playwright/test, since CI's `pnpm --filter e2e exec playwright` step was failing with "Command playwright not found" — the CLI bin was only present transitively via @playwright/test.
JavierRibaldelRio
marked this pull request as ready for review
August 2, 2026 20:33
The e2e build only produces a testing-view folder, so getAvailableViews()
returns exactly one view and the selector's own renderer auto-sends that
mode as soon as it loads (renderer/mode-selector/index.html's
`views.length === 1` shortcut). That can win the race against our
explicit setInitialMode() call and close the selector window first,
which was failing every CI run with "Target page ... has been closed"
right at the selector interaction step.
Make the explicit selector call best-effort (same outcome either way),
and buffer window-open events instead of awaiting them sequentially,
since window creation can likewise outrun sequential
waitForEvent("window") calls once a mode is picked.
Playwright doesn't surface the launched Electron app's own console output or exit reason by default, which leaves "Target page, context or browser has been closed" failures with no clue why the process actually went away. Pipe stdout/stderr and log the exit code/signal so the next CI run shows what's actually happening inside the app.
The selector's own renderer auto-selects a mode and closes itself with zero user interaction whenever only one view is built (see renderer/mode-selector/index.html's `views.length === 1` shortcut) — true for the e2e "testing" build. That renderer-driven window.close() races ahead of the main process's handling of the "mode-selected" IPC event and can complete before createWindow() creates the replacement window. setupLifecycleHandlers() — the only place window-all-closed is handled — was only registered after showModeSelector() resolved, so during that race there was no listener at all: Electron's documented default (quit when all windows close and nothing is listening) fired silently, with exit code 0 and no error logged anywhere. This is what was causing every e2e test to fail with "Target page, context or browser has been closed" — confirmed via added stdout/stderr forwarding in the e2e fixture showing the process exit immediately after "Mode selector found", on a real CI Windows runner. Register lifecycle handlers before showing the selector, and reuse the existing isInTransition guard (the same one the "return-to-selector" flow already relies on for this exact class of problem) around the selector-to-main-window handoff, so a momentary zero-window state during mode selection no longer triggers an unwanted quit.
Collaborator
Author
|
Problem of H12 |
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
Root cause turned out to be a genuine startup race condition in the app itself, not just the test fixture — see the investigation trail below.
electron-app/main.js,electron-app/src/app/modeSelector.js): the mode selector's own renderer auto-selects a mode and closes itself with zero user interaction whenever only one view is built (renderer/mode-selector/index.html'sviews.length === 1shortcut — true for the e2e "testing" build). That renderer-drivenwindow.close()can complete before the main process'screateWindow()creates the replacement window.setupLifecycleHandlers()(the only placewindow-all-closedis handled) was registered aftershowModeSelector()resolved, so during that race no listener existed at all — Electron's documented default (quit when all windows close and nothing is listening) fired silently: exit code 0, nothing logged. This is what was causing every e2e test to fail with "Target page, context or browser has been closed" on a real CI Windows runner. Fixed by registering lifecycle handlers before showing the selector, and reusing the existingisInTransitionguard (the same onereturn-to-selectoralready relies on) around the selector→main-window handoff.e2e/fixtures/electron.ts): updated to drive mode selection viawindow.electronAPI.setInitialMode("testing")instead of assuming windows open automatically (broken by143a00bf, "feat: selector mode"), made that call best-effort and buffered window-open events (since the app can auto-select faster than sequentialwaitForEventcalls), and added stdout/stderr forwarding from the Electron process — this is what surfaced the real bug above, since Playwright doesn't forward it by default.e2e/package.json): addedplaywrightas an explicit devDependency alongside@playwright/test, since CI was separately failing withERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL Command "playwright" not found— the CLI bin was only present transitively.Test plan
git log/git showon the relevant commits, full source trace ofmain.js→modeSelector.js→preload.js/renderer/mode-selector/index.html→lifecycle.js, and live CI log evidence (electron process exit right after "Mode selector found", no error, code 0).node --checkon both modified electron-app files (no electron-app test suite exists).modeSelector.jsandlifecycle.js.tests/ui/*.test.tspass — could not verify end-to-end locally (this dev sandbox can't sustain a real Electron GUI window even for a fully unmodified app launched standalone, unrelated to this fix).