diff --git a/packages/playwright/src/common/process.ts b/packages/playwright/src/common/process.ts index 4e1e2672ff..14ad995fed 100644 --- a/packages/playwright/src/common/process.ts +++ b/packages/playwright/src/common/process.ts @@ -44,11 +44,12 @@ export class ProcessRunner { } } -let closed = false; +let gracefullyCloseCalled = false; +let forceExitInitiated = false; sendMessageToParent({ method: 'ready' }); -process.on('disconnect', gracefullyCloseAndExit); +process.on('disconnect', () => gracefullyCloseAndExit(true)); process.on('SIGINT', () => {}); process.on('SIGTERM', () => {}); @@ -76,7 +77,7 @@ process.on('message', async (message: any) => { const keys = new Set([...Object.keys(process.env), ...Object.keys(startingEnv)]); const producedEnv: EnvProducedPayload = [...keys].filter(key => startingEnv[key] !== process.env[key]).map(key => [key, process.env[key] ?? null]); sendMessageToParent({ method: '__env_produced__', params: producedEnv }); - await gracefullyCloseAndExit(); + await gracefullyCloseAndExit(false); return; } if (message.method === '__dispatch__') { @@ -92,19 +93,24 @@ process.on('message', async (message: any) => { } }); -async function gracefullyCloseAndExit() { - if (closed) - return; - closed = true; - // Force exit after 30 seconds. - // eslint-disable-next-line no-restricted-properties - setTimeout(() => process.exit(0), 30000); - // Meanwhile, try to gracefully shutdown. - await processRunner?.gracefullyClose().catch(() => {}); - if (processName) - await stopProfiling(processName).catch(() => {}); - // eslint-disable-next-line no-restricted-properties - process.exit(0); +const kForceExitTimeout = +(process.env.PWTEST_FORCE_EXIT_TIMEOUT || 30000); + +async function gracefullyCloseAndExit(forceExit: boolean) { + if (forceExit && !forceExitInitiated) { + forceExitInitiated = true; + // Force exit after 30 seconds. + // eslint-disable-next-line no-restricted-properties + setTimeout(() => process.exit(0), kForceExitTimeout); + } + if (!gracefullyCloseCalled) { + gracefullyCloseCalled = true; + // Meanwhile, try to gracefully shutdown. + await processRunner?.gracefullyClose().catch(() => {}); + if (processName) + await stopProfiling(processName).catch(() => {}); + // eslint-disable-next-line no-restricted-properties + process.exit(0); + } } function sendMessageToParent(message: { method: string, params?: any }) { diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index 9ae9a86207..ea2cdbaebc 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -100,12 +100,17 @@ export class WorkerMain extends ProcessRunner { override async gracefullyClose() { try { await this._stop(); + // Ignore top-level errors, they are already inside TestInfo.errors. + const fakeTestInfo = new TestInfoImpl(this._config, this._project, this._params, undefined, 0, () => {}, () => {}, () => {}); + const runnable = { type: 'teardown' } as const; // We have to load the project to get the right deadline below. - await this._loadIfNeeded(); - await this._teardownScopes(); + await fakeTestInfo._runAsStage({ title: 'worker cleanup', runnable }, () => this._loadIfNeeded()).catch(() => {}); + await this._fixtureRunner.teardownScope('test', fakeTestInfo, runnable).catch(() => {}); + await this._fixtureRunner.teardownScope('worker', fakeTestInfo, runnable).catch(() => {}); // Close any other browsers launched in this process. This includes anything launched // manually in the test/hooks and internal browsers like Playwright Inspector. - await gracefullyCloseAll(); + await fakeTestInfo._runAsStage({ title: 'worker cleanup', runnable }, () => gracefullyCloseAll()).catch(() => {}); + this._fatalErrors.push(...fakeTestInfo.errors); } catch (e) { this._fatalErrors.push(serializeError(e)); } @@ -144,15 +149,6 @@ export class WorkerMain extends ProcessRunner { } } - private async _teardownScopes() { - const fakeTestInfo = new TestInfoImpl(this._config, this._project, this._params, undefined, 0, () => {}, () => {}, () => {}); - const runnable = { type: 'teardown' } as const; - // Ignore top-level errors, they are already inside TestInfo.errors. - await this._fixtureRunner.teardownScope('test', fakeTestInfo, runnable).catch(() => {}); - await this._fixtureRunner.teardownScope('worker', fakeTestInfo, runnable).catch(() => {}); - this._fatalErrors.push(...fakeTestInfo.errors); - } - unhandledError(error: Error | any) { // No current test - fatal error. if (!this._currentTest) { diff --git a/tests/playwright-test/timeout.spec.ts b/tests/playwright-test/timeout.spec.ts index 0ffa2164ce..50069b3280 100644 --- a/tests/playwright-test/timeout.spec.ts +++ b/tests/playwright-test/timeout.spec.ts @@ -514,3 +514,47 @@ test('should report up to 3 timeout errors', async ({ runInlineTest }) => { expect(result.output).toContain('Test timeout of 1000ms exceeded while running "afterEach" hook.'); expect(result.output).toContain('Worker teardown timeout of 1000ms exceeded while tearing down "autoWorker".'); }); + +test('should complain when worker fixture times out during worker cleanup', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + import { test as base, expect } from '@playwright/test'; + const test = base.extend({ + slowTeardown: [async ({}, use) => { + await use('hey'); + await new Promise(f => setTimeout(f, 2000)); + }, { scope: 'worker', auto: true, timeout: 400 }], + }); + test('test ok', async ({ slowTeardown }) => { + expect(slowTeardown).toBe('hey'); + }); + ` + }); + expect(result.exitCode).toBe(1); + expect(result.passed).toBe(1); + expect(result.output).toContain(`Fixture "slowTeardown" timeout of 400ms exceeded during teardown.`); +}); + +test('should allow custom worker fixture timeout longer than force exit cap', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + import { test as base, expect } from '@playwright/test'; + const test = base.extend({ + slowTeardown: [async ({}, use) => { + await use('hey'); + await new Promise(f => setTimeout(f, 1500)); + console.log('output from teardown'); + throw new Error('Oh my!'); + }, { scope: 'worker', auto: true, timeout: 2000 }], + }); + test('test ok', async ({ slowTeardown }) => { + expect(slowTeardown).toBe('hey'); + }); + ` + }, {}, { PWTEST_FORCE_EXIT_TIMEOUT: '400' }); + expect(result.exitCode).toBe(1); + expect(result.passed).toBe(1); + expect(result.output).toContain(`output from teardown`); + expect(result.output).toContain(`Error: Oh my!`); + expect(result.output).toContain(`1 error was not a part of any test, see above for details`); +});