diff --git a/packages/playwright/src/runner/dispatcher.ts b/packages/playwright/src/runner/dispatcher.ts index 85a9c1357e..7645754b8c 100644 --- a/packages/playwright/src/runner/dispatcher.ts +++ b/packages/playwright/src/runner/dispatcher.ts @@ -117,21 +117,27 @@ export class Dispatcher { return; } + const stopCallback = () => this.stop().catch(() => {}); + const jobDispatcher = new JobDispatcher(job, this._reporter, this._failureTracker, stopCallback); + this._workerSlots[index].jobDispatcher = jobDispatcher; + // 2. Start the worker if it is down. + let startError; if (!worker) { worker = this._createWorker(job, index, serializeConfig(this._config)); this._workerSlots[index].worker = worker; worker.on('exit', () => this._workerSlots[index].worker = undefined); - await worker.start(); + startError = await worker.start(); if (this._isStopped) // Check stopped signal after async hop. return; } // 3. Run the job. - const stopCallback = () => this.stop().catch(() => {}); - const jobDispatcher = new JobDispatcher(job, this._reporter, this._failureTracker, stopCallback); - this._workerSlots[index].jobDispatcher = jobDispatcher; - const result = await jobDispatcher.runInWorker(worker); + if (startError) + jobDispatcher.onExit(startError); + else + jobDispatcher.runInWorker(worker); + const result = await jobDispatcher.jobResult; this._workerSlots[index].jobDispatcher = undefined; this._updateCounterForWorkerHash(job.workerHash, -1); @@ -265,7 +271,8 @@ export class Dispatcher { } class JobDispatcher { - private _jobResult = new ManualPromise<{ newJob?: TestGroup, didFail: boolean }>(); + jobResult = new ManualPromise<{ newJob?: TestGroup, didFail: boolean }>(); + private _listeners: RegisteredListener[] = []; private _failedTests = new Set(); private _remainingByTestId = new Map(); @@ -498,19 +505,19 @@ class JobDispatcher { this._finished({ didFail: true, newJob }); } - private _onExit(data: ProcessExitData) { + onExit(data: ProcessExitData) { const unexpectedExitError: TestError | undefined = data.unexpectedly ? { - message: `Internal error: worker process exited unexpectedly (code=${data.code}, signal=${data.signal})` + message: `Error: worker process exited unexpectedly (code=${data.code}, signal=${data.signal})` } : undefined; this._onDone({ skipTestsDueToSetupFailure: [], fatalErrors: [], unexpectedExitError }); } private _finished(result: { newJob?: TestGroup, didFail: boolean }) { eventsHelper.removeEventListeners(this._listeners); - this._jobResult.resolve(result); + this.jobResult.resolve(result); } - async runInWorker(worker: WorkerHost): Promise<{ newJob?: TestGroup, didFail: boolean }> { + runInWorker(worker: WorkerHost) { this._parallelIndex = worker.parallelIndex; this._workerIndex = worker.workerIndex; @@ -529,9 +536,8 @@ class JobDispatcher { eventsHelper.addEventListener(worker, 'stepEnd', this._onStepEnd.bind(this)), eventsHelper.addEventListener(worker, 'attach', this._onAttach.bind(this)), eventsHelper.addEventListener(worker, 'done', this._onDone.bind(this)), - eventsHelper.addEventListener(worker, 'exit', this._onExit.bind(this)), + eventsHelper.addEventListener(worker, 'exit', this.onExit.bind(this)), ]; - return this._jobResult; } currentlyRunning() { diff --git a/packages/playwright/src/runner/loadUtils.ts b/packages/playwright/src/runner/loadUtils.ts index a940b89d71..0b38552d7d 100644 --- a/packages/playwright/src/runner/loadUtils.ts +++ b/packages/playwright/src/runner/loadUtils.ts @@ -90,13 +90,14 @@ export async function loadFileSuites(testRun: TestRun, mode: 'out-of-process' | // Load test files. const fileSuiteByFile = new Map(); const loaderHost = mode === 'out-of-process' ? new OutOfProcessLoaderHost(config) : new InProcessLoaderHost(config); - await loaderHost.start(); - for (const file of allTestFiles) { - const fileSuite = await loaderHost.loadTestFile(file, errors); - fileSuiteByFile.set(file, fileSuite); - errors.push(...createDuplicateTitlesErrors(config, fileSuite)); + if (await loaderHost.start(errors)) { + for (const file of allTestFiles) { + const fileSuite = await loaderHost.loadTestFile(file, errors); + fileSuiteByFile.set(file, fileSuite); + errors.push(...createDuplicateTitlesErrors(config, fileSuite)); + } + await loaderHost.stop(); } - await loaderHost.stop(); // Check that no test file imports another test file. // Loader must be stopped first, since it popuplates the dependency tree. diff --git a/packages/playwright/src/runner/loaderHost.ts b/packages/playwright/src/runner/loaderHost.ts index d1e94688ed..0b2bac5dad 100644 --- a/packages/playwright/src/runner/loaderHost.ts +++ b/packages/playwright/src/runner/loaderHost.ts @@ -33,8 +33,9 @@ export class InProcessLoaderHost { this._poolBuilder = PoolBuilder.createForLoader(); } - async start() { + async start(errors: TestError[]) { await initializeEsmLoader(); + return true; } async loadTestFile(file: string, testErrors: TestError[]): Promise { @@ -57,8 +58,15 @@ export class OutOfProcessLoaderHost { this._processHost = new ProcessHost(require.resolve('../loader/loaderMain.js'), 'loader', {}); } - async start() { - await this._processHost.startRunner(serializeConfig(this._config)); + async start(errors: TestError[]) { + const startError = await this._processHost.startRunner(serializeConfig(this._config)); + if (startError) { + errors.push({ + message: `Test loader process failed to start with code "${startError.code}" and signal "${startError.signal}"`, + }); + return false; + } + return true; } async loadTestFile(file: string, testErrors: TestError[]): Promise { diff --git a/packages/playwright/src/runner/processHost.ts b/packages/playwright/src/runner/processHost.ts index 2e19bd1e7a..2773a72e1b 100644 --- a/packages/playwright/src/runner/processHost.ts +++ b/packages/playwright/src/runner/processHost.ts @@ -45,7 +45,7 @@ export class ProcessHost extends EventEmitter { this._extraEnv = env; } - async startRunner(runnerParams: any, options: { onStdOut?: (chunk: Buffer | string) => void, onStdErr?: (chunk: Buffer | string) => void } = {}) { + async startRunner(runnerParams: any, options: { onStdOut?: (chunk: Buffer | string) => void, onStdErr?: (chunk: Buffer | string) => void } = {}): Promise { this.process = child_process.fork(require.resolve('../common/process'), { detached: false, env: { ...process.env, ...this._extraEnv }, @@ -93,11 +93,14 @@ export class ProcessHost extends EventEmitter { if (options.onStdErr) this.process.stderr?.on('data', options.onStdErr); - await new Promise((resolve, reject) => { - this.process!.once('exit', (code, signal) => reject(new Error(`process exited with code "${code}" and signal "${signal}" before it became ready`))); - this.once('ready', () => resolve()); + const error = await new Promise(resolve => { + this.process!.once('exit', (code, signal) => resolve({ unexpectedly: true, code, signal })); + this.once('ready', () => resolve(undefined)); }); + if (error) + return error; + const processParams: ProcessInitParams = { stdoutParams: { rows: process.stdout.rows, diff --git a/packages/playwright/src/runner/workerHost.ts b/packages/playwright/src/runner/workerHost.ts index b6efc1d1e6..8df07fada9 100644 --- a/packages/playwright/src/runner/workerHost.ts +++ b/packages/playwright/src/runner/workerHost.ts @@ -55,7 +55,7 @@ export class WorkerHost extends ProcessHost { async start() { await fs.promises.mkdir(this._params.artifactsDir, { recursive: true }); - await this.startRunner(this._params, { + return await this.startRunner(this._params, { onStdOut: chunk => this.emit('stdOut', stdioChunkToParams(chunk)), onStdErr: chunk => this.emit('stdErr', stdioChunkToParams(chunk)), }); diff --git a/tests/playwright-test/expect.spec.ts b/tests/playwright-test/expect.spec.ts index e5dcd266b5..d2977dae25 100644 --- a/tests/playwright-test/expect.spec.ts +++ b/tests/playwright-test/expect.spec.ts @@ -649,7 +649,7 @@ test('should print expected/received on Ctrl+C', async ({ interactWithTestRunner `, }, { workers: 1 }); await testProcess.waitForOutput('%%SEND-SIGINT%%'); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); const { exitCode } = await testProcess.exited; expect(exitCode).toBe(130); diff --git a/tests/playwright-test/hooks.spec.ts b/tests/playwright-test/hooks.spec.ts index af5228f36a..6697482713 100644 --- a/tests/playwright-test/hooks.spec.ts +++ b/tests/playwright-test/hooks.spec.ts @@ -660,7 +660,7 @@ test('should not hang and report results when worker process suddenly exits duri expect(result.exitCode).toBe(1); expect(result.passed).toBe(0); expect(result.failed).toBe(1); - expect(result.output).toContain('Internal error: worker process exited unexpectedly'); + expect(result.output).toContain('Error: worker process exited unexpectedly'); expect(result.output).toContain('[1/1] a.spec.js:3:11 › failing due to afterall'); }); diff --git a/tests/playwright-test/playwright.spec.ts b/tests/playwright-test/playwright.spec.ts index 691149a8c1..3e259018cc 100644 --- a/tests/playwright-test/playwright.spec.ts +++ b/tests/playwright-test/playwright.spec.ts @@ -447,7 +447,7 @@ test('should report click error on sigint', async ({ interactWithTestRunner }) = `, }, { workers: 1 }); await testProcess.waitForOutput('%%SEND-SIGINT%%'); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); const { exitCode } = await testProcess.exited; expect(exitCode).toBe(130); diff --git a/tests/playwright-test/runner.spec.ts b/tests/playwright-test/runner.spec.ts index 0a576f97f8..72a401a1cc 100644 --- a/tests/playwright-test/runner.spec.ts +++ b/tests/playwright-test/runner.spec.ts @@ -92,7 +92,66 @@ test('should continue with other tests after worker process suddenly exits', asy expect(result.passed).toBe(4); expect(result.failed).toBe(1); expect(result.skipped).toBe(0); - expect(result.output).toContain('Internal error: worker process exited unexpectedly'); + expect(result.output).toContain('Error: worker process exited unexpectedly'); +}); + +test('should report subprocess creation error', async ({ runInlineTest }, testInfo) => { + const result = await runInlineTest({ + 'preload.js': ` + process.exit(42); + `, + 'a.spec.js': ` + import { test, expect } from '@playwright/test'; + test('fails', () => {}); + test('skipped', () => {}); + // Infect subprocesses to immediately exit when spawning a worker. + process.env.NODE_OPTIONS = '--require ${JSON.stringify(testInfo.outputPath('preload.js').replace(/\\/g, '\\\\'))}'; + ` + }); + expect(result.exitCode).toBe(1); + expect(result.passed).toBe(0); + expect(result.failed).toBe(1); + expect(result.skipped).toBe(1); + expect(result.output).toContain('Error: worker process exited unexpectedly (code=42, signal=null)'); +}); + +test('should ignore subprocess creation error because of SIGINT', async ({ interactWithTestRunner }, testInfo) => { + test.skip(process.platform === 'win32', 'No sending SIGINT on Windows'); + + const readyFile = testInfo.outputPath('ready.txt'); + const testProcess = await interactWithTestRunner({ + 'hang.js': ` + require('fs').writeFileSync(${JSON.stringify(readyFile)}, 'ready'); + setInterval(() => {}, 1000); + `, + 'preload.js': ` + require('child_process').spawnSync( + process.argv[0], + [require('path').resolve('./hang.js')], + { env: { ...process.env, NODE_OPTIONS: '' } }, + ); + `, + 'a.spec.js': ` + import { test, expect } from '@playwright/test'; + test('fails', () => {}); + test('skipped', () => {}); + // Infect subprocesses to immediately hang when spawning a worker. + process.env.NODE_OPTIONS = '--require ${JSON.stringify(testInfo.outputPath('preload.js'))}'; + ` + }); + + while (!fs.existsSync(readyFile)) + await new Promise(f => setTimeout(f, 100)); + process.kill(-testProcess.process.pid!, 'SIGINT'); + + const { exitCode } = await testProcess.exited; + expect(exitCode).toBe(130); + + const result = parseTestRunnerOutput(testProcess.output); + expect(result.passed).toBe(0); + expect(result.failed).toBe(0); + expect(result.skipped).toBe(2); + expect(result.output).not.toContain('worker process exited unexpectedly'); }); test('sigint should stop workers', async ({ interactWithTestRunner }) => { @@ -124,7 +183,7 @@ test('sigint should stop workers', async ({ interactWithTestRunner }) => { PLAYWRIGHT_JSON_OUTPUT_NAME: 'report.json', }); await testProcess.waitForOutput('%%SEND-SIGINT%%', 2); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); const { exitCode } = await testProcess.exited; expect(exitCode).toBe(130); @@ -200,7 +259,7 @@ test('worker interrupt should report errors', async ({ interactWithTestRunner }) `, }); await testProcess.waitForOutput('%%SEND-SIGINT%%'); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); const { exitCode } = await testProcess.exited; expect(exitCode).toBe(130); @@ -378,7 +437,7 @@ test('sigint should stop global setup', async ({ interactWithTestRunner }) => { `, }, { 'workers': 1 }); await testProcess.waitForOutput('%%SEND-SIGINT%%'); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); const { exitCode } = await testProcess.exited; expect(exitCode).toBe(130); @@ -425,7 +484,7 @@ test('sigint should stop plugins', async ({ interactWithTestRunner }) => { `, }, { 'workers': 1 }); await testProcess.waitForOutput('%%SEND-SIGINT%%'); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); const { exitCode } = await testProcess.exited; expect(exitCode).toBe(130); @@ -473,7 +532,7 @@ test('sigint should stop plugins 2', async ({ interactWithTestRunner }) => { `, }, { 'workers': 1 }); await testProcess.waitForOutput('%%SEND-SIGINT%%'); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); const { exitCode } = await testProcess.exited; expect(exitCode).toBe(130); @@ -578,7 +637,7 @@ test('should not hang on worker error in test file', async ({ runInlineTest }) = }, { 'timeout': 3000 }); expect(result.exitCode).toBe(1); expect(result.results[0].status).toBe('failed'); - expect(result.results[0].error.message).toContain('Internal error: worker process exited unexpectedly'); + expect(result.results[0].error.message).toContain('Error: worker process exited unexpectedly'); expect(result.results[1].status).toBe('skipped'); }); @@ -606,8 +665,8 @@ test('fast double SIGINT should be ignored', async ({ interactWithTestRunner }) }); await testProcess.waitForOutput('%%SEND-SIGINT%%'); // Send SIGINT twice in quick succession. - process.kill(testProcess.process.pid!, 'SIGINT'); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); const { exitCode } = await testProcess.exited; expect(exitCode).toBe(130); @@ -639,9 +698,9 @@ test('slow double SIGINT should be respected', async ({ interactWithTestRunner } `, }); await testProcess.waitForOutput('%%SEND-SIGINT%%'); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); await new Promise(f => setTimeout(f, 2000)); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); const { exitCode } = await testProcess.exited; expect(exitCode).toBe(130); @@ -680,10 +739,10 @@ test('slow double SIGINT should be respected in reporter.onExit', async ({ inter `, }, { reporter: '' }); await testProcess.waitForOutput('%%SEND-SIGINT%%'); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); await new Promise(f => setTimeout(f, 2000)); await testProcess.waitForOutput('MyReporter.onExit started'); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); const { exitCode, signal } = await testProcess.exited; expect(exitCode).toBe(null); expect(signal).toBe('SIGINT'); // Default handler should report the signal. diff --git a/tests/playwright-test/web-server.spec.ts b/tests/playwright-test/web-server.spec.ts index bb8d49c274..5fac838dac 100644 --- a/tests/playwright-test/web-server.spec.ts +++ b/tests/playwright-test/web-server.spec.ts @@ -735,7 +735,7 @@ test('should forward stdout when set to "pipe" before server is ready', async ({ `, }, { workers: 1 }); await testProcess.waitForOutput('%%SEND-SIGINT%%'); - process.kill(testProcess.process.pid!, 'SIGINT'); + process.kill(-testProcess.process.pid!, 'SIGINT'); await testProcess.exited; const result = parseTestRunnerOutput(testProcess.output);