diff --git a/packages/playwright/src/matchers/expect.ts b/packages/playwright/src/matchers/expect.ts index 2bf73ef3ac2833..178f40db508fd9 100644 --- a/packages/playwright/src/matchers/expect.ts +++ b/packages/playwright/src/matchers/expect.ts @@ -248,45 +248,66 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler { if (!testInfo) return matcher.call(target, ...args); - const rawStack = captureRawStack(); - const stackFrames = filteredStackTrace(rawStack); const customMessage = this._info.message || ''; const argsSuffix = computeArgsSuffix(matcherName, args); const defaultTitle = `expect${this._info.isPoll ? '.poll' : ''}${this._info.isSoft ? '.soft' : ''}${this._info.isNot ? '.not' : ''}.${matcherName}${argsSuffix}`; const title = customMessage || defaultTitle; const wallTime = Date.now(); - const step = matcherName !== 'toPass' ? testInfo._addStep({ - location: stackFrames[0], + + // This looks like it is unnecessary, but it isn't - we need to filter + // out all the frames that belong to the test runner from caught runtime errors. + const rawStack = captureRawStack(); + const stackFrames = filteredStackTrace(rawStack); + + // Enclose toPass in a step to maintain async stacks, toPass matcher is always async. + if (matcherName === 'toPass') { + return testInfo._runAsStep({ + category: 'expect', + title: trimLongString(title, 1024), + wallTime, + infectParentStepsWithError: this._info.isSoft, + laxParent: true, + isSoft: this._info.isSoft, + }, async step => { + try { + const result = await matcher.call(target, ...args); + step.complete({}); + return result; + } catch (jestError) { + const error = new ExpectError(jestError, customMessage, stackFrames); + step.complete({ error }); + if (!this._info.isSoft) + throw error; + } + }); + } + + const step = testInfo._addStep({ category: 'expect', title: trimLongString(title, 1024), params: args[0] ? { expected: args[0] } : undefined, wallTime, infectParentStepsWithError: this._info.isSoft, laxParent: true, - }) : undefined; + isSoft: this._info.isSoft, + }); const reportStepError = (jestError: ExpectError) => { const error = new ExpectError(jestError, customMessage, stackFrames); - const serializedError = { - message: error.message, - stack: error.stack, - }; - step?.complete({ error: serializedError }); - if (this._info.isSoft) - testInfo._failWithError(serializedError, false /* isHardError */); - else + step.complete({ error }); + if (!this._info.isSoft) throw error; }; const finalizer = () => { - step?.complete({}); + step.complete({}); }; - const expectZone: ExpectZone = { title, wallTime }; - try { - const result = zones.run('expectZone', expectZone, () => matcher.call(target, ...args)); + const expectZone: ExpectZone | null = matcherName !== 'toPass' ? { title, wallTime } : null; + const callback = () => matcher.call(target, ...args); + const result = expectZone ? zones.run('expectZone', expectZone, callback) : zones.preserve(callback); if (result instanceof Promise) return result.then(finalizer).catch(reportStepError); finalizer(); diff --git a/packages/playwright/src/matchers/matcherHint.ts b/packages/playwright/src/matchers/matcherHint.ts index 8a345537449f1c..229ef6ca2863b8 100644 --- a/packages/playwright/src/matchers/matcherHint.ts +++ b/packages/playwright/src/matchers/matcherHint.ts @@ -49,6 +49,7 @@ export class ExpectError extends Error { log?: string[]; timeout?: number; }; + constructor(jestError: ExpectError, customMessage: string, stackFrames: StackFrame[]) { super(''); // Copy to erase the JestMatcherError constructor name from the console.log(error). diff --git a/packages/playwright/src/matchers/matchers.ts b/packages/playwright/src/matchers/matchers.ts index cd8e54a37d6fd4..91bdc3701bdda5 100644 --- a/packages/playwright/src/matchers/matchers.ts +++ b/packages/playwright/src/matchers/matchers.ts @@ -17,13 +17,13 @@ import type { Locator, Page, APIResponse } from 'playwright-core'; import type { FrameExpectOptions } from 'playwright-core/lib/client/types'; import { colors } from 'playwright-core/lib/utilsBundle'; -import { expectTypes, callLogText, filteredStackTrace } from '../util'; +import { expectTypes, callLogText } from '../util'; import { toBeTruthy } from './toBeTruthy'; import { toEqual } from './toEqual'; import { toExpectedTextValues, toMatchText } from './toMatchText'; -import { captureRawStack, constructURLBasedOnBaseURL, isRegExp, isTextualMimeType, pollAgainstDeadline } from 'playwright-core/lib/utils'; +import { constructURLBasedOnBaseURL, isRegExp, isTextualMimeType, pollAgainstDeadline } from 'playwright-core/lib/utils'; import { currentTestInfo } from '../common/globals'; -import { TestInfoImpl, type TestStepInternal } from '../worker/testInfo'; +import { TestInfoImpl } from '../worker/testInfo'; import type { ExpectMatcherContext } from './expect'; interface LocatorEx extends Locator { @@ -369,42 +369,26 @@ export async function toPass( const testInfo = currentTestInfo(); const timeout = options.timeout !== undefined ? options.timeout : 0; - const rawStack = captureRawStack(); - const stackFrames = filteredStackTrace(rawStack); - - const runWithOrWithoutStep = async (callback: (step: TestStepInternal | undefined) => Promise<{ pass: boolean; message: () => string; }>) => { - if (!testInfo) - return await callback(undefined); - return await testInfo._runAsStep({ - title: 'expect.toPass', - category: 'expect', - location: stackFrames[0], - }, callback); - }; - - return await runWithOrWithoutStep(async (step: TestStepInternal | undefined) => { - const { deadline, timeoutMessage } = testInfo ? testInfo._deadlineForMatcher(timeout) : TestInfoImpl._defaultDeadlineForMatcher(timeout); - const result = await pollAgainstDeadline(async () => { - if (testInfo && currentTestInfo() !== testInfo) - return { continuePolling: false, result: undefined }; - try { - await callback(); - return { continuePolling: !!this.isNot, result: undefined }; - } catch (e) { - return { continuePolling: !this.isNot, result: e }; - } - }, deadline, options.intervals || [100, 250, 500, 1000]); - - if (result.timedOut) { - const message = result.result ? [ - result.result.message, - '', - `Call Log:`, - `- ${timeoutMessage}`, - ].join('\n') : timeoutMessage; - step?.complete({ error: { message } }); - return { message: () => message, pass: !!this.isNot }; + const { deadline, timeoutMessage } = testInfo ? testInfo._deadlineForMatcher(timeout) : TestInfoImpl._defaultDeadlineForMatcher(timeout); + const result = await pollAgainstDeadline(async () => { + if (testInfo && currentTestInfo() !== testInfo) + return { continuePolling: false, result: undefined }; + try { + await callback(); + return { continuePolling: !!this.isNot, result: undefined }; + } catch (e) { + return { continuePolling: !this.isNot, result: e }; } - return { pass: !this.isNot, message: () => '' }; - }); + }, deadline, options.intervals || [100, 250, 500, 1000]); + + if (result.timedOut) { + const message = result.result ? [ + result.result.message, + '', + `Call Log:`, + `- ${timeoutMessage}`, + ].join('\n') : timeoutMessage; + return { message: () => message, pass: !!this.isNot }; + } + return { pass: !this.isNot, message: () => '' }; } diff --git a/packages/playwright/src/matchers/toMatchSnapshot.ts b/packages/playwright/src/matchers/toMatchSnapshot.ts index 511a42395eb3d4..e551ebf9c13b88 100644 --- a/packages/playwright/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright/src/matchers/toMatchSnapshot.ts @@ -22,7 +22,7 @@ import type { ImageComparatorOptions, Comparator } from 'playwright-core/lib/uti import { getComparator, sanitizeForFilePath, zones } from 'playwright-core/lib/utils'; import type { PageScreenshotOptions } from 'playwright-core/types/types'; import { - addSuffixToFilePath, serializeError, + addSuffixToFilePath, trimLongString, callLogText, expectTypes } from '../util'; import { colors } from 'playwright-core/lib/utilsBundle'; @@ -206,7 +206,7 @@ class SnapshotHelper { return this.createMatcherResult(message, true); } if (this.updateSnapshots === 'missing') { - this.testInfo._failWithError(serializeError(new Error(message)), false /* isHardError */); + this.testInfo._failWithError(new Error(message), false /* isHardError */); return this.createMatcherResult('', true); } return this.createMatcherResult(message, false); diff --git a/packages/playwright/src/worker/fixtureRunner.ts b/packages/playwright/src/worker/fixtureRunner.ts index 2e5e0d09c46e2a..520921722d4a1f 100644 --- a/packages/playwright/src/worker/fixtureRunner.ts +++ b/packages/playwright/src/worker/fixtureRunner.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { formatLocation, debugTest, filterStackFile, serializeError } from '../util'; +import { formatLocation, debugTest, filterStackFile } from '../util'; import { ManualPromise, zones } from 'playwright-core/lib/utils'; import type { TestInfoImpl, TestStepInternal } from './testInfo'; import type { FixtureDescription, TimeoutManager } from './timeoutManager'; @@ -143,7 +143,7 @@ class Fixture { this._selfTeardownComplete?.then(() => { afterStep?.complete({}); }).catch(e => { - afterStep?.complete({ error: serializeError(e) }); + afterStep?.complete({ error: e }); }); } testInfo._timeoutManager.setCurrentFixture(undefined); diff --git a/packages/playwright/src/worker/testInfo.ts b/packages/playwright/src/worker/testInfo.ts index f9f5ce95ca93a0..c402f04ef0c755 100644 --- a/packages/playwright/src/worker/testInfo.ts +++ b/packages/playwright/src/worker/testInfo.ts @@ -30,7 +30,7 @@ import type { Attachment } from './testTracing'; import type { StackFrame } from '@protocol/channels'; export interface TestStepInternal { - complete(result: { error?: Error | TestInfoError, attachments?: Attachment[] }): void; + complete(result: { error?: Error, attachments?: Attachment[] }): void; stepId: string; title: string; category: string; @@ -45,6 +45,7 @@ export interface TestStepInternal { error?: TestInfoError; infectParentStepsWithError?: boolean; box?: boolean; + isSoft?: boolean; } export class TestInfoImpl implements TestInfo { @@ -231,7 +232,7 @@ export class TestInfoImpl implements TestInfo { this.duration = this._timeoutManager.defaultSlotTimings().elapsed | 0; } - async _runAndFailOnError(fn: () => Promise, skips?: 'allowSkips'): Promise { + async _runAndFailOnError(fn: () => Promise, skips?: 'allowSkips'): Promise { try { await fn(); } catch (error) { @@ -239,9 +240,8 @@ export class TestInfoImpl implements TestInfo { if (this.status === 'passed') this.status = 'skipped'; } else { - const serialized = serializeError(error); - this._failWithError(serialized, true /* isHardError */); - return serialized; + this._failWithError(error, true /* isHardError */); + return error; } } } @@ -283,24 +283,18 @@ export class TestInfoImpl implements TestInfo { complete: result => { if (step.endWallTime) return; + step.endWallTime = Date.now(); - let error: TestInfoError | undefined; - if (result.error instanceof Error) { - // Step function threw an error. - if (data.boxedStack) { - const errorTitle = `${result.error.name}: ${result.error.message}`; - result.error.stack = `${errorTitle}\n${stringifyStackFrames(data.boxedStack).join('\n')}`; - } - error = serializeError(result.error); - } else if (result.error) { - // Internal API step reported an error. + if (result.error) { + if (!(result.error as any)[stepSymbol]) + (result.error as any)[stepSymbol] = step; + const error = serializeError(result.error); if (data.boxedStack) - result.error.stack = `${result.error.message}\n${stringifyStackFrames(data.boxedStack).join('\n')}`; - error = result.error; + error.stack = `${error.message}\n${stringifyStackFrames(data.boxedStack).join('\n')}`; + step.error = error; } - step.error = error; - if (!error) { + if (!step.error) { // Soft errors inside try/catch will make the test fail. // In order to locate the failing step, we are marking all the parent // steps as failing unconditionally. @@ -311,18 +305,20 @@ export class TestInfoImpl implements TestInfo { break; } } - error = step.error; } const payload: StepEndPayload = { testId: this._test.id, stepId, wallTime: step.endWallTime, - error, + error: step.error, }; this._onStepEnd(payload); - const errorForTrace = error ? { name: '', message: error.message || '', stack: error.stack } : undefined; + const errorForTrace = step.error ? { name: '', message: step.error.message || '', stack: step.error.stack } : undefined; this._tracing.appendAfterActionForStep(stepId, errorForTrace, result.attachments); + + if (step.isSoft && result.error) + this._failWithError(result.error, false /* isHardError */); } }; const parentStepList = parentStep ? parentStep.steps : this._steps; @@ -350,7 +346,7 @@ export class TestInfoImpl implements TestInfo { this.status = 'interrupted'; } - _failWithError(error: TestInfoError, isHardError: boolean) { + _failWithError(error: Error, isHardError: boolean) { // Do not overwrite any previous hard errors. // Some (but not all) scenarios include: // - expect() that fails after uncaught exception. @@ -361,7 +357,11 @@ export class TestInfoImpl implements TestInfo { this._hasHardError = true; if (this.status === 'passed' || this.status === 'skipped') this.status = 'failed'; - this.errors.push(error); + const serialized = serializeError(error); + const step = (error as any)[stepSymbol] as TestStepInternal | undefined; + if (step && step.boxedStack) + serialized.stack = `${error.name}: ${error.message}\n${stringifyStackFrames(step.boxedStack).join('\n')}`; + this.errors.push(serialized); } async _runAsStepWithRunnable( @@ -486,3 +486,5 @@ export class TestInfoImpl implements TestInfo { class SkipError extends Error { } + +const stepSymbol = Symbol('step'); diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index b3bb77a0bc4a89..bd529bbfd8f2b6 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -170,7 +170,7 @@ export class WorkerMain extends ProcessRunner { // and unhandled errors - both lead to the test failing. This is good for regular tests, // so that you can, e.g. expect() from inside an event handler. The test fails, // and we restart the worker. - this._currentTest._failWithError(serializeError(error), true /* isHardError */); + this._currentTest._failWithError(error, true /* isHardError */); // For tests marked with test.fail(), this might be a problem when unhandled error // is not coming from the user test code (legit failure), but from fixtures or test runner. @@ -395,7 +395,7 @@ export class WorkerMain extends ProcessRunner { const afterHooksSlot = testInfo._didTimeout ? { timeout: this._project.project.timeout, elapsed: 0 } : undefined; await testInfo._runAsStepWithRunnable({ category: 'hook', title: 'After Hooks', runnableType: 'afterHooks', runnableSlot: afterHooksSlot }, async step => { testInfo._afterHooksStep = step; - let firstAfterHooksError: TestInfoError | undefined; + let firstAfterHooksError: Error | undefined; await testInfo._runWithTimeout(async () => { // Note: do not wrap all teardown steps together, because failure in any of them // does not prevent further teardown steps from running. @@ -541,11 +541,11 @@ export class WorkerMain extends ProcessRunner { throw beforeAllError; } - private async _runAfterAllHooksForSuite(suite: Suite, testInfo: TestInfoImpl) { + private async _runAfterAllHooksForSuite(suite: Suite, testInfo: TestInfoImpl): Promise { if (!this._activeSuites.has(suite)) return; this._activeSuites.delete(suite); - let firstError: TestInfoError | undefined; + let firstError: Error | undefined; for (const hook of suite._hooks) { if (hook.type !== 'afterAll') continue; diff --git a/tests/playwright-test/reporter.spec.ts b/tests/playwright-test/reporter.spec.ts index fa303bf7b8355b..4157310dd9e5ae 100644 --- a/tests/playwright-test/reporter.spec.ts +++ b/tests/playwright-test/reporter.spec.ts @@ -274,7 +274,7 @@ for (const useIntermediateMergeReport of [false, true] as const) { `begin {\"title\":\"expect.toBeTruthy\",\"category\":\"expect\"}`, `end {\"title\":\"expect.toBeTruthy\",\"category\":\"expect\"}`, `begin {\"title\":\"expect.toBeTruthy\",\"category\":\"expect\"}`, - `end {\"title\":\"expect.toBeTruthy\",\"category\":\"expect\",\"error\":{\"message\":\"\\u001b[2mexpect(\\u001b[22m\\u001b[31mreceived\\u001b[39m\\u001b[2m).\\u001b[22mtoBeTruthy\\u001b[2m()\\u001b[22m\\n\\nReceived: \\u001b[31mfalse\\u001b[39m\",\"stack\":\"\",\"location\":\"\",\"snippet\":\"\"}}`, + `end {\"title\":\"expect.toBeTruthy\",\"category\":\"expect\",\"error\":{\"message\":\"Error: \\u001b[2mexpect(\\u001b[22m\\u001b[31mreceived\\u001b[39m\\u001b[2m).\\u001b[22mtoBeTruthy\\u001b[2m()\\u001b[22m\\n\\nReceived: \\u001b[31mfalse\\u001b[39m\",\"stack\":\"\",\"location\":\"\",\"snippet\":\"\"}}`, `begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, `end {\"title\":\"After Hooks\",\"category\":\"hook\"}`, `begin {\"title\":\"Before Hooks\",\"category\":\"hook\"}`, diff --git a/tests/playwright-test/test-step.spec.ts b/tests/playwright-test/test-step.spec.ts index 5acf1b6f382060..af4a168ecf2fc7 100644 --- a/tests/playwright-test/test-step.spec.ts +++ b/tests/playwright-test/test-step.spec.ts @@ -44,6 +44,15 @@ class Reporter { }; } + distillError(error) { + return { + error: { + message: stripAnsi(error.message), + stack: stripAnsi(error.stack), + } + }; + } + onStdOut(data) { process.stdout.write(data.toString()); } @@ -61,6 +70,9 @@ class Reporter { for (const step of result.steps) { console.log('%% ' + JSON.stringify(this.distillStep(step))); } + for (const error of result.errors) { + console.log('%% ' + JSON.stringify(this.distillError(error))); + } } } }; @@ -249,6 +261,9 @@ test('should report before hooks step error', async ({ runInlineTest }) => { category: 'hook', title: 'After Hooks', }, + { + error: expect.any(Object) + } ]); }); @@ -556,6 +571,9 @@ test('should report custom expect steps', async ({ runInlineTest }) => { category: 'hook', title: 'After Hooks', }, + { + error: expect.any(Object) + } ]); }); @@ -633,7 +651,8 @@ test('should mark step as failed when soft expect fails', async ({ runInlineTest category: 'test.step', location: { file: 'a.test.ts', line: expect.any(Number), column: expect.any(Number) } }, - { title: 'After Hooks', category: 'hook' } + { title: 'After Hooks', category: 'hook' }, + { error: expect.any(Object) } ]); }); @@ -1131,6 +1150,13 @@ test('should show final toPass error', async ({ runInlineTest }) => { title: 'After Hooks', category: 'hook', }, + { + error: { + message: expect.stringContaining('Error: expect(received).toBe(expected)'), + stack: expect.stringContaining('a.test.ts:4'), + } + } + ]); }); @@ -1211,6 +1237,18 @@ test('should propagate nested soft errors', async ({ runInlineTest }) => { category: 'hook', title: 'After Hooks', }, + { + error: { + message: expect.stringContaining('Error: expect(received).toBe(expected)'), + stack: expect.stringContaining('a.test.ts:6'), + } + }, + { + error: { + message: expect.stringContaining('Error: expect(received).toBe(expected)'), + stack: expect.stringContaining('a.test.ts:12'), + } + } ]); }); @@ -1292,6 +1330,12 @@ test('should not propagate nested hard errors', async ({ runInlineTest }) => { category: 'hook', title: 'After Hooks', }, + { + error: { + message: expect.stringContaining('Error: expect(received).toBe(expected)'), + stack: expect.stringContaining('a.test.ts:13'), + } + } ]); }); @@ -1342,6 +1386,12 @@ test('should step w/o box', async ({ runInlineTest }) => { category: 'hook', title: 'After Hooks', }, + { + error: { + message: expect.stringContaining('Error: expect(received).toBe(expected)'), + stack: expect.stringContaining('a.test.ts:3'), + } + } ]); }); @@ -1385,6 +1435,12 @@ test('should step w/ box', async ({ runInlineTest }) => { category: 'hook', title: 'After Hooks', }, + { + error: { + message: expect.stringContaining('expect(received).toBe(expected)'), + stack: expect.not.stringMatching(/a.test.ts:[^8]/), + } + } ]); }); @@ -1428,6 +1484,12 @@ test('should soft step w/ box', async ({ runInlineTest }) => { category: 'hook', title: 'After Hooks', }, + { + error: { + message: expect.stringContaining('Error: expect(received).toBe(expected)'), + stack: expect.not.stringMatching(/a.test.ts:[^8]/), + } + } ]); });