diff --git a/src/client/channelOwner.ts b/src/client/channelOwner.ts index a828852639..0793519282 100644 --- a/src/client/channelOwner.ts +++ b/src/client/channelOwner.ts @@ -72,15 +72,22 @@ export abstract class ChannelOwner { if (prop === 'debugScopeState') return (params: any) => this._connection.sendMessageToServer(this, prop, params, stackTrace); if (typeof prop === 'string') { const validator = scheme[paramsName(this._type, prop)]; - if (validator) - return (params: any) => this._connection.sendMessageToServer(this, prop, validator(params, ''), stackTrace); + if (validator) { + return (params: any) => { + if (callCookie && csi) { + callCookie.userObject = csi.onApiCallBegin(renderCallWithParams(stackTrace!.apiName, params)).userObject; + csi = undefined; + } + return this._connection.sendMessageToServer(this, prop, validator(params, ''), stackTrace); + }; + } } return obj[prop]; }, @@ -97,22 +104,25 @@ export abstract class ChannelOwner = this; while (!ancestorWithCSI._csi && ancestorWithCSI._parent) ancestorWithCSI = ancestorWithCSI._parent; - let csiCallback: ((e?: Error) => void) | undefined; + + // Do not report nested async calls to _wrapApiCall. + const isNested = stackTrace.allFrames.filter(f => f.function?.includes('_wrapApiCall')).length > 1; + const csi = isNested ? undefined : ancestorWithCSI._csi; + const callCookie: { userObject: any } = { userObject: null }; try { - logApiCall(logger, `=> ${apiName} started`); - csiCallback = ancestorWithCSI._csi?.onApiCall(stackTrace); - const channel = this._createChannel({}, stackTrace); + logApiCall(logger, `=> ${apiName} started`, isNested); + const channel = this._createChannel({}, stackTrace, csi, callCookie); const result = await func(channel as any, stackTrace); - csiCallback?.(); - logApiCall(logger, `<= ${apiName} succeeded`); + csi?.onApiCallEnd(callCookie); + logApiCall(logger, `<= ${apiName} succeeded`, isNested); return result; } catch (e) { const innerError = ((process.env.PWDEBUGIMPL || isUnderTest()) && e.stack) ? '\n\n' + e.stack : ''; e.message = apiName + ': ' + e.message; e.stack = e.message + '\n' + frameTexts.join('\n') + innerError; - csiCallback?.(e); - logApiCall(logger, `<= ${apiName} failed`); + csi?.onApiCallEnd(callCookie, e); + logApiCall(logger, `<= ${apiName} failed`, isNested); throw e; } } @@ -129,7 +139,9 @@ export abstract class ChannelOwner { return (arg: any, path: string) => { if (arg._object instanceof ChannelOwner && (name === '*' || arg._object._type === name)) diff --git a/src/client/connection.ts b/src/client/connection.ts index 4d52be1105..9de2b92116 100644 --- a/src/client/connection.ts +++ b/src/client/connection.ts @@ -83,7 +83,7 @@ export class Connection extends EventEmitter { async sendMessageToServer(object: ChannelOwner, method: string, params: any, maybeStackTrace: ParsedStackTrace | null): Promise { const guid = object._guid; - const stackTrace = maybeStackTrace || { frameTexts: [], frames: [], apiName: '' }; + const stackTrace: ParsedStackTrace = maybeStackTrace || { frameTexts: [], frames: [], apiName: '', allFrames: [] }; const { frames, apiName } = stackTrace; const id = ++this._lastId; diff --git a/src/client/page.ts b/src/client/page.ts index ad9d80e013..8591146ea3 100644 --- a/src/client/page.ts +++ b/src/client/page.ts @@ -383,9 +383,7 @@ export class Page extends ChannelOwner { - return this._wrapApiCall(async (channel: channels.PageChannel) => { - return this._waitForEvent(event, optionsOrPredicate, `waiting for event "${event}"`); - }); + return this._waitForEvent(event, optionsOrPredicate, `waiting for event "${event}"`); } private async _waitForEvent(event: string, optionsOrPredicate: WaitForEventOptions, logLine?: string): Promise { diff --git a/src/client/types.ts b/src/client/types.ts index 67cf21a3c3..d6f0eef5a9 100644 --- a/src/client/types.ts +++ b/src/client/types.ts @@ -17,7 +17,6 @@ import * as channels from '../protocol/channels'; import type { Size } from '../common/types'; -import type { ParsedStackTrace } from '../utils/stackTrace'; export { Size, Point, Rect, Quad, URLMatch, TimeoutOptions, HeadersArray } from '../common/types'; type LoggerSeverity = 'verbose' | 'info' | 'warning' | 'error'; @@ -27,7 +26,8 @@ export interface Logger { } export interface ClientSideInstrumentation { - onApiCall(stackTrace: ParsedStackTrace): ((error?: Error) => void) | undefined; + onApiCallBegin(apiCall: string): { userObject: any }; + onApiCallEnd(userData: { userObject: any }, error?: Error): any; } export type StrictOptions = { strict?: boolean }; diff --git a/src/test/dispatcher.ts b/src/test/dispatcher.ts index 172e4e00cd..59c897df13 100644 --- a/src/test/dispatcher.ts +++ b/src/test/dispatcher.ts @@ -313,7 +313,8 @@ export class Dispatcher { }; steps.set(params.stepId, step); (parentStep || result).steps.push(step); - stepStack.add(step); + if (params.canHaveChildren) + stepStack.add(step); this._reporter.onStepBegin?.(test, result, step); }); worker.on('stepEnd', (params: StepEndPayload) => { diff --git a/src/test/expect.ts b/src/test/expect.ts index 4ce40fa0e2..6740c7a656 100644 --- a/src/test/expect.ts +++ b/src/test/expect.ts @@ -75,7 +75,7 @@ function wrap(matcherName: string, matcher: any) { const INTERNAL_STACK_LENGTH = 3; const stackLines = new Error().stack!.split('\n').slice(INTERNAL_STACK_LENGTH + 1); - const step = testInfo._addStep('expect', `expect${this.isNot ? '.not' : ''}.${matcherName}`); + const step = testInfo._addStep('expect', `expect${this.isNot ? '.not' : ''}.${matcherName}`, true); const reportStepEnd = (result: any) => { const success = result.pass !== this.isNot; diff --git a/src/test/index.ts b/src/test/index.ts index ea96b71e2b..3aae49b277 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -20,7 +20,6 @@ import type { LaunchOptions, BrowserContextOptions, Page, BrowserContext, Browse import type { TestType, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, TestInfo } from '../../types/test'; import { rootTestType } from './testType'; import { createGuid, removeFolders } from '../utils/utils'; -import { TestInfoImpl } from './types'; export { expect } from './expect'; export const _baseTest: TestType<{}, {}> = rootTestType.test; @@ -203,13 +202,14 @@ export const test = _baseTest.extend({ await context.tracing.stop(); } (context as any)._csi = { - onApiCall: (stackTrace: ParsedStackTrace) => { - const testInfoImpl = testInfo as TestInfoImpl; - const existingStep = testInfoImpl._currentSteps().find(step => step.category === 'pw:api' || step.category === 'expect'); - const newStep = existingStep ? undefined : testInfoImpl._addStep('pw:api', stackTrace.apiName); - return (error?: Error) => { - newStep?.complete(error); - }; + onApiCallBegin: (apiCall: string) => { + const testInfoImpl = testInfo as any; + const step = testInfoImpl._addStep('pw:api', apiCall, false); + return { userObject: step }; + }, + onApiCallEnd: (data: { userObject: any }, error?: Error) => { + const step = data.userObject; + step?.complete(error); }, }; }; diff --git a/src/test/ipc.ts b/src/test/ipc.ts index ccdb7e7ca9..e42a295deb 100644 --- a/src/test/ipc.ts +++ b/src/test/ipc.ts @@ -51,6 +51,7 @@ export type StepBeginPayload = { stepId: string; title: string; category: string; + canHaveChildren: boolean; wallTime: number; // milliseconds since unix epoch }; diff --git a/src/test/testType.ts b/src/test/testType.ts index 201fd3a88b..98a6220ff8 100644 --- a/src/test/testType.ts +++ b/src/test/testType.ts @@ -186,7 +186,7 @@ export class TestTypeImpl { const testInfo = currentTestInfo(); if (!testInfo) throw errorWithLocation(location, `test.step() can only be called from a test`); - const step = testInfo._addStep('test.step', title); + const step = testInfo._addStep('test.step', title, true); try { await body(); step.complete(); diff --git a/src/test/types.ts b/src/test/types.ts index 4f3c185b6e..113e5674e0 100644 --- a/src/test/types.ts +++ b/src/test/types.ts @@ -28,10 +28,10 @@ export type Annotations = { type: string, description?: string }[]; export interface TestStepInternal { complete(error?: Error | TestError): void; category: string; + canHaveChildren: boolean; } export interface TestInfoImpl extends TestInfo { _testFinished: Promise; - _addStep: (category: string, title: string) => TestStepInternal; - _currentSteps(): TestStepInternal[]; + _addStep: (category: string, title: string, canHaveChildren: boolean) => TestStepInternal; } diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index 0c1b8cdcc9..c8f5a35ca4 100644 --- a/src/test/workerRunner.ts +++ b/src/test/workerRunner.ts @@ -219,7 +219,6 @@ export class WorkerRunner extends EventEmitter { let testFinishedCallback = () => {}; let lastStepId = 0; - const stepStack = new Set(); const testInfo: TestInfoImpl = { workerIndex: this._params.workerIndex, project: this._project.config, @@ -268,18 +267,18 @@ export class WorkerRunner extends EventEmitter { deadlineRunner.updateDeadline(deadline()); }, _testFinished: new Promise(f => testFinishedCallback = f), - _addStep: (category: string, title: string) => { + _addStep: (category: string, title: string, canHaveChildren: boolean) => { const stepId = `${category}@${title}@${++lastStepId}`; let callbackHandled = false; const step: TestStepInternal = { category, + canHaveChildren, complete: (error?: Error | TestError) => { if (callbackHandled) return; callbackHandled = true; if (error instanceof Error) error = serializeError(error); - stepStack.delete(step); const payload: StepEndPayload = { testId, stepId, @@ -290,11 +289,11 @@ export class WorkerRunner extends EventEmitter { this.emit('stepEnd', payload); } }; - stepStack.add(step); const payload: StepBeginPayload = { testId, stepId, category, + canHaveChildren, title, wallTime: Date.now(), }; @@ -302,7 +301,6 @@ export class WorkerRunner extends EventEmitter { this.emit('stepBegin', payload); return step; }, - _currentSteps: () => [...stepStack], }; // Inherit test.setTimeout() from parent suites. @@ -425,7 +423,7 @@ export class WorkerRunner extends EventEmitter { } private async _runTestWithBeforeHooks(test: TestCase, testInfo: TestInfoImpl) { - const step = testInfo._addStep('hook', 'Before Hooks'); + const step = testInfo._addStep('hook', 'Before Hooks', true); if (test._type === 'test') await this._runBeforeHooks(test, testInfo); @@ -459,7 +457,7 @@ export class WorkerRunner extends EventEmitter { let step: TestStepInternal | undefined; let teardownError: TestError | undefined; try { - step = testInfo._addStep('hook', 'After Hooks'); + step = testInfo._addStep('hook', 'After Hooks', true); if (test._type === 'test') await this._runHooks(test.parent!, 'afterEach', testInfo); } catch (error) { diff --git a/src/utils/stackTrace.ts b/src/utils/stackTrace.ts index 6e0c71f51b..9110ec2f3e 100644 --- a/src/utils/stackTrace.ts +++ b/src/utils/stackTrace.ts @@ -35,6 +35,7 @@ const CLIENT_LIB = path.join(ROOT_DIR, 'lib', 'client'); const CLIENT_SRC = path.join(ROOT_DIR, 'src', 'client'); export type ParsedStackTrace = { + allFrames: StackFrame[]; frames: StackFrame[]; frameTexts: string[]; apiName: string; @@ -78,6 +79,7 @@ export function captureStackTrace(): ParsedStackTrace { 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; @@ -88,6 +90,7 @@ export function captureStackTrace(): ParsedStackTrace { } return { + allFrames: allFrames.map(p => p.frame), frames: parsedFrames.map(p => p.frame), frameTexts: parsedFrames.map(p => p.frameText), apiName diff --git a/src/web/htmlReport/htmlReport.tsx b/src/web/htmlReport/htmlReport.tsx index be3db0a212..93806a91e7 100644 --- a/src/web/htmlReport/htmlReport.tsx +++ b/src/web/htmlReport/htmlReport.tsx @@ -80,7 +80,7 @@ const ProjectTreeItemView: React.FC<{ } loadChildren={hasChildren ? () => { - return project.suites.filter(s => !(failingOnly && s.stats.ok)).map((s, i) => ) || []; + return project.suites.filter(s => !(failingOnly && s.stats.ok)).map((s, i) => ) || []; } : undefined} depth={0} expandByDefault={true}>; }; @@ -90,17 +90,14 @@ const SuiteTreeItemView: React.FC<{ setTestId: (id: TestId) => void; failingOnly: boolean; depth: number, - showFileName: boolean, -}> = ({ suite, testId, setTestId, showFileName, failingOnly, depth }) => { - const location = renderLocation(suite.location, showFileName); +}> = ({ suite, testId, setTestId, failingOnly, depth }) => { return
{suite.title}
- {!!suite.location?.line && location &&
{location}
} } loadChildren={() => { - const suiteChildren = suite.suites.filter(s => !(failingOnly && s.stats.ok)).map((s, i) => ) || []; + const suiteChildren = suite.suites.filter(s => !(failingOnly && s.stats.ok)).map((s, i) => ) || []; const suiteCount = suite.suites.length; const testChildren = suite.tests.filter(t => !(failingOnly && t.ok)).map((t, i) => ) || []; return [...suiteChildren, ...testChildren]; diff --git a/tests/config/browserTest.ts b/tests/config/browserTest.ts index 3ca7a4c82a..2cf8701350 100644 --- a/tests/config/browserTest.ts +++ b/tests/config/browserTest.ts @@ -141,11 +141,14 @@ export const playwrightFixtures: Fixtures { + onApiCallBegin: (apiCall: string) => { const testInfoImpl = testInfo as any; - const existingStep = testInfoImpl._currentSteps().find(step => step.category === 'pw:api' || step.category === 'expect'); - const newStep = existingStep ? undefined : testInfoImpl._addStep('pw:api', stackTrace.apiName); - return (error?: Error) => newStep?.complete(error); + const step = testInfoImpl._addStep('pw:api', apiCall, false); + return { userObject: step }; + }, + onApiCallEnd: (data: { userObject: any }, error?: Error) => { + const step = data.userObject; + step?.complete(error); }, }; return context; diff --git a/tests/playwright-test/reporter.spec.ts b/tests/playwright-test/reporter.spec.ts index b807a0f353..036996af78 100644 --- a/tests/playwright-test/reporter.spec.ts +++ b/tests/playwright-test/reporter.spec.ts @@ -229,7 +229,9 @@ test('should report expect steps', async ({ runInlineTest }) => { `%% end {\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"Before Hooks\",\"category\":\"hook\",\"steps\":[{\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"}]}`, `%% begin {\"title\":\"expect.not.toHaveTitle\",\"category\":\"expect\"}`, - `%% end {\"title\":\"expect.not.toHaveTitle\",\"category\":\"expect\"}`, + `%% begin {\"title\":\"page.title\",\"category\":\"pw:api\"}`, + `%% end {\"title\":\"page.title\",\"category\":\"pw:api\"}`, + `%% end {\"title\":\"expect.not.toHaveTitle\",\"category\":\"expect\",\"steps\":[{\"title\":\"page.title\",\"category\":\"pw:api\"}]}`, `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, `%% begin {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`, @@ -281,22 +283,22 @@ test('should report api steps', async ({ runInlineTest }) => { `%% end {\"title\":\"Before Hooks\",\"category\":\"hook\",\"steps\":[{\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"}]}`, `%% begin {\"title\":\"page.setContent\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"page.setContent\",\"category\":\"pw:api\"}`, - `%% begin {\"title\":\"page.click\",\"category\":\"pw:api\"}`, - `%% end {\"title\":\"page.click\",\"category\":\"pw:api\"}`, + `%% begin {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, + `%% end {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, `%% begin {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"After Hooks\",\"category\":\"hook\",\"steps\":[{\"title\":\"browserContext.close\",\"category\":\"pw:api\"}]}`, `%% begin {\"title\":\"Before Hooks\",\"category\":\"hook\"}`, `%% end {\"title\":\"Before Hooks\",\"category\":\"hook\"}`, - `%% begin {\"title\":\"page.click\",\"category\":\"pw:api\"}`, - `%% end {\"title\":\"page.click\",\"category\":\"pw:api\"}`, + `%% begin {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, + `%% end {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, `%% end {\"title\":\"After Hooks\",\"category\":\"hook\"}`, `%% begin {\"title\":\"Before Hooks\",\"category\":\"hook\"}`, `%% end {\"title\":\"Before Hooks\",\"category\":\"hook\"}`, - `%% begin {\"title\":\"page.click\",\"category\":\"pw:api\"}`, - `%% end {\"title\":\"page.click\",\"category\":\"pw:api\"}`, + `%% begin {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, + `%% end {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, `%% end {\"title\":\"After Hooks\",\"category\":\"hook\"}`, ]); @@ -328,8 +330,8 @@ test('should report api step failure', async ({ runInlineTest }) => { `%% end {\"title\":\"Before Hooks\",\"category\":\"hook\",\"steps\":[{\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"}]}`, `%% begin {\"title\":\"page.setContent\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"page.setContent\",\"category\":\"pw:api\"}`, - `%% begin {\"title\":\"page.click\",\"category\":\"pw:api\"}`, - `%% end {\"title\":\"page.click\",\"category\":\"pw:api\",\"error\":{\"message\":\"page.click: Timeout 1ms exceeded.\\n=========================== logs ===========================\\nwaiting for selector \\\"input\\\"\\n============================================================\",\"stack\":\"\"}}`, + `%% begin {\"title\":\"page.click(input)\",\"category\":\"pw:api\"}`, + `%% end {\"title\":\"page.click(input)\",\"category\":\"pw:api\",\"error\":{\"message\":\"page.click: Timeout 1ms exceeded.\\n=========================== logs ===========================\\nwaiting for selector \\\"input\\\"\\n============================================================\",\"stack\":\"\"}}`, `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, `%% begin {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`,