Skip to content

Commit

Permalink
cherry-pick(9735): fix(stack): hide test runner stack frames
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman committed Oct 25, 2021
1 parent c75bdde commit 18a0a4a
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 32 deletions.
17 changes: 16 additions & 1 deletion packages/playwright-core/src/utils/stackTrace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export function rewriteErrorMessage<E extends Error>(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[];
Expand All @@ -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')))
Expand Down Expand Up @@ -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),
Expand Down
1 change: 0 additions & 1 deletion tests/playwright-test/reporter-html.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
30 changes: 0 additions & 30 deletions tests/playwright-test/test-step.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
],
Expand Down Expand Up @@ -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',
},
],
Expand Down Expand Up @@ -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',
},
},
],
},
Expand All @@ -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',
},
},
],
},
Expand Down Expand Up @@ -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',
},
],
Expand All @@ -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',
},
],
Expand Down
46 changes: 46 additions & 0 deletions tests/tracing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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(`<div onclick='window.alert(123)'>Click me</div>`);
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(`<title>Hello</title><div onclick='window.alert(123)'>Click me</div>`);
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<string, Buffer> }> {
const zipFS = new ZipFileSystem(file);
const resources = new Map<string, Buffer>();
Expand Down Expand Up @@ -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, ''));
}

0 comments on commit 18a0a4a

Please sign in to comment.