chore: do not use library stack capturer in test runner (#21075)

This commit is contained in:
Pavel Feldman 2023-02-21 19:24:17 -08:00 committed by GitHub
parent 07bb483156
commit 3f8f2a0fdd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 79 additions and 60 deletions

View file

@ -19,7 +19,7 @@ import type * as channels from '@protocol/channels';
import { maybeFindValidator, ValidationError, type ValidatorContext } from '../protocol/validator';
import { debugLogger } from '../common/debugLogger';
import type { ParsedStackTrace } from '../utils/stackTrace';
import { captureRawStack, captureStackTrace } from '../utils/stackTrace';
import { captureRawStack, captureLibraryStackTrace } from '../utils/stackTrace';
import { isUnderTest } from '../utils';
import { zones } from '../utils/zones';
import type { ClientInstrumentation } from './clientInstrumentation';
@ -161,7 +161,7 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
if (apiZone)
return func(apiZone);
const stackTrace = captureStackTrace(stack);
const stackTrace = captureLibraryStackTrace(stack);
if (isInternal)
delete stackTrace.apiName;
const csi = isInternal ? undefined : this._instrumentation;

View file

@ -36,7 +36,7 @@ import { WritableStream } from './writableStream';
import { debugLogger } from '../common/debugLogger';
import { SelectorsOwner } from './selectors';
import { Android, AndroidSocket, AndroidDevice } from './android';
import { captureStackTrace, type ParsedStackTrace } from '../utils/stackTrace';
import { captureLibraryStackTrace, type ParsedStackTrace } from '../utils/stackTrace';
import { Artifact } from './artifact';
import { EventEmitter } from 'events';
import { JsonPipe } from './jsonPipe';
@ -166,7 +166,7 @@ export class Connection extends EventEmitter {
}
close(errorMessage: string = 'Connection closed') {
const stack = captureStackTrace().frameTexts.join('\n');
const stack = captureLibraryStackTrace().frameTexts.join('\n');
if (stack)
errorMessage += '\n ==== Closed by ====\n' + stack + '\n';
this._closedErrorMessage = errorMessage;

View file

@ -58,17 +58,7 @@ export function captureRawStack(): string {
return stack;
}
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;
return false;
}
export function captureStackTrace(rawStack?: string): ParsedStackTrace {
export function captureLibraryStackTrace(rawStack?: string): ParsedStackTrace {
const stack = rawStack || captureRawStack();
const isTesting = isUnderTest();
@ -78,17 +68,15 @@ export function captureStackTrace(rawStack?: string): ParsedStackTrace {
isPlaywrightLibrary: boolean;
};
let parsedFrames = stack.split('\n').map(line => {
const { frame, fileName } = parseStackTraceLine(line);
if (!frame || !frame.file || !fileName)
const frame = parseStackTraceLine(line);
if (!frame || !frame.fileName)
return null;
if (!process.env.PWDEBUGIMPL && isInternalFileName(frame.file, frame.function))
if (!process.env.PWDEBUGIMPL && isTesting && frame.fileName.includes(COVERAGE_PATH))
return null;
if (!process.env.PWDEBUGIMPL && isTesting && fileName.includes(COVERAGE_PATH))
return null;
const isPlaywrightLibrary = fileName.startsWith(CORE_DIR);
const isPlaywrightLibrary = frame.fileName.startsWith(CORE_DIR);
const parsed: ParsedFrame = {
frame: {
file: fileName,
file: frame.fileName,
line: frame.line,
column: frame.column,
function: frame.function,

View file

@ -38,20 +38,37 @@ 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';
const StackUtils: typeof import('../bundles/utils/node_modules/@types/stack-utils') = require('./utilsBundleImpl').StackUtils;
const stackUtils = new StackUtils();
const stackUtils = new StackUtils({ internals: StackUtils.nodeInternals() });
const nodeInternals = StackUtils.nodeInternals();
const nodeMajorVersion = +process.versions.node.split('.')[0];
export function parseStackTraceLine(line: string): { frame: import('../bundles/utils/node_modules/@types/stack-utils').StackLineData | null, fileName: string | null } {
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 {
if (!process.env.PWDEBUGIMPL && nodeMajorVersion < 16 && nodeInternals.some(internal => internal.test(line)))
return null;
const frame = stackUtils.parseLine(line);
if (!frame)
return { frame: null, fileName: null };
let fileName = 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);
}
return {
frame,
line: frame.line,
column: frame.column,
fileName,
fileOrUrl: frame.file,
function: frame.function,
};
}

View file

@ -14,7 +14,7 @@
* limitations under the License.
*/
import { captureStackTrace, pollAgainstTimeout } from 'playwright-core/lib/utils';
import { pollAgainstTimeout } from 'playwright-core/lib/utils';
import path from 'path';
import {
toBeChecked,
@ -44,7 +44,7 @@ import {
import { toMatchSnapshot, toHaveScreenshot } from './toMatchSnapshot';
import type { Expect } from '../common/types';
import { currentTestInfo, currentExpectTimeout } from '../common/globals';
import { serializeError, trimLongString } from '../util';
import { filteredStackTrace, serializeError, stringifyStackFrames, trimLongString } from '../util';
import {
expect as expectLibrary,
INVERTED_COLOR,
@ -196,13 +196,12 @@ class ExpectMetaInfoProxyHandler {
if (!testInfo)
return matcher.call(target, ...args);
const stackTrace = captureStackTrace();
const stackLines = stackTrace.frameTexts;
const frame = stackTrace.frames[0];
const stackFrames = filteredStackTrace(new Error());
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 step = testInfo._addStep({
location: frame && frame.file ? { file: path.resolve(process.cwd(), frame.file), line: frame.line || 0, column: frame.column || 0 } : undefined,
location: frame && frame.fileName ? { file: path.resolve(process.cwd(), frame.fileName), line: frame.line || 0, column: frame.column || 0 } : undefined,
category: 'expect',
title: trimLongString(customMessage || defaultTitle, 1024),
canHaveChildren: true,
@ -230,7 +229,7 @@ class ExpectMetaInfoProxyHandler {
...messageLines,
].join('\n');
jestError.message = newMessage;
jestError.stack = jestError.name + ': ' + newMessage + '\n' + stackLines.join('\n');
jestError.stack = jestError.name + ': ' + newMessage + '\n' + stringifyStackFrames(stackFrames).join('\n');
}
const serializerError = serializeError(jestError);

View file

@ -135,7 +135,7 @@ export function toContainText(
options: { timeout?: number, useInnerText?: boolean, ignoreCase?: boolean } = {},
) {
if (Array.isArray(expected)) {
return toEqual.call(this, 'toContainText', locator, 'Locator', async (isNot, timeout, customStackTrace) => {
return toEqual.call(this, 'toContainText', locator, 'Locator', async (isNot, timeout) => {
const expectedText = toExpectedTextValues(expected, { matchSubstring: true, normalizeWhiteSpace: true, ignoreCase: options.ignoreCase });
return await locator._expect('to.contain.text.array', { expectedText, isNot, useInnerText: options.useInnerText, timeout });
}, expected, { ...options, contains: true });
@ -264,7 +264,7 @@ export function toHaveValues(
expected: (string | RegExp)[],
options?: { timeout?: number },
) {
return toEqual.call(this, 'toHaveValues', locator, 'Locator', async (isNot, timeout, customStackTrace) => {
return toEqual.call(this, 'toHaveValues', locator, 'Locator', async (isNot, timeout) => {
const expectedText = toExpectedTextValues(expected);
return await locator._expect('to.have.values', { expectedText, isNot, timeout });
}, expected, options);

View file

@ -17,8 +17,6 @@
import type { Expect } from '../common/types';
import { expectTypes } from '../util';
import { callLogText } from '../util';
import type { ParsedStackTrace } from 'playwright-core/lib/utils';
import { captureStackTrace } from 'playwright-core/lib/utils';
import { matcherHint } from './matcherHint';
import { currentExpectTimeout } from '../common/globals';
@ -34,7 +32,7 @@ export async function toEqual<T>(
matcherName: string,
receiver: any,
receiverType: string,
query: (isNot: boolean, timeout: number, customStackTrace: ParsedStackTrace) => Promise<{ matches: boolean, received?: any, log?: string[], timedOut?: boolean }>,
query: (isNot: boolean, timeout: number) => Promise<{ matches: boolean, received?: any, log?: string[], timedOut?: boolean }>,
expected: T,
options: { timeout?: number, contains?: boolean } = {},
) {
@ -48,9 +46,7 @@ export async function toEqual<T>(
const timeout = currentExpectTimeout(options);
const customStackTrace = captureStackTrace();
customStackTrace.apiName = 'expect.' + matcherName;
const { matches: pass, received, log, timedOut } = await query(this.isNot, timeout, customStackTrace);
const { matches: pass, received, log, timedOut } = await query(this.isNot, timeout);
const message = pass
? () =>

View file

@ -441,12 +441,12 @@ export function prepareErrorStack(stack: string): {
const stackLines = lines.slice(firstStackLine);
let location: Location | undefined;
for (const line of stackLines) {
const { frame: parsed, fileName: resolvedFile } = parseStackTraceLine(line);
if (!parsed || !resolvedFile)
const frame = parseStackTraceLine(line);
if (!frame || !frame.fileName)
continue;
if (belongsToNodeModules(resolvedFile))
if (belongsToNodeModules(frame.fileName))
continue;
location = { file: resolvedFile, column: parsed.column || 0, line: parsed.line || 0 };
location = { file: frame.fileName, column: frame.column || 0, line: frame.line || 0 };
break;
}
return { message, stackLines, location };

View file

@ -16,33 +16,52 @@
import fs from 'fs';
import { mime } from 'playwright-core/lib/utilsBundle';
import type { StackFrameData } from 'playwright-core/lib/utilsBundle';
import util from 'util';
import path from 'path';
import url from 'url';
import { colors, debug, minimatch } from 'playwright-core/lib/utilsBundle';
import { colors, debug, minimatch, parseStackTraceLine } from 'playwright-core/lib/utilsBundle';
import type { TestInfoError, Location } from './common/types';
import { calculateSha1, captureStackTrace, isRegExp, isString } from 'playwright-core/lib/utils';
import type { ParsedStackTrace } from 'playwright-core/lib/utils';
export type { ParsedStackTrace };
import { calculateSha1, isRegExp, isString } from 'playwright-core/lib/utils';
const PLAYWRIGHT_TEST_PATH = path.join(__dirname, '..');
const PLAYWRIGHT_CORE_PATH = path.dirname(require.resolve('playwright-core/package.json'));
export function filterStackTrace(e: Error) {
if (process.env.PWDEBUGIMPL)
return;
const stack = captureStackTrace(e.stack);
const stackLines = stack.frames.filter(f => !f.file.startsWith(PLAYWRIGHT_TEST_PATH)).map(f => {
if (f.function)
return ` at ${f.function} (${f.file}:${f.line}:${f.column})`;
return ` at ${f.file}:${f.line}:${f.column}`;
});
const stackLines = stringifyStackFrames(filteredStackTrace(e));
const message = e.message;
e.stack = `${e.name}: ${e.message}\n${stackLines.join('\n')}`;
e.message = message;
}
export function filteredStackTrace(e: Error): StackFrameData[] {
const frames: StackFrameData[] = [];
for (const line of e.stack?.split('\n') || []) {
const frame = parseStackTraceLine(line);
if (!frame || !frame.fileName)
continue;
if (!process.env.PWDEBUGIMPL && frame.fileName.startsWith(PLAYWRIGHT_TEST_PATH))
continue;
if (!process.env.PWDEBUGIMPL && frame.fileName.startsWith(PLAYWRIGHT_CORE_PATH))
continue;
frames.push(frame);
}
return frames;
}
export function stringifyStackFrames(frames: StackFrameData[]): string[] {
const stackLines: string[] = [];
for (const frame of frames) {
if (frame.function)
stackLines.push(` at ${frame.function} (${frame.fileName}:${frame.line}:${frame.column})`);
else
stackLines.push(` at ${frame.fileName}:${frame.line}:${frame.column}`);
}
return stackLines;
}
export function serializeError(error: Error | any): TestInfoError {
if (error instanceof Error) {
filterStackTrace(error);

View file

@ -233,7 +233,8 @@ export class WorkerMain extends ProcessRunner {
// In theory, we should run above code without any errors.
// However, in the case we screwed up, or loadTestFile failed in the worker
// but not in the runner, let's do a fatal error.
this.unhandledError(e);
this._fatalErrors.push(serializeError(e));
this._stop();
} finally {
const donePayload: DonePayload = {
fatalErrors: this._fatalErrors,

View file

@ -278,7 +278,6 @@ it('should dispatch mouse move after context menu was opened', async ({ page, br
const angle = 2 * Math.PI * i / N;
const x = CX + Math.round(radius * Math.cos(angle));
const y = CY + Math.round(radius * Math.sin(angle));
console.log(x, y);
await page.mouse.move(x, y);
}
}