feat(test): adopt _electron.launch() Playwright API in e2e config#244
Merged
CoderCoco merged 27 commits intoJun 18, 2026
Merged
Conversation
Adopt electron.launch() via Playwright's experimental _electron API for native Electron app testing. Enables end-to-end testing of the desktop app with full browser context isolation and proper window lifecycle mgmt. Refs #188 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Apollo's gate skipped window creation in test mode, so Playwright's _electron.launch() could never resolve firstWindow(). Keep the window in test mode (the flag is a forward-looking seam for F.7/#198) and retain the preload-path fix (../preload/index.js -> ../preload/preload.js). Add unit tests for the flag-on and flag-off branches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adopt _electron.launch() without breaking the 55 existing stub-based specs. The 'chromium' project keeps the vite preview + page.route() model for the current specs; a new 'electron' project runs only electron-smoke.spec.ts against the packaged out/main/index.js with HYVEON_TEST_MODE=1. Existing specs migrate to Electron under their own Epic F children (F.2-F.6, gated on F.7's window.gsd.__test.mock surface). Gitignore the electron-vite out/ dir. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
One self-contained spec proving the packaged Electron app launches: a BrowserWindow opens and window.gsd is exposed by the preload bridge. Ignore playwright-report/ and test-results/ in eslint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The electron project needs a display server on Linux; xvfb-run supplies a headless one. Install xvfb alongside the Playwright system deps. No OS matrix added (left to a later Epic F child). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Update the tier-1 e2e row to cover the new _electron.launch() electron project alongside the chromium stub project, and reference HYVEON_TEST_MODE. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The global webServer block was running 'npm run build && npm run preview' unconditionally, even when PLAYWRIGHT_PROJECT=electron. This adds overhead and is unnecessary when the electron project launches the app directly via _electron.launch(). Refs #188
The comment previously claimed that --project=electron alone triggers the Vite build skip, but actually PLAYWRIGHT_PROJECT must be set in the environment explicitly. Updated comment to reflect the correct invocation pattern. Refs #188
Avoids an extra top-level 'playwright' devDependency that would have to be version-locked against @playwright/test; _electron is re-exported there. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
All 8 test cases that manually manipulated process.env now use vi.stubEnv() instead, eliminating the need for manual cleanup via Object.defineProperty. This follows Vitest best practices and makes the code cleaner. Refs #188 Co-Authored-By: undefined (via PR comment)
…g it Removes dev-mode renderer URL from inherited environment so packaged-renderer smoke tests always exercise the loadFile() path even when run from a shell that still has dev-server variables set. Refs #188 Co-Authored-By: Copilot (via PR comment)
The electron-smoke spec imported test/expect/_electron directly from @playwright/test, diverging from the convention that non-auth-gate specs go through e2e/fixtures/index.ts. Re-export _electron from the shared fixtures index and import the whole Playwright surface from there. The extended test carries browser-page fixtures, but they are lazy and never instantiate for an _electron.launch() spec that requests no page fixtures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
test:e2e previously dropped the desktop:build step entirely, relying on CI's explicit build. That broke local 'npm run app:test:e2e' runs, which no longer produced out/main/index.js for _electron.launch(). Guard the build behind an existsSync check so it runs locally on demand and is skipped in CI where the explicit step already built the bundle — single build in both environments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The ELECTRON_RENDERER_URL strip is now a plain inline comment on the destructuring statement, and the TSDoc block sits directly above `export const electronEnv` with no intervening code, satisfying the adjacent-TSDoc convention for exported declarations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ript Refs #188 Co-Authored-By: undefined (via PR comment)
The web test:e2e script always builds the desktop bundle before Playwright, so the bundle is freshly built (never stale) for the electron-smoke spec. The explicit workflow build step was therefore a second, redundant build — removing it halves build time and resolves both the double-build perf concern and the stale-reuse reliability concern in one coherent change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduces a two-project Playwright configuration ( ChangesElectron E2E Testing Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/packages/desktop-main/src/electron-entry.ts`:
- Line 46: The code directly accesses process.env.HYVEON_TEST_MODE in the
business logic of electron-entry.ts, which violates coding guidelines. Instead
of reading process.env directly, create or use a dedicated environment service
or helper function that wraps this access. Replace the line where isTestMode is
assigned with a call to this service method (for example, something like
getEnvironmentConfig().isTestMode or a similar accessor pattern). This allows
tests to use vi.spyOn on the service/helper to stub the return value rather than
mutating process.env directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 92849849-88c3-4492-bbef-af0b4db75237
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.github/workflows/e2e.yml.gitignoreCLAUDE.mdapp/eslint.config.jsapp/packages/desktop-main/src/electron-entry.test.tsapp/packages/desktop-main/src/electron-entry.tsapp/packages/web/e2e/fixtures/index.tsapp/packages/web/e2e/specs/electron-smoke.spec.tsapp/packages/web/package.jsonapp/packages/web/playwright.config.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
ESLint configuration uses flat config (app/eslint.config.js) with
@eslint/js, typescript-eslint recommended presets, eslint-plugin-react, and eslint-plugin-react-hooks recommended rules. Run via npm run app:lint or npm run app:lint:fix
Files:
app/eslint.config.jsapp/packages/web/e2e/fixtures/index.tsapp/packages/web/e2e/specs/electron-smoke.spec.tsapp/packages/desktop-main/src/electron-entry.tsapp/packages/web/playwright.config.tsapp/packages/desktop-main/src/electron-entry.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Document non-trivial functions, helpers, and notable constants/variables with TSDoc (/** ... */) including in test-file helpers, stub factories, and fixtures
Files:
app/packages/web/e2e/fixtures/index.tsapp/packages/web/e2e/specs/electron-smoke.spec.tsapp/packages/desktop-main/src/electron-entry.tsapp/packages/web/playwright.config.tsapp/packages/desktop-main/src/electron-entry.test.ts
app/packages/web/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Playwright page objects must be created for each route under app/packages/web/e2e/pages/. Specs must reach for elements through page-object methods rather than calling page.getByX(...) directly. Each page object receives a Page and exposes goto() plus typed Locator-returning methods
Files:
app/packages/web/e2e/specs/electron-smoke.spec.ts
app/packages/web/e2e/specs/**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Import { test, expect, stubApis } plus the page-object fixture from ../fixtures/index.js for authenticated Playwright specs. Use
@playwright/testdirectly only for auth-gate specs
Files:
app/packages/web/e2e/specs/electron-smoke.spec.ts
app/packages/*/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use raw process.env in business logic; wrap environment access behind a service method so tests can stub it via vi.spyOn instead of mutating process.env
Files:
app/packages/desktop-main/src/electron-entry.tsapp/packages/desktop-main/src/electron-entry.test.ts
app/packages/desktop-main/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
app/packages/desktop-main/src/**/*.ts: Neither Discord secret (bot token or public key) is ever sent to client. Use getRedacted() which returns botTokenSet: boolean and publicKeySet: boolean instead of actual values. Preserve this in API response shapes
Use Nest.js built-in DI (@Injectable()) for services and Winston for structured logging. Feature modules group related providers; HTTP handlers live in controllers/ as@Controller-decoratedclasses wired through AppModule
Files:
app/packages/desktop-main/src/electron-entry.tsapp/packages/desktop-main/src/electron-entry.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Test names must read as natural-language sentences starting with 'should' (e.g., 'should return null when state file is missing'), not 'returns null...'
Files:
app/packages/desktop-main/src/electron-entry.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
In tests, avoid 'as unknown as SomeType' casts. Prefer vi.mocked(fn) for mocked modules and Partial + a single 'as T' for service-shaped stubs
Files:
app/packages/desktop-main/src/electron-entry.test.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: CoderCoco/Hyveon
Timestamp: 2026-06-16T22:11:37.377Z
Learning: Add Terraform variables to all four of these files in the same commit: terraform/variables.tf, terraform/terraform.tfvars.example, docs/docs/components/terraform.md, and docs/docs/setup.md
Learnt from: CR
Repo: CoderCoco/Hyveon
Timestamp: 2026-06-16T22:11:37.377Z
Learning: The full AWS IAM deploy policy (GameServerDeployAll) lives in docs/docs/setup.md. When a new AWS service or action is needed, update the policy JSON there and nowhere else
Learnt from: CR
Repo: CoderCoco/Hyveon
Timestamp: 2026-06-16T22:11:37.377Z
Learning: PR titles MUST use Conventional Commits format: <type>(<optional-scope>): <imperative summary>. Types are feat, fix, refactor, docs, test, chore, perf, build, ci, style. Keep subject under ~70 characters. Before creating PR, verify title matches ^(feat|fix|refactor|docs|test|chore|perf|build|ci|style)(\([^)]+\))?: .+$
Learnt from: CR
Repo: CoderCoco/Hyveon
Timestamp: 2026-06-16T22:11:37.377Z
Learning: Always include 'Closes `#N`' in PR body when PR resolves a GitHub issue. Place it as first line so GitHub auto-closes the issue on merge
Learnt from: CR
Repo: CoderCoco/Hyveon
Timestamp: 2026-06-16T22:11:37.377Z
Learning: There is no persistent ECS Service. Servers run only when user clicks Start or invokes /server-start. The app/followup-Lambda calls ecs.run_task() / ecs.stop_task() against per-game task definitions. Do not introduce a long-running Service
Learnt from: CR
Repo: CoderCoco/Hyveon
Timestamp: 2026-06-16T22:11:37.377Z
Learning: Use git worktrees for feature work at .worktrees/<branch-name>. Create with: git worktree add .worktrees/<branch> -b <branch>. The .worktrees directory is gitignored
Learnt from: CR
Repo: CoderCoco/Hyveon
Timestamp: 2026-06-16T22:11:37.377Z
Learning: All changes go through PR, including trivial chores (.gitignore entries, config tweaks). Never commit directly to main; main is protected and requires PR review
🔇 Additional comments (9)
app/packages/desktop-main/src/electron-entry.test.ts (1)
137-138: LGTM!Also applies to: 152-153, 168-169, 187-188, 207-208, 227-228, 243-244, 258-300
app/packages/web/playwright.config.ts (1)
2-95: LGTM!app/packages/web/e2e/fixtures/index.ts (1)
262-268: LGTM!app/packages/web/e2e/specs/electron-smoke.spec.ts (1)
1-38: LGTM!app/packages/web/package.json (1)
10-10: LGTM!.github/workflows/e2e.yml (1)
26-42: LGTM!.gitignore (1)
44-45: LGTM!app/eslint.config.js (1)
17-18: LGTM!CLAUDE.md (1)
185-185: LGTM!
Centralizes process.env access for test-mode gating and renderer URL, allowing tests to stub these via vi.spyOn() instead of mutating process.env directly. Refs #188 Co-Authored-By: coderabbitai[bot] (via PR comment)
Owner
Author
|
I've addressed the following feedback:
Please re-review when you get a chance. |
The e2e job has been red on every commit of this branch. After a fresh npm ci, @hyveon/shared has no dist/ (its package.json resolves to ./dist/index.js), so electron-vite's desktop:build fails with 'Failed to resolve entry for package @hyveon/shared'. The integration job already pre-builds (npm run build -w @hyveon/desktop-main) for the same reason; e2e had no equivalent. Build @hyveon/shared first in app:test:e2e — leaner than building desktop-main since electron-vite bundles the main process from source and only needs shared's compiled dist for module resolution. Reproduced the failure locally (rm shared/dist -> desktop:build fails with the exact CI error) and verified the fix builds out/main/index.js from a clean state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
With the build fixed, the electron-smoke spec failed at runtime with 'Electron failed to install correctly' — electron is a hoisted devDependency whose postinstall binary download didn't materialize under npm ci in CI, so _electron.launch() found no executable (55 chromium specs passed; only the 2 electron specs failed). Run electron's install script explicitly after npm ci to fetch the platform binary. Verified locally: removing node_modules/electron/dist reproduces the failure, and install.js re-downloads the binary (idempotent when already present). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Previous attempt no-op'd in 0s: the runner environment carries
ELECTRON_SKIP_BINARY_DOWNLOAD, so both npm ci's postinstall and a plain
install.js exit before writing node_modules/electron/path.txt. Playwright
resolves the executable via require('electron/index.js'), which throws
'Electron failed to install correctly' when path.txt is absent — and
re-throws it verbatim (only MODULE_NOT_FOUND is remapped), which is the
exact error the electron-smoke spec hit (55 chromium specs passed).
Clear the skip flag at step scope, remove the partial install, and run
install.js so it actually downloads; then assert path.txt exists so any
regression fails loudly here instead of mid-test. Verified locally.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The forced install.js still produced no path.txt and exited 0 silently, so the download is being skipped/failing inside @electron/get rather than at the skip-flag gate. Enable DEBUG=@electron/get:* and dump the electron dir state to capture the actual cause in CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DEBUG output revealed the real cause: npm ci caches a corrupt/truncated
electron zip in ~/.cache/electron, so install.js gets a '@electron/get
Cache hit' and extracts an incomplete dist (only locales/ — no binary, no
version, no path.txt), exiting 0. require('electron/index.js') then throws
'Electron failed to install correctly', failing the 2 electron-smoke
specs (the 55 chromium specs pass).
Remove ~/.cache/electron and set force_no_cache=true so install.js
re-downloads fresh with checksum validation; a truncated re-download now
fails loudly on checksum mismatch instead of silently caching garbage.
Assert the binary resolves at this step.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
force_no_cache still produced no path.txt in ~1s with no output. Add a network reachability probe (curl to the github release asset) plus @electron/get debug and full dist/cache dumps, all non-fatal, to pinpoint whether this is a network-egress restriction or a download/extract issue. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DEBUG proved the download is fine (full 111MB zip lands in
~/.cache/electron, http 200) but electron's install.js exits ~20ms after
the download — its un-awaited .then(extractFile) promise doesn't keep the
event loop alive, so the process exits mid-extraction, leaving dist/ with
only locales/ and no path.txt. require('electron/index.js') then throws
'Electron failed to install correctly', failing the 2 electron-smoke specs.
Run install.js only to populate the download cache, then extract the
cached zip with unzip ourselves and write path.txt — deterministic and
complete. Assert the binary resolves so regressions fail at this step.
Verified locally end-to-end.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The electron-smoke spec timed out: the app launched then crashed before opening a window with ERR_MODULE_NOT_FOUND for @grpc/proto-loader. The main bundle (which inlines @nestjs/microservices) hoists that package's normally-lazy loadPackage() require into a top-level ES import; it wasn't installed, so rollup left it as an unresolved external and the main process threw on startup. We never use gRPC (custom Electron IPC transport), but the import must still resolve. Add @grpc/proto-loader as a desktop-main dependency and document why in electron.vite.config.ts. Verified locally under xvfb: the app boots (HYVEON_TEST_MODE seam logs, window opens) and both electron-smoke specs pass (2/2). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CoderCoco
deleted the
claude/issue-188-feat-test---adopt--electron-launch---playwright-ap
branch
This was referenced Jun 18, 2026
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
1 participant
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.
Closes #188
Summary
playwright.config.tsto use_electron.launch()as a dual Chromium + Electron Playwright project setup, with aPLAYWRIGHT_PROJECTenv var to select which project runsHYVEON_TEST_MODEgate to the Electron main entry (electron-entry.ts) so the test harness can launch Electron without a real tfstate/AWS surface, and covers it with a unit testelectron-smoke.spec.ts) that proves a BrowserWindow opens andwindow.gsdis defined; wires CI to run Playwright underxvfbfor the Electron projectChanges
Test plan
npm run app:test— all unit tests pass, including the newelectron-entry.test.tscoveringHYVEON_TEST_MODEnpm run app:lint— 0 errorsPLAYWRIGHT_PROJECT=electron npm run app:test:e2e(withdesktop:buildrun first) — Electron smoke spec passes: BrowserWindow opens andwindow.gsdis definedPLAYWRIGHT_PROJECT=chromium npm run app:test:e2e— existing Chromium e2e specs continue to pass unaffectedxvfb-runfor the Electron project without failure🤖 Generated with Claude Code