From 0a690778e4b91f15f78426835bd28c11ca50e7e8 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 28 Sep 2021 16:02:34 -0700 Subject: [PATCH] fix(expect): beautiful expect stacks (#9204) We now mark our wrapper as `__PWTRAP__[expect.toHaveText]` and find it later in the stack trace. Added trace/inspector tests to ensure this behavior in the future. --- src/test/expect.ts | 5 +++ src/utils/stackTrace.ts | 47 ++++++++++++------------- tests/inspector/pause.spec.ts | 19 ++++++++++ tests/trace-viewer/trace-viewer.spec.ts | 2 ++ 4 files changed, 48 insertions(+), 25 deletions(-) diff --git a/src/test/expect.ts b/src/test/expect.ts index f8bab74da6..121313ee47 100644 --- a/src/test/expect.ts +++ b/src/test/expect.ts @@ -74,6 +74,10 @@ function wrap(matcherName: string, matcher: any) { return matcher.call(this, ...args); const INTERNAL_STACK_LENGTH = 3; + // at Object.__PWTRAP__[expect.toHaveText] (...) + // at __EXTERNAL_MATCHER_TRAP__ (...) + // at Object.throwingMatcher [as toHaveText] (...) + // at (...) const stackLines = new Error().stack!.split('\n').slice(INTERNAL_STACK_LENGTH + 1); const step = testInfo._addStep({ category: 'expect', @@ -107,6 +111,7 @@ function wrap(matcherName: string, matcher: any) { reportStepError(e); } }; + result.displayName = '__PWTRAP__[expect.' + matcherName + ']'; return result; } diff --git a/src/utils/stackTrace.ts b/src/utils/stackTrace.ts index 7aaa5db11d..a64ed2a7f1 100644 --- a/src/utils/stackTrace.ts +++ b/src/utils/stackTrace.ts @@ -33,8 +33,6 @@ export function rewriteErrorMessage(e: E, newMessage: string): const ROOT_DIR = path.resolve(__dirname, '..', '..'); const CLIENT_LIB = path.join(ROOT_DIR, 'lib', 'client'); const CLIENT_SRC = path.join(ROOT_DIR, 'src', 'client'); -const TEST_LIB = path.join(ROOT_DIR, 'lib', 'test'); -const TEST_SRC = path.join(ROOT_DIR, 'src', 'test'); export type ParsedStackTrace = { allFrames: StackFrame[]; @@ -65,14 +63,7 @@ export function captureStackTrace(): ParsedStackTrace { const fileName = path.resolve(process.cwd(), frame.file); if (isTesting && fileName.includes(path.join('playwright', 'tests', 'config', 'coverage.js'))) return null; - const inClient = - // Allow fixtures in the reported stacks. - (!fileName.includes('test/index') && !fileName.includes('test\\index')) && ( - frame.file.includes(path.join('node_modules', 'expect')) - || fileName.startsWith(CLIENT_LIB) - || fileName.startsWith(CLIENT_SRC) - || fileName.startsWith(TEST_LIB) - || fileName.startsWith(TEST_SRC)); + const inClient = fileName.startsWith(CLIENT_LIB) || fileName.startsWith(CLIENT_SRC); const parsed: ParsedFrame = { frame: { file: fileName, @@ -87,23 +78,29 @@ export function captureStackTrace(): ParsedStackTrace { }).filter(Boolean) as ParsedFrame[]; let apiName = ''; - // Deepest transition between non-client code calling into client code - // is the api entry. const allFrames = parsedFrames; - for (let i = 0; i < parsedFrames.length - 1; i++) { - if (parsedFrames[i].inClient && !parsedFrames[i + 1].inClient) { - const frame = parsedFrames[i].frame; - const text = parsedFrames[i].frameText; - // expect matchers have the following stack structure: - // at __EXTERNAL_MATCHER_TRAP__ (.../index.js:342:30) - // at Object.throwingMatcher [as toBeChecked] (.../index.js:343:15) - const aliasIndex = text.indexOf('[as '); - if (aliasIndex !== -1) - apiName = 'expect.' + text.substring(aliasIndex + 4, text.indexOf(']')); - else + + // expect matchers have the following stack structure: + // at Object.__PWTRAP__[expect.toHaveText] (...) + // at __EXTERNAL_MATCHER_TRAP__ (...) + // at Object.throwingMatcher [as toHaveText] (...) + const TRAP = '__PWTRAP__['; + const expectIndex = parsedFrames.findIndex(f => f.frameText.includes(TRAP)); + if (expectIndex !== -1) { + const text = parsedFrames[expectIndex].frameText; + const aliasIndex = text.indexOf(TRAP); + apiName = text.substring(aliasIndex + TRAP.length, text.indexOf(']')); + parsedFrames = parsedFrames.slice(expectIndex + 3); + } else { + // Deepest transition between non-client code calling into client code + // is the api entry. + for (let i = 0; i < parsedFrames.length - 1; i++) { + if (parsedFrames[i].inClient && !parsedFrames[i + 1].inClient) { + const frame = parsedFrames[i].frame; apiName = frame.function ? frame.function[0].toLowerCase() + frame.function.slice(1) : ''; - parsedFrames = parsedFrames.slice(i + 1); - break; + parsedFrames = parsedFrames.slice(i + 1); + break; + } } } diff --git a/tests/inspector/pause.spec.ts b/tests/inspector/pause.spec.ts index 422930176d..93e108d7d7 100644 --- a/tests/inspector/pause.spec.ts +++ b/tests/inspector/pause.spec.ts @@ -173,6 +173,25 @@ it.describe('pause', () => { await scriptPromise; }); + it('should show expect.toHaveText', async ({ page, recorderPageGetter }) => { + await page.setContent(''); + const scriptPromise = (async () => { + await page.pause(); + await expect(page.locator('button')).toHaveText('Submit'); + await page.pause(); // 2 + })(); + const recorderPage = await recorderPageGetter(); + await recorderPage.click('[title="Resume"]'); + await recorderPage.waitForSelector('.source-line-paused:has-text("page.pause(); // 2")'); + expect(await sanitizeLog(recorderPage)).toEqual([ + 'page.pause- XXms', + 'expect.toHaveText(button)- XXms', + 'page.pause', + ]); + await recorderPage.click('[title="Resume"]'); + await scriptPromise; + }); + it('should highlight waitForEvent', async ({ page, recorderPageGetter }) => { await page.setContent(''); const scriptPromise = (async () => { diff --git a/tests/trace-viewer/trace-viewer.spec.ts b/tests/trace-viewer/trace-viewer.spec.ts index 1093c4aeff..b84b81235e 100644 --- a/tests/trace-viewer/trace-viewer.spec.ts +++ b/tests/trace-viewer/trace-viewer.spec.ts @@ -105,6 +105,7 @@ test.beforeAll(async function recordTrace({ browser, browserName, browserType, s const page = await context.newPage(); await page.goto('data:text/html,Hello world'); await page.setContent(''); + await expect(page.locator('button')).toHaveText('Click'); await page.evaluate(({ a }) => { console.log('Info'); console.warn('Warning'); @@ -158,6 +159,7 @@ test('should open simple trace viewer', async ({ showTraceViewer }) => { await expect(traceViewer.actionTitles).toHaveText([ /page.gotodata:text\/html,Hello world<\/html>— [\d.ms]+/, /page.setContent— [\d.ms]+/, + /expect.toHaveTextbutton— [\d.ms]+/, /page.evaluate— [\d.ms]+/, /page.click"Click"— [\d.ms]+/, /page.waitForEvent— [\d.ms]+/,