From afa25aee961607e73d1afa42086cd000b73c77c9 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 8 Oct 2026 18:27:03 -0500 Subject: [PATCH] test: run spawned Electron apps with a timeout instead of spawnSync (#54735) Several specs started a whole Electron app with spawnSync() and no timeout. spawnSync() blocks the spec worker's event loop until the app exits, so a hung app also blocks the test's own timeout. Nothing then fails or reports until the whole run is killed. Most of them also ignored the app's exit status. Use spawnAndWait() with a 20 second timeout instead. It kills a hung app and fails the test with the app's output. Also check that each app exits with code 0, which all of these fixtures do on success: - node.spec.ts: "Can find a module using a package.json main field" - api-protocol.spec.ts: "throws for invalid scheme names" and the custom protocol code cache tests - api-app.spec.ts: the setPath() session data tests - modules.spec.ts: "can be required if electron.exe is renamed" Co-authored-by: Claude Opus 5.5 --- spec/api-app.spec.ts | 20 +++++++++++++------- spec/api-protocol.spec.ts | 22 ++++++++++++++-------- spec/modules.spec.ts | 8 ++++---- spec/node.spec.ts | 9 +++++---- 4 files changed, 36 insertions(+), 23 deletions(-) diff --git a/spec/api-app.spec.ts b/spec/api-app.spec.ts index a193a4520b..e5386d29d4 100644 --- a/spec/api-app.spec.ts +++ b/spec/api-app.spec.ts @@ -16,7 +16,7 @@ import { setTimeout } from 'node:timers/promises'; import { promisify } from 'node:util'; import { collectStreamBody, getResponse } from './lib/net-helpers.ts'; -import { defer, ifdescribe, ifit, isWayland, listen, waitUntil } from './lib/spec-helpers.ts'; +import { defer, ifdescribe, ifit, isWayland, listen, spawnAndWait, waitUntil } from './lib/spec-helpers.ts'; import { runInUtilityProcess } from './lib/utility-process-helpers.ts'; import { closeWindow, closeAllWindows } from './lib/window-helpers.ts'; import { @@ -1428,22 +1428,28 @@ describe('app module', () => { fs.rmSync(tempBrowserDataPath, { force: true, recursive: true }); }); - it('writes to userData by default', () => { + // The app quits once it has written its session data. + const runApp = async (args: string[]) => { + const { code, stdout, stderr } = await spawnAndWait(process.execPath, [appPath, ...args], { timeout: 20_000 }); + expect(code).to.equal(0, `stdout:\n${stdout}\nstderr:\n${stderr}`); + }; + + it('writes to userData by default', async () => { expect(hasSessionFiles(userDataPath)).to.equal(false); - cp.spawnSync(process.execPath, [appPath]); + await runApp([]); expect(hasSessionFiles(userDataPath)).to.equal(true); }); - it('can be changed', () => { + it('can be changed', async () => { expect(hasSessionFiles(userDataPath)).to.equal(false); - cp.spawnSync(process.execPath, [appPath, 'sessionData', tempBrowserDataPath]); + await runApp(['sessionData', tempBrowserDataPath]); expect(hasSessionFiles(userDataPath)).to.equal(false); expect(hasSessionFiles(tempBrowserDataPath)).to.equal(true); }); - it('changing userData affects default sessionData', () => { + it('changing userData affects default sessionData', async () => { expect(hasSessionFiles(userDataPath)).to.equal(false); - cp.spawnSync(process.execPath, [appPath, 'userData', tempBrowserDataPath]); + await runApp(['userData', tempBrowserDataPath]); expect(hasSessionFiles(userDataPath)).to.equal(false); expect(hasSessionFiles(tempBrowserDataPath)).to.equal(true); }); diff --git a/spec/api-protocol.spec.ts b/spec/api-protocol.spec.ts index 803ff5bab1..de984686b8 100644 --- a/spec/api-protocol.spec.ts +++ b/spec/api-protocol.spec.ts @@ -19,7 +19,7 @@ import * as url from 'node:url'; import * as zlib from 'node:zlib'; import { collectStreamBody, getResponse } from './lib/net-helpers.ts'; -import { listen, defer, ifit } from './lib/spec-helpers.ts'; +import { listen, defer, ifit, spawnAndWait } from './lib/spec-helpers.ts'; import { WebmGenerator } from './lib/video-helpers.js'; import { closeAllWindows, closeWindow } from './lib/window-helpers.ts'; @@ -962,7 +962,7 @@ describe('protocol module', () => { expect(stderr).to.not.contain('VALIDATION_ERROR_DESERIALIZATION_FAILED'); }); - it('throws for invalid scheme names', () => { + it('throws for invalid scheme names', async () => { const appPath = path.join(fixturesPath, 'apps', 'remote-control'); const bootEval = ` const { protocol } = require('electron'); @@ -978,10 +978,10 @@ describe('protocol module', () => { console.log(JSON.stringify(results)); process.exit(0); `; - const result = ChildProcess.spawnSync(process.execPath, [appPath, `--boot-eval=${bootEval}`]); - - const stdout = result.stdout.toString(); - expect(result.status, `stdout: ${stdout}\nstderr: ${result.stderr.toString()}`).to.equal(0); + const { code, stdout, stderr } = await spawnAndWait(process.execPath, [appPath, `--boot-eval=${bootEval}`], { + timeout: 20_000 + }); + expect(code, `stdout: ${stdout}\nstderr: ${stderr}`).to.equal(0); const line = stdout.split('\n').find((l) => l.startsWith('{')); expect(line, `unexpected stdout: ${stdout}`).to.be.a('string'); @@ -1817,12 +1817,18 @@ describe('protocol module', () => { }); it('code cache in custom protocol is disabled by default', async () => { - ChildProcess.spawnSync(process.execPath, [appPath, 'false', codeCachePath]); + const { code, stdout, stderr } = await spawnAndWait(process.execPath, [appPath, 'false', codeCachePath], { + timeout: 20_000 + }); + expect(code).to.equal(0, `stdout:\n${stdout}\nstderr:\n${stderr}`); expect(fs.readdirSync(path.join(codeCachePath, 'js')).length).to.equal(2); }); it('codeCache:true enables codeCache in custom protocol', async () => { - ChildProcess.spawnSync(process.execPath, [appPath, 'true', codeCachePath]); + const { code, stdout, stderr } = await spawnAndWait(process.execPath, [appPath, 'true', codeCachePath], { + timeout: 20_000 + }); + expect(code).to.equal(0, `stdout:\n${stdout}\nstderr:\n${stderr}`); expect(fs.readdirSync(path.join(codeCachePath, 'js')).length).to.above(2); }); }); diff --git a/spec/modules.spec.ts b/spec/modules.spec.ts index e418b4e477..2475619a81 100644 --- a/spec/modules.spec.ts +++ b/spec/modules.spec.ts @@ -8,7 +8,7 @@ import * as fs from 'node:fs'; import { createRequire } from 'node:module'; import * as path from 'node:path'; -import { ifdescribe, ifit } from './lib/spec-helpers.ts'; +import { ifdescribe, ifit, spawnAndWait } from './lib/spec-helpers.ts'; import { closeAllWindows } from './lib/window-helpers.ts'; const require = createRequire(import.meta.url); @@ -38,14 +38,14 @@ describe('modules support', () => { expect(msg).to.equal('ok'); }); - ifit(process.platform === 'win32')('can be required if electron.exe is renamed', () => { + ifit(process.platform === 'win32')('can be required if electron.exe is renamed', async () => { const testExecPath = path.join(path.dirname(process.execPath), 'test.exe'); fs.copyFileSync(process.execPath, testExecPath); try { const fixture = path.join(fixtures, 'module', 'echo-renamed.js'); expect(fs.existsSync(fixture)).to.be.true(); - const child = childProcess.spawnSync(testExecPath, [fixture]); - expect(child.status).to.equal(0); + const { code, stdout, stderr } = await spawnAndWait(testExecPath, [fixture], { timeout: 20_000 }); + expect(code).to.equal(0, `stdout:\n${stdout}\nstderr:\n${stderr}`); } finally { fs.unlinkSync(testExecPath); } diff --git a/spec/node.spec.ts b/spec/node.spec.ts index bcfd2d49c8..40fdd8a208 100644 --- a/spec/node.spec.ts +++ b/spec/node.spec.ts @@ -25,6 +25,7 @@ import { ifit, isTestingBindingAvailable, itremote, + spawnAndWait, startRemoteControlApp, useRemoteContext } from './lib/spec-helpers.ts'; @@ -1537,13 +1538,13 @@ describe('node feature', () => { } }); - it('Can find a module using a package.json main field', () => { - const result = childProcess.spawnSync( + it('Can find a module using a package.json main field', async () => { + const { code, stdout, stderr } = await spawnAndWait( process.execPath, [path.resolve(fixtures, 'api', 'electron-main-module', 'app.asar')], - { stdio: 'inherit' } + { timeout: 20_000 } ); - expect(result.status).to.equal(0); + expect(code).to.equal(0, `stdout:\n${stdout}\nstderr:\n${stderr}`); }); it('handles Promise timeouts correctly', async () => {