chore: remove dependency from library on expectZone, straighten csi handling (#34211)

This commit is contained in:
Dmitry Gozman 2025-01-07 13:30:04 +00:00 committed by GitHub
parent 527505e67b
commit 63329a3471
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 84 additions and 82 deletions

View File

@ -18,7 +18,6 @@ import { EventEmitter } from './eventEmitter';
import type * as channels from '@protocol/channels'; import type * as channels from '@protocol/channels';
import { maybeFindValidator, ValidationError, type ValidatorContext } from '../protocol/validator'; import { maybeFindValidator, ValidationError, type ValidatorContext } from '../protocol/validator';
import { debugLogger } from '../utils/debugLogger'; import { debugLogger } from '../utils/debugLogger';
import type { ExpectZone } from '../utils/stackTrace';
import { captureLibraryStackTrace, stringifyStackFrames } from '../utils/stackTrace'; import { captureLibraryStackTrace, stringifyStackFrames } from '../utils/stackTrace';
import { isUnderTest } from '../utils'; import { isUnderTest } from '../utils';
import { zones } from '../utils/zones'; import { zones } from '../utils/zones';
@ -148,15 +147,18 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
if (validator) { if (validator) {
return async (params: any) => { return async (params: any) => {
return await this._wrapApiCall(async apiZone => { return await this._wrapApiCall(async apiZone => {
const { apiName, frames, csi, callCookie, stepId } = apiZone.reported ? { apiName: undefined, csi: undefined, callCookie: undefined, frames: [], stepId: undefined } : apiZone; const validatedParams = validator(params, '', { tChannelImpl: tChannelImplToWire, binary: this._connection.rawBuffers() ? 'buffer' : 'toBase64' });
apiZone.reported = true; if (!apiZone.isInternal && !apiZone.reported) {
let currentStepId = stepId; // Reporting/tracing/logging this api call for the first time.
if (csi && apiName) { apiZone.params = params;
const out: { stepId?: string } = {}; apiZone.reported = true;
csi.onApiCallBegin(apiName, params, frames, callCookie, out); this._instrumentation.onApiCallBegin(apiZone);
currentStepId = out.stepId; logApiCall(this._logger, `=> ${apiZone.apiName} started`);
return await this._connection.sendMessageToServer(this, prop, validatedParams, apiZone.apiName, apiZone.frames, apiZone.stepId);
} }
return await this._connection.sendMessageToServer(this, prop, validator(params, '', { tChannelImpl: tChannelImplToWire, binary: this._connection.rawBuffers() ? 'buffer' : 'toBase64' }), apiName, frames, currentStepId); // Since this api call is either internal, or has already been reported/traced once,
// passing undefined apiName will avoid an extra unneeded tracing entry.
return await this._connection.sendMessageToServer(this, prop, validatedParams, undefined, [], undefined);
}); });
}; };
} }
@ -170,48 +172,36 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
async _wrapApiCall<R>(func: (apiZone: ApiZone) => Promise<R>, isInternal?: boolean): Promise<R> { async _wrapApiCall<R>(func: (apiZone: ApiZone) => Promise<R>, isInternal?: boolean): Promise<R> {
const logger = this._logger; const logger = this._logger;
const apiZone = zones.zoneData<ApiZone>('apiZone'); const existingApiZone = zones.zoneData<ApiZone>('apiZone');
if (apiZone) if (existingApiZone)
return await func(apiZone); return await func(existingApiZone);
const stackTrace = captureLibraryStackTrace();
let apiName: string | undefined = stackTrace.apiName;
const frames: channels.StackFrame[] = stackTrace.frames;
if (isInternal === undefined) if (isInternal === undefined)
isInternal = this._isInternalType; isInternal = this._isInternalType;
if (isInternal) const stackTrace = captureLibraryStackTrace();
apiName = undefined; const apiZone: ApiZone = { apiName: stackTrace.apiName, frames: stackTrace.frames, isInternal, reported: false, userData: undefined, stepId: undefined };
// Enclosing zone could have provided the apiName and wallTime.
const expectZone = zones.zoneData<ExpectZone>('expectZone');
const stepId = expectZone?.stepId;
if (!isInternal && expectZone)
apiName = expectZone.title;
// If we are coming from the expectZone, there is no need to generate a new
// step for the API call, since it will be generated by the expect itself.
const csi = isInternal || expectZone ? undefined : this._instrumentation;
const callCookie: any = {};
try { try {
logApiCall(logger, `=> ${apiName} started`, isInternal);
const apiZone: ApiZone = { apiName, frames, isInternal, reported: false, csi, callCookie, stepId };
const result = await zones.run('apiZone', apiZone, async () => await func(apiZone)); const result = await zones.run('apiZone', apiZone, async () => await func(apiZone));
csi?.onApiCallEnd(callCookie); if (!isInternal) {
logApiCall(logger, `<= ${apiName} succeeded`, isInternal); logApiCall(logger, `<= ${apiZone.apiName} succeeded`);
this._instrumentation.onApiCallEnd(apiZone);
}
return result; return result;
} catch (e) { } catch (e) {
const innerError = ((process.env.PWDEBUGIMPL || isUnderTest()) && e.stack) ? '\n<inner error>\n' + e.stack : ''; const innerError = ((process.env.PWDEBUGIMPL || isUnderTest()) && e.stack) ? '\n<inner error>\n' + e.stack : '';
if (apiName && !apiName.includes('<anonymous>')) if (apiZone.apiName && !apiZone.apiName.includes('<anonymous>'))
e.message = apiName + ': ' + e.message; e.message = apiZone.apiName + ': ' + e.message;
const stackFrames = '\n' + stringifyStackFrames(stackTrace.frames).join('\n') + innerError; const stackFrames = '\n' + stringifyStackFrames(stackTrace.frames).join('\n') + innerError;
if (stackFrames.trim()) if (stackFrames.trim())
e.stack = e.message + stackFrames; e.stack = e.message + stackFrames;
else else
e.stack = ''; e.stack = '';
csi?.onApiCallEnd(callCookie, e); if (!isInternal) {
logApiCall(logger, `<= ${apiName} failed`, isInternal); apiZone.error = e;
logApiCall(logger, `<= ${apiZone.apiName} failed`);
this._instrumentation.onApiCallEnd(apiZone);
}
throw e; throw e;
} }
} }
@ -232,9 +222,7 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
} }
} }
function logApiCall(logger: Logger | undefined, message: string, isNested: boolean) { function logApiCall(logger: Logger | undefined, message: string) {
if (isNested)
return;
if (logger && logger.isEnabled('api', 'info')) if (logger && logger.isEnabled('api', 'info'))
logger.log('api', 'info', message, [], { color: 'cyan' }); logger.log('api', 'info', message, [], { color: 'cyan' });
debugLogger.log('api', message); debugLogger.log('api', message);
@ -247,11 +235,12 @@ function tChannelImplToWire(names: '*' | string[], arg: any, path: string, conte
} }
type ApiZone = { type ApiZone = {
apiName: string | undefined; apiName: string;
params?: Record<string, any>;
frames: channels.StackFrame[]; frames: channels.StackFrame[];
isInternal: boolean; isInternal: boolean;
reported: boolean; reported: boolean;
csi: ClientInstrumentation | undefined; userData: any;
callCookie: any;
stepId?: string; stepId?: string;
error?: Error;
}; };

View File

@ -18,12 +18,22 @@ import type { StackFrame } from '@protocol/channels';
import type { BrowserContext } from './browserContext'; import type { BrowserContext } from './browserContext';
import type { APIRequestContext } from './fetch'; import type { APIRequestContext } from './fetch';
// Instrumentation can mutate the data, for example change apiName or stepId.
export interface ApiCallData {
apiName: string;
params?: Record<string, any>;
frames: StackFrame[];
userData: any;
stepId?: string;
error?: Error;
}
export interface ClientInstrumentation { export interface ClientInstrumentation {
addListener(listener: ClientInstrumentationListener): void; addListener(listener: ClientInstrumentationListener): void;
removeListener(listener: ClientInstrumentationListener): void; removeListener(listener: ClientInstrumentationListener): void;
removeAllListeners(): void; removeAllListeners(): void;
onApiCallBegin(apiCall: string, params: Record<string, any>, frames: StackFrame[], userData: any, out: { stepId?: string }): void; onApiCallBegin(apiCall: ApiCallData): void;
onApiCallEnd(userData: any, error?: Error): void; onApiCallEnd(apiCal: ApiCallData): void;
onWillPause(options: { keepTestTimeout: boolean }): void; onWillPause(options: { keepTestTimeout: boolean }): void;
runAfterCreateBrowserContext(context: BrowserContext): Promise<void>; runAfterCreateBrowserContext(context: BrowserContext): Promise<void>;
@ -33,8 +43,8 @@ export interface ClientInstrumentation {
} }
export interface ClientInstrumentationListener { export interface ClientInstrumentationListener {
onApiCallBegin?(apiName: string, params: Record<string, any>, frames: StackFrame[], userData: any, out: { stepId?: string }): void; onApiCallBegin?(apiCall: ApiCallData): void;
onApiCallEnd?(userData: any, error?: Error): void; onApiCallEnd?(apiCall: ApiCallData): void;
onWillPause?(options: { keepTestTimeout: boolean }): void; onWillPause?(options: { keepTestTimeout: boolean }): void;
runAfterCreateBrowserContext?(context: BrowserContext): Promise<void>; runAfterCreateBrowserContext?(context: BrowserContext): Promise<void>;

View File

@ -78,9 +78,9 @@ export class Connection extends EventEmitter {
constructor(localUtils: LocalUtils | undefined, instrumentation: ClientInstrumentation | undefined) { constructor(localUtils: LocalUtils | undefined, instrumentation: ClientInstrumentation | undefined) {
super(); super();
this._rootObject = new Root(this);
this._localUtils = localUtils;
this._instrumentation = instrumentation || createInstrumentation(); this._instrumentation = instrumentation || createInstrumentation();
this._localUtils = localUtils;
this._rootObject = new Root(this);
} }
markAsRemote() { markAsRemote() {

View File

@ -18,12 +18,13 @@ import * as fs from 'fs';
import * as path from 'path'; import * as path from 'path';
import type { APIRequestContext, BrowserContext, Browser, BrowserContextOptions, LaunchOptions, Page, Tracing, Video } from 'playwright-core'; import type { APIRequestContext, BrowserContext, Browser, BrowserContextOptions, LaunchOptions, Page, Tracing, Video } from 'playwright-core';
import * as playwrightLibrary from 'playwright-core'; import * as playwrightLibrary from 'playwright-core';
import { createGuid, debugMode, addInternalStackPrefix, isString, asLocator, jsonStringifyForceASCII } from 'playwright-core/lib/utils'; import { createGuid, debugMode, addInternalStackPrefix, isString, asLocator, jsonStringifyForceASCII, zones } from 'playwright-core/lib/utils';
import type { ExpectZone } from 'playwright-core/lib/utils';
import type { Fixtures, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, ScreenshotMode, TestInfo, TestType, VideoMode } from '../types/test'; import type { Fixtures, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, ScreenshotMode, TestInfo, TestType, VideoMode } from '../types/test';
import type { TestInfoImpl, TestStepInternal } from './worker/testInfo'; import type { TestInfoImpl, TestStepInternal } from './worker/testInfo';
import { rootTestType } from './common/testType'; import { rootTestType } from './common/testType';
import type { ContextReuseMode } from './common/config'; import type { ContextReuseMode } from './common/config';
import type { ClientInstrumentation, ClientInstrumentationListener } from '../../playwright-core/src/client/clientInstrumentation'; import type { ApiCallData, ClientInstrumentation, ClientInstrumentationListener } from '../../playwright-core/src/client/clientInstrumentation';
import { currentTestInfo } from './common/globals'; import { currentTestInfo } from './common/globals';
export { expect } from './matchers/expect'; export { expect } from './matchers/expect';
export const _baseTest: TestType<{}, {}> = rootTestType.test; export const _baseTest: TestType<{}, {}> = rootTestType.test;
@ -258,34 +259,43 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
const tracingGroupSteps: TestStepInternal[] = []; const tracingGroupSteps: TestStepInternal[] = [];
const csiListener: ClientInstrumentationListener = { const csiListener: ClientInstrumentationListener = {
onApiCallBegin: (apiName: string, params: Record<string, any>, frames: StackFrame[], userData: any, out: { stepId?: string }) => { onApiCallBegin: (data: ApiCallData) => {
userData.apiName = apiName;
const testInfo = currentTestInfo(); const testInfo = currentTestInfo();
if (!testInfo || apiName.includes('setTestIdAttribute') || apiName === 'tracing.groupEnd') // Some special calls do not get into steps.
if (!testInfo || data.apiName.includes('setTestIdAttribute') || data.apiName === 'tracing.groupEnd')
return; return;
const step = testInfo._addStep({ const expectZone = zones.zoneData<ExpectZone>('expectZone');
location: frames[0] as any, if (expectZone) {
category: 'pw:api', // Display the internal locator._expect call under the name of the enclosing expect call,
title: renderApiCall(apiName, params), // and connect it to the existing expect step.
apiName, data.apiName = expectZone.title;
params, data.stepId = expectZone.stepId;
}, tracingGroupSteps[tracingGroupSteps.length - 1]);
userData.step = step;
out.stepId = step.stepId;
if (apiName === 'tracing.group')
tracingGroupSteps.push(step);
},
onApiCallEnd: (userData: any, error?: Error) => {
// "tracing.group" step will end later, when "tracing.groupEnd" finishes.
if (userData.apiName === 'tracing.group')
return;
if (userData.apiName === 'tracing.groupEnd') {
const step = tracingGroupSteps.pop();
step?.complete({ error });
return; return;
} }
const step = userData.step; // In the general case, create a step for each api call and connect them through the stepId.
step?.complete({ error }); const step = testInfo._addStep({
location: data.frames[0],
category: 'pw:api',
title: renderApiCall(data.apiName, data.params),
apiName: data.apiName,
params: data.params,
}, tracingGroupSteps[tracingGroupSteps.length - 1]);
data.userData = step;
data.stepId = step.stepId;
if (data.apiName === 'tracing.group')
tracingGroupSteps.push(step);
},
onApiCallEnd: (data: ApiCallData) => {
// "tracing.group" step will end later, when "tracing.groupEnd" finishes.
if (data.apiName === 'tracing.group')
return;
if (data.apiName === 'tracing.groupEnd') {
const step = tracingGroupSteps.pop();
step?.complete({ error: data.error });
return;
}
const step = data.userData;
step?.complete({ error: data.error });
}, },
onWillPause: ({ keepTestTimeout }) => { onWillPause: ({ keepTestTimeout }) => {
if (!keepTestTimeout) if (!keepTestTimeout)
@ -441,13 +451,6 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
}, },
}); });
type StackFrame = {
file: string,
line?: number,
column?: number,
function?: string,
};
type ScreenshotOption = PlaywrightWorkerOptions['screenshot'] | undefined; type ScreenshotOption = PlaywrightWorkerOptions['screenshot'] | undefined;
type Playwright = PlaywrightWorkerArgs['playwright']; type Playwright = PlaywrightWorkerArgs['playwright'];