Skip to content

test(e2e): replace Cypress with Playwright Electron E2E suite - #875

Open
raul-oliveira wants to merge 2 commits into
masterfrom
raul-oliveira/feat/electron-e2e-playwright
Open

test(e2e): replace Cypress with Playwright Electron E2E suite#875
raul-oliveira wants to merge 2 commits into
masterfrom
raul-oliveira/feat/electron-e2e-playwright

Conversation

@raul-oliveira

@raul-oliveira raul-oliveira commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Replaces the Cypress browser-based E2E suite with Playwright driving the real Electron desktop app. Built on the four-layer architecture established for the rpc-lib web-wallet in HathorNetwork/hathor-rpc-lib#169 (Electron app driver / Electron-blind page object / provisioning journeys / specs), and reused for the explorer in HathorNetwork/hathor-explorer#519.

This is PR #1 of a series: it lays the harness + conventions and ships the two onboarding journeys. Funded sends and token-lifecycle are deferred to follow-ups.

How it works: Playwright's webServer boots the CRA dev server; a worker-scoped fixture launches the installed Electron 27 binary via _electron.launch with a fresh --user-data-dir per project (= one isolated wallet). The testnet switch is an explicit spec verb (per the #169 doctrine). Selectors are hybrid (roles/text/ids), isolated in one selectors.ts; timeouts are centralized and env-tunable (E2E_TIMEOUT_SCALE).

Test evidence (real runs against public nodes):

  • onboarding1 passed — new wallet → dashboard → Total: 0.00 HTR on mainnet.
  • import2 passed — funded seed → mainnet dashboard → switch to testnet → re-sync → transaction history rendered.
  • Skip-path verified: with E2E_IMPORT_SEED unset, the import journey is skipped.

Run locally: cp .env.e2e.example .env.e2e (set E2E_IMPORT_SEED to a funded testnet stub seed), then npm run e2e (or npm run e2e -- --project=onboarding, npm run e2e:headed).

Acceptance Criteria

  • Cypress fully removed: cypress/, cypress.config.ts, the three Cypress devDependencies, and the Cypress ESLint override; lavamoat policy regenerated without Cypress entries.
  • Playwright E2E harness in place under tests/e2e/: config, worker-scoped Electron fixture (fresh --user-data-dir per project), Electron app driver, Electron-blind page object, provisioning journeys, centralized env-tunable timeouts, hybrid selectors, dependency-free .env.e2e loader, wallets registry, and a read-first e2e.md conventions doc.
  • e2e / e2e:headed / e2e:ui npm scripts added; .env.e2e, playwright-report/, test-results/, blob-report/ gitignored.
  • Onboarding journey: create a new wallet → password → PIN → dashboard on the mainnet connect-default → assert Total: 0.00 HTR.
  • Import journey: import a funded testnet seed (from E2E_IMPORT_SEED) → mainnet dashboard → switch network to testnet as an explicit spec verb → assert re-sync renders the transaction history. Skipped when the seed env var is unset.
  • Single product-source change: data-testid="wallet-balance-total" on the dashboard Total balance (history reuses the existing #token-history table — no extra testid).
  • CI: the Cypress workflow job is replaced with a Playwright job running headlessly under xvfb-run, with E2E_IMPORT_SEED provided via a repository secret (import journey skips green until the secret is set).

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. The only new dependency is @playwright/test (a devDependency, test-only); three Cypress devDependencies were removed. No production dependencies were added or changed. The funded import seed is a public testnet stub kept out of the repo — it is loaded from a gitignored .env.e2e locally and a CI secret in the workflow; the import journey skips when it is unset.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • End-to-end testing now supports both onboarding and wallet import flows, including a testnet switch scenario.
    • Added options for headed and UI-based test runs, plus faster/safer setup for different environments.
  • Bug Fixes

    • Improved test stability with clearer timeouts, isolated app sessions, and more reliable app startup/cleanup.
  • Documentation

    • Added guidance for running E2E tests and configuring optional seeds and timeout scaling.
  • Chores

    • Updated ignore rules and CI to match the new test setup.

Drive the real Electron desktop app end-to-end with Playwright, replacing the
Cypress browser-based suite. Built on the four-layer architecture from
rpc-lib PR #169: an Electron app driver, an Electron-blind page object,
provisioning journeys, and specs.

Journeys (green against public nodes):
- onboarding: create a new wallet, reach the dashboard on the mainnet
  connect-default, assert a zero HTR total.
- import: import a funded testnet seed (from E2E_IMPORT_SEED / .env.e2e),
  load on mainnet, then switch to testnet as an explicit spec verb and assert
  re-sync renders the transaction history.

Changes:
- Remove Cypress: delete cypress/, cypress.config.ts, the cypress devDeps and
  ESLint override; regenerate lavamoat policy.
- Add tests/e2e/ harness: playwright.e2e.config.ts, worker-scoped Electron
  fixture (fresh --user-data-dir per project), page object, journeys,
  centralized env-tunable timeouts, hybrid selectors, .env.e2e loader,
  wallets registry, and e2e.md conventions.
- Add a single data-testid="wallet-balance-total" to the dashboard Total
  balance (history reuses the existing #token-history table).
- Replace the Cypress CI job with a Playwright job running under xvfb, with
  E2E_IMPORT_SEED provided via secret (import journey skips when unset).
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR replaces the Cypress E2E testing stack with a Playwright + Electron setup. It removes Cypress config/tests/support files, updates CI workflow, gitignore/eslintignore, package scripts/dependencies, and LavaMoat policy allowlists. It adds Playwright config, env/build support, Electron driver, wallet UI helpers, journeys, fixtures, onboarding/import specs, documentation, and a test id on WalletBalance.

Changes

Cypress to Playwright E2E Migration

Layer / File(s) Summary
Remove Cypress CI/config, update ignore rules
.github/workflows/e2e.yml, .gitignore, .eslintignore, .env.e2e.example
CI workflow switches from a Cypress job to a Playwright job (installs deps, runs headless via xvfb, uploads reports); gitignore/eslintignore drop Cypress artifact patterns and add Playwright/e2e patterns; env example documents E2E_IMPORT_SEED/E2E_TIMEOUT_SCALE.
Update package scripts and dependencies
package.json
Cypress e2e script and devDependencies (cypress, @testing-library/cypress, eslint-plugin-cypress) are replaced with Playwright scripts (e2e, e2e:headed, e2e:ui, e2e:release*) and @playwright/test; the Cypress ESLint override is removed.
Update LavaMoat dependency policy
lavamoat/webpack/policy.json
Numerous Cypress-sourced allowlist entries (safe-buffer, lodash, qs, randombytes, has-tostringtag) are replaced with equivalents sourced through ttag-cli and @hathor/wallet-lib.
Add Playwright E2E config
playwright.e2e.config.ts
New config loads .env.e2e, resolves dev/build target, configures test directories, timeouts, tracing, two projects (onboarding/import) with distinct walletSetup, and a conditional webServer for the dev target.
Add env loader, build setup, and TS config
tests/e2e/support/env.ts, tests/e2e/support/build.ts, tests/e2e/tsconfig.json
Adds .env parsing helper, a globalSetup that conditionally runs npm run build for the build target, and TypeScript config for the e2e test project.
Add Electron driver and timeout scaling
tests/e2e/driver/electronApp.ts, tests/e2e/driver/timeouts.ts
Adds launchWallet/closeWallet to manage Electron app lifecycle with a temp user-data dir, and a TIMEOUTS object scaled via E2E_TIMEOUT_SCALE.
Add wallet UI helpers, selectors, journeys, seed lookup
tests/e2e/helpers/selectors.ts, tests/e2e/helpers/walletApp.ts, tests/e2e/helpers/wallets.ts, tests/e2e/helpers/journeys.ts
Adds centralized selectors/credentials, a WalletApp class wrapping onboarding/dashboard/history/network UI actions, wallet seed lookup helpers, and shared provisioning/testnet-switch journeys.
Wire fixtures and add specs
tests/e2e/fixtures/electron-fixture.ts, tests/e2e/onboarding.spec.ts, tests/e2e/import.spec.ts, tests/e2e/wallets.config.json
Adds worker-scoped Playwright fixtures for launching/provisioning the wallet, onboarding and import spec files, and a funded-wallet config entry.
Add E2E docs and balance test id
tests/e2e/e2e.md, src/components/WalletBalance.js
Documents the E2E architecture/conventions and adds a data-testid on the total balance element for test assertions.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Spec as Test Spec
  participant Fixture as electron-fixture
  participant Journeys as journeys.ts
  participant WalletApp as WalletApp
  participant Electron as electronApp driver

  Spec->>Fixture: request wallet fixture
  Fixture->>Electron: launchWallet()
  Electron-->>Fixture: LaunchedApp (app, page)
  Fixture->>WalletApp: new WalletApp(page)
  Fixture->>Journeys: provisionNewWallet / provisionImportedWallet
  Journeys->>WalletApp: chooseSoftwareWallet, createNewWords/enterSeed, setPassword, setPin
  WalletApp-->>Journeys: expectDashboardLoaded
  Fixture-->>Spec: provisioned WalletApp
  Spec->>Journeys: switchToTestnet (import spec)
  Journeys->>WalletApp: selectNetwork, enterNetworkPin, confirmTestnetModal
Loading

Suggested reviewers: pedroferreira1, tuliomir

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change from Cypress to a Playwright Electron E2E suite.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch raul-oliveira/feat/electron-e2e-playwright
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch raul-oliveira/feat/electron-e2e-playwright

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Add an E2E_TARGET=build mode so the same specs run against the
LavaMoat-hardened production build (build/index.html via file://), not just
the dev server. A Playwright globalSetup runs `npm run build`; the webServer
is skipped and Electron launches without ELECTRON_START_URL so it loads the
production bundle. Both journeys pass under LavaMoat's SES lockdown.

- npm run e2e         -> dev server (no LavaMoat)
- npm run e2e:release -> production build with LavaMoat
- E2E_SKIP_BUILD=1 reuses an existing build/ for faster iteration

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (4)
.github/workflows/e2e.yml (1)

16-49: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider adding job-level timeout-minutes and least-privilege permissions.

The job has no timeout, so a hung Electron/Playwright process (e.g. a stalled xvfb-run -a npm run e2e) could run until the default GitHub Actions limit. Also no permissions: block is set, so the job defaults to broader token scope than needed (this workflow only checks out code and runs tests).

🛡️ Suggested additions
 jobs:
   playwright-run:
     runs-on: ubuntu-22.04
+    timeout-minutes: 30
+    permissions:
+      contents: read
     strategy:
🤖 Prompt for 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.

In @.github/workflows/e2e.yml around lines 16 - 49, The playwright-run job in
the e2e workflow is missing guardrails: add a job-level timeout to prevent a
hung xvfb-run / npm run e2e from running indefinitely, and add an explicit
least-privilege permissions block since the job only needs to check out code and
execute tests. Update the job definition itself so the timeout and permissions
apply to all steps, keeping the existing Checkout, Setup nodejs, and Playwright
test steps unchanged.
tests/e2e/import.spec.ts (1)

15-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid duplicating the PIN literal; use the shared default.

switchToTestnet already defaults pin to CREDENTIALS.pin (the same value used by provisionImportedWallet to set up the wallet). Passing '123456' explicitly duplicates that value outside its single source of truth — if CREDENTIALS.pin ever changes, this call silently uses a stale PIN and the testnet switch will fail.

♻️ Proposed fix
-    await switchToTestnet(wallet, '123456');
+    await switchToTestnet(wallet);
🤖 Prompt for 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.

In `@tests/e2e/import.spec.ts` around lines 15 - 21, The test is duplicating the
wallet PIN by passing a hardcoded value into switchToTestnet even though that
helper already defaults to CREDENTIALS.pin. Update the call in the test that
switches the Hathor network to testnet so it relies on switchToTestnet’s default
PIN instead of supplying a literal, keeping it aligned with
provisionImportedWallet and the shared credential source.
tests/e2e/helpers/walletApp.ts (1)

90-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Direct hash navigation bypasses real UI flows.

openNetworkSettings, expectConnectedToTestnet, and goToDashboard navigate by setting window.location.hash via page.evaluate rather than clicking through the app's own nav/menu. This is a reasonable pragmatic shortcut for E2E setup, but it means these paths never exercise the actual navigation UI (menu links, routing guards). Worth keeping in mind if a future regression in the nav UI itself goes undetected by this suite.

🤖 Prompt for 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.

In `@tests/e2e/helpers/walletApp.ts` around lines 90 - 139, The navigation helpers
in walletApp use direct hash assignment via page.evaluate in
openNetworkSettings, expectConnectedToTestnet, and goToDashboard, which bypasses
the app’s real navigation UI. Update these helpers to drive navigation through
the actual visible menu/link controls or a shared UI navigation helper so the
E2E flow exercises routing guards and menu behavior instead of only the
hash-based route changes.
playwright.e2e.config.ts (1)

46-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Inline env-var shell syntax breaks on Windows.

PORT=${devServerPort} BROWSER=none npm start relies on POSIX shell VAR=value cmd syntax, which fails under Windows cmd.exe/PowerShell. Playwright's webServer.env option handles this portably.

♻️ Proposed fix
       ? {
-          command: `PORT=${devServerPort} BROWSER=none npm start`,
+          command: 'npm start',
+          env: { PORT: devServerPort, BROWSER: 'none' },
           url: devServerUrl,
🤖 Prompt for 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.

In `@playwright.e2e.config.ts` around lines 46 - 56, The webServer command in
playwright.e2e.config.ts uses POSIX inline env assignment, which breaks on
Windows. Update the webServer configuration for the target === 'dev' branch to
pass PORT and BROWSER through the webServer env option instead of prefixing them
in the command string, and keep the existing command focused on starting the app
with npm start.
🤖 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 @.github/workflows/e2e.yml:
- Around line 24-25: The Checkout step in the e2e workflow should disable git
credential persistence. Update the actions/checkout usage in the Checkout job to
set persist-credentials to false so the job does not retain the token after
cloning. Keep the change scoped to the checkout step only, since no later steps
in this workflow need to push or reuse git credentials.

In `@tests/e2e/driver/electronApp.ts`:
- Around line 24-53: The temporary userDataDir created in launchWallet can leak
when electron.launch() or app.firstWindow() throws before closeWallet ever runs.
Update launchWallet to ensure cleanup happens on all failure paths, ideally by
wrapping the launch and firstWindow flow in try/finally and removing the temp
directory if startup fails, while preserving the returned app/page/userDataDir
shape on success. Use the launchWallet helper and the userDataDir variable as
the main points to locate and adjust the lifecycle handling.

---

Nitpick comments:
In @.github/workflows/e2e.yml:
- Around line 16-49: The playwright-run job in the e2e workflow is missing
guardrails: add a job-level timeout to prevent a hung xvfb-run / npm run e2e
from running indefinitely, and add an explicit least-privilege permissions block
since the job only needs to check out code and execute tests. Update the job
definition itself so the timeout and permissions apply to all steps, keeping the
existing Checkout, Setup nodejs, and Playwright test steps unchanged.

In `@playwright.e2e.config.ts`:
- Around line 46-56: The webServer command in playwright.e2e.config.ts uses
POSIX inline env assignment, which breaks on Windows. Update the webServer
configuration for the target === 'dev' branch to pass PORT and BROWSER through
the webServer env option instead of prefixing them in the command string, and
keep the existing command focused on starting the app with npm start.

In `@tests/e2e/helpers/walletApp.ts`:
- Around line 90-139: The navigation helpers in walletApp use direct hash
assignment via page.evaluate in openNetworkSettings, expectConnectedToTestnet,
and goToDashboard, which bypasses the app’s real navigation UI. Update these
helpers to drive navigation through the actual visible menu/link controls or a
shared UI navigation helper so the E2E flow exercises routing guards and menu
behavior instead of only the hash-based route changes.

In `@tests/e2e/import.spec.ts`:
- Around line 15-21: The test is duplicating the wallet PIN by passing a
hardcoded value into switchToTestnet even though that helper already defaults to
CREDENTIALS.pin. Update the call in the test that switches the Hathor network to
testnet so it relies on switchToTestnet’s default PIN instead of supplying a
literal, keeping it aligned with provisionImportedWallet and the shared
credential source.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: b75e5993-27ec-4120-9000-29b16fea0dc7

📥 Commits

Reviewing files that changed from the base of the PR and between 23f902a and 80d0f16.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (26)
  • .env.e2e.example
  • .eslintignore
  • .github/workflows/e2e.yml
  • .gitignore
  • cypress.config.ts
  • cypress/e2e/00-welcome.cy.js
  • cypress/support/commands.js
  • cypress/support/e2e.js
  • lavamoat/webpack/policy.json
  • package.json
  • playwright.e2e.config.ts
  • src/components/WalletBalance.js
  • tests/e2e/driver/electronApp.ts
  • tests/e2e/driver/timeouts.ts
  • tests/e2e/e2e.md
  • tests/e2e/fixtures/electron-fixture.ts
  • tests/e2e/helpers/journeys.ts
  • tests/e2e/helpers/selectors.ts
  • tests/e2e/helpers/walletApp.ts
  • tests/e2e/helpers/wallets.ts
  • tests/e2e/import.spec.ts
  • tests/e2e/onboarding.spec.ts
  • tests/e2e/support/build.ts
  • tests/e2e/support/env.ts
  • tests/e2e/tsconfig.json
  • tests/e2e/wallets.config.json
💤 Files with no reviewable changes (4)
  • cypress/support/commands.js
  • cypress/e2e/00-welcome.cy.js
  • cypress/support/e2e.js
  • cypress.config.ts

Comment thread .github/workflows/e2e.yml
Comment on lines 24 to 25
- name: Checkout
# https://github.com/actions/checkout/releases/tag/v4.2.2
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Set persist-credentials: false on checkout.

Static analysis flags that the checkout step doesn't disable credential persistence. Since this job doesn't need to push or use git credentials afterward, disable persistence to reduce token exposure surface.

🔒 Suggested fix
       - name: Checkout
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
+        with:
+          persist-credentials: false
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Checkout
# https://github.com/actions/checkout/releases/tag/v4.2.2
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
- name: Checkout
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
with:
persist-credentials: false
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 24-25: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 Prompt for 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.

In @.github/workflows/e2e.yml around lines 24 - 25, The Checkout step in the e2e
workflow should disable git credential persistence. Update the actions/checkout
usage in the Checkout job to set persist-credentials to false so the job does
not retain the token after cloning. Keep the change scoped to the checkout step
only, since no later steps in this workflow need to push or reuse git
credentials.

Source: Linters/SAST tools

Comment on lines +24 to +53
export async function launchWallet(): Promise<LaunchedApp> {
const userDataDir = mkdtempSync(join(tmpdir(), 'hathor-wallet-e2e-'));

// build target: no ELECTRON_START_URL -> public/electron.js loads the production
// build/index.html via file:// (LavaMoat applied). dev target: point Electron at
// the CRA dev server (no LavaMoat).
const target = process.env.E2E_TARGET === 'build' ? 'build' : 'dev';
const env: NodeJS.ProcessEnv = { ...process.env, SENTRY_DSN: DUMMY_SENTRY_DSN };
if (target === 'build') {
delete env.ELECTRON_START_URL;
env.NODE_ENV = 'production';
} else {
env.ELECTRON_START_URL = env.ELECTRON_START_URL ?? 'http://localhost:3000';
env.NODE_ENV = 'dev';
}

const app = await electron.launch({
executablePath: electronBinary,
cwd: repoRoot,
args: ['.', '--no-sandbox', `--user-data-dir=${userDataDir}`],
// electron.launch types env as { [k: string]: string }; Node's process.env is
// { [k: string]: string | undefined }. At runtime present keys are strings, so
// reconcile the two here rather than filtering undefined values away.
env: env as { [key: string]: string },
timeout: TIMEOUTS.appLaunch,
});
const page = await app.firstWindow();
await page.waitForLoadState('domcontentloaded');
return { app, page, userDataDir };
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Temp userDataDir leaks if electron.launch() or firstWindow() throws.

mkdtempSync runs before the launch; if electron.launch times out/fails or app.firstWindow() rejects, the temp directory is never removed (closeWallet is never reached since launchWallet never returns an app to close). Over repeated CI failures this accumulates stray temp dirs.

🔧 Proposed fix
 export async function launchWallet(): Promise<LaunchedApp> {
   const userDataDir = mkdtempSync(join(tmpdir(), 'hathor-wallet-e2e-'));
-
-  // build target: ...
-  const target = process.env.E2E_TARGET === 'build' ? 'build' : 'dev';
-  const env: NodeJS.ProcessEnv = { ...process.env, SENTRY_DSN: DUMMY_SENTRY_DSN };
-  if (target === 'build') {
-    delete env.ELECTRON_START_URL;
-    env.NODE_ENV = 'production';
-  } else {
-    env.ELECTRON_START_URL = env.ELECTRON_START_URL ?? 'http://localhost:3000';
-    env.NODE_ENV = 'dev';
-  }
-
-  const app = await electron.launch({
-    executablePath: electronBinary,
-    cwd: repoRoot,
-    args: ['.', '--no-sandbox', `--user-data-dir=${userDataDir}`],
-    env: env as { [key: string]: string },
-    timeout: TIMEOUTS.appLaunch,
-  });
-  const page = await app.firstWindow();
-  await page.waitForLoadState('domcontentloaded');
-  return { app, page, userDataDir };
+  try {
+    const target = process.env.E2E_TARGET === 'build' ? 'build' : 'dev';
+    const env: NodeJS.ProcessEnv = { ...process.env, SENTRY_DSN: DUMMY_SENTRY_DSN };
+    if (target === 'build') {
+      delete env.ELECTRON_START_URL;
+      env.NODE_ENV = 'production';
+    } else {
+      env.ELECTRON_START_URL = env.ELECTRON_START_URL ?? 'http://localhost:3000';
+      env.NODE_ENV = 'dev';
+    }
+
+    const app = await electron.launch({
+      executablePath: electronBinary,
+      cwd: repoRoot,
+      args: ['.', '--no-sandbox', `--user-data-dir=${userDataDir}`],
+      env: env as { [key: string]: string },
+      timeout: TIMEOUTS.appLaunch,
+    });
+    const page = await app.firstWindow();
+    await page.waitForLoadState('domcontentloaded');
+    return { app, page, userDataDir };
+  } catch (err) {
+    rmSync(userDataDir, { recursive: true, force: true });
+    throw err;
+  }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function launchWallet(): Promise<LaunchedApp> {
const userDataDir = mkdtempSync(join(tmpdir(), 'hathor-wallet-e2e-'));
// build target: no ELECTRON_START_URL -> public/electron.js loads the production
// build/index.html via file:// (LavaMoat applied). dev target: point Electron at
// the CRA dev server (no LavaMoat).
const target = process.env.E2E_TARGET === 'build' ? 'build' : 'dev';
const env: NodeJS.ProcessEnv = { ...process.env, SENTRY_DSN: DUMMY_SENTRY_DSN };
if (target === 'build') {
delete env.ELECTRON_START_URL;
env.NODE_ENV = 'production';
} else {
env.ELECTRON_START_URL = env.ELECTRON_START_URL ?? 'http://localhost:3000';
env.NODE_ENV = 'dev';
}
const app = await electron.launch({
executablePath: electronBinary,
cwd: repoRoot,
args: ['.', '--no-sandbox', `--user-data-dir=${userDataDir}`],
// electron.launch types env as { [k: string]: string }; Node's process.env is
// { [k: string]: string | undefined }. At runtime present keys are strings, so
// reconcile the two here rather than filtering undefined values away.
env: env as { [key: string]: string },
timeout: TIMEOUTS.appLaunch,
});
const page = await app.firstWindow();
await page.waitForLoadState('domcontentloaded');
return { app, page, userDataDir };
}
export async function launchWallet(): Promise<LaunchedApp> {
const userDataDir = mkdtempSync(join(tmpdir(), 'hathor-wallet-e2e-'));
try {
// build target: no ELECTRON_START_URL -> public/electron.js loads the production
// build/index.html via file:// (LavaMoat applied). dev target: point Electron at
// the CRA dev server (no LavaMoat).
const target = process.env.E2E_TARGET === 'build' ? 'build' : 'dev';
const env: NodeJS.ProcessEnv = { ...process.env, SENTRY_DSN: DUMMY_SENTRY_DSN };
if (target === 'build') {
delete env.ELECTRON_START_URL;
env.NODE_ENV = 'production';
} else {
env.ELECTRON_START_URL = env.ELECTRON_START_URL ?? 'http://localhost:3000';
env.NODE_ENV = 'dev';
}
const app = await electron.launch({
executablePath: electronBinary,
cwd: repoRoot,
args: ['.', '--no-sandbox', `--user-data-dir=${userDataDir}`],
// electron.launch types env as { [k: string]: string }; Node's process.env is
// { [k: string]: string | undefined }. At runtime present keys are strings, so
// reconcile the two here rather than filtering undefined values away.
env: env as { [key: string]: string },
timeout: TIMEOUTS.appLaunch,
});
const page = await app.firstWindow();
await page.waitForLoadState('domcontentloaded');
return { app, page, userDataDir };
} catch (err) {
rmSync(userDataDir, { recursive: true, force: true });
throw err;
}
}
🤖 Prompt for 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.

In `@tests/e2e/driver/electronApp.ts` around lines 24 - 53, The temporary
userDataDir created in launchWallet can leak when electron.launch() or
app.firstWindow() throws before closeWallet ever runs. Update launchWallet to
ensure cleanup happens on all failure paths, ideally by wrapping the launch and
firstWindow flow in try/finally and removing the temp directory if startup
fails, while preserving the returned app/page/userDataDir shape on success. Use
the launchWallet helper and the userDataDir variable as the main points to
locate and adjust the lifecycle handling.

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