From 1b21ec9cd84387abe1537d15862e3b9bf5f1f387 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 17 Jan 2025 10:17:49 -0800 Subject: [PATCH] chore: remove --save-trace codegen option (#34362) --- packages/playwright-core/src/cli/program.ts | 7 - .../playwright-core/src/server/recorder.ts | 3 +- .../src/server/recorder/recorderCollection.ts | 22 +-- .../src/server/recorder/recorderRunner.ts | 8 +- .../src/server/recorder/recorderUtils.ts | 26 +-- .../src/utils/isomorphic/recorderUtils.ts | 163 ------------------ tests/library/inspector/cli-codegen-2.spec.ts | 13 +- 7 files changed, 16 insertions(+), 226 deletions(-) delete mode 100644 packages/playwright-core/src/utils/isomorphic/recorderUtils.ts diff --git a/packages/playwright-core/src/cli/program.ts b/packages/playwright-core/src/cli/program.ts index a69414f2b2..ce568b8dd3 100644 --- a/packages/playwright-core/src/cli/program.ts +++ b/packages/playwright-core/src/cli/program.ts @@ -65,7 +65,6 @@ commandWithOpenOptions('codegen [url]', 'open page and generate code for user ac [ ['-o, --output ', 'saves the generated script to a file'], ['--target ', `language to generate, one of javascript, playwright-test, python, python-async, python-pytest, csharp, csharp-mstest, csharp-nunit, java, java-junit`, codegenId()], - ['--save-trace ', 'record a trace for the session and save it to a file'], ['--test-id-attribute ', 'use the specified attribute to generate data test ID selectors'], ]).action(function(url, options) { codegen(options, url).catch(logErrorAndExit); @@ -353,7 +352,6 @@ type Options = { saveHar?: string; saveHarGlob?: string; saveStorage?: string; - saveTrace?: string; timeout: string; timezone?: string; viewportSize?: string; @@ -508,8 +506,6 @@ async function launchContext(options: Options, extraOptions: LaunchOptions): Pro if (closingBrowser) return; closingBrowser = true; - if (options.saveTrace) - await context.tracing.stop({ path: options.saveTrace }); if (options.saveStorage) await context.storageState({ path: options.saveStorage }).catch(e => null); if (options.saveHar) @@ -536,9 +532,6 @@ async function launchContext(options: Options, extraOptions: LaunchOptions): Pro context.setDefaultTimeout(timeout); context.setDefaultNavigationTimeout(timeout); - if (options.saveTrace) - await context.tracing.start({ screenshots: true, snapshots: true }); - // Omit options that we add automatically for presentation purpose. delete launchOptions.headless; delete launchOptions.executablePath; diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index f763d5491b..59705c2eb4 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -27,9 +27,8 @@ import { Debugger } from './debugger'; import type { CallMetadata, InstrumentationListener, SdkObject } from './instrumentation'; import { ContextRecorder, generateFrameSelector } from './recorder/contextRecorder'; import type { IRecorderAppFactory, IRecorderApp, IRecorder } from './recorder/recorderFrontend'; -import { metadataToCallLog } from './recorder/recorderUtils'; +import { buildFullSelector, metadataToCallLog } from './recorder/recorderUtils'; import type * as actions from '@recorder/actions'; -import { buildFullSelector } from '../utils/isomorphic/recorderUtils'; import { stringifySelector } from '../utils/isomorphic/selectorParser'; import type { Frame } from './frames'; import type { AriaTemplateNode } from '@isomorphic/ariaSnapshot'; diff --git a/packages/playwright-core/src/server/recorder/recorderCollection.ts b/packages/playwright-core/src/server/recorder/recorderCollection.ts index 2643dece91..162d5d6813 100644 --- a/packages/playwright-core/src/server/recorder/recorderCollection.ts +++ b/packages/playwright-core/src/server/recorder/recorderCollection.ts @@ -20,10 +20,8 @@ import type { Page } from '../page'; import type { Signal } from '../../../../recorder/src/actions'; import type * as actions from '@recorder/actions'; import { monotonicTime } from '../../utils/time'; -import { callMetadataForAction, collapseActions } from './recorderUtils'; -import { serializeError } from '../errors'; +import { collapseActions } from './recorderUtils'; import { performAction } from './recorderRunner'; -import type { CallMetadata } from '@protocol/callMetadata'; import { isUnderTest } from '../../utils/debug'; export class RecorderCollection extends EventEmitter { @@ -46,8 +44,8 @@ export class RecorderCollection extends EventEmitter { } async performAction(actionInContext: actions.ActionInContext) { - await this._addAction(actionInContext, async callMetadata => { - await performAction(callMetadata, this._pageAliases, actionInContext); + await this._addAction(actionInContext, async () => { + await performAction(this._pageAliases, actionInContext); }); } @@ -60,7 +58,7 @@ export class RecorderCollection extends EventEmitter { this._addAction(actionInContext).catch(() => {}); } - private async _addAction(actionInContext: actions.ActionInContext, callback?: (callMetadata: CallMetadata) => Promise) { + private async _addAction(actionInContext: actions.ActionInContext, callback?: () => Promise) { if (!this._enabled) return; if (actionInContext.action.name === 'openPage' || actionInContext.action.name === 'closePage') { @@ -69,18 +67,10 @@ export class RecorderCollection extends EventEmitter { return; } - const { callMetadata, mainFrame } = callMetadataForAction(this._pageAliases, actionInContext); - await mainFrame.instrumentation.onBeforeCall(mainFrame, callMetadata); this._actions.push(actionInContext); this._fireChange(); - const error = await callback?.(callMetadata).catch((e: Error) => e); - callMetadata.endTime = monotonicTime(); - actionInContext.endTime = callMetadata.endTime; - callMetadata.error = error ? serializeError(error) : undefined; - // Do not wait for onAfterCall so that performAction returned immediately after the action. - mainFrame.instrumentation.onAfterCall(mainFrame, callMetadata).then(() => { - this._fireChange(); - }).catch(() => {}); + await callback?.().catch(); + actionInContext.endTime = monotonicTime(); } signal(pageAlias: string, frame: Frame, signal: Signal) { diff --git a/packages/playwright-core/src/server/recorder/recorderRunner.ts b/packages/playwright-core/src/server/recorder/recorderRunner.ts index 1b33895f98..bb225ec5ab 100644 --- a/packages/playwright-core/src/server/recorder/recorderRunner.ts +++ b/packages/playwright-core/src/server/recorder/recorderRunner.ts @@ -16,14 +16,14 @@ import { serializeExpectedTextValues } from '../../utils'; import { toKeyboardModifiers } from '../codegen/language'; -import type { CallMetadata } from '../instrumentation'; +import { serverSideCallMetadata } from '../instrumentation'; import type { Page } from '../page'; import type * as actions from '@recorder/actions'; import type * as types from '../types'; -import { mainFrameForAction } from './recorderUtils'; -import { buildFullSelector } from '../../utils/isomorphic/recorderUtils'; +import { buildFullSelector, mainFrameForAction } from './recorderUtils'; -export async function performAction(callMetadata: CallMetadata, pageAliases: Map, actionInContext: actions.ActionInContext) { +export async function performAction(pageAliases: Map, actionInContext: actions.ActionInContext) { + const callMetadata = serverSideCallMetadata(); const mainFrame = mainFrameForAction(pageAliases, actionInContext); const { action } = actionInContext; diff --git a/packages/playwright-core/src/server/recorder/recorderUtils.ts b/packages/playwright-core/src/server/recorder/recorderUtils.ts index 990ba959d6..77ef329d27 100644 --- a/packages/playwright-core/src/server/recorder/recorderUtils.ts +++ b/packages/playwright-core/src/server/recorder/recorderUtils.ts @@ -19,8 +19,10 @@ import type { CallLog, CallLogStatus } from '@recorder/recorderTypes'; import type { Page } from '../page'; import type { Frame } from '../frames'; import type * as actions from '@recorder/actions'; -import { createGuid } from '../../utils'; -import { buildFullSelector, traceParamsForAction } from '../../utils/isomorphic/recorderUtils'; + +export function buildFullSelector(framePath: string[], selector: string) { + return [...framePath, selector].join(' >> internal:control=enter-frame >> '); +} export function metadataToCallLog(metadata: CallMetadata, status: CallLogStatus): CallLog { let title = metadata.apiName || metadata.method; @@ -70,26 +72,6 @@ export async function frameForAction(pageAliases: Map, actionInCon return result.frame; } -export function callMetadataForAction(pageAliases: Map, actionInContext: actions.ActionInContext): { callMetadata: CallMetadata, mainFrame: Frame } { - const mainFrame = mainFrameForAction(pageAliases, actionInContext); - const { method, apiName, params } = traceParamsForAction(actionInContext); - - const callMetadata: CallMetadata = { - id: `call@${createGuid()}`, - apiName, - objectId: mainFrame.guid, - pageId: mainFrame._page.guid, - frameId: mainFrame.guid, - startTime: actionInContext.startTime, - endTime: 0, - type: 'Frame', - method, - params, - log: [], - }; - return { callMetadata, mainFrame }; -} - export function collapseActions(actions: actions.ActionInContext[]): actions.ActionInContext[] { const result: actions.ActionInContext[] = []; for (const action of actions) { diff --git a/packages/playwright-core/src/utils/isomorphic/recorderUtils.ts b/packages/playwright-core/src/utils/isomorphic/recorderUtils.ts deleted file mode 100644 index 7ef45e5ece..0000000000 --- a/packages/playwright-core/src/utils/isomorphic/recorderUtils.ts +++ /dev/null @@ -1,163 +0,0 @@ -/** - * Copyright (c) Microsoft Corporation. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import type * as recorderActions from '@recorder/actions'; -import type * as channels from '@protocol/channels'; -import type * as types from '../../server/types'; - -export function buildFullSelector(framePath: string[], selector: string) { - return [...framePath, selector].join(' >> internal:control=enter-frame >> '); -} - -const kDefaultTimeout = 5_000; - -export function traceParamsForAction(actionInContext: recorderActions.ActionInContext): { method: string, apiName: string, params: any } { - const { action } = actionInContext; - - switch (action.name) { - case 'navigate': { - const params: channels.FrameGotoParams = { - url: action.url, - }; - return { method: 'goto', apiName: 'page.goto', params }; - } - case 'openPage': - case 'closePage': - throw new Error('Not reached'); - } - - const selector = buildFullSelector(actionInContext.frame.framePath, action.selector); - switch (action.name) { - case 'click': { - const params: channels.FrameClickParams = { - selector, - strict: true, - modifiers: toKeyboardModifiers(action.modifiers), - button: action.button, - clickCount: action.clickCount, - position: action.position, - }; - return { method: 'click', apiName: 'locator.click', params }; - } - case 'press': { - const params: channels.FramePressParams = { - selector, - strict: true, - key: [...toKeyboardModifiers(action.modifiers), action.key].join('+'), - }; - return { method: 'press', apiName: 'locator.press', params }; - } - case 'fill': { - const params: channels.FrameFillParams = { - selector, - strict: true, - value: action.text, - }; - return { method: 'fill', apiName: 'locator.fill', params }; - } - case 'setInputFiles': { - const params: channels.FrameSetInputFilesParams = { - selector, - strict: true, - localPaths: action.files, - }; - return { method: 'setInputFiles', apiName: 'locator.setInputFiles', params }; - } - case 'check': { - const params: channels.FrameCheckParams = { - selector, - strict: true, - }; - return { method: 'check', apiName: 'locator.check', params }; - } - case 'uncheck': { - const params: channels.FrameUncheckParams = { - selector, - strict: true, - }; - return { method: 'uncheck', apiName: 'locator.uncheck', params }; - } - case 'select': { - const params: channels.FrameSelectOptionParams = { - selector, - strict: true, - options: action.options.map(option => ({ value: option })), - }; - return { method: 'selectOption', apiName: 'locator.selectOption', params }; - } - case 'assertChecked': { - const params: channels.FrameExpectParams = { - selector: action.selector, - expression: 'to.be.checked', - isNot: !action.checked, - timeout: kDefaultTimeout, - }; - return { method: 'expect', apiName: 'expect.toBeChecked', params }; - } - case 'assertText': { - const params: channels.FrameExpectParams = { - selector, - expression: 'to.have.text', - expectedText: [], - isNot: false, - timeout: kDefaultTimeout, - }; - return { method: 'expect', apiName: 'expect.toContainText', params }; - } - case 'assertValue': { - const params: channels.FrameExpectParams = { - selector, - expression: 'to.have.value', - expectedValue: undefined, - isNot: false, - timeout: kDefaultTimeout, - }; - return { method: 'expect', apiName: 'expect.toHaveValue', params }; - } - case 'assertVisible': { - const params: channels.FrameExpectParams = { - selector, - expression: 'to.be.visible', - isNot: false, - timeout: kDefaultTimeout, - }; - return { method: 'expect', apiName: 'expect.toBeVisible', params }; - } - case 'assertSnapshot': { - const params: channels.FrameExpectParams = { - selector, - expression: 'to.match.snapshot', - expectedText: [], - isNot: false, - timeout: kDefaultTimeout, - }; - return { method: 'expect', apiName: 'expect.toMatchAriaSnapshot', params }; - } - } -} - -export function toKeyboardModifiers(modifiers: number): types.SmartKeyboardModifier[] { - const result: types.SmartKeyboardModifier[] = []; - if (modifiers & 1) - result.push('Alt'); - if (modifiers & 2) - result.push('ControlOrMeta'); - if (modifiers & 4) - result.push('ControlOrMeta'); - if (modifiers & 8) - result.push('Shift'); - return result; -} diff --git a/tests/library/inspector/cli-codegen-2.spec.ts b/tests/library/inspector/cli-codegen-2.spec.ts index 920ecbdf76..205bbbae5e 100644 --- a/tests/library/inspector/cli-codegen-2.spec.ts +++ b/tests/library/inspector/cli-codegen-2.spec.ts @@ -451,27 +451,16 @@ await page1.GotoAsync("about:blank?foo");`); await recorder.waitForOutput('JavaScript', `await page.goto('${server.PREFIX}/page2.html');`); }); - test('should --save-trace', async ({ runCLI }, testInfo) => { - const traceFileName = testInfo.outputPath('trace.zip'); - const cli = runCLI([`--save-trace=${traceFileName}`], { - autoExitWhen: ' ', - }); - await cli.waitForCleanExit(); - expect(fs.existsSync(traceFileName)).toBeTruthy(); - }); - test('should save assets via SIGINT', async ({ runCLI, platform }, testInfo) => { test.skip(platform === 'win32', 'SIGINT not supported on Windows'); - const traceFileName = testInfo.outputPath('trace.zip'); const storageFileName = testInfo.outputPath('auth.json'); const harFileName = testInfo.outputPath('har.har'); - const cli = runCLI([`--save-trace=${traceFileName}`, `--save-storage=${storageFileName}`, `--save-har=${harFileName}`]); + const cli = runCLI([`--save-storage=${storageFileName}`, `--save-har=${harFileName}`]); await cli.waitFor(`import { test, expect } from '@playwright/test'`); await cli.process.kill('SIGINT'); const { exitCode } = await cli.process.exited; expect(exitCode).toBe(130); - expect(fs.existsSync(traceFileName)).toBeTruthy(); expect(fs.existsSync(storageFileName)).toBeTruthy(); expect(fs.existsSync(harFileName)).toBeTruthy(); });