Skip to content

fix(e2e): adapt Playwright fixture to mode-selector startup flow - #599

Closed
JavierRibaldelRio wants to merge 4 commits into
developfrom
worktree-fix-e2e-mode-selector
Closed

fix(e2e): adapt Playwright fixture to mode-selector startup flow#599
JavierRibaldelRio wants to merge 4 commits into
developfrom
worktree-fix-e2e-mode-selector

Conversation

@JavierRibaldelRio

@JavierRibaldelRio JavierRibaldelRio commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

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.

  • App fix (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's views.length === 1 shortcut — true for the e2e "testing" build). That renderer-driven window.close() can complete before the main process's createWindow() creates the replacement window. setupLifecycleHandlers() (the only place window-all-closed is handled) was registered after showModeSelector() 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 existing isInTransition guard (the same one return-to-selector already relies on) around the selector→main-window handoff.
  • e2e fixture (e2e/fixtures/electron.ts): updated to drive mode selection via window.electronAPI.setInitialMode("testing") instead of assuming windows open automatically (broken by 143a00bf, "feat: selector mode"), made that call best-effort and buffered window-open events (since the app can auto-select faster than sequential waitForEvent calls), 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.
  • CI dependency fix (e2e/package.json): added playwright as an explicit devDependency alongside @playwright/test, since CI was separately failing with ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL Command "playwright" not found — the CLI bin was only present transitively.

Test plan

  • Root-caused via git log/git show on the relevant commits, full source trace of main.jsmodeSelector.jspreload.js/renderer/mode-selector/index.htmllifecycle.js, and live CI log evidence (electron process exit right after "Mode selector found", no error, code 0).
  • node --check on both modified electron-app files (no electron-app test suite exists).
  • Confirmed no circular import between modeSelector.js and lifecycle.js.
  • Needs a CI run to confirm all 12 tests/ui/*.test.ts pass — 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).

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
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.
@JavierRibaldelRio

Copy link
Copy Markdown
Collaborator Author

Problem of H12

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.

1 participant