From 08afb34c14ae8da33697a5ebcf8ddbfd2edf965f Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 15 Feb 2024 11:38:13 -0800 Subject: [PATCH] chore(test runner): make timeout error an Error (#29515) --- packages/playwright/src/util.ts | 7 ++++--- packages/playwright/src/worker/testInfo.ts | 5 +++-- packages/playwright/src/worker/timeoutManager.ts | 15 +++++++-------- packages/playwright/src/worker/workerMain.ts | 2 +- tests/playwright-test/test-step.spec.ts | 4 ++-- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/packages/playwright/src/util.ts b/packages/playwright/src/util.ts index 17e406d514..f6e12f202e 100644 --- a/packages/playwright/src/util.ts +++ b/packages/playwright/src/util.ts @@ -31,13 +31,14 @@ const PLAYWRIGHT_TEST_PATH = path.join(__dirname, '..'); const PLAYWRIGHT_CORE_PATH = path.dirname(require.resolve('playwright-core/package.json')); export function filterStackTrace(e: Error): { message: string, stack: string } { + const name = e.name ? e.name + ': ' : ''; if (process.env.PWDEBUGIMPL) - return { message: e.name + ': ' + e.message, stack: e.stack || '' }; + return { message: name + e.message, stack: e.stack || '' }; const stackLines = stringifyStackFrames(filteredStackTrace(e.stack?.split('\n') || [])); return { - message: e.name + ': ' + e.message, - stack: `${e.name}: ${e.message}\n${stackLines.join('\n')}` + message: name + e.message, + stack: `${name}${e.message}${stackLines.map(line => '\n' + line).join('')}` }; } diff --git a/packages/playwright/src/worker/testInfo.ts b/packages/playwright/src/worker/testInfo.ts index 45fe34d9b0..0f00006e1e 100644 --- a/packages/playwright/src/worker/testInfo.ts +++ b/packages/playwright/src/worker/testInfo.ts @@ -226,8 +226,9 @@ export class TestInfoImpl implements TestInfo { // consider it a timeout. if (!this._wasInterrupted && timeoutError && !this._didTimeout) { this._didTimeout = true; - this.errors.push(timeoutError); - this._tracing.appendForError(timeoutError); + const serialized = serializeError(timeoutError); + this.errors.push(serialized); + this._tracing.appendForError(serialized); // Do not overwrite existing failure upon hook/teardown timeout. if (this.status === 'passed' || this.status === 'skipped') this.status = 'timedOut'; diff --git a/packages/playwright/src/worker/timeoutManager.ts b/packages/playwright/src/worker/timeoutManager.ts index 56b0272eba..94e182f682 100644 --- a/packages/playwright/src/worker/timeoutManager.ts +++ b/packages/playwright/src/worker/timeoutManager.ts @@ -16,7 +16,6 @@ import { colors } from 'playwright-core/lib/utilsBundle'; import { TimeoutRunner, TimeoutRunnerError } from 'playwright-core/lib/utils'; -import type { TestInfoError } from '../../types/test'; import type { Location } from '../../types/testReporter'; export type TimeSlot = { @@ -86,7 +85,7 @@ export class TimeoutManager { this._timeoutRunner.updateTimeout(slot.timeout); } - async runWithTimeout(cb: () => Promise): Promise { + async runWithTimeout(cb: () => Promise): Promise { try { await this._timeoutRunner.run(cb); } catch (error) { @@ -127,7 +126,7 @@ export class TimeoutManager { this._timeoutRunner.updateTimeout(slot.timeout, slot.elapsed); } - private _createTimeoutError(): TestInfoError { + private _createTimeoutError(): Error { let message = ''; const timeout = this._currentSlot().timeout; switch (this._runnable.type) { @@ -174,10 +173,10 @@ export class TimeoutManager { message = `Fixture "${fixtureWithSlot.title}" timeout of ${timeout}ms exceeded during ${fixtureWithSlot.phase}.`; message = colors.red(message); const location = (fixtureWithSlot || this._runnable).location; - return { - message, - // Include location for hooks, modifiers and fixtures to distinguish between them. - stack: location ? message + `\n at ${location.file}:${location.line}:${location.column}` : undefined - }; + const error = new Error(message); + error.name = ''; + // Include location for hooks, modifiers and fixtures to distinguish between them. + error.stack = message + (location ? `\n at ${location.file}:${location.line}:${location.column}` : ''); + return error; } } diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index 45867cc4d6..f080d08e0c 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -170,7 +170,7 @@ export class WorkerMain extends ProcessRunner { await this._fixtureRunner.teardownScope('worker', timeoutManager, e => this._fatalErrors.push(serializeError(e))); }); if (timeoutError) - this._fatalErrors.push(timeoutError); + this._fatalErrors.push(serializeError(timeoutError)); }); } diff --git a/tests/playwright-test/test-step.spec.ts b/tests/playwright-test/test-step.spec.ts index 5f1b386ce3..c00521e9ae 100644 --- a/tests/playwright-test/test-step.spec.ts +++ b/tests/playwright-test/test-step.spec.ts @@ -354,7 +354,7 @@ test('should not report nested after hooks', async ({ runInlineTest }) => { { error: { message: 'Test timeout of 2000ms exceeded.', - stack: '' + stack: 'Test timeout of 2000ms exceeded.', }, }, ]); @@ -981,7 +981,7 @@ test('should not mark page.close as failed when page.click fails', async ({ runI { error: { message: 'Test timeout of 2000ms exceeded.', - stack: '', + stack: 'Test timeout of 2000ms exceeded.', }, }, {