Skip to content

Commit

Permalink
chore: allow starting recorder in traceviewer (#32741)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman committed Sep 20, 2024
1 parent dfb3fdf commit 418d1c0
Show file tree
Hide file tree
Showing 19 changed files with 159 additions and 102 deletions.
1 change: 1 addition & 0 deletions packages/playwright-core/src/cli/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,7 @@ async function codegen(options: Options & { target: string, output?: string, tes
device: options.device,
saveStorage: options.saveStorage,
mode: 'recording',
codegenMode: process.env.PW_RECORDER_IS_TRACE_VIEWER ? 'trace-events' : 'actions',
testIdAttributeName,
outputFile: outputFile ? path.resolve(outputFile) : undefined,
});
Expand Down
13 changes: 2 additions & 11 deletions packages/playwright-core/src/client/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,17 +492,8 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
await this._closedPromise;
}

async _enableRecorder(params: {
language: string,
launchOptions?: LaunchOptions,
contextOptions?: BrowserContextOptions,
device?: string,
saveStorage?: string,
mode?: 'recording' | 'inspecting',
testIdAttributeName?: string,
outputFile?: string,
}) {
await this._channel.recorderSupplementEnable(params);
async _enableRecorder(params: channels.BrowserContextEnableRecorderParams) {
await this._channel.enableRecorder(params);
}
}

Expand Down
5 changes: 3 additions & 2 deletions packages/playwright-core/src/protocol/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -965,9 +965,10 @@ scheme.BrowserContextStorageStateResult = tObject({
});
scheme.BrowserContextPauseParams = tOptional(tObject({}));
scheme.BrowserContextPauseResult = tOptional(tObject({}));
scheme.BrowserContextRecorderSupplementEnableParams = tObject({
scheme.BrowserContextEnableRecorderParams = tObject({
language: tOptional(tString),
mode: tOptional(tEnum(['inspecting', 'recording'])),
codegenMode: tOptional(tEnum(['actions', 'trace-events'])),
pauseOnNextStatement: tOptional(tBoolean),
testIdAttributeName: tOptional(tString),
launchOptions: tOptional(tAny),
Expand All @@ -977,7 +978,7 @@ scheme.BrowserContextRecorderSupplementEnableParams = tObject({
outputFile: tOptional(tString),
omitCallTracking: tOptional(tBoolean),
});
scheme.BrowserContextRecorderSupplementEnableResult = tOptional(tObject({}));
scheme.BrowserContextEnableRecorderResult = tOptional(tObject({}));
scheme.BrowserContextNewCDPSessionParams = tObject({
page: tOptional(tChannel(['Page'])),
frame: tOptional(tChannel(['Frame'])),
Expand Down
6 changes: 3 additions & 3 deletions packages/playwright-core/src/server/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,15 @@ export abstract class BrowserContext extends SdkObject {

// When PWDEBUG=1, show inspector for each context.
if (debugMode() === 'inspector')
await Recorder.show(this, RecorderApp.factory(this), { pauseOnNextStatement: true });
await Recorder.show('actions', this, RecorderApp.factory(this), { pauseOnNextStatement: true });

// When paused, show inspector.
if (this._debugger.isPaused())
Recorder.showInspector(this, RecorderApp.factory(this));
Recorder.showInspectorNoReply(this, RecorderApp.factory(this));

this._debugger.on(Debugger.Events.PausedStateChanged, () => {
if (this._debugger.isPaused())
Recorder.showInspector(this, RecorderApp.factory(this));
Recorder.showInspectorNoReply(this, RecorderApp.factory(this));
});

if (debugMode() === 'console')
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/server/debugController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export class DebugController extends SdkObject {
const contexts = new Set<BrowserContext>();
for (const page of this._playwright.allPages())
contexts.add(page.context());
const result = await Promise.all([...contexts].map(c => Recorder.show(c, () => Promise.resolve(new InspectingRecorderApp(this)), { omitCallTracking: true })));
const result = await Promise.all([...contexts].map(c => Recorder.showInspector(c, { omitCallTracking: true }, () => Promise.resolve(new InspectingRecorderApp(this)))));
return result.filter(Boolean) as Recorder[];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import { serializeError } from '../errors';
import { ElementHandleDispatcher } from './elementHandlerDispatcher';
import { RecorderInTraceViewer } from '../recorder/recorderInTraceViewer';
import { RecorderApp } from '../recorder/recorderApp';
import type { IRecorderAppFactory } from '../recorder/recorderFrontend';
import { WebSocketRouteDispatcher } from './webSocketRouteDispatcher';

export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channels.BrowserContextChannel, DispatcherScope> implements channels.BrowserContextChannel {
Expand Down Expand Up @@ -301,21 +300,19 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
await this._context.close(params);
}

async recorderSupplementEnable(params: channels.BrowserContextRecorderSupplementEnableParams): Promise<void> {
let factory: IRecorderAppFactory;
if (process.env.PW_RECORDER_IS_TRACE_VIEWER) {
factory = RecorderInTraceViewer.factory(this._context);
async enableRecorder(params: channels.BrowserContextEnableRecorderParams): Promise<void> {
if (params.codegenMode === 'trace-events') {
await this._context.tracing.start({
name: 'trace',
snapshots: true,
screenshots: false,
screenshots: true,
live: true,
});
await this._context.tracing.startChunk({ name: 'trace', title: 'trace' });
await Recorder.show('trace-events', this._context, RecorderInTraceViewer.factory(this._context), params);
} else {
factory = RecorderApp.factory(this._context);
await Recorder.show('actions', this._context, RecorderApp.factory(this._context), params);
}
await Recorder.show(this._context, factory, params);
}

async pause(params: channels.BrowserContextPauseParams, metadata: CallMetadata) {
Expand Down
21 changes: 12 additions & 9 deletions packages/playwright-core/src/server/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,32 +45,35 @@ export class Recorder implements InstrumentationListener, IRecorder {
private _omitCallTracking = false;
private _currentLanguage: Language;

static showInspector(context: BrowserContext, recorderAppFactory: IRecorderAppFactory) {
const params: channels.BrowserContextRecorderSupplementEnableParams = {};
static async showInspector(context: BrowserContext, params: channels.BrowserContextEnableRecorderParams, recorderAppFactory: IRecorderAppFactory) {
if (isUnderTest())
params.language = process.env.TEST_INSPECTOR_LANGUAGE;
Recorder.show(context, recorderAppFactory, params).catch(() => {});
return await Recorder.show('actions', context, recorderAppFactory, params);
}

static show(context: BrowserContext, recorderAppFactory: IRecorderAppFactory, params: channels.BrowserContextRecorderSupplementEnableParams = {}): Promise<Recorder> {
static showInspectorNoReply(context: BrowserContext, recorderAppFactory: IRecorderAppFactory) {
Recorder.showInspector(context, {}, recorderAppFactory).catch(() => {});
}

static show(codegenMode: 'actions' | 'trace-events', context: BrowserContext, recorderAppFactory: IRecorderAppFactory, params: channels.BrowserContextEnableRecorderParams): Promise<Recorder> {
let recorderPromise = (context as any)[recorderSymbol] as Promise<Recorder>;
if (!recorderPromise) {
recorderPromise = Recorder._create(context, recorderAppFactory, params);
recorderPromise = Recorder._create(codegenMode, context, recorderAppFactory, params);
(context as any)[recorderSymbol] = recorderPromise;
}
return recorderPromise;
}

private static async _create(context: BrowserContext, recorderAppFactory: IRecorderAppFactory, params: channels.BrowserContextRecorderSupplementEnableParams = {}): Promise<Recorder> {
const recorder = new Recorder(context, params);
private static async _create(codegenMode: 'actions' | 'trace-events', context: BrowserContext, recorderAppFactory: IRecorderAppFactory, params: channels.BrowserContextEnableRecorderParams = {}): Promise<Recorder> {
const recorder = new Recorder(codegenMode, context, params);
const recorderApp = await recorderAppFactory(recorder);
await recorder._install(recorderApp);
return recorder;
}

constructor(context: BrowserContext, params: channels.BrowserContextRecorderSupplementEnableParams) {
constructor(codegenMode: 'actions' | 'trace-events', context: BrowserContext, params: channels.BrowserContextEnableRecorderParams) {
this._mode = params.mode || 'none';
this._contextRecorder = new ContextRecorder(context, params, {});
this._contextRecorder = new ContextRecorder(codegenMode, context, params, {});
this._context = context;
this._omitCallTracking = !!params.omitCallTracking;
this._debugger = context.debugger();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ export class ContextRecorder extends EventEmitter {
private _lastDialogOrdinal = -1;
private _lastDownloadOrdinal = -1;
private _context: BrowserContext;
private _params: channels.BrowserContextRecorderSupplementEnableParams;
private _params: channels.BrowserContextEnableRecorderParams;
private _delegate: ContextRecorderDelegate;
private _recorderSources: Source[];
private _throttledOutputFile: ThrottledFile | null = null;
private _orderedLanguages: LanguageGenerator[] = [];
private _listeners: RegisteredListener[] = [];

constructor(context: BrowserContext, params: channels.BrowserContextRecorderSupplementEnableParams, delegate: ContextRecorderDelegate) {
constructor(codegenMode: 'actions' | 'trace-events', context: BrowserContext, params: channels.BrowserContextEnableRecorderParams, delegate: ContextRecorderDelegate) {
super();
this._context = context;
this._params = params;
Expand All @@ -73,7 +73,7 @@ export class ContextRecorder extends EventEmitter {
saveStorage: params.saveStorage,
};

const collection = new RecorderCollection(context, this._pageAliases, params.mode === 'recording');
const collection = new RecorderCollection(codegenMode, context, this._pageAliases, params.mode === 'recording');
collection.on('change', (actions: ActionInContext[]) => {
this._recorderSources = [];
for (const languageGenerator of this._orderedLanguages) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ export class RecorderCollection extends EventEmitter {
private _pageAliases: Map<Page, string>;
private _context: BrowserContext;

constructor(context: BrowserContext, pageAliases: Map<Page, string>, enabled: boolean) {
constructor(codegenMode: 'actions' | 'trace-events', context: BrowserContext, pageAliases: Map<Page, string>, enabled: boolean) {
super();
this._context = context;
this._enabled = enabled;
this._pageAliases = pageAliases;

if (process.env.PW_RECORDER_IS_TRACE_VIEWER) {
if (codegenMode === 'trace-events') {
this._context.tracing.onMemoryEvents(events => {
this._actions = traceEventsToAction(events);
this._fireChange();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,12 @@ export function traceParamsForAction(actionInContext: ActionInContext): { method

export function callMetadataForAction(pageAliases: Map<Page, string>, actionInContext: ActionInContext): { callMetadata: CallMetadata, mainFrame: Frame } {
const mainFrame = mainFrameForAction(pageAliases, actionInContext);
const { action } = actionInContext;
const { method, params } = traceParamsForAction(actionInContext);

const callMetadata: CallMetadata = {
id: `call@${createGuid()}`,
stepId: `recorder@${createGuid()}`,
apiName: 'frame.' + action.name,
apiName: 'page.' + method,
objectId: mainFrame.guid,
pageId: mainFrame._page.guid,
frameId: mainFrame.guid,
Expand Down
10 changes: 6 additions & 4 deletions packages/protocol/src/channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1526,7 +1526,7 @@ export interface BrowserContextChannel extends BrowserContextEventTarget, EventT
setOffline(params: BrowserContextSetOfflineParams, metadata?: CallMetadata): Promise<BrowserContextSetOfflineResult>;
storageState(params?: BrowserContextStorageStateParams, metadata?: CallMetadata): Promise<BrowserContextStorageStateResult>;
pause(params?: BrowserContextPauseParams, metadata?: CallMetadata): Promise<BrowserContextPauseResult>;
recorderSupplementEnable(params: BrowserContextRecorderSupplementEnableParams, metadata?: CallMetadata): Promise<BrowserContextRecorderSupplementEnableResult>;
enableRecorder(params: BrowserContextEnableRecorderParams, metadata?: CallMetadata): Promise<BrowserContextEnableRecorderResult>;
newCDPSession(params: BrowserContextNewCDPSessionParams, metadata?: CallMetadata): Promise<BrowserContextNewCDPSessionResult>;
harStart(params: BrowserContextHarStartParams, metadata?: CallMetadata): Promise<BrowserContextHarStartResult>;
harExport(params: BrowserContextHarExportParams, metadata?: CallMetadata): Promise<BrowserContextHarExportResult>;
Expand Down Expand Up @@ -1766,9 +1766,10 @@ export type BrowserContextStorageStateResult = {
export type BrowserContextPauseParams = {};
export type BrowserContextPauseOptions = {};
export type BrowserContextPauseResult = void;
export type BrowserContextRecorderSupplementEnableParams = {
export type BrowserContextEnableRecorderParams = {
language?: string,
mode?: 'inspecting' | 'recording',
codegenMode?: 'actions' | 'trace-events',
pauseOnNextStatement?: boolean,
testIdAttributeName?: string,
launchOptions?: any,
Expand All @@ -1778,9 +1779,10 @@ export type BrowserContextRecorderSupplementEnableParams = {
outputFile?: string,
omitCallTracking?: boolean,
};
export type BrowserContextRecorderSupplementEnableOptions = {
export type BrowserContextEnableRecorderOptions = {
language?: string,
mode?: 'inspecting' | 'recording',
codegenMode?: 'actions' | 'trace-events',
pauseOnNextStatement?: boolean,
testIdAttributeName?: string,
launchOptions?: any,
Expand All @@ -1790,7 +1792,7 @@ export type BrowserContextRecorderSupplementEnableOptions = {
outputFile?: string,
omitCallTracking?: boolean,
};
export type BrowserContextRecorderSupplementEnableResult = void;
export type BrowserContextEnableRecorderResult = void;
export type BrowserContextNewCDPSessionParams = {
page?: PageChannel,
frame?: FrameChannel,
Expand Down
7 changes: 6 additions & 1 deletion packages/protocol/src/protocol.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,7 @@ BrowserContext:
pause:
experimental: True

recorderSupplementEnable:
enableRecorder:
experimental: True
parameters:
language: string?
Expand All @@ -1196,6 +1196,11 @@ BrowserContext:
literals:
- inspecting
- recording
codegenMode:
type: enum?
literals:
- actions
- trace-events
pauseOnNextStatement: boolean?
testIdAttributeName: string?
launchOptions: json?
Expand Down
4 changes: 3 additions & 1 deletion packages/trace-viewer/src/sw.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,14 @@ async function loadTrace(traceUrl: string, traceFileName: string | null, clientI
}
set.add(traceUrl);

const isRecorderMode = traceUrl.includes('/recorder-trace-');

const traceModel = new TraceModel();
try {
// Allow 10% to hop from sw to page.
const [fetchProgress, unzipProgress] = splitProgress(progress, [0.5, 0.4, 0.1]);
const backend = traceUrl.endsWith('json') ? new FetchTraceModelBackend(traceUrl) : new ZipTraceModelBackend(traceUrl, fetchProgress);
await traceModel.load(backend, unzipProgress);
await traceModel.load(backend, isRecorderMode, unzipProgress);
} catch (error: any) {
// eslint-disable-next-line no-console
console.error(error);
Expand Down
23 changes: 20 additions & 3 deletions packages/trace-viewer/src/traceModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { parseClientSideCallMetadata } from '../../../packages/playwright-core/src/utils/isomorphic/traceUtils';
import type { ContextEntry } from './entries';
import type { ActionEntry, ContextEntry } from './entries';
import { createEmptyContext } from './entries';
import { SnapshotStorage } from './snapshotStorage';
import { TraceModernizer } from './traceModernizer';
Expand All @@ -38,7 +38,7 @@ export class TraceModel {
constructor() {
}

async load(backend: TraceModelBackend, unzipProgress: (done: number, total: number) => void) {
async load(backend: TraceModelBackend, isRecorderMode: boolean, unzipProgress: (done: number, total: number) => void) {
this._backend = backend;

const ordinals: string[] = [];
Expand Down Expand Up @@ -72,7 +72,8 @@ export class TraceModel {
modernizer.appendTrace(network);
unzipProgress(++done, total);

contextEntry.actions = modernizer.actions().sort((a1, a2) => a1.startTime - a2.startTime);
const actions = modernizer.actions().sort((a1, a2) => a1.startTime - a2.startTime);
contextEntry.actions = isRecorderMode ? collapseActionsForRecorder(actions) : actions;

if (!backend.isLive()) {
// Terminate actions w/o after event gracefully.
Expand Down Expand Up @@ -133,3 +134,19 @@ function stripEncodingFromContentType(contentType: string) {
return charset[1];
return contentType;
}

function collapseActionsForRecorder(actions: ActionEntry[]): ActionEntry[] {
const result: ActionEntry[] = [];
for (const action of actions) {
const lastAction = result[result.length - 1];
const isSameAction = lastAction && lastAction.method === action.method && lastAction.pageId === action.pageId;
const isSameSelector = lastAction && 'selector' in lastAction.params && 'selector' in action.params && action.params.selector === lastAction.params.selector;
const shouldMerge = isSameAction && (action.method === 'goto' || (action.method === 'fill' && isSameSelector));
if (!shouldMerge) {
result.push(action);
continue;
}
result[result.length - 1] = action;
}
return result;
}
2 changes: 2 additions & 0 deletions tests/config/testModeFixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import * as playwrightLibrary from 'playwright-core';

export type TestModeWorkerOptions = {
mode: TestModeName;
codegenMode: 'trace-events' | 'actions';
};

export type TestModeTestFixtures = {
Expand Down Expand Up @@ -48,6 +49,7 @@ export const testModeTest = test.extend<TestModeTestFixtures, TestModeWorkerOpti
await run(playwright);
await testMode.teardown();
}, { scope: 'worker' }],
codegenMode: ['actions', { scope: 'worker', option: true }],

toImplInWorkerScope: [async ({ playwright }, use) => {
await use((playwright as any)._toImpl);
Expand Down
2 changes: 1 addition & 1 deletion tests/config/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export async function parseTraceRaw(file: string): Promise<{ events: any[], reso
export async function parseTrace(file: string): Promise<{ resources: Map<string, Buffer>, events: (EventTraceEvent | ConsoleMessageTraceEvent)[], actions: ActionTraceEvent[], apiNames: string[], traceModel: TraceModel, model: MultiTraceModel, actionTree: string[], errors: string[] }> {
const backend = new TraceBackend(file);
const traceModel = new TraceModel();
await traceModel.load(backend, () => {});
await traceModel.load(backend, false, () => {});
const model = new MultiTraceModel(traceModel.contextEntries);
const { rootItem } = buildActionTree(model.actions);
const actionTree: string[] = [];
Expand Down
9 changes: 6 additions & 3 deletions tests/library/inspector/cli-codegen-2.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,8 @@ await page.GetByRole(AriaRole.Button, new() { Name = "click me" }).ClickAsync();
}
});

test('should record open in a new tab with url', async ({ openRecorder, browserName }) => {
test('should record open in a new tab with url', async ({ openRecorder, browserName, codegenMode }) => {
test.skip(codegenMode === 'trace-events');
const { page, recorder } = await openRecorder();
await recorder.setContentAndWait(`<a href="about:blank?foo">link</a>`);

Expand Down Expand Up @@ -490,7 +491,8 @@ await page1.GotoAsync("about:blank?foo");`);
await recorder.waitForOutput('JavaScript', `await page.goto('${server.PREFIX}/page2.html');`);
});

test('should --save-trace', async ({ runCLI }, testInfo) => {
test('should --save-trace', async ({ runCLI, codegenMode }, testInfo) => {
test.skip(codegenMode === 'trace-events');
const traceFileName = testInfo.outputPath('trace.zip');
const cli = runCLI([`--save-trace=${traceFileName}`], {
autoExitWhen: ' ',
Expand All @@ -499,7 +501,8 @@ await page1.GotoAsync("about:blank?foo");`);
expect(fs.existsSync(traceFileName)).toBeTruthy();
});

test('should save assets via SIGINT', async ({ runCLI, platform }, testInfo) => {
test('should save assets via SIGINT', async ({ runCLI, platform, codegenMode }, testInfo) => {
test.skip(codegenMode === 'trace-events');
test.skip(platform === 'win32', 'SIGINT not supported on Windows');

const traceFileName = testInfo.outputPath('trace.zip');
Expand Down
Loading

0 comments on commit 418d1c0

Please sign in to comment.