diff --git a/src/client/channelOwner.ts b/src/client/channelOwner.ts index 8b2d603739..a828852639 100644 --- a/src/client/channelOwner.ts +++ b/src/client/channelOwner.ts @@ -21,7 +21,7 @@ import { debugLogger } from '../utils/debugLogger'; import { captureStackTrace, ParsedStackTrace } from '../utils/stackTrace'; import { isUnderTest } from '../utils/utils'; import type { Connection } from './connection'; -import type { ClientSideInstrumentation, LogContainer, Logger } from './types'; +import type { ClientSideInstrumentation, Logger } from './types'; export abstract class ChannelOwner extends EventEmitter { protected _connection: Connection; @@ -72,15 +72,15 @@ export abstract class ChannelOwner { if (prop === 'debugScopeState') - return (params: any) => this._connection.sendMessageToServer(this, prop, params, stackTrace, logContainer); + 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, logContainer); + return (params: any) => this._connection.sendMessageToServer(this, prop, validator(params, ''), stackTrace); } return obj[prop]; }, @@ -97,22 +97,21 @@ export abstract class ChannelOwner = this; while (!ancestorWithCSI._csi && ancestorWithCSI._parent) ancestorWithCSI = ancestorWithCSI._parent; - let csiCallback: ((log: string[], e?: Error) => void) | undefined; - const logContainer = { log: [] }; + let csiCallback: ((e?: Error) => void) | undefined; try { logApiCall(logger, `=> ${apiName} started`); csiCallback = ancestorWithCSI._csi?.onApiCall(stackTrace); - const channel = this._createChannel({}, stackTrace, csiCallback ? logContainer : undefined); + const channel = this._createChannel({}, stackTrace); const result = await func(channel as any, stackTrace); - csiCallback?.(logContainer.log); + csiCallback?.(); logApiCall(logger, `<= ${apiName} succeeded`); 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?.(logContainer.log, e); + csiCallback?.(e); logApiCall(logger, `<= ${apiName} failed`); throw e; } diff --git a/src/client/connection.ts b/src/client/connection.ts index f1411f3205..500581d041 100644 --- a/src/client/connection.ts +++ b/src/client/connection.ts @@ -39,7 +39,6 @@ import { ParsedStackTrace } from '../utils/stackTrace'; import { Artifact } from './artifact'; import { EventEmitter } from 'events'; import { JsonPipe } from './jsonPipe'; -import type { LogContainer } from './types'; class Root extends ChannelOwner { constructor(connection: Connection) { @@ -58,7 +57,7 @@ export class Connection extends EventEmitter { private _waitingForObject = new Map(); onmessage = (message: object): void => {}; private _lastId = 0; - private _callbacks = new Map void, reject: (a: Error) => void, stackTrace: ParsedStackTrace, logContainer: LogContainer | undefined }>(); + private _callbacks = new Map void, reject: (a: Error) => void, stackTrace: ParsedStackTrace }>(); private _rootObject: Root; private _disconnectedErrorMessage: string | undefined; private _onClose?: () => void; @@ -81,7 +80,7 @@ export class Connection extends EventEmitter { return this._objects.get(guid)!; } - async sendMessageToServer(object: ChannelOwner, method: string, params: any, maybeStackTrace: ParsedStackTrace | null, logContainer?: LogContainer): Promise { + async sendMessageToServer(object: ChannelOwner, method: string, params: any, maybeStackTrace: ParsedStackTrace | null): Promise { const guid = object._guid; const stackTrace = maybeStackTrace || { frameTexts: [], frames: [], apiName: '' }; const { frames, apiName } = stackTrace; @@ -90,12 +89,12 @@ export class Connection extends EventEmitter { const converted = { id, guid, method, params }; // Do not include metadata in debug logs to avoid noise. debugLogger.log('channel:command', converted); - const metadata: channels.Metadata = { stack: frames, apiName, collectLogs: logContainer ? true : undefined }; + const metadata: channels.Metadata = { stack: frames, apiName }; this.onmessage({ ...converted, metadata }); if (this._disconnectedErrorMessage) throw new Error(this._disconnectedErrorMessage); - return await new Promise((resolve, reject) => this._callbacks.set(id, { resolve, reject, stackTrace, logContainer })); + return await new Promise((resolve, reject) => this._callbacks.set(id, { resolve, reject, stackTrace })); } _debugScopeState(): any { @@ -103,15 +102,13 @@ export class Connection extends EventEmitter { } dispatch(message: object) { - const { id, guid, method, params, result, error, log } = message as any; + const { id, guid, method, params, result, error } = message as any; if (id) { debugLogger.log('channel:response', message); const callback = this._callbacks.get(id); if (!callback) throw new Error(`Cannot find command to respond: ${id}`); this._callbacks.delete(id); - if (log && callback.logContainer) - callback.logContainer.log.push(...log); if (error) callback.reject(parseError(error)); else diff --git a/src/client/types.ts b/src/client/types.ts index a4ade36d9a..67cf21a3c3 100644 --- a/src/client/types.ts +++ b/src/client/types.ts @@ -27,11 +27,8 @@ export interface Logger { } export interface ClientSideInstrumentation { - onApiCall(stackTrace: ParsedStackTrace): ((log: string[], error?: Error) => void) | undefined; + onApiCall(stackTrace: ParsedStackTrace): ((error?: Error) => void) | undefined; } -export type LogContainer = { - log: string[]; -}; export type StrictOptions = { strict?: boolean }; export type Headers = { [key: string]: string }; diff --git a/src/test/dispatcher.ts b/src/test/dispatcher.ts index 718bd0ddcf..172e4e00cd 100644 --- a/src/test/dispatcher.ts +++ b/src/test/dispatcher.ts @@ -331,7 +331,6 @@ export class Dispatcher { step.duration = params.wallTime - step.startTime.getTime(); if (params.error) step.error = params.error; - step.data = params.data; stepStack.delete(step); steps.delete(params.stepId); this._reporter.onStepEnd?.(test, result, step); diff --git a/src/test/expect.ts b/src/test/expect.ts index 8156d2efce..4ce40fa0e2 100644 --- a/src/test/expect.ts +++ b/src/test/expect.ts @@ -41,8 +41,6 @@ import type { Expect, TestError } from './types'; import matchers from 'expect/build/matchers'; import { currentTestInfo } from './globals'; import { serializeError } from './util'; -import StackUtils from 'stack-utils'; -import path from 'path'; export const expect: Expect = expectLibrary as any; expectLibrary.setState({ expand: false }); @@ -77,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}`, prepareExpectStepData(stackLines)); + const step = testInfo._addStep('expect', `expect${this.isNot ? '.not' : ''}.${matcherName}`); const reportStepEnd = (result: any) => { const success = result.pass !== this.isNot; @@ -106,22 +104,6 @@ function wrap(matcherName: string, matcher: any) { }; } -const stackUtils = new StackUtils(); - -function prepareExpectStepData(lines: string[]) { - const frames = lines.map(line => { - const parsed = stackUtils.parseLine(line); - if (!parsed) - return; - return { - file: parsed.file ? path.resolve(process.cwd(), parsed.file) : undefined, - line: parsed.line, - column: parsed.column - }; - }).filter(frame => !!frame); - return { stack: frames, log: [] }; -} - const wrappedMatchers: any = {}; for (const matcherName in matchers) wrappedMatchers[matcherName] = wrap(matcherName, matchers[matcherName]); diff --git a/src/test/index.ts b/src/test/index.ts index c6c67d5244..ea96b71e2b 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -206,9 +206,8 @@ export const test = _baseTest.extend({ 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, { stack: stackTrace.frames, log: [] }); - return (log: string[], error?: Error) => { - (existingStep || newStep)?.data.log?.push(...log); + const newStep = existingStep ? undefined : testInfoImpl._addStep('pw:api', stackTrace.apiName); + return (error?: Error) => { newStep?.complete(error); }; }, diff --git a/src/test/ipc.ts b/src/test/ipc.ts index 4b2e89508f..ccdb7e7ca9 100644 --- a/src/test/ipc.ts +++ b/src/test/ipc.ts @@ -59,7 +59,6 @@ export type StepEndPayload = { stepId: string; wallTime: number; // milliseconds since unix epoch error?: TestError; - data: { [key: string]: any }; }; export type TestEntry = { diff --git a/src/test/types.ts b/src/test/types.ts index 751b34f975..4f3c185b6e 100644 --- a/src/test/types.ts +++ b/src/test/types.ts @@ -28,11 +28,10 @@ export type Annotations = { type: string, description?: string }[]; export interface TestStepInternal { complete(error?: Error | TestError): void; category: string; - data: { [key: string]: any }; } export interface TestInfoImpl extends TestInfo { _testFinished: Promise; - _addStep: (category: string, title: string, data?: { [key: string]: any }) => TestStepInternal; + _addStep: (category: string, title: string) => TestStepInternal; _currentSteps(): TestStepInternal[]; } diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index 7fcf0a863c..0c1b8cdcc9 100644 --- a/src/test/workerRunner.ts +++ b/src/test/workerRunner.ts @@ -268,11 +268,10 @@ export class WorkerRunner extends EventEmitter { deadlineRunner.updateDeadline(deadline()); }, _testFinished: new Promise(f => testFinishedCallback = f), - _addStep: (category: string, title: string, data: { [key: string]: any } = {}) => { + _addStep: (category: string, title: string) => { const stepId = `${category}@${title}@${++lastStepId}`; let callbackHandled = false; const step: TestStepInternal = { - data, category, complete: (error?: Error | TestError) => { if (callbackHandled) @@ -286,7 +285,6 @@ export class WorkerRunner extends EventEmitter { stepId, wallTime: Date.now(), error, - data, }; if (reportEvents) this.emit('stepEnd', payload); diff --git a/src/web/htmlReport/htmlReport.tsx b/src/web/htmlReport/htmlReport.tsx index 292adc624d..8860c12473 100644 --- a/src/web/htmlReport/htmlReport.tsx +++ b/src/web/htmlReport/htmlReport.tsx @@ -272,14 +272,9 @@ const TestStepDetails: React.FC<{ } })(); }, [step]); - const [selectedTab, setSelectedTab] = React.useState('log'); + const [selectedTab, setSelectedTab] = React.useState('errors'); return
{step?.log ? step.log.join('\n') : ''}
- }, { id: 'errors', title: 'Errors', diff --git a/src/web/htmlReport2/htmlReport.tsx b/src/web/htmlReport2/htmlReport.tsx index 74a4f43cc7..5e00758b03 100644 --- a/src/web/htmlReport2/htmlReport.tsx +++ b/src/web/htmlReport2/htmlReport.tsx @@ -181,23 +181,14 @@ const StepTreeItem: React.FC<{ {step.title}
{msToString(step.duration)}
-
} loadChildren={step.steps.length + (step.log || []).length + (step.error ? 1 : 0) ? () => { - const stepChildren = step.steps.map((s, i) => ); - const logChildren = (step.log || []).map((l, i) => ); - const children = [...stepChildren, ...logChildren]; + } loadChildren={step.steps.length + (step.error ? 1 : 0) ? () => { + const children = step.steps.map((s, i) => ); if (step.error) children.unshift(); return children; } : undefined} depth={depth}>; }; -const LogTreeItem: React.FC<{ - log: string; - depth: number, -}> = ({ log, depth }) => { - return { log }} depth={depth}>; -}; - function statusIconForFailedTests(failedTests: number) { return failedTests ? statusIcon('failed') : statusIcon('passed'); } diff --git a/tests/config/browserTest.ts b/tests/config/browserTest.ts index 8d9e195588..bf0e5feee2 100644 --- a/tests/config/browserTest.ts +++ b/tests/config/browserTest.ts @@ -142,11 +142,8 @@ export const playwrightFixtures: Fixtures { 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, { stack: stackTrace.frames, log: [] }); - return (log: string[], error?: Error) => { - (existingStep || newStep)?.data.log?.push(...log); - newStep?.complete(error); - }; + const newStep = existingStep ? undefined : testInfoImpl._addStep('pw:api', stackTrace.apiName); + return (error?: Error) => newStep?.complete(error); }, }; contexts.push(context); diff --git a/tests/playwright-test/reporter.spec.ts b/tests/playwright-test/reporter.spec.ts index a42eeb814e..b807a0f353 100644 --- a/tests/playwright-test/reporter.spec.ts +++ b/tests/playwright-test/reporter.spec.ts @@ -359,51 +359,6 @@ test('should not have internal error when steps are finished after timeout', asy expect(result.output).not.toContain('Internal error'); }); -test('should report expect and pw:api stacks and logs', async ({ runInlineTest }, testInfo) => { - const expectReporterJS = ` - class Reporter { - stepStack(step) { - if (!step.data.stack || !step.data.stack[0]) - return step.title + ' '; - const frame = step.data.stack[0] - return step.title + ' stack: ' + frame.file + ':' + frame.line + ':' + frame.column; - } - onStepEnd(test, result, step) { - console.log('%%%% ' + this.stepStack(step)); - console.log('%%%% ' + step.title + ' log: ' + (step.data.log || []).join('')); - } - } - module.exports = Reporter; - `; - - const result = await runInlineTest({ - 'reporter.ts': expectReporterJS, - 'playwright.config.ts': ` - module.exports = { - reporter: './reporter', - }; - `, - 'a.test.ts': ` - const { test } = pwt; - test('pass', async ({ page }) => { - await page.setContent('hello
Click me
'); - await page.click('text=Click me'); - expect(1).toBe(1); - await expect(page.locator('div')).toHaveText('Click me'); - }); - ` - }, { reporter: '', workers: 1 }); - - expect(result.exitCode).toBe(0); - expect(result.output).toContain(`%% page.setContent stack: ${testInfo.outputPath('a.test.ts:7:20')}`); - expect(result.output).toContain(`%% page.setContent log: setting frame content, waiting until "load"`); - expect(result.output).toContain(`%% page.click stack: ${testInfo.outputPath('a.test.ts:8:20')}`); - expect(result.output).toContain(`%% page.click log: waiting for selector "text=Click me"`); - expect(result.output).toContain(`%% expect.toBe stack: ${testInfo.outputPath('a.test.ts:9:19')}`); - expect(result.output).toContain(`%% expect.toHaveText stack: ${testInfo.outputPath('a.test.ts:10:43')}`); - expect(result.output).toContain(`%% expect.toHaveText log: retrieving textContent from "div"`); -}); - function stripEscapedAscii(str: string) { return str.replace(/\\u00[a-z0-9][a-z0-9]\[[^m]+m/g, ''); }