From 2d4db7a6f0c7919ab7db83bbc26c658ae4c4e5c9 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 25 Oct 2021 10:49:59 -0800 Subject: [PATCH] fix(stack): hide test runner stack frames (#9735) --- .../playwright-core/src/utils/stackTrace.ts | 17 ++++++- tests/playwright-test/reporter-html.spec.ts | 1 - tests/playwright-test/test-step.spec.ts | 30 ------------ tests/tracing.spec.ts | 46 +++++++++++++++++++ 4 files changed, 62 insertions(+), 32 deletions(-) diff --git a/packages/playwright-core/src/utils/stackTrace.ts b/packages/playwright-core/src/utils/stackTrace.ts index 05bedcf724..3dcba129cc 100644 --- a/packages/playwright-core/src/utils/stackTrace.ts +++ b/packages/playwright-core/src/utils/stackTrace.ts @@ -33,6 +33,8 @@ export function rewriteErrorMessage(e: E, newMessage: string): const CORE_DIR = path.resolve(__dirname, '..', '..'); const CLIENT_LIB = path.join(CORE_DIR, 'lib', 'client'); const CLIENT_SRC = path.join(CORE_DIR, 'src', 'client'); +const TEST_DIR_SRC = path.resolve(CORE_DIR, '..', 'playwright-test'); +const TEST_DIR_LIB = path.resolve(CORE_DIR, '..', '@playwright', 'test'); export type ParsedStackTrace = { allFrames: StackFrame[]; @@ -58,7 +60,11 @@ export function captureStackTrace(): ParsedStackTrace { const frame = stackUtils.parseLine(line); if (!frame || !frame.file) return null; - if (frame.file.startsWith('internal')) + // 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; const fileName = path.resolve(process.cwd(), frame.file); if (isTesting && fileName.includes(path.join('playwright', 'tests', 'config', 'coverage.js'))) @@ -104,6 +110,15 @@ export function captureStackTrace(): ParsedStackTrace { } } + // Hide all test runner and library frames in the user stack (event handlers produce them). + parsedFrames = parsedFrames.filter((f, i) => { + if (f.frame.file.startsWith(TEST_DIR_SRC) || f.frame.file.startsWith(TEST_DIR_LIB)) + return false; + if (i && f.frame.file.startsWith(CORE_DIR)) + return false; + return true; + }); + return { allFrames: allFrames.map(p => p.frame), frames: parsedFrames.map(p => p.frame), diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts index d0c30563c6..7927adaaf7 100644 --- a/tests/playwright-test/reporter-html.spec.ts +++ b/tests/playwright-test/reporter-html.spec.ts @@ -287,7 +287,6 @@ test('should show trace source', async ({ runInlineTest, page, showReport }) => await expect(page.locator('.stack-trace-frame')).toContainText([ /a.test.js:[\d]+/, - /fixtures.[tj]s:[\d]+/, ]); await expect(page.locator('.stack-trace-frame.selected')).toContainText('a.test.js'); }); diff --git a/tests/playwright-test/test-step.spec.ts b/tests/playwright-test/test-step.spec.ts index 6635aca4b8..a56a8151ef 100644 --- a/tests/playwright-test/test-step.spec.ts +++ b/tests/playwright-test/test-step.spec.ts @@ -88,11 +88,6 @@ test('should report api step hierarchy', async ({ runInlineTest }) => { steps: [ { category: 'pw:api', - location: { - column: 'number', - file: 'index.ts', - line: 'number', - }, title: 'browserContext.newPage', }, ], @@ -161,11 +156,6 @@ test('should report api step hierarchy', async ({ runInlineTest }) => { steps: [ { category: 'pw:api', - location: { - column: 'number', - file: 'index.ts', - line: 'number', - }, title: 'browserContext.close', }, ], @@ -201,11 +191,6 @@ test('should not report nested after hooks', async ({ runInlineTest }) => { { category: 'pw:api', title: 'browserContext.newPage', - location: { - column: 'number', - file: 'index.ts', - line: 'number', - }, }, ], }, @@ -225,11 +210,6 @@ test('should not report nested after hooks', async ({ runInlineTest }) => { { category: 'pw:api', title: 'browserContext.close', - location: { - column: 'number', - file: 'index.ts', - line: 'number', - }, }, ], }, @@ -316,11 +296,6 @@ test('should report expect step locations', async ({ runInlineTest }) => { steps: [ { category: 'pw:api', - location: { - column: 'number', - file: 'index.ts', - line: 'number', - }, title: 'browserContext.newPage', }, ], @@ -340,11 +315,6 @@ test('should report expect step locations', async ({ runInlineTest }) => { steps: [ { category: 'pw:api', - location: { - column: 'number', - file: 'index.ts', - line: 'number', - }, title: 'browserContext.close', }, ], diff --git a/tests/tracing.spec.ts b/tests/tracing.spec.ts index 20e421c0cf..2816c70aff 100644 --- a/tests/tracing.spec.ts +++ b/tests/tracing.spec.ts @@ -17,6 +17,7 @@ import { expect, contextTest as test, browserTest } from './config/browserTest'; import { ZipFileSystem } from '../packages/playwright-core/lib/utils/vfs'; import jpeg from 'jpeg-js'; +import path from 'path'; test.skip(({ trace }) => !!trace); @@ -287,6 +288,47 @@ test('should not hang for clicks that open dialogs', async ({ context, page }) = await context.tracing.stop(); }); +test('should hide internal stack frames', async ({ context, page }, testInfo) => { + await context.tracing.start({ screenshots: true, snapshots: true }); + let evalPromise; + page.on('dialog', dialog => { + evalPromise = page.evaluate('2+2'); + dialog.dismiss(); + }); + await page.setContent(`
Click me
`); + await page.click('div'); + await evalPromise; + const tracePath = testInfo.outputPath('trace.zip'); + await context.tracing.stop({ path: tracePath }); + + const trace = await parseTrace(tracePath); + const actions = trace.events.filter(e => e.type === 'action' && !e.metadata.apiName.startsWith('tracing.')); + expect(actions).toHaveLength(4); + for (const action of actions) + expect(relativeStack(action)).toEqual(['tracing.spec.ts']); +}); + +test('should hide internal stack frames in expect', async ({ context, page }, testInfo) => { + await context.tracing.start({ screenshots: true, snapshots: true }); + let expectPromise; + page.on('dialog', dialog => { + expectPromise = expect(page).toHaveTitle('Hello'); + dialog.dismiss(); + }); + await page.setContent(`Hello
Click me
`); + await page.click('div'); + await expect(page.locator('div')).toBeVisible(); + await expectPromise; + const tracePath = testInfo.outputPath('trace.zip'); + await context.tracing.stop({ path: tracePath }); + + const trace = await parseTrace(tracePath); + const actions = trace.events.filter(e => e.type === 'action' && !e.metadata.apiName.startsWith('tracing.')); + expect(actions).toHaveLength(5); + for (const action of actions) + expect(relativeStack(action)).toEqual(['tracing.spec.ts']); +}); + async function parseTrace(file: string): Promise<{ events: any[], resources: Map }> { const zipFS = new ZipFileSystem(file); const resources = new Map(); @@ -330,3 +372,7 @@ function expectBlue(pixels: Buffer, offset: number) { expect(b).toBeGreaterThan(200); expect(a).toBe(255); } + +function relativeStack(action: any): string[] { + return action.metadata.stack.map(f => f.file.replace(__dirname + path.sep, '')); +}