test(electron): require confirmed process exit before relaunch

This commit is contained in:
4gray committed 2026-09-22 21:34:46 +02:00
1 parent 19ff9a9e5b
commit f48be2ac63
6 files changed
+79 -60

No files matched your search

+1 -1
View File
@@ -1152,7 +1152,7 @@ jobs:
- name: Upload packaged frame-copy smoke diagnostics
if: always() && matrix.os == 'linux' && matrix.linux_profile == 'portable'
uses: actions/upload-artifact@v7
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a
with:
name: packaged-frame-copy-smoke
path: |
+1 -1
View File
@@ -69,7 +69,7 @@ jobs:
run: pnpm nx build electron-backend
- name: Verify Electron process cleanup
run: pnpm exec tsx --test apps/electron-backend-e2e/src/performance/electron-process-termination.spec.ts
run: pnpm exec tsx --test apps/electron-backend-e2e/src/performance/electron-process-lifecycle.spec.ts apps/electron-backend-e2e/src/performance/electron-process-termination.spec.ts
- name: Install Playwright Browsers
run: pnpm exec playwright install --with-deps
@@ -1,8 +1,9 @@
import type { ChildProcess } from 'node:child_process';
import { terminateElectronProcess } from './electron-process-termination';
type ElectronChildProcess = Pick<
ChildProcess,
'exitCode' | 'kill' | 'once' | 'removeListener' | 'signalCode'
'exitCode' | 'kill' | 'once' | 'pid' | 'removeListener' | 'signalCode'
>;
export interface ClosableElectronApplication {
@@ -49,7 +50,11 @@ export async function prepareElectronApplication<Application, Prepared>(
export async function closeElectronApplicationAndConfirmExit(
application: ClosableElectronApplication,
options: ElectronExitConfirmationOptions
options: ElectronExitConfirmationOptions,
terminate: (
child: ElectronChildProcess,
signal: NodeJS.Signals
) => void = terminateElectronProcess
): Promise<void> {
assertTimeout(options.closeTimeoutMs);
assertTimeout(options.exitTimeoutMs);
@@ -82,14 +87,19 @@ export async function closeElectronApplicationAndConfirmExit(
}
if (first.kind === 'close-rejected') failure = first.failure;
try {
child.kill();
} catch (killFailure) {
failure = failure
? new AggregateError([failure, killFailure])
: killFailure;
// A failed termination must not skip exit observation or let a
// caller restart against a profile that is still owned by Electron.
for (const signal of ['SIGTERM', 'SIGKILL'] as const) {
try {
terminate(child, signal);
} catch (killFailure) {
failure = failure
? new AggregateError([failure, killFailure])
: killFailure;
}
if (await resolvesWithin(exit.promise, options.exitTimeoutMs))
return;
}
if (await resolvesWithin(exit.promise, options.exitTimeoutMs)) return;
} finally {
exit.cancel();
}
@@ -31,7 +31,6 @@ import {
closeElectronApplicationAndConfirmExit,
prepareElectronApplication,
} from './electron-process-lifecycle';
import { terminateElectronProcess } from './electron-process-termination';
export const workspaceRoot = resolve(__dirname, '../../..');
export const electronMainPath = join(
@@ -565,52 +564,7 @@ export async function launchCompetingElectronInstance(
export async function closeElectronApp(
app: LaunchedElectronApp
): Promise<void> {
try {
const closePromise = app.electronApp.close();
const closed = await waitForPromiseWithTimeout(
closePromise,
electronAppCloseTimeoutMs
);
if (closed) {
return;
}
console.warn(
`Electron app did not close within ${electronAppCloseTimeoutMs}ms; killing process`
);
const childProcess = app.electronApp.process();
if (!childProcess.killed) {
terminateElectronProcess(childProcess);
}
await waitForPromiseWithTimeout(
closePromise.catch(() => undefined),
electronAppKillWaitMs
);
// SIGTERM asks Electron for a graceful quit, which the app can
// legitimately refuse — the unsaved-settings close guard cancels the
// quit while it waits for an answer. A process that survives here
// would outlive the test, hold its data dir, and time out the worker
// teardown, so escalate to SIGKILL.
if (
childProcess.exitCode === null &&
childProcess.signalCode === null
) {
console.warn(
'Electron app survived SIGTERM; escalating to SIGKILL'
);
terminateElectronProcess(childProcess, 'SIGKILL');
await waitForPromiseWithTimeout(
closePromise.catch(() => undefined),
electronAppKillWaitMs
);
}
} catch (error) {
console.warn('Failed to close Electron app cleanly:', error);
}
await closeElectronAppAndConfirmExit(app);
}
export async function closeElectronAppAndConfirmExit(
@@ -1,13 +1,27 @@
import assert from 'node:assert/strict';
import { EventEmitter } from 'node:events';
import { describe, it } from 'node:test';
import {
closeElectronApp,
type LaunchedElectronApp,
} from '../electron-test-fixtures';
import {
closeElectronApplicationAndConfirmExit,
closeElectronApplicationAndConfirmExit as closeApplication,
ElectronApplicationDisposalError,
prepareElectronApplication,
} from '../electron-process-lifecycle';
// Fake children have no OS PID; exercise lifecycle decisions independently
// of the platform process-tree integration test.
const closeElectronApplicationAndConfirmExit: typeof closeApplication = (
app,
options
) =>
closeApplication(app, options, (child, signal) => {
child.kill(signal);
});
class FakeChildProcess extends EventEmitter {
exitCode: number | null = null;
signalCode: NodeJS.Signals | null = null;
@@ -25,6 +39,25 @@ class FakeChildProcess extends EventEmitter {
}
describe('Electron process lifecycle', () => {
it('does not return from public cleanup when close and termination both fail', async () => {
const child = new FakeChildProcess();
child.kill = () => {
throw new Error('termination failed');
};
const app = {
electronApp: {
close: async () => {
throw new Error('CDP disconnected');
},
process: () => child,
},
} as unknown as LaunchedElectronApp;
await assert.rejects(
closeElectronApp(app),
/electron-process-exit-unconfirmed/
);
});
it('closes and confirms exit when post-spawn launch preparation fails', async () => {
const child = new FakeChildProcess();
const launchFailure = new Error('renderer readiness failed');
@@ -71,7 +104,7 @@ describe('Electron process lifecycle', () => {
}),
/electron-process-exit-unconfirmed/
);
assert.equal(child.killCalls, 1);
assert.equal(child.killCalls, 2);
});
it('preserves both preparation and unconfirmed-disposal failures', async () => {
@@ -137,4 +170,23 @@ describe('Electron process lifecycle', () => {
assert.equal(child.killCalls, 1);
assert.equal(child.signalCode, 'SIGTERM');
});
it('still observes exit after the first termination attempt throws', async () => {
const child = new FakeChildProcess();
const signals: NodeJS.Signals[] = [];
await closeApplication(
{
close: () => new Promise<void>(() => undefined),
process: () => child,
},
{ closeTimeoutMs: 1, exitTimeoutMs: 1 },
(_child, signal) => {
signals.push(signal);
if (signal === 'SIGTERM') throw new Error('termination failed');
child.kill();
}
);
assert.deepEqual(signals, ['SIGTERM', 'SIGKILL']);
assert.equal(child.signalCode, 'SIGTERM');
});
});
+3
View File
@@ -75,6 +75,9 @@ only `electronApp.process()` can leave Electron holding the temporary profile
and make the next launch exit on the single-instance lock. The E2E workflow
checks this against a real Windows shell and child process. Test cleanup must
target only its own launch PID, never all Electron or IPTVnator processes.
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.
The Linux portable build uploads `packaged-frame-copy-smoke` reports and traces
even when the smoke fails. Check the paused-frame screenshot and trace before