From dfb3fdf217a7923affb0accfd82e708a16274412 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 20 Sep 2024 13:08:33 -0700 Subject: [PATCH] chore: iterate towards recording into trace (3) (#32718) --- packages/playwright-core/src/cli/program.ts | 4 +- packages/playwright-core/src/server/dialog.ts | 1 + .../playwright-core/src/server/download.ts | 13 +- .../src/server/instrumentation.ts | 6 + .../src/server/recorder/recorderApp.ts | 6 +- .../server/recorder/recorderInTraceViewer.ts | 64 ++++--- .../src/server/recorder/recorderUtils.ts | 158 ++++++++++++++---- .../src/server/trace/recorder/tracing.ts | 24 +++ .../src/server/trace/viewer/traceViewer.ts | 3 + .../playwright-core/src/utils/httpServer.ts | 4 +- packages/playwright/src/runner/testServer.ts | 1 + packages/trace-viewer/src/ui/recorderView.tsx | 3 +- 12 files changed, 224 insertions(+), 63 deletions(-) diff --git a/packages/playwright-core/src/cli/program.ts b/packages/playwright-core/src/cli/program.ts index fb27b14231b94..9d7abf7b9232c 100644 --- a/packages/playwright-core/src/cli/program.ts +++ b/packages/playwright-core/src/cli/program.ts @@ -397,7 +397,7 @@ async function launchContext(options: Options, extraOptions: LaunchOptions): Pro process.stdout.write('\n-------------8<-------------\n'); const autoExitCondition = process.env.PWTEST_CLI_AUTO_EXIT_WHEN; if (autoExitCondition && text.includes(autoExitCondition)) - Promise.all(context.pages().map(async p => p.close())); + closeBrowser(); }; // Make sure we exit abnormally when browser crashes. const logs: string[] = []; @@ -504,7 +504,7 @@ async function launchContext(options: Options, extraOptions: LaunchOptions): Pro if (hasPage) return; // Avoid the error when the last page is closed because the browser has been closed. - closeBrowser().catch(e => null); + closeBrowser().catch(() => {}); }); }); process.on('SIGINT', async () => { diff --git a/packages/playwright-core/src/server/dialog.ts b/packages/playwright-core/src/server/dialog.ts index 51dcfc2fc961e..f0793d43fb70c 100644 --- a/packages/playwright-core/src/server/dialog.ts +++ b/packages/playwright-core/src/server/dialog.ts @@ -39,6 +39,7 @@ export class Dialog extends SdkObject { this._onHandle = onHandle; this._defaultValue = defaultValue || ''; this._page._frameManager.dialogDidOpen(this); + this.instrumentation.onDialog(this); } page() { diff --git a/packages/playwright-core/src/server/download.ts b/packages/playwright-core/src/server/download.ts index f7a92c8c7ddf3..78a9c015dce95 100644 --- a/packages/playwright-core/src/server/download.ts +++ b/packages/playwright-core/src/server/download.ts @@ -35,16 +35,25 @@ export class Download { this._suggestedFilename = suggestedFilename; page._browserContext._downloads.add(this); if (suggestedFilename !== undefined) - this._page.emit(Page.Events.Download, this); + this._fireDownloadEvent(); + } + + page(): Page { + return this._page; } _filenameSuggested(suggestedFilename: string) { assert(this._suggestedFilename === undefined); this._suggestedFilename = suggestedFilename; - this._page.emit(Page.Events.Download, this); + this._fireDownloadEvent(); } suggestedFilename(): string { return this._suggestedFilename!; } + + private _fireDownloadEvent() { + this._page.instrumentation.onDownload(this._page, this); + this._page.emit(Page.Events.Download, this); + } } diff --git a/packages/playwright-core/src/server/instrumentation.ts b/packages/playwright-core/src/server/instrumentation.ts index b4628ac904b3e..4d29be028436d 100644 --- a/packages/playwright-core/src/server/instrumentation.ts +++ b/packages/playwright-core/src/server/instrumentation.ts @@ -35,6 +35,8 @@ export type Attribution = { }; import type { CallMetadata } from '@protocol/callMetadata'; +import type { Dialog } from './dialog'; +import type { Download } from './download'; export type { CallMetadata } from '@protocol/callMetadata'; export class SdkObject extends EventEmitter { @@ -62,6 +64,8 @@ export interface Instrumentation { onPageClose(page: Page): void; onBrowserOpen(browser: Browser): void; onBrowserClose(browser: Browser): void; + onDialog(dialog: Dialog): void; + onDownload(page: Page, download: Download): void; } export interface InstrumentationListener { @@ -73,6 +77,8 @@ export interface InstrumentationListener { onPageClose?(page: Page): void; onBrowserOpen?(browser: Browser): void; onBrowserClose?(browser: Browser): void; + onDialog?(dialog: Dialog): void; + onDownload?(page: Page, download: Download): void; } export function createInstrumentation(): Instrumentation { diff --git a/packages/playwright-core/src/server/recorder/recorderApp.ts b/packages/playwright-core/src/server/recorder/recorderApp.ts index c7120ef408c0d..5892a7f637e69 100644 --- a/packages/playwright-core/src/server/recorder/recorderApp.ts +++ b/packages/playwright-core/src/server/recorder/recorderApp.ts @@ -162,8 +162,10 @@ export class RecorderApp extends EventEmitter implements IRecorderApp { }).toString(), { isFunction: true }, sources).catch(() => {}); // Testing harness for runCLI mode. - if (process.env.PWTEST_CLI_IS_UNDER_TEST && sources.length) - (process as any)._didSetSourcesForTest(sources[0].text); + if (process.env.PWTEST_CLI_IS_UNDER_TEST && sources.length) { + if ((process as any)._didSetSourcesForTest(sources[0].text)) + this.close(); + } } async setSelector(selector: string, userGesture?: boolean): Promise { diff --git a/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts b/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts index 8f08b969e1f4a..8da0896497829 100644 --- a/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts +++ b/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts @@ -21,77 +21,97 @@ import type { IRecorder, IRecorderApp, IRecorderAppFactory } from './recorderFro import { installRootRedirect, openTraceViewerApp, startTraceViewerServer } from '../trace/viewer/traceViewer'; import type { TraceViewerServerOptions } from '../trace/viewer/traceViewer'; import type { BrowserContext } from '../browserContext'; -import { gracefullyProcessExitDoNotHang } from '../../utils/processLauncher'; -import type { Transport } from '../../utils/httpServer'; +import type { HttpServer, Transport } from '../../utils/httpServer'; +import type { Page } from '../page'; +import { ManualPromise } from '../../utils/manualPromise'; export class RecorderInTraceViewer extends EventEmitter implements IRecorderApp { readonly wsEndpointForTest: string | undefined; - private _recorder: IRecorder; - private _transport: Transport; + private _transport: RecorderTransport; + private _tracePage: Page; + private _traceServer: HttpServer; static factory(context: BrowserContext): IRecorderAppFactory { return async (recorder: IRecorder) => { const transport = new RecorderTransport(); const trace = path.join(context._browser.options.tracesDir, 'trace'); - const wsEndpointForTest = await openApp(trace, { transport, headless: !context._browser.options.headful }); - return new RecorderInTraceViewer(context, recorder, transport, wsEndpointForTest); + const { wsEndpointForTest, tracePage, traceServer } = await openApp(trace, { transport, headless: !context._browser.options.headful }); + return new RecorderInTraceViewer(transport, tracePage, traceServer, wsEndpointForTest); }; } - constructor(context: BrowserContext, recorder: IRecorder, transport: Transport, wsEndpointForTest: string | undefined) { + constructor(transport: RecorderTransport, tracePage: Page, traceServer: HttpServer, wsEndpointForTest: string | undefined) { super(); - this._recorder = recorder; this._transport = transport; + this._tracePage = tracePage; + this._traceServer = traceServer; this.wsEndpointForTest = wsEndpointForTest; + this._tracePage.once('close', () => { + this.close(); + }); } async close(): Promise { - this._transport.sendEvent?.('close', {}); + await this._tracePage.context().close({ reason: 'Recorder window closed' }); + await this._traceServer.stop(); } async setPaused(paused: boolean): Promise { - this._transport.sendEvent?.('setPaused', { paused }); + this._transport.deliverEvent('setPaused', { paused }); } async setMode(mode: Mode): Promise { - this._transport.sendEvent?.('setMode', { mode }); + this._transport.deliverEvent('setMode', { mode }); } async setFile(file: string): Promise { - this._transport.sendEvent?.('setFileIfNeeded', { file }); + this._transport.deliverEvent('setFileIfNeeded', { file }); } async setSelector(selector: string, userGesture?: boolean): Promise { - this._transport.sendEvent?.('setSelector', { selector, userGesture }); + this._transport.deliverEvent('setSelector', { selector, userGesture }); } async updateCallLogs(callLogs: CallLog[]): Promise { - this._transport.sendEvent?.('updateCallLogs', { callLogs }); + this._transport.deliverEvent('updateCallLogs', { callLogs }); } async setSources(sources: Source[]): Promise { - this._transport.sendEvent?.('setSources', { sources }); + this._transport.deliverEvent('setSources', { sources }); + if (process.env.PWTEST_CLI_IS_UNDER_TEST && sources.length) { + if ((process as any)._didSetSourcesForTest(sources[0].text)) + this.close(); + } } } -async function openApp(trace: string, options?: TraceViewerServerOptions & { headless?: boolean }): Promise { - const server = await startTraceViewerServer(options); - await installRootRedirect(server, [trace], { ...options, webApp: 'recorder.html' }); - const page = await openTraceViewerApp(server.urlPrefix('precise'), 'chromium', options); - page.on('close', () => gracefullyProcessExitDoNotHang(0)); - return page.context()._browser.options.wsEndpoint; +async function openApp(trace: string, options?: TraceViewerServerOptions & { headless?: boolean }): Promise<{ wsEndpointForTest: string | undefined, tracePage: Page, traceServer: HttpServer }> { + const traceServer = await startTraceViewerServer(options); + await installRootRedirect(traceServer, [trace], { ...options, webApp: 'recorder.html' }); + const page = await openTraceViewerApp(traceServer.urlPrefix('precise'), 'chromium', options); + return { wsEndpointForTest: page.context()._browser.options.wsEndpoint, tracePage: page, traceServer }; } class RecorderTransport implements Transport { + private _connected = new ManualPromise(); + constructor() { } - async dispatch(method: string, params: any) { + onconnect() { + this._connected.resolve(); + } + + async dispatch(method: string, params: any): Promise { } onclose() { } + deliverEvent(method: string, params: any) { + this._connected.then(() => this.sendEvent?.(method, params)); + } + sendEvent?: (method: string, params: any) => void; close?: () => void; } diff --git a/packages/playwright-core/src/server/recorder/recorderUtils.ts b/packages/playwright-core/src/server/recorder/recorderUtils.ts index 29186e9f96601..d2ed8208769dd 100644 --- a/packages/playwright-core/src/server/recorder/recorderUtils.ts +++ b/packages/playwright-core/src/server/recorder/recorderUtils.ts @@ -25,7 +25,7 @@ import type * as trace from '@trace/trace'; import { fromKeyboardModifiers, toKeyboardModifiers } from '../codegen/language'; import { serializeExpectedTextValues } from '../../utils/expectUtils'; import { createGuid, monotonicTime } from '../../utils'; -import { serializeValue } from '../../protocol/serializers'; +import { parseSerializedValue, serializeValue } from '../../protocol/serializers'; import type { SmartKeyboardModifier } from '../types'; export function metadataToCallLog(metadata: CallMetadata, status: CallLogStatus): CallLog { @@ -158,7 +158,7 @@ export function traceParamsForAction(actionInContext: ActionInContext): { method const params: channels.FrameExpectParams = { selector: action.selector, expression: 'to.be.checked', - isNot: action.checked, + isNot: !action.checked, }; return { method: 'expect', params }; } @@ -166,7 +166,7 @@ export function traceParamsForAction(actionInContext: ActionInContext): { method const params: channels.FrameExpectParams = { selector, expression: 'to.have.text', - expectedText: serializeExpectedTextValues([action.text], { matchSubstring: true, normalizeWhiteSpace: true }), + expectedText: serializeExpectedTextValues([action.text], { matchSubstring: action.substring, normalizeWhiteSpace: true }), isNot: false, }; return { method: 'expect', params }; @@ -195,6 +195,7 @@ export function callMetadataForAction(pageAliases: Map, actionInCo const mainFrame = mainFrameForAction(pageAliases, actionInContext); const { action } = actionInContext; const { method, params } = traceParamsForAction(actionInContext); + const callMetadata: CallMetadata = { id: `call@${createGuid()}`, stepId: `recorder@${createGuid()}`, @@ -215,38 +216,70 @@ export function callMetadataForAction(pageAliases: Map, actionInCo export function traceEventsToAction(events: trace.TraceEvent[]): ActionInContext[] { const result: ActionInContext[] = []; const pageAliases = new Map(); + let lastDownloadOrdinal = 0; + let lastDialogOrdinal = 0; + + const addSignal = (signal: actions.Signal) => { + const lastAction = result[result.length - 1]; + if (!lastAction) + return; + lastAction.action.signals.push(signal); + }; for (const event of events) { - if (event.type === 'event' && event.class === 'BrowserContext' && event.method === 'page') { - const pageAlias = 'page' + pageAliases.size; - pageAliases.set(event.params.pageId, pageAlias); - const lastAction = result[result.length - 1]; - lastAction.action.signals.push({ - name: 'popup', - popupAlias: pageAlias, - }); - result.push({ - frame: { pageAlias, framePath: [] }, - action: { - name: 'openPage', - url: '', - signals: [], - }, - timestamp: event.time, - }); - continue; - } + if (event.type === 'event' && event.class === 'BrowserContext') { + const { method, params } = event; + if (method === 'page') { + const pageAlias = 'page' + (pageAliases.size || ''); + pageAliases.set(params.pageId, pageAlias); + addSignal({ + name: 'popup', + popupAlias: pageAlias, + }); + result.push({ + frame: { pageAlias, framePath: [] }, + action: { + name: 'openPage', + url: '', + signals: [], + }, + timestamp: event.time, + }); + continue; + } - if (event.type === 'event' && event.class === 'BrowserContext' && event.method === 'pageClosed') { - const pageAlias = pageAliases.get(event.params.pageId) || 'page'; - result.push({ - frame: { pageAlias, framePath: [] }, - action: { - name: 'closePage', - signals: [], - }, - timestamp: event.time, - }); + if (method === 'pageClosed') { + const pageAlias = pageAliases.get(event.params.pageId) || 'page'; + result.push({ + frame: { pageAlias, framePath: [] }, + action: { + name: 'closePage', + signals: [], + }, + timestamp: event.time, + }); + continue; + } + + if (method === 'download') { + const downloadAlias = lastDownloadOrdinal ? String(lastDownloadOrdinal) : ''; + ++lastDownloadOrdinal; + addSignal({ + name: 'download', + downloadAlias, + }); + continue; + } + + if (method === 'dialog') { + const dialogAlias = lastDialogOrdinal ? String(lastDialogOrdinal) : ''; + ++lastDialogOrdinal; + addSignal({ + name: 'dialog', + dialogAlias, + }); + continue; + } continue; } @@ -389,6 +422,67 @@ export function traceEventsToAction(events: trace.TraceEvent[]): ActionInContext }); continue; } + if (method === 'expect') { + const params = untypedParams as channels.FrameExpectParams; + if (params.expression === 'to.have.text') { + const entry = params.expectedText?.[0]; + result.push({ + frame: { pageAlias, framePath: [] }, + action: { + name: 'assertText', + selector: params.selector, + signals: [], + text: entry?.string!, + substring: !!entry?.matchSubstring, + }, + timestamp: event.startTime + }); + continue; + } + + if (params.expression === 'to.have.value') { + result.push({ + frame: { pageAlias, framePath: [] }, + action: { + name: 'assertValue', + selector: params.selector, + signals: [], + value: parseSerializedValue(params.expectedValue!.value, params.expectedValue!.handles), + }, + timestamp: event.startTime + }); + continue; + } + + if (params.expression === 'to.be.checked') { + result.push({ + frame: { pageAlias, framePath: [] }, + action: { + name: 'assertChecked', + selector: params.selector, + signals: [], + checked: !params.isNot, + }, + timestamp: event.startTime + }); + continue; + } + + if (params.expression === 'to.be.visible') { + result.push({ + frame: { pageAlias, framePath: [] }, + action: { + name: 'assertVisible', + selector: params.selector, + signals: [], + }, + timestamp: event.startTime + }); + continue; + } + + continue; + } } return result; diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts index 44e6bd7f1f090..83b7fe612005e 100644 --- a/packages/playwright-core/src/server/trace/recorder/tracing.ts +++ b/packages/playwright-core/src/server/trace/recorder/tracing.ts @@ -38,6 +38,8 @@ import { Snapshotter } from './snapshotter'; import type { ConsoleMessage } from '../../console'; import { Dispatcher } from '../../dispatchers/dispatcher'; import { serializeError } from '../../errors'; +import type { Dialog } from '../../dialog'; +import type { Download } from '../../download'; const version: trace.VERSION = 7; @@ -454,6 +456,28 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps this._appendTraceEvent(event); } + onDialog(dialog: Dialog) { + const event: trace.EventTraceEvent = { + type: 'event', + time: monotonicTime(), + class: 'BrowserContext', + method: 'dialog', + params: { pageId: dialog.page().guid, type: dialog.type(), message: dialog.message(), defaultValue: dialog.defaultValue() }, + }; + this._appendTraceEvent(event); + } + + onDownload(page: Page, download: Download) { + const event: trace.EventTraceEvent = { + type: 'event', + time: monotonicTime(), + class: 'BrowserContext', + method: 'download', + params: { pageId: page.guid, url: download.url, suggestedFilename: download.suggestedFilename() }, + }; + this._appendTraceEvent(event); + } + onPageOpen(page: Page) { const event: trace.EventTraceEvent = { type: 'event', diff --git a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts index 6ca0319aa313c..ee2ccac593ba2 100644 --- a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts +++ b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts @@ -223,6 +223,9 @@ class StdinServer implements Transport { process.stdin.on('close', () => gracefullyProcessExitDoNotHang(0)); } + onconnect() { + } + async dispatch(method: string, params: any) { if (method === 'initialize') { if (this._traceUrl) diff --git a/packages/playwright-core/src/utils/httpServer.ts b/packages/playwright-core/src/utils/httpServer.ts index 24a84ea50271c..8da2a0e0d0b0e 100644 --- a/packages/playwright-core/src/utils/httpServer.ts +++ b/packages/playwright-core/src/utils/httpServer.ts @@ -27,8 +27,9 @@ export type ServerRouteHandler = (request: http.IncomingMessage, response: http. export type Transport = { sendEvent?: (method: string, params: any) => void; - dispatch: (method: string, params: any) => Promise; close?: () => void; + onconnect: () => void; + dispatch: (method: string, params: any) => Promise; onclose: () => void; }; @@ -82,6 +83,7 @@ export class HttpServer { this._wsGuid = guid || createGuid(); const wss = new wsServer({ server: this._server, path: '/' + this._wsGuid }); wss.on('connection', ws => { + transport.onconnect(); transport.sendEvent = (method, params) => ws.send(JSON.stringify({ method, params })); transport.close = () => ws.close(); ws.on('message', async message => { diff --git a/packages/playwright/src/runner/testServer.ts b/packages/playwright/src/runner/testServer.ts index 5d67385dc58bb..d4433b2d85e55 100644 --- a/packages/playwright/src/runner/testServer.ts +++ b/packages/playwright/src/runner/testServer.ts @@ -84,6 +84,7 @@ export class TestServerDispatcher implements TestServerInterface { constructor(configLocation: ConfigLocation) { this._configLocation = configLocation; this.transport = { + onconnect: () => {}, dispatch: (method, params) => (this as any)[method](params), onclose: () => { if (this._closeOnDisconnect) diff --git a/packages/trace-viewer/src/ui/recorderView.tsx b/packages/trace-viewer/src/ui/recorderView.tsx index 4d8ec8d2975c8..6111ea3dde4a6 100644 --- a/packages/trace-viewer/src/ui/recorderView.tsx +++ b/packages/trace-viewer/src/ui/recorderView.tsx @@ -45,8 +45,6 @@ export const RecorderView: React.FunctionComponent = () => { connection.setMode('recording'); }, [connection]); - window.playwrightSourcesEchoForTest = sources; - return