From 09f77c41dd61c06d0dcdf2d7617fca7622ecd6ba Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 24 Feb 2023 18:36:15 -0800 Subject: [PATCH] chore: migrate to protocol's StackFrame type (#21198) --- .../playwright-core/src/protocol/validator.ts | 4 +-- .../playwright-core/src/utils/stackTrace.ts | 21 ++++----------- packages/playwright-core/src/utilsBundle.ts | 27 +++++++------------ .../playwright-test/src/matchers/expect.ts | 4 +-- .../playwright-test/src/reporters/base.ts | 6 ++--- packages/playwright-test/src/util.ts | 18 ++++++------- packages/protocol/src/channels.ts | 4 +-- packages/protocol/src/protocol.yml | 4 +-- 8 files changed, 33 insertions(+), 55 deletions(-) diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index bb4a464702..93076afb9d 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -22,8 +22,8 @@ export { ValidationError, findValidator, maybeFindValidator, createMetadataValid scheme.StackFrame = tObject({ file: tString, - line: tOptional(tNumber), - column: tOptional(tNumber), + line: tNumber, + column: tNumber, function: tOptional(tString), }); scheme.Metadata = tObject({ diff --git a/packages/playwright-core/src/utils/stackTrace.ts b/packages/playwright-core/src/utils/stackTrace.ts index 58fabc8b9f..64b259d96c 100644 --- a/packages/playwright-core/src/utils/stackTrace.ts +++ b/packages/playwright-core/src/utils/stackTrace.ts @@ -17,6 +17,7 @@ import path from 'path'; import { parseStackTraceLine } from '../utilsBundle'; import { isUnderTest } from './'; +import type { StackFrame } from '@protocol/channels'; export function rewriteErrorMessage(e: E, newMessage: string): E { const lines: string[] = (e.stack?.split('\n') || []).filter(l => l.startsWith(' at ')); @@ -35,13 +36,6 @@ const internalStackPrefixes = [ ]; export const addInternalStackPrefix = (prefix: string) => internalStackPrefixes.push(prefix); -export type StackFrame = { - file: string, - line?: number, - column?: number, - function?: string, -}; - export type ParsedStackTrace = { allFrames: StackFrame[]; frames: StackFrame[]; @@ -71,18 +65,13 @@ export function captureLibraryStackTrace(rawStack?: RawStack): ParsedStackTrace }; let parsedFrames = stack.map(line => { const frame = parseStackTraceLine(line); - if (!frame || !frame.fileName) + if (!frame || !frame.file) return null; - if (!process.env.PWDEBUGIMPL && isTesting && frame.fileName.includes(COVERAGE_PATH)) + if (!process.env.PWDEBUGIMPL && isTesting && frame.file.includes(COVERAGE_PATH)) return null; - const isPlaywrightLibrary = frame.fileName.startsWith(CORE_DIR); + const isPlaywrightLibrary = frame.file.startsWith(CORE_DIR); const parsed: ParsedFrame = { - frame: { - file: frame.fileName, - line: frame.line, - column: frame.column, - function: frame.function, - }, + frame, frameText: line, isPlaywrightLibrary }; diff --git a/packages/playwright-core/src/utilsBundle.ts b/packages/playwright-core/src/utilsBundle.ts index cfc3301bfd..0d750c0593 100644 --- a/packages/playwright-core/src/utilsBundle.ts +++ b/packages/playwright-core/src/utilsBundle.ts @@ -36,21 +36,14 @@ export const wsReceiver = require('./utilsBundleImpl').wsReceiver; export const wsSender = require('./utilsBundleImpl').wsSender; export type { Command } from '../bundles/utils/node_modules/commander'; export type { WebSocket, WebSocketServer, RawData as WebSocketRawData, EventEmitter as WebSocketEventEmitter } from '../bundles/utils/node_modules/@types/ws'; +import type { StackFrame } from '@protocol/channels'; const StackUtils: typeof import('../bundles/utils/node_modules/@types/stack-utils') = require('./utilsBundleImpl').StackUtils; const stackUtils = new StackUtils({ internals: StackUtils.nodeInternals() }); const nodeInternals = StackUtils.nodeInternals(); const nodeMajorVersion = +process.versions.node.split('.')[0]; -export type StackFrameData = { - line?: number | undefined; - column?: number | undefined; - fileName?: string | undefined; - fileOrUrl?: string | undefined; - function?: string | undefined; -}; - -export function parseStackTraceLine(line: string): StackFrameData | null { +export function parseStackTraceLine(line: string): StackFrame | null { if (!process.env.PWDEBUGIMPL && nodeMajorVersion < 16 && nodeInternals.some(internal => internal.test(line))) return null; const frame = stackUtils.parseLine(line); @@ -58,16 +51,14 @@ export function parseStackTraceLine(line: string): StackFrameData | null { return null; if (!process.env.PWDEBUGIMPL && (frame.file?.startsWith('internal') || frame.file?.startsWith('node:'))) return null; - let fileName: string | undefined = undefined; - if (frame.file) { - // ESM files return file:// URLs, see here: https://github.com/tapjs/stack-utils/issues/60 - fileName = frame.file.startsWith('file://') ? url.fileURLToPath(frame.file) : path.resolve(process.cwd(), frame.file); - } + if (!frame.file) + return null; + // ESM files return file:// URLs, see here: https://github.com/tapjs/stack-utils/issues/60 + const file = frame.file.startsWith('file://') ? url.fileURLToPath(frame.file) : path.resolve(process.cwd(), frame.file); return { - line: frame.line, - column: frame.column, - fileName, - fileOrUrl: frame.file, + file, + line: frame.line || 0, + column: frame.column || 0, function: frame.function, }; } diff --git a/packages/playwright-test/src/matchers/expect.ts b/packages/playwright-test/src/matchers/expect.ts index 1162028836..f6a3707101 100644 --- a/packages/playwright-test/src/matchers/expect.ts +++ b/packages/playwright-test/src/matchers/expect.ts @@ -16,7 +16,6 @@ import { captureRawStack, pollAgainstTimeout } from 'playwright-core/lib/utils'; import type { ExpectZone } from 'playwright-core/lib/utils'; -import path from 'path'; import { toBeChecked, toBeDisabled, @@ -200,12 +199,11 @@ class ExpectMetaInfoProxyHandler { const rawStack = captureRawStack(); const stackFrames = filteredStackTrace(rawStack); - const frame = stackFrames[0]; const customMessage = this._info.message || ''; const defaultTitle = `expect${this._info.isPoll ? '.poll' : ''}${this._info.isSoft ? '.soft' : ''}${this._info.isNot ? '.not' : ''}.${matcherName}`; const wallTime = Date.now(); const step = testInfo._addStep({ - location: frame && frame.fileName ? { file: path.resolve(process.cwd(), frame.fileName), line: frame.line || 0, column: frame.column || 0 } : undefined, + location: stackFrames[0], category: 'expect', title: trimLongString(customMessage || defaultTitle, 1024), canHaveChildren: true, diff --git a/packages/playwright-test/src/reporters/base.ts b/packages/playwright-test/src/reporters/base.ts index dcf999be52..6e95b12edc 100644 --- a/packages/playwright-test/src/reporters/base.ts +++ b/packages/playwright-test/src/reporters/base.ts @@ -442,11 +442,11 @@ export function prepareErrorStack(stack: string): { let location: Location | undefined; for (const line of stackLines) { const frame = parseStackTraceLine(line); - if (!frame || !frame.fileName) + if (!frame || !frame.file) continue; - if (belongsToNodeModules(frame.fileName)) + if (belongsToNodeModules(frame.file)) continue; - location = { file: frame.fileName, column: frame.column || 0, line: frame.line || 0 }; + location = { file: frame.file, column: frame.column || 0, line: frame.line || 0 }; break; } return { message, stackLines, location }; diff --git a/packages/playwright-test/src/util.ts b/packages/playwright-test/src/util.ts index aa1013b067..50bc893ba1 100644 --- a/packages/playwright-test/src/util.ts +++ b/packages/playwright-test/src/util.ts @@ -16,7 +16,7 @@ import fs from 'fs'; import { mime } from 'playwright-core/lib/utilsBundle'; -import type { StackFrameData } from 'playwright-core/lib/utilsBundle'; +import type { StackFrame } from '@protocol/channels'; import util from 'util'; import path from 'path'; import url from 'url'; @@ -37,28 +37,28 @@ export function filterStackTrace(e: Error) { e.message = message; } -export function filteredStackTrace(rawStack: RawStack): StackFrameData[] { - const frames: StackFrameData[] = []; +export function filteredStackTrace(rawStack: RawStack): StackFrame[] { + const frames: StackFrame[] = []; for (const line of rawStack) { const frame = parseStackTraceLine(line); - if (!frame || !frame.fileName) + if (!frame || !frame.file) continue; - if (!process.env.PWDEBUGIMPL && frame.fileName.startsWith(PLAYWRIGHT_TEST_PATH)) + if (!process.env.PWDEBUGIMPL && frame.file.startsWith(PLAYWRIGHT_TEST_PATH)) continue; - if (!process.env.PWDEBUGIMPL && frame.fileName.startsWith(PLAYWRIGHT_CORE_PATH)) + if (!process.env.PWDEBUGIMPL && frame.file.startsWith(PLAYWRIGHT_CORE_PATH)) continue; frames.push(frame); } return frames; } -export function stringifyStackFrames(frames: StackFrameData[]): string[] { +export function stringifyStackFrames(frames: StackFrame[]): string[] { const stackLines: string[] = []; for (const frame of frames) { if (frame.function) - stackLines.push(` at ${frame.function} (${frame.fileName}:${frame.line}:${frame.column})`); + stackLines.push(` at ${frame.function} (${frame.file}:${frame.line}:${frame.column})`); else - stackLines.push(` at ${frame.fileName}:${frame.line}:${frame.column}`); + stackLines.push(` at ${frame.file}:${frame.line}:${frame.column}`); } return stackLines; } diff --git a/packages/protocol/src/channels.ts b/packages/protocol/src/channels.ts index fdffc270dc..0bfd8b6941 100644 --- a/packages/protocol/src/channels.ts +++ b/packages/protocol/src/channels.ts @@ -139,8 +139,8 @@ export type EventTargetTraits = export type StackFrame = { file: string, - line?: number, - column?: number, + line: number, + column: number, function?: string, }; diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml index f1f14d5653..a5acefd06b 100644 --- a/packages/protocol/src/protocol.yml +++ b/packages/protocol/src/protocol.yml @@ -16,8 +16,8 @@ StackFrame: type: object properties: file: string - line: number? - column: number? + line: number + column: number function: string? # This object can be send with any rpc call in the "metadata" field.