diff --git a/packages/playwright-core/src/server/codegen/language.ts b/packages/playwright-core/src/server/codegen/language.ts index 3e0c8f71e5..4b1ba99b6f 100644 --- a/packages/playwright-core/src/server/codegen/language.ts +++ b/packages/playwright-core/src/server/codegen/language.ts @@ -20,6 +20,7 @@ import type * as types from '../types'; import type { ActionInContext, LanguageGenerator, LanguageGeneratorOptions } from './types'; export function generateCode(actions: ActionInContext[], languageGenerator: LanguageGenerator, options: LanguageGeneratorOptions) { + actions = collapseActions(actions); const header = languageGenerator.generateHeader(options); const footer = languageGenerator.generateFooter(options.saveStorage); const actionTexts = actions.map(a => languageGenerator.generateAction(a)).filter(Boolean); @@ -83,3 +84,19 @@ export function toClickOptionsForSourceCode(action: actions.ClickAction): types. options.position = action.position; return options; } + +function collapseActions(actions: ActionInContext[]): ActionInContext[] { + const result: ActionInContext[] = []; + for (const action of actions) { + const lastAction = result[result.length - 1]; + const isSameAction = lastAction && lastAction.action.name === action.action.name && lastAction.frame.pageAlias === action.frame.pageAlias && lastAction.frame.framePath.join('|') === action.frame.framePath.join('|'); + const isSameSelector = lastAction && 'selector' in lastAction.action && 'selector' in action.action && action.action.selector === lastAction.action.selector; + const shouldMerge = isSameAction && (action.action.name === 'navigate' || (action.action.name === 'fill' && isSameSelector)); + if (!shouldMerge) { + result.push(action); + continue; + } + result[result.length - 1] = action; + } + return result; +} diff --git a/packages/playwright-core/src/server/codegen/types.ts b/packages/playwright-core/src/server/codegen/types.ts index 96f2aa85d1..48c1141a3e 100644 --- a/packages/playwright-core/src/server/codegen/types.ts +++ b/packages/playwright-core/src/server/codegen/types.ts @@ -36,7 +36,7 @@ export type ActionInContext = { frame: FrameDescription; description?: string; action: actions.Action; - committed?: boolean; + timestamp: number; }; export interface LanguageGenerator { diff --git a/packages/playwright-core/src/server/recorder/contextRecorder.ts b/packages/playwright-core/src/server/recorder/contextRecorder.ts index 6642f13a62..71a1d3ec75 100644 --- a/packages/playwright-core/src/server/recorder/contextRecorder.ts +++ b/packages/playwright-core/src/server/recorder/contextRecorder.ts @@ -18,7 +18,7 @@ import type * as channels from '@protocol/channels'; import type { Source } from '@recorder/recorderTypes'; import { EventEmitter } from 'events'; import * as recorderSource from '../../generated/recorderSource'; -import { eventsHelper, isUnderTest, monotonicTime, quoteCSSAttributeValue, type RegisteredListener } from '../../utils'; +import { eventsHelper, monotonicTime, quoteCSSAttributeValue, type RegisteredListener } from '../../utils'; import { raceAgainstDeadline } from '../../utils/timeoutRunner'; import { BrowserContext } from '../browserContext'; import type { ActionInContext, FrameDescription, LanguageGeneratorOptions, Language, LanguageGenerator } from '../codegen/types'; @@ -27,7 +27,6 @@ import type { Dialog } from '../dialog'; import { Frame } from '../frames'; import { Page } from '../page'; import type * as actions from './recorderActions'; -import { performAction } from './recorderRunner'; import { ThrottledFile } from './throttledFile'; import { RecorderCollection } from './recorderCollection'; import { generateCode } from '../codegen/language'; @@ -48,7 +47,6 @@ export class ContextRecorder extends EventEmitter { private _lastPopupOrdinal = 0; private _lastDialogOrdinal = -1; private _lastDownloadOrdinal = -1; - private _timers = new Set(); private _context: BrowserContext; private _params: channels.BrowserContextRecorderSupplementEnableParams; private _delegate: ContextRecorderDelegate; @@ -150,9 +148,6 @@ export class ContextRecorder extends EventEmitter { } dispose() { - for (const timer of this._timers) - clearTimeout(timer); - this._timers.clear(); eventsHelper.removeEventListeners(this._listeners); } @@ -162,11 +157,11 @@ export class ContextRecorder extends EventEmitter { page.on('close', () => { this._collection.addRecordedAction({ frame: this._describeMainFrame(page), - committed: true, action: { name: 'closePage', signals: [], - } + }, + timestamp: monotonicTime() }); this._pageAliases.delete(page); }); @@ -184,12 +179,12 @@ export class ContextRecorder extends EventEmitter { } else { this._collection.addRecordedAction({ frame: this._describeMainFrame(page), - committed: true, action: { name: 'openPage', url: page.mainFrame().url(), signals: [], - } + }, + timestamp: monotonicTime() }); } } @@ -220,54 +215,24 @@ export class ContextRecorder extends EventEmitter { return this._params.testIdAttributeName || this._context.selectors().testIdAttributeName() || 'data-testid'; } - private async _performAction(frame: Frame, action: actions.PerformOnRecordAction) { - // Commit last action so that no further signals are added to it. - this._collection.commitLastAction(); - + private async _createActionInContext(frame: Frame, action: actions.Action): Promise { const frameDescription = await this._describeFrame(frame); const actionInContext: ActionInContext = { frame: frameDescription, action, description: undefined, + timestamp: monotonicTime() }; - await this._delegate.rewriteActionInContext?.(this._pageAliases, actionInContext); + return actionInContext; + } - const callMetadata = await this._collection.willPerformAction(actionInContext); - if (!callMetadata) - return; - const error = await performAction(callMetadata, this._pageAliases, actionInContext).then(() => undefined).catch((e: Error) => e); - await this._collection.didPerformAction(callMetadata, actionInContext, error); - if (error) - actionInContext.committed = true; - else - this._setCommittedAfterTimeout(actionInContext); + private async _performAction(frame: Frame, action: actions.PerformOnRecordAction) { + await this._collection.performAction(await this._createActionInContext(frame, action)); } private async _recordAction(frame: Frame, action: actions.Action) { - // Commit last action so that no further signals are added to it. - this._collection.commitLastAction(); - - const frameDescription = await this._describeFrame(frame); - const actionInContext: ActionInContext = { - frame: frameDescription, - action, - description: undefined, - }; - - await this._delegate.rewriteActionInContext?.(this._pageAliases, actionInContext); - - this._setCommittedAfterTimeout(actionInContext); - this._collection.addRecordedAction(actionInContext); - } - - private _setCommittedAfterTimeout(actionInContext: ActionInContext) { - const timer = setTimeout(() => { - // Commit the action after 5 seconds so that no further signals are added to it. - actionInContext.committed = true; - this._timers.delete(timer); - }, isUnderTest() ? 500 : 5000); - this._timers.add(timer); + this._collection.addRecordedAction(await this._createActionInContext(frame, action)); } private _onFrameNavigated(frame: Frame, page: Page) { diff --git a/packages/playwright-core/src/server/recorder/recorderCollection.ts b/packages/playwright-core/src/server/recorder/recorderCollection.ts index fbfbf8f26e..e9c2b31427 100644 --- a/packages/playwright-core/src/server/recorder/recorderCollection.ts +++ b/packages/playwright-core/src/server/recorder/recorderCollection.ts @@ -19,13 +19,14 @@ import type { Frame } from '../frames'; import type { Page } from '../page'; import type { Signal } from './recorderActions'; import type { ActionInContext } from '../codegen/types'; -import type { CallMetadata } from '@protocol/callMetadata'; -import { createGuid } from '../../utils/crypto'; import { monotonicTime } from '../../utils/time'; -import { mainFrameForAction, traceParamsForAction } from './recorderUtils'; +import { callMetadataForAction } from './recorderUtils'; +import { serializeError } from '../errors'; +import { performAction } from './recorderRunner'; +import type { CallMetadata } from '@protocol/callMetadata'; +import { isUnderTest } from '../../utils/debug'; export class RecorderCollection extends EventEmitter { - private _lastAction: ActionInContext | null = null; private _actions: ActionInContext[] = []; private _enabled: boolean; private _pageAliases: Map; @@ -38,7 +39,6 @@ export class RecorderCollection extends EventEmitter { } restart() { - this._lastAction = null; this._actions = []; this.emit('change'); } @@ -51,98 +51,73 @@ export class RecorderCollection extends EventEmitter { this._enabled = enabled; } - async willPerformAction(actionInContext: ActionInContext): Promise { - if (!this._enabled) - return null; - const { callMetadata, mainFrame } = this._callMetadataForAction(actionInContext); - await mainFrame.instrumentation.onBeforeCall(mainFrame, callMetadata); - this._lastAction = actionInContext; - return callMetadata; - } - - private _callMetadataForAction(actionInContext: ActionInContext): { callMetadata: CallMetadata, mainFrame: Frame } { - const mainFrame = mainFrameForAction(this._pageAliases, actionInContext); - const { action } = actionInContext; - const callMetadata: CallMetadata = { - id: `call@${createGuid()}`, - apiName: 'frame.' + action.name, - objectId: mainFrame.guid, - pageId: mainFrame._page.guid, - frameId: mainFrame.guid, - startTime: monotonicTime(), - endTime: 0, - type: 'Frame', - method: action.name, - params: traceParamsForAction(actionInContext), - log: [], - }; - return { callMetadata, mainFrame }; - } - - async didPerformAction(callMetadata: CallMetadata, actionInContext: ActionInContext, error?: Error) { - if (!this._enabled) - return; - - if (!error) - this._actions.push(actionInContext); - - const mainFrame = mainFrameForAction(this._pageAliases, actionInContext); - callMetadata.endTime = monotonicTime(); - await mainFrame.instrumentation.onAfterCall(mainFrame, callMetadata); - - this.emit('change'); + async performAction(actionInContext: ActionInContext) { + await this._addAction(actionInContext, async callMetadata => { + await performAction(callMetadata, this._pageAliases, actionInContext); + }); } addRecordedAction(actionInContext: ActionInContext) { - if (!this._enabled) - return; - const action = actionInContext.action; - - const lastAction = this._lastAction && this._lastAction.frame.pageAlias === actionInContext.frame.pageAlias ? this._lastAction.action : undefined; - if (lastAction && action.name === 'navigate' && lastAction.name === 'navigate' && action.url === lastAction.url) { - // Already at a target URL. + if (['openPage', 'closePage'].includes(actionInContext.action.name)) { + this._actions.push(actionInContext); + this.emit('change'); return; } - - if (lastAction && action.name === 'fill' && lastAction.name === 'fill' && action.selector === lastAction.selector) - this._actions.pop(); - - this._lastAction = actionInContext; - this._actions.push(actionInContext); - this.emit('change'); + this._addAction(actionInContext).catch(() => {}); } - commitLastAction() { + private async _addAction(actionInContext: ActionInContext, callback?: (callMetadata: CallMetadata) => Promise) { if (!this._enabled) return; - const action = this._lastAction; - if (action) - action.committed = true; + + const { callMetadata, mainFrame } = callMetadataForAction(this._pageAliases, actionInContext); + await mainFrame.instrumentation.onBeforeCall(mainFrame, callMetadata); + this._actions.push(actionInContext); + this.emit('change'); + const error = await callback?.(callMetadata).catch((e: Error) => e); + callMetadata.endTime = monotonicTime(); + callMetadata.error = error ? serializeError(error) : undefined; + await mainFrame.instrumentation.onAfterCall(mainFrame, callMetadata); } signal(pageAlias: string, frame: Frame, signal: Signal) { if (!this._enabled) return; - if (this._lastAction && !this._lastAction.committed) { - this._lastAction.action.signals.push(signal); - this.emit('change'); + if (signal.name === 'navigation' && frame._page.mainFrame() === frame) { + const timestamp = monotonicTime(); + const lastAction = this._actions[this._actions.length - 1]; + const signalThreshold = isUnderTest() ? 500 : 5000; + + let generateGoto = false; + if (!lastAction) + generateGoto = true; + else if (lastAction.action.name !== 'click' && lastAction.action.name !== 'press') + generateGoto = true; + else if (timestamp - lastAction.timestamp > signalThreshold) + generateGoto = true; + + if (generateGoto) { + this.addRecordedAction({ + frame: { + pageAlias, + framePath: [], + }, + action: { + name: 'navigate', + url: frame.url(), + signals: [], + }, + timestamp + }); + } return; } - if (signal.name === 'navigation' && frame._page.mainFrame() === frame) { - this.addRecordedAction({ - frame: { - pageAlias, - framePath: [], - }, - committed: true, - action: { - name: 'navigate', - url: frame.url(), - signals: [], - }, - }); + if (this._actions.length) { + this._actions[this._actions.length - 1].action.signals.push(signal); + this.emit('change'); + return; } } } diff --git a/packages/playwright-core/src/server/recorder/recorderUtils.ts b/packages/playwright-core/src/server/recorder/recorderUtils.ts index 234fc79a0f..ac6c970489 100644 --- a/packages/playwright-core/src/server/recorder/recorderUtils.ts +++ b/packages/playwright-core/src/server/recorder/recorderUtils.ts @@ -22,6 +22,7 @@ import type { Frame } from '../frames'; import type * as actions from './recorderActions'; import { toKeyboardModifiers } from '../codegen/language'; import { serializeExpectedTextValues } from '../../utils/expectUtils'; +import { createGuid, monotonicTime } from '../../utils'; export function metadataToCallLog(metadata: CallMetadata, status: CallLogStatus): CallLog { let title = metadata.apiName || metadata.method; @@ -59,7 +60,7 @@ export function mainFrameForAction(pageAliases: Map, actionInConte const pageAlias = actionInContext.frame.pageAlias; const page = [...pageAliases.entries()].find(([, alias]) => pageAlias === alias)?.[0]; if (!page) - throw new Error('Internal error: page not found'); + throw new Error(`Internal error: page ${pageAlias} not found in [${[...pageAliases.values()]}]`); return page.mainFrame(); } @@ -129,3 +130,22 @@ export function traceParamsForAction(actionInContext: ActionInContext) { } } } + +export function callMetadataForAction(pageAliases: Map, actionInContext: ActionInContext): { callMetadata: CallMetadata, mainFrame: Frame } { + const mainFrame = mainFrameForAction(pageAliases, actionInContext); + const { action } = actionInContext; + const callMetadata: CallMetadata = { + id: `call@${createGuid()}`, + apiName: 'frame.' + action.name, + objectId: mainFrame.guid, + pageId: mainFrame._page.guid, + frameId: mainFrame.guid, + startTime: monotonicTime(), + endTime: 0, + type: 'Frame', + method: action.name, + params: traceParamsForAction(actionInContext), + log: [], + }; + return { callMetadata, mainFrame }; +} diff --git a/tests/library/inspector/cli-codegen-1.spec.ts b/tests/library/inspector/cli-codegen-1.spec.ts index 4320aac5e8..c8046ef9b4 100644 --- a/tests/library/inspector/cli-codegen-1.spec.ts +++ b/tests/library/inspector/cli-codegen-1.spec.ts @@ -706,7 +706,7 @@ var page1 = await page.RunAndWaitForPopupAsync(async () => expect(popup.url()).toBe('about:blank'); }); - test('should assert navigation', async ({ page, openRecorder }) => { + test('should attribute navigation to click', async ({ page, openRecorder }) => { const recorder = await openRecorder(); await recorder.setContentAndWait(`link`); @@ -720,24 +720,42 @@ var page1 = await page.RunAndWaitForPopupAsync(async () => ]); expect.soft(sources.get('JavaScript')!.text).toContain(` - await page.getByText('link').click();`); + await page.goto('about:blank'); + await page.getByText('link').click(); + + // --------------------- + await context.close();`); expect.soft(sources.get('Playwright Test')!.text).toContain(` - await page.getByText('link').click();`); + await page.goto('about:blank'); + await page.getByText('link').click(); +});`); expect.soft(sources.get('Java')!.text).toContain(` - page.getByText("link").click();`); + page.navigate(\"about:blank\"); + page.getByText(\"link\").click(); + }`); expect.soft(sources.get('Python')!.text).toContain(` - page.get_by_text("link").click()`); + page.goto("about:blank") + page.get_by_text("link").click() + + # --------------------- + context.close()`); expect.soft(sources.get('Python Async')!.text).toContain(` - await page.get_by_text("link").click()`); + await page.goto("about:blank") + await page.get_by_text("link").click() + + # --------------------- + await context.close()`); expect.soft(sources.get('Pytest')!.text).toContain(` + page.goto("about:blank") page.get_by_text("link").click()`); expect.soft(sources.get('C#')!.text).toContain(` +await page.GotoAsync("about:blank"); await page.GetByText("link").ClickAsync();`); expect(page.url()).toContain('about:blank#foo');