test: guarantee e2e teardown, bound the job, skip unused browsers

Address review feedback. Close the Electron app in a finally block in the
shutdown test so the process is always terminated even if an earlier step
throws. Add a job timeout-minutes so a stuck launch/close fails fast instead of
tying up the runner. Set PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1 since only Electron
is launched, never Playwright's bundled browsers, so their download during
yarn install is wasted time and network flakiness. Rename the launch helper's
`home` to `userDataDir` since it is a temp userData dir, not the home path.

Note: AI-assisted (Claude Code). Manually verified: 3 e2e tests pass locally;
tsc, eslint, prettier clean.

Author: notluquis

.github/workflows/e2e.yml

@@ -17,12 +17,19 @@ jobs:
   e2e:
     name: 'Playwright (Linux)'
     runs-on: ubuntu-latest
+    # Bound the job so a stuck Electron launch/close fails fast instead of
+    # tying up a runner; this suite specifically guards shutdown hangs.
+    timeout-minutes: 15
+    env:
+      # Only Electron is launched here, never Playwright's bundled browsers, so
+      # skip their download during yarn install (faster, fewer network flakes).
+      PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: '1'
     steps:
       - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5
       - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5
         with:
-          # Node 24 (active LTS), not 26 (Current): keep the e2e runner on the
-          # stable line for Electron's install tooling and Playwright.
+          # Match the Node that the target Electron bundles (Electron 42 -> Node
+          # 24); do not run ahead of it. Node 26 broke Electron's install tooling.
           node-version: '24.x'
           cache: 'yarn'
       # Cache the @electron/get binary download so launch does not depend on a

test/e2e/helpers.ts

@@ -7,27 +7,28 @@ import { tmpdir } from 'os';
 // A fresh userData dir per launch keeps tests independent. Electron's native
 // --user-data-dir flag is honored because the app reads app.getPath('userData')
 // (see getUserDataDir in utils) and only overrides it on Snap. Returns the app
-// plus the temp dir so the caller can clean it up in a finally block. If launch
-// fails, the temp dir is removed here so repeated runs don't accumulate dirs.
+// plus the temp userData dir so the caller can clean it up in a finally block.
+// If launch fails, the temp dir is removed here so repeated runs don't
+// accumulate dirs.
 export async function launchApp(): Promise<{
   app: ElectronApplication;
-  home: string;
+  userDataDir: string;
 }> {
-  const home = mkdtempSync(join(tmpdir(), 'jlab-e2e-'));
+  const userDataDir = mkdtempSync(join(tmpdir(), 'jlab-e2e-'));
   try {
     const app = await electron.launch({
-      args: ['.', `--user-data-dir=${home}`]
+      args: ['.', `--user-data-dir=${userDataDir}`]
     });
     await stubAllDialogs(app);
-    return { app, home };
+    return { app, userDataDir };
   } catch (error) {
-    cleanup(home);
+    cleanup(userDataDir);
     throw error;
   }
 }
 
-export function cleanup(home: string): void {
-  rmSync(home, { recursive: true, force: true });
+export function cleanup(userDataDir: string): void {
+  rmSync(userDataDir, { recursive: true, force: true });
 }
 
 // The app opens several windows (titlebar, welcome, session, manager) and most

test/e2e/smoke.test.ts

@@ -2,18 +2,18 @@ import { expect, test } from '@playwright/test';
 import { cleanup, launchApp, pageByTitle } from './helpers';
 
 test('app launches and opens at least one window', async () => {
-  const { app, home } = await launchApp();
+  const { app, userDataDir } = await launchApp();
   try {
     await app.firstWindow();
     expect(app.windows().length).toBeGreaterThan(0);
   } finally {
     await app.close();
-    cleanup(home);
+    cleanup(userDataDir);
   }
 });
 
 test('on first run the Welcome window renders', async () => {
-  const { app, home } = await launchApp();
+  const { app, userDataDir } = await launchApp();
   try {
     // Selecting by title rather than firstWindow(): the app opens several
     // windows and only this one is the welcome view.
@@ -24,19 +24,20 @@ test('on first run the Welcome window renders', async () => {
     await expect(welcome.locator('#new-notebook-link')).toBeVisible();
   } finally {
     await app.close();
-    cleanup(home);
+    cleanup(userDataDir);
   }
 });
 
 test('app shuts down cleanly without hanging', async () => {
-  const { app, home } = await launchApp();
+  const { app, userDataDir } = await launchApp();
   try {
     await app.firstWindow();
-    // app.close() resolves once the process exits; a hang would fail via the
-    // suite timeout. Assert close completes and the windows are gone.
-    await app.close();
-    expect(app.windows().length).toBe(0);
   } finally {
-    cleanup(home);
+    // Close in finally so the Electron process is always terminated, even if
+    // an earlier step throws. A hang here surfaces via the job timeout.
+    await app.close();
+    cleanup(userDataDir);
   }
+  // close() has resolved, so the process exited and the windows are gone.
+  expect(app.windows().length).toBe(0);
 });