diff --git a/apps/electron-backend-e2e/src/electron-process-lifecycle.ts b/apps/electron-backend-e2e/src/electron-process-lifecycle.ts index 11bb38a3e..d0302668b 100644 --- a/apps/electron-backend-e2e/src/electron-process-lifecycle.ts +++ b/apps/electron-backend-e2e/src/electron-process-lifecycle.ts @@ -11,11 +11,23 @@ export interface ClosableElectronApplication { } export interface ElectronExitConfirmationOptions { - readonly childProcess: ElectronChildProcess; readonly closeTimeoutMs: number; readonly exitTimeoutMs: number; } +const electronProcesses = new WeakMap< + ClosableElectronApplication, + ElectronChildProcess +>(); + +export function captureElectronProcess( + application: ClosableElectronApplication & { process(): Process } +): Process { + const child = application.process(); + electronProcesses.set(application, child); + return child; +} + export interface PrepareElectronApplicationOptions { readonly application: Application; readonly dispose: (application: Application) => Promise; @@ -61,7 +73,8 @@ export async function closeElectronApplicationAndConfirmExit( // Playwright discards its process dispatcher when Electron exits. Keep // using the Node handle captured immediately after launch, including when // the application's last window already caused a normal process exit. - const child = options.childProcess; + const child = electronProcesses.get(application); + if (!child) throw new Error('electron-process-handle-not-captured'); if (hasExited(child)) return; const exit = observeProcessExit(child); diff --git a/apps/electron-backend-e2e/src/electron-test-fixtures.ts b/apps/electron-backend-e2e/src/electron-test-fixtures.ts index be8ffc951..6aa458b81 100644 --- a/apps/electron-backend-e2e/src/electron-test-fixtures.ts +++ b/apps/electron-backend-e2e/src/electron-test-fixtures.ts @@ -28,6 +28,7 @@ import { writeDataDirOwnerMarker, } from './data-dir-reaper'; import { + captureElectronProcess, closeElectronApplicationAndConfirmExit, prepareElectronApplication, } from './electron-process-lifecycle'; @@ -136,7 +137,6 @@ declare global { export type LaunchedElectronApp = { electronApp: ElectronApplication; - electronProcess: ChildProcess; mainWindow: Page; }; @@ -235,12 +235,11 @@ export async function launchElectronApp( args, env: buildElectronLaunchEnvironment(dataDir, options), }); - const electronProcess = electronApp.process(); + const electronProcess = captureElectronProcess(electronApp); return prepareElectronApplication({ application: electronApp, dispose: (application) => closeElectronApplicationAndConfirmExit(application, { - childProcess: electronProcess, closeTimeoutMs: electronAppCloseTimeoutMs, exitTimeoutMs: electronAppKillWaitMs, }), @@ -253,7 +252,6 @@ export async function launchElectronApp( await startRendererFrameCapture(mainWindow); return { electronApp: application, - electronProcess, mainWindow, }; }, @@ -462,12 +460,11 @@ export async function launchPackagedElectronApp( NODE_ENV: 'test', }, }); - const electronProcess = electronApp.process(); + const electronProcess = captureElectronProcess(electronApp); return prepareElectronApplication({ application: electronApp, dispose: (application) => closeElectronApplicationAndConfirmExit(application, { - childProcess: electronProcess, closeTimeoutMs: electronAppCloseTimeoutMs, exitTimeoutMs: electronAppKillWaitMs, }), @@ -477,7 +474,6 @@ export async function launchPackagedElectronApp( await waitForAppReady(mainWindow); return { electronApp: application, - electronProcess, mainWindow, }; }, @@ -582,7 +578,6 @@ export async function closeElectronAppAndConfirmExit( app: LaunchedElectronApp ): Promise { await closeElectronApplicationAndConfirmExit(app.electronApp, { - childProcess: app.electronProcess, closeTimeoutMs: electronAppCloseTimeoutMs, exitTimeoutMs: electronAppKillWaitMs, }); diff --git a/apps/electron-backend-e2e/src/legacy-playlist-migration.e2e.ts b/apps/electron-backend-e2e/src/legacy-playlist-migration.e2e.ts index 1e91bd8dc..2a9e44037 100644 --- a/apps/electron-backend-e2e/src/legacy-playlist-migration.e2e.ts +++ b/apps/electron-backend-e2e/src/legacy-playlist-migration.e2e.ts @@ -18,6 +18,7 @@ import { openSettingsSection, } from './electron-test-fixtures'; import { seedLegacyProfile, legacyPlaylists } from './legacy-profile-fixture'; +import { captureElectronProcess } from './electron-process-lifecycle'; import { applyTheme } from './theme-contrast'; interface StartupTestGlobals { @@ -94,6 +95,7 @@ require(${JSON.stringify(electronMainPath)});` ], env: buildElectronLaunchEnvironment(dataDir), }); + captureElectronProcess(app); const page = await app.firstWindow(); try { // The route is intentionally still empty while recovery is pending. diff --git a/apps/electron-backend-e2e/src/performance/electron-process-lifecycle.spec.ts b/apps/electron-backend-e2e/src/performance/electron-process-lifecycle.spec.ts index d7e3024a1..025c937bc 100644 --- a/apps/electron-backend-e2e/src/performance/electron-process-lifecycle.spec.ts +++ b/apps/electron-backend-e2e/src/performance/electron-process-lifecycle.spec.ts @@ -7,6 +7,7 @@ import { } from '../electron-test-fixtures'; import { + captureElectronProcess, closeElectronApplicationAndConfirmExit as closeApplication, ElectronApplicationDisposalError, prepareElectronApplication, @@ -39,25 +40,67 @@ class FakeChildProcess extends EventEmitter { } describe('Electron process lifecycle', () => { + it('rejects cleanup when no process was captured for that application', async () => { + await assert.rejects( + closeApplication( + { close: async () => assert.fail('exit cannot be confirmed') }, + { closeTimeoutMs: 1, exitTimeoutMs: 1 } + ), + /electron-process-handle-not-captured/ + ); + }); + + it('cleans the replacement application after a partial restart assignment', async () => { + const oldChild = new FakeChildProcess(); + const newChild = new FakeChildProcess(); + const oldApplication = { + close: async () => { + oldChild.exitCode = 0; + oldChild.emit('exit', 0, null); + }, + process: () => oldChild, + }; + const newApplication = { + close: async () => { + newChild.exitCode = 0; + newChild.emit('exit', 0, null); + }, + process: () => newChild, + }; + const app = { + electronApp: oldApplication, + } as unknown as LaunchedElectronApp; + captureElectronProcess(oldApplication); + captureElectronProcess(newApplication); + await closeElectronApp(app); + // Existing E2E callers replace the application/window fields only. + app.electronApp = + newApplication as unknown as LaunchedElectronApp['electronApp']; + + await closeElectronApp(app); + assert.equal(newChild.exitCode, 0); + }); + it('confirms an already exited process after Playwright disposes its dispatcher', async () => { const child = new FakeChildProcess(); - child.exitCode = 0; const app = { - electronProcess: child, electronApp: { close: async () => { assert.fail( 'an exited application must not be closed again' ); }, - process: () => { - throw new TypeError( - "Cannot read properties of undefined (reading '_object')" - ); - }, + process: () => child, }, } as unknown as LaunchedElectronApp; + captureElectronProcess(app.electronApp); + child.exitCode = 0; + app.electronApp.process = () => { + throw new TypeError( + "Cannot read properties of undefined (reading '_object')" + ); + }; await closeElectronApp(app); assert.equal(child.killCalls, 0); }); @@ -68,17 +111,18 @@ describe('Electron process lifecycle', () => { throw new Error('termination failed'); }; const app = { - electronProcess: child, electronApp: { close: async () => { throw new Error('CDP disconnected'); }, - process: () => { - assert.fail('cleanup must use the retained child process'); - }, + process: () => child, }, } as unknown as LaunchedElectronApp; + captureElectronProcess(app.electronApp); + app.electronApp.process = () => { + assert.fail('cleanup must use the retained child process'); + }; await assert.rejects( closeElectronApp(app), /electron-process-exit-unconfirmed/ @@ -96,13 +140,13 @@ describe('Electron process lifecycle', () => { }, process: () => child, }; + captureElectronProcess(application); await assert.rejects( prepareElectronApplication({ application, dispose: (app) => closeElectronApplicationAndConfirmExit(app, { - childProcess: child, closeTimeoutMs: 10, exitTimeoutMs: 10, }), @@ -123,22 +167,25 @@ describe('Electron process lifecycle', () => { close: async () => { assert.fail('an exited application must not be closed again'); }, - process: () => { - assert.fail('the Playwright dispatcher is already disposed'); - }, + process: () => child, }; + captureElectronProcess(application); await assert.rejects( prepareElectronApplication({ application, dispose: (app) => closeElectronApplicationAndConfirmExit(app, { - childProcess: child, closeTimeoutMs: 10, exitTimeoutMs: 10, }), prepare: async () => { child.exitCode = 0; + application.process = () => { + assert.fail( + 'the Playwright dispatcher is already disposed' + ); + }; throw launchFailure; }, }), @@ -154,10 +201,10 @@ describe('Electron process lifecycle', () => { close: () => new Promise(() => undefined), process: () => child, }; + captureElectronProcess(application); await assert.rejects( closeElectronApplicationAndConfirmExit(application, { - childProcess: child, closeTimeoutMs: 1, exitTimeoutMs: 1, }), @@ -199,10 +246,10 @@ describe('Electron process lifecycle', () => { }, process: () => child, }; + captureElectronProcess(application); await Promise.race([ closeElectronApplicationAndConfirmExit(application, { - childProcess: child, closeTimeoutMs: 1_000, exitTimeoutMs: 10, }), @@ -222,9 +269,9 @@ describe('Electron process lifecycle', () => { close: () => new Promise(() => undefined), process: () => child, }; + captureElectronProcess(application); await closeElectronApplicationAndConfirmExit(application, { - childProcess: child, closeTimeoutMs: 1, exitTimeoutMs: 10, }); @@ -235,11 +282,14 @@ describe('Electron process lifecycle', () => { it('still observes exit after the first termination attempt throws', async () => { const child = new FakeChildProcess(); const signals: NodeJS.Signals[] = []; + const application = { + close: () => new Promise(() => undefined), + process: () => child, + }; + captureElectronProcess(application); await closeApplication( - { - close: () => new Promise(() => undefined), - }, - { childProcess: child, closeTimeoutMs: 1, exitTimeoutMs: 1 }, + application, + { closeTimeoutMs: 1, exitTimeoutMs: 1 }, (_child, signal) => { signals.push(signal); if (signal === 'SIGTERM') throw new Error('termination failed'); diff --git a/docs/development/electron-debugging.md b/docs/development/electron-debugging.md index 200f3378e..ee1e35ba3 100644 --- a/docs/development/electron-debugging.md +++ b/docs/development/electron-debugging.md @@ -79,7 +79,10 @@ Cleanup confirms process exit even if Playwright's close promise fails or never settles. If termination fails, it retries and then throws instead of allowing a relaunch against a potentially locked profile. Capture the Node child-process handle immediately after launch and retain it -for cleanup and preparation failures. After the last window closes on Linux, +in a WeakMap keyed by the Electron application for cleanup and preparation +failures. This binding follows the application when restart callers replace +only the application/window fields on their fixture. After the last window +closes on Linux, Electron can exit before cleanup begins; Playwright disposes its dispatcher, so calling `electronApp.process()` at that point can throw even after a clean exit. The retained handle still provides the actual exit code and signal.