From b0daa7754fe257b08466662f6e43d3de523be61d Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Tue, 1 Feb 2022 19:40:44 -0700 Subject: [PATCH] feat: filter stack traces to exclude test runner frames (#11795) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: ```bash Running 1 test using 1 worker 1) [chromium] › tests/example.spec.ts:3:1 › should work ========================================== Error: expect(received).toBe(expected) // Object.is equality Expected: 2 Received: 1 2 | 3 | test('should work', async({page}) => { > 4 | expect(1).toBe(2); | ^ 5 | }); 6 | at Proxy. (/Users/andreylushnikov/prog/playwright/packages/playwright-test/src/expect.ts:151:30) at /Users/andreylushnikov/tmp/tests/example.spec.ts:4:13 at /Users/andreylushnikov/prog/playwright/packages/playwright-test/src/workerRunner.ts:335:13 at runNextTicks (node:internal/process/task_queues:61:5) at processImmediate (node:internal/timers:437:9) at TestInfoImpl._runFn (/Users/andreylushnikov/prog/playwright/packages/playwright-test/src/testInfo.ts:164:7) at WorkerRunner._runTestWithBeforeHooks (/Users/andreylushnikov/prog/playwright/packages/playwright-test/src/workerRunner.ts:317:24) at TimeoutRunner.run (/Users/andreylushnikov/prog/playwright/packages/playwright-core/src/utils/async.ts:48:14) at TestInfoImpl._runWithTimeout (/Users/andreylushnikov/prog/playwright/packages/playwright-test/src/testInfo.ts:151:7) at WorkerRunner._runTestOrAllHook (/Users/andreylushnikov/prog/playwright/packages/playwright-test/src/workerRunner.ts:276:5) at WorkerRunner._runSuite (/Users/andreylushnikov/prog/playwright/packages/playwright-test/src/workerRunner.ts:190:11) at WorkerRunner.run (/Users/andreylushnikov/prog/playwright/packages/playwright-test/src/workerRunner.ts:137:9) at process. (/Users/andreylushnikov/prog/playwright/packages/playwright-test/src/worker.ts:87:5) ``` after: ``` Running 1 test using 1 worker 1) [chromium] › tests/example.spec.ts:3:1 › should work ========================================== Error: expect(received).toBe(expected) // Object.is equality Expected: 2 Received: 1 2 | 3 | test('should work', async({page}) => { > 4 | expect(1).toBe(2); | ^ 5 | }); 6 | at /Users/andreylushnikov/tmp/tests/example.spec.ts:4:13 ``` --- packages/playwright-core/package.json | 1 + .../playwright-core/src/utils/stackTrace.ts | 26 ++-- packages/playwright-test/src/util.ts | 33 ++++++ tests/playwright-test/loader.spec.ts | 111 +++++++++++++++++- 4 files changed, 160 insertions(+), 11 deletions(-) diff --git a/packages/playwright-core/package.json b/packages/playwright-core/package.json index bc6bf05e17..cd96911c1d 100644 --- a/packages/playwright-core/package.json +++ b/packages/playwright-core/package.json @@ -28,6 +28,7 @@ "./lib/utils/processLauncher": "./lib/utils/processLauncher.js", "./lib/utils/registry": "./lib/utils/registry.js", "./lib/utils/utils": "./lib/utils/utils.js", + "./lib/utils/stackTrace": "./lib/utils/stackTrace.js", "./lib/remote/playwrightServer": "./lib/remote/playwrightServer.js", "./lib/remote/playwrightClient": "./lib/remote/playwrightClient.js" }, diff --git a/packages/playwright-core/src/utils/stackTrace.ts b/packages/playwright-core/src/utils/stackTrace.ts index a0e1036176..1aed8bf500 100644 --- a/packages/playwright-core/src/utils/stackTrace.ts +++ b/packages/playwright-core/src/utils/stackTrace.ts @@ -56,6 +56,21 @@ export function captureRawStack(): string { return stack; } +export function isInternalFileName(file: string, functionName?: string): boolean { + // Node 16+ has node:internal. + if (file.startsWith('internal') || file.startsWith('node:')) + return true; + // EventEmitter.emit has 'events.js' file. + if (file === 'events.js' && functionName?.endsWith('emit')) + return true; + // Node 12 + if (file === '_stream_readable.js' || file === '_stream_writable.js') + return true; + if (file.startsWith(WS_LIB)) + return true; + return false; +} + export function captureStackTrace(rawStack?: string): ParsedStackTrace { const stack = rawStack || captureRawStack(); @@ -69,16 +84,7 @@ export function captureStackTrace(rawStack?: string): ParsedStackTrace { const frame = stackUtils.parseLine(line); if (!frame || !frame.file) return null; - // Node 16+ has node:internal. - if (frame.file.startsWith('internal') || frame.file.startsWith('node:')) - return null; - // EventEmitter.emit has 'events.js' file. - if (frame.file === 'events.js' && frame.function?.endsWith('.emit')) - return null; - // Node 12 - if (frame.file === '_stream_readable.js' || frame.file === '_stream_writable.js') - return null; - if (frame.file.startsWith(WS_LIB)) + if (isInternalFileName(frame.file, frame.function)) return null; // Workaround for https://github.com/tapjs/stack-utils/issues/60 let fileName: string; diff --git a/packages/playwright-test/src/util.ts b/packages/playwright-test/src/util.ts index 42731e9543..29e5556e45 100644 --- a/packages/playwright-test/src/util.ts +++ b/packages/playwright-test/src/util.ts @@ -21,9 +21,42 @@ import type { TestError, Location } from './types'; import { default as minimatch } from 'minimatch'; import debug from 'debug'; import { calculateSha1, isRegExp } from 'playwright-core/lib/utils/utils'; +import { isInternalFileName } from 'playwright-core/lib/utils/stackTrace'; + +const PLAYWRIGHT_CORE_PATH = path.dirname(require.resolve('playwright-core')); +const PLAYWRIGHT_TEST_PATH = path.join(__dirname, '..'); + +function filterStackTrace(e: Error) { + // This method filters internal stack frames using Error.prepareStackTrace + // hook. Read more about the hook: https://v8.dev/docs/stack-trace-api + // + // NOTE: Error.prepareStackTrace will only be called if `e.stack` has not + // been accessed before. This is the case for Jest Expect and simple throw + // statements. + // + // If `e.stack` has been accessed, this method will be NOOP. + const oldPrepare = Error.prepareStackTrace; + const stackFormatter = oldPrepare || ((error, structuredStackTrace) => [ + `${error.name}: ${error.message}`, + ...structuredStackTrace.map(callSite => ' at ' + callSite.toString()), + ].join('\n')); + Error.prepareStackTrace = (error, structuredStackTrace) => { + return stackFormatter(error, structuredStackTrace.filter(callSite => { + const fileName = callSite.getFileName(); + const functionName = callSite.getFunctionName() || undefined; + if (!fileName) + return true; + return !fileName.startsWith(PLAYWRIGHT_TEST_PATH) && !fileName.startsWith(PLAYWRIGHT_CORE_PATH) && !isInternalFileName(fileName, functionName); + })); + }; + // eslint-disable-next-line + e.stack; // trigger Error.prepareStackTrace + Error.prepareStackTrace = oldPrepare; +} export function serializeError(error: Error | any): TestError { if (error instanceof Error) { + filterStackTrace(error); return { message: error.message, stack: error.stack diff --git a/tests/playwright-test/loader.spec.ts b/tests/playwright-test/loader.spec.ts index 48cca50775..ad6a4f449f 100644 --- a/tests/playwright-test/loader.spec.ts +++ b/tests/playwright-test/loader.spec.ts @@ -14,7 +14,8 @@ * limitations under the License. */ -import { test, expect } from './playwright-test-fixtures'; +import { test, expect, stripAnsi } from './playwright-test-fixtures'; +import path from 'path'; test('should return the location of a syntax error', async ({ runInlineTest }) => { const result = await runInlineTest({ @@ -265,3 +266,111 @@ test('should import esm from ts when package.json has type module in experimenta expect(result.exitCode).toBe(0); }); + +test('should filter stack trace for simple expect', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'expect-test.spec.ts': ` + const { test } = pwt; + test('should work', () => { + test.expect(1+1).toEqual(3); + }); + ` + }); + expect(result.exitCode).toBe(1); + expect(stripAnsi(result.output)).not.toContain(path.sep + `playwright-test`); + expect(stripAnsi(result.output)).not.toContain(path.sep + `playwright-core`); + expect(stripAnsi(result.output)).not.toContain('internal'); +}); + +test('should filter stack trace for web-first assertions', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'expect-test.spec.ts': ` + const { test } = pwt; + test('should work', async ({page}) => { + await expect(page.locator('x-foo'), 'x-foo must be visible').toBeVisible({timeout: 1}); + }); + ` + }); + expect(result.exitCode).toBe(1); + expect(stripAnsi(result.output)).not.toContain(path.sep + `playwright-test`); + expect(stripAnsi(result.output)).not.toContain(path.sep + `playwright-core`); + expect(stripAnsi(result.output)).not.toContain('internal'); +}); + +test('should filter out event emitter from stack traces', async ({ runInlineTest }, testInfo) => { + const result = await runInlineTest({ + 'expect-test.spec.ts': ` + const { test } = pwt; + const EventEmitter = require('events'); + test('should work', async ({}) => { + const emitter = new EventEmitter(); + emitter.on('event', function handle() { expect(1).toBe(2); }); + emitter.emit('event'); + }); + ` + }); + expect(result.exitCode).toBe(1); + const outputWithoutGoodStackFrames = stripAnsi(result.output).split('\n').filter(line => !line.includes(testInfo.outputPath())).join('\n'); + expect(outputWithoutGoodStackFrames).not.toContain('EventEmitter.emit'); +}); + +test('should filter stack trace for raw errors', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'expect-test.spec.ts': ` + const { test } = pwt; + test('should work', async ({}) => { + throw new Error('foobar!'); + }); + ` + }); + expect(result.exitCode).toBe(1); + expect(stripAnsi(result.output)).toContain('foobar!'); + expect(stripAnsi(result.output)).not.toContain(path.sep + `playwright-test`); + expect(stripAnsi(result.output)).not.toContain(path.sep + `playwright-core`); + expect(stripAnsi(result.output)).not.toContain('internal'); +}); + +test('should not filter out POM', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'helper.ts': ` + export function foo() { + throw new Error('foo'); + } + `, + 'expect-test.spec.ts': ` + const { test } = pwt; + const { foo } = require('./helper'); + test('should work', ({}) => { + foo(); + }); + ` + }); + expect(result.exitCode).toBe(1); + expect(stripAnsi(result.output)).toContain('foo'); + expect(stripAnsi(result.output)).toContain('helper.ts'); + expect(stripAnsi(result.output)).toContain('expect-test.spec.ts'); + expect(stripAnsi(result.output)).not.toContain(path.sep + `playwright-test`); + expect(stripAnsi(result.output)).not.toContain(path.sep + `playwright-core`); + expect(stripAnsi(result.output)).not.toContain('internal'); +}); + +test('should filter stack even without default Error.prepareStackTrace', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'expect-test.spec.ts': ` + const { test } = pwt; + test('should work', ({}) => { + Error.prepareStackTrace = undefined; + throw new Error('foobar'); + }); + ` + }); + expect(result.exitCode).toBe(1); + expect(stripAnsi(result.output)).toContain('foobar'); + expect(stripAnsi(result.output)).toContain('expect-test.spec.ts'); + expect(stripAnsi(result.output)).not.toContain(path.sep + `playwright-test`); + expect(stripAnsi(result.output)).not.toContain(path.sep + `playwright-core`); + expect(stripAnsi(result.output)).not.toContain('internal'); + const stackLines = stripAnsi(result.output).split('\n').filter(line => line.includes(' at ')); + expect(stackLines.length).toBe(1); +}); +