From 37eb66df10d962390cd4d5a74844c244702b591a Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 23 Aug 2024 10:19:44 -0700 Subject: [PATCH] chore: extract performAction in recorder (#32279) --- .../src/server/injected/recorder/recorder.ts | 8 +- .../playwright-core/src/server/recorder.ts | 156 ++++++++++++------ .../src/server/recorder/recorderActions.ts | 1 + .../playwright-core/src/utils/expectUtils.ts | 29 ++++ packages/playwright-core/src/utils/index.ts | 1 + packages/playwright/src/matchers/matchers.ts | 36 ++-- .../playwright/src/matchers/toMatchText.ts | 13 -- 7 files changed, 157 insertions(+), 87 deletions(-) create mode 100644 packages/playwright-core/src/utils/expectUtils.ts diff --git a/packages/playwright-core/src/server/injected/recorder/recorder.ts b/packages/playwright-core/src/server/injected/recorder/recorder.ts index 3432f159dd..6e573c3c5a 100644 --- a/packages/playwright-core/src/server/injected/recorder/recorder.ts +++ b/packages/playwright-core/src/server/injected/recorder/recorder.ts @@ -23,7 +23,7 @@ import type { Highlight, HighlightOptions } from '../highlight'; import clipPaths from './clipPaths'; interface RecorderDelegate { - performAction?(action: actions.Action): Promise; + performAction?(action: actions.PerformOnRecordAction): Promise; recordAction?(action: actions.Action): Promise; setSelector?(selector: string): Promise; setMode?(mode: Mode): Promise; @@ -483,7 +483,7 @@ class RecordActionTool implements RecorderTool { return true; } - private async _performAction(action: actions.Action) { + private async _performAction(action: actions.PerformOnRecordAction) { this._hoveredElement = null; this._hoveredModel = null; this._activeModel = null; @@ -1361,7 +1361,7 @@ function createSvgElement(doc: Document, { tagName, attrs, children }: SvgJson): } interface Embedder { - __pw_recorderPerformAction(action: actions.Action): Promise; + __pw_recorderPerformAction(action: actions.PerformOnRecordAction): Promise; __pw_recorderRecordAction(action: actions.Action): Promise; __pw_recorderState(): Promise; __pw_recorderSetSelector(selector: string): Promise; @@ -1407,7 +1407,7 @@ export class PollingRecorder implements RecorderDelegate { this._pollRecorderModeTimer = this._recorder.injectedScript.builtinSetTimeout(() => this._pollRecorderMode(), pollPeriod); } - async performAction(action: actions.Action) { + async performAction(action: actions.PerformOnRecordAction) { await this._embedder.__pw_recorderPerformAction(action); } diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index 93f706581f..9e2174303b 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -36,7 +36,7 @@ import { RecorderApp } from './recorder/recorderApp'; import type { CallMetadata, InstrumentationListener, SdkObject } from './instrumentation'; import type { Point } from '../common/types'; import type { CallLog, CallLogStatus, EventData, Mode, OverlayState, Source, UIState } from '@recorder/recorderTypes'; -import { createGuid, isUnderTest, monotonicTime } from '../utils'; +import { createGuid, isUnderTest, monotonicTime, serializeExpectedTextValues } from '../utils'; import { metadataToCallLog } from './recorder/recorderUtils'; import { Debugger } from './debugger'; import { EventEmitter } from 'events'; @@ -470,7 +470,7 @@ class ContextRecorder extends EventEmitter { // Input actions that potentially lead to navigation are intercepted on the page and are // performed by the Playwright. await this._context.exposeBinding('__pw_recorderPerformAction', false, - (source: BindingSource, action: actions.Action) => this._performAction(source.frame, action)); + (source: BindingSource, action: actions.PerformOnRecordAction) => this._performAction(source.frame, action)); // Other non-essential actions are simply being recorded. await this._context.exposeBinding('__pw_recorderRecordAction', false, @@ -585,7 +585,7 @@ class ContextRecorder extends EventEmitter { return this._params.testIdAttributeName || this._context.selectors().testIdAttributeName() || 'data-testid'; } - private async _performAction(frame: Frame, action: actions.Action) { + private async _performAction(frame: Frame, action: actions.PerformOnRecordAction) { // Commit last action so that no further signals are added to it. this._generator.commitLastAction(); @@ -595,56 +595,13 @@ class ContextRecorder extends EventEmitter { action }; - const perform = async (action: string, params: any, cb: (callMetadata: CallMetadata) => Promise) => { - const callMetadata: CallMetadata = { - id: `call@${createGuid()}`, - apiName: 'frame.' + action, - objectId: frame.guid, - pageId: frame._page.guid, - frameId: frame.guid, - startTime: monotonicTime(), - endTime: 0, - type: 'Frame', - method: action, - params, - log: [], - }; - this._generator.willPerformAction(actionInContext); - - try { - await frame.instrumentation.onBeforeCall(frame, callMetadata); - await cb(callMetadata); - } catch (e) { - callMetadata.endTime = monotonicTime(); - await frame.instrumentation.onAfterCall(frame, callMetadata); - this._generator.performedActionFailed(actionInContext); - return; - } - - callMetadata.endTime = monotonicTime(); - await frame.instrumentation.onAfterCall(frame, callMetadata); - - this._setCommittedAfterTimeout(actionInContext); + this._generator.willPerformAction(actionInContext); + const success = await performAction(frame, action); + if (success) { this._generator.didPerformAction(actionInContext); - }; - - const kActionTimeout = 5000; - if (action.name === 'click') { - const { options } = toClickOptions(action); - await perform('click', { selector: action.selector }, callMetadata => frame.click(callMetadata, action.selector, { ...options, timeout: kActionTimeout, strict: true })); - } - if (action.name === 'press') { - const modifiers = toModifiers(action.modifiers); - const shortcut = [...modifiers, action.key].join('+'); - await perform('press', { selector: action.selector, key: shortcut }, callMetadata => frame.press(callMetadata, action.selector, shortcut, { timeout: kActionTimeout, strict: true })); - } - if (action.name === 'check') - await perform('check', { selector: action.selector }, callMetadata => frame.check(callMetadata, action.selector, { timeout: kActionTimeout, strict: true })); - if (action.name === 'uncheck') - await perform('uncheck', { selector: action.selector }, callMetadata => frame.uncheck(callMetadata, action.selector, { timeout: kActionTimeout, strict: true })); - if (action.name === 'select') { - const values = action.options.map(value => ({ value })); - await perform('selectOption', { selector: action.selector, values }, callMetadata => frame.selectOption(callMetadata, action.selector, [], values, { timeout: kActionTimeout, strict: true })); + this._setCommittedAfterTimeout(actionInContext); + } else { + this._generator.performedActionFailed(actionInContext); } } @@ -749,3 +706,98 @@ async function findFrameSelector(frame: Frame): Promise { } catch (e) { } } + +async function innerPerformAction(frame: Frame, action: string, params: any, cb: (callMetadata: CallMetadata) => Promise): Promise { + const callMetadata: CallMetadata = { + id: `call@${createGuid()}`, + apiName: 'frame.' + action, + objectId: frame.guid, + pageId: frame._page.guid, + frameId: frame.guid, + startTime: monotonicTime(), + endTime: 0, + type: 'Frame', + method: action, + params, + log: [], + }; + + try { + await frame.instrumentation.onBeforeCall(frame, callMetadata); + await cb(callMetadata); + } catch (e) { + callMetadata.endTime = monotonicTime(); + await frame.instrumentation.onAfterCall(frame, callMetadata); + return false; + } + + callMetadata.endTime = monotonicTime(); + await frame.instrumentation.onAfterCall(frame, callMetadata); + return true; +} + +async function performAction(frame: Frame, action: actions.Action): Promise { + const kActionTimeout = 5000; + if (action.name === 'click') { + const { options } = toClickOptions(action); + return await innerPerformAction(frame, 'click', { selector: action.selector }, callMetadata => frame.click(callMetadata, action.selector, { ...options, timeout: kActionTimeout, strict: true })); + } + if (action.name === 'press') { + const modifiers = toModifiers(action.modifiers); + const shortcut = [...modifiers, action.key].join('+'); + return await innerPerformAction(frame, 'press', { selector: action.selector, key: shortcut }, callMetadata => frame.press(callMetadata, action.selector, shortcut, { timeout: kActionTimeout, strict: true })); + } + if (action.name === 'fill') + return await innerPerformAction(frame, 'fill', { selector: action.selector, text: action.text }, callMetadata => frame.fill(callMetadata, action.selector, action.text, { timeout: kActionTimeout, strict: true })); + if (action.name === 'setInputFiles') + return await innerPerformAction(frame, 'setInputFiles', { selector: action.selector, files: action.files }, callMetadata => frame.setInputFiles(callMetadata, action.selector, { selector: action.selector, payloads: [], timeout: kActionTimeout, strict: true })); + if (action.name === 'check') + return await innerPerformAction(frame, 'check', { selector: action.selector }, callMetadata => frame.check(callMetadata, action.selector, { timeout: kActionTimeout, strict: true })); + if (action.name === 'uncheck') + return await innerPerformAction(frame, 'uncheck', { selector: action.selector }, callMetadata => frame.uncheck(callMetadata, action.selector, { timeout: kActionTimeout, strict: true })); + if (action.name === 'select') { + const values = action.options.map(value => ({ value })); + return await innerPerformAction(frame, 'selectOption', { selector: action.selector, values }, callMetadata => frame.selectOption(callMetadata, action.selector, [], values, { timeout: kActionTimeout, strict: true })); + } + if (action.name === 'navigate') + return await innerPerformAction(frame, 'goto', { url: action.url }, callMetadata => frame.goto(callMetadata, action.url, { timeout: kActionTimeout })); + if (action.name === 'closePage') + return await innerPerformAction(frame, 'close', {}, callMetadata => frame._page.close(callMetadata)); + if (action.name === 'openPage') + throw Error('Not reached'); + if (action.name === 'assertChecked') { + return await innerPerformAction(frame, 'expect', { selector: action.selector }, callMetadata => frame.expect(callMetadata, action.selector, { + selector: action.selector, + expression: 'to.be.checked', + isNot: !action.checked, + timeout: kActionTimeout, + })); + } + if (action.name === 'assertText') { + return await innerPerformAction(frame, 'expect', { selector: action.selector }, callMetadata => frame.expect(callMetadata, action.selector, { + selector: action.selector, + expression: 'to.have.text', + expectedText: serializeExpectedTextValues([action.text], { matchSubstring: true, normalizeWhiteSpace: true }), + isNot: false, + timeout: kActionTimeout, + })); + } + if (action.name === 'assertValue') { + return await innerPerformAction(frame, 'expect', { selector: action.selector }, callMetadata => frame.expect(callMetadata, action.selector, { + selector: action.selector, + expression: 'to.have.value', + expectedValue: action.value, + isNot: false, + timeout: kActionTimeout, + })); + } + if (action.name === 'assertVisible') { + return await innerPerformAction(frame, 'expect', { selector: action.selector }, callMetadata => frame.expect(callMetadata, action.selector, { + selector: action.selector, + expression: 'to.be.visible', + isNot: false, + timeout: kActionTimeout, + })); + } + throw new Error('Internal error: unexpected action ' + (action as any).name); +} diff --git a/packages/playwright-core/src/server/recorder/recorderActions.ts b/packages/playwright-core/src/server/recorder/recorderActions.ts index 3c9720cbc4..295758aaeb 100644 --- a/packages/playwright-core/src/server/recorder/recorderActions.ts +++ b/packages/playwright-core/src/server/recorder/recorderActions.ts @@ -121,6 +121,7 @@ export type AssertVisibleAction = ActionBase & { export type Action = ClickAction | CheckAction | ClosesPageAction | OpenPageAction | UncheckAction | FillAction | NavigateAction | PressAction | SelectAction | SetInputFilesAction | AssertTextAction | AssertValueAction | AssertCheckedAction | AssertVisibleAction; export type AssertAction = AssertCheckedAction | AssertValueAction | AssertTextAction | AssertVisibleAction; +export type PerformOnRecordAction = ClickAction | CheckAction | UncheckAction | PressAction | SelectAction; // Signals. diff --git a/packages/playwright-core/src/utils/expectUtils.ts b/packages/playwright-core/src/utils/expectUtils.ts new file mode 100644 index 0000000000..0ae21e8602 --- /dev/null +++ b/packages/playwright-core/src/utils/expectUtils.ts @@ -0,0 +1,29 @@ +/** + * 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 { ExpectedTextValue } from '@protocol/channels'; +import { isRegExp, isString } from './rtti'; + +export function serializeExpectedTextValues(items: (string | RegExp)[], options: { matchSubstring?: boolean, normalizeWhiteSpace?: boolean, ignoreCase?: boolean } = {}): ExpectedTextValue[] { + return items.map(i => ({ + string: isString(i) ? i : undefined, + regexSource: isRegExp(i) ? i.source : undefined, + regexFlags: isRegExp(i) ? i.flags : undefined, + matchSubstring: options.matchSubstring, + ignoreCase: options.ignoreCase, + normalizeWhiteSpace: options.normalizeWhiteSpace, + })); +} diff --git a/packages/playwright-core/src/utils/index.ts b/packages/playwright-core/src/utils/index.ts index 372922ec59..0bc7a75b08 100644 --- a/packages/playwright-core/src/utils/index.ts +++ b/packages/playwright-core/src/utils/index.ts @@ -21,6 +21,7 @@ export * from './debug'; export * from './debugLogger'; export * from './env'; export * from './eventsHelper'; +export * from './expectUtils'; export * from './fileUtils'; export * from './headers'; export * from './hostPlatform'; diff --git a/packages/playwright/src/matchers/matchers.ts b/packages/playwright/src/matchers/matchers.ts index 08ae8b6385..3ca9180ae2 100644 --- a/packages/playwright/src/matchers/matchers.ts +++ b/packages/playwright/src/matchers/matchers.ts @@ -20,8 +20,8 @@ import { colors } from 'playwright-core/lib/utilsBundle'; import { expectTypes, callLogText } from '../util'; import { toBeTruthy } from './toBeTruthy'; import { toEqual } from './toEqual'; -import { toExpectedTextValues, toMatchText } from './toMatchText'; -import { constructURLBasedOnBaseURL, isRegExp, isString, isTextualMimeType, pollAgainstDeadline } from 'playwright-core/lib/utils'; +import { toMatchText } from './toMatchText'; +import { constructURLBasedOnBaseURL, isRegExp, isString, isTextualMimeType, pollAgainstDeadline, serializeExpectedTextValues } from 'playwright-core/lib/utils'; import { currentTestInfo } from '../common/globals'; import { TestInfoImpl } from '../worker/testInfo'; import type { ExpectMatcherState } from '../../types/test'; @@ -163,12 +163,12 @@ export function toContainText( ) { if (Array.isArray(expected)) { return toEqual.call(this, 'toContainText', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues(expected, { matchSubstring: true, normalizeWhiteSpace: true, ignoreCase: options.ignoreCase }); + const expectedText = serializeExpectedTextValues(expected, { matchSubstring: true, normalizeWhiteSpace: true, ignoreCase: options.ignoreCase }); return await locator._expect('to.contain.text.array', { expectedText, isNot, useInnerText: options.useInnerText, timeout }); }, expected, { ...options, contains: true }); } else { return toMatchText.call(this, 'toContainText', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected], { matchSubstring: true, normalizeWhiteSpace: true, ignoreCase: options.ignoreCase }); + const expectedText = serializeExpectedTextValues([expected], { matchSubstring: true, normalizeWhiteSpace: true, ignoreCase: options.ignoreCase }); return await locator._expect('to.have.text', { expectedText, isNot, useInnerText: options.useInnerText, timeout }); }, expected, options); } @@ -181,7 +181,7 @@ export function toHaveAccessibleDescription( options?: { timeout?: number, ignoreCase?: boolean }, ) { return toMatchText.call(this, 'toHaveAccessibleDescription', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected], { ignoreCase: options?.ignoreCase }); + const expectedText = serializeExpectedTextValues([expected], { ignoreCase: options?.ignoreCase }); return await locator._expect('to.have.accessible.description', { expectedText, isNot, timeout }); }, expected, options); } @@ -193,7 +193,7 @@ export function toHaveAccessibleName( options?: { timeout?: number, ignoreCase?: boolean }, ) { return toMatchText.call(this, 'toHaveAccessibleName', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected], { ignoreCase: options?.ignoreCase }); + const expectedText = serializeExpectedTextValues([expected], { ignoreCase: options?.ignoreCase }); return await locator._expect('to.have.accessible.name', { expectedText, isNot, timeout }); }, expected, options); } @@ -218,7 +218,7 @@ export function toHaveAttribute( }, options); } return toMatchText.call(this, 'toHaveAttribute', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected as (string | RegExp)], { ignoreCase: options?.ignoreCase }); + const expectedText = serializeExpectedTextValues([expected as (string | RegExp)], { ignoreCase: options?.ignoreCase }); return await locator._expect('to.have.attribute.value', { expressionArg: name, expectedText, isNot, timeout }); }, expected as (string | RegExp), options); } @@ -231,12 +231,12 @@ export function toHaveClass( ) { if (Array.isArray(expected)) { return toEqual.call(this, 'toHaveClass', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues(expected); + const expectedText = serializeExpectedTextValues(expected); return await locator._expect('to.have.class.array', { expectedText, isNot, timeout }); }, expected, options); } else { return toMatchText.call(this, 'toHaveClass', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected]); + const expectedText = serializeExpectedTextValues([expected]); return await locator._expect('to.have.class', { expectedText, isNot, timeout }); }, expected, options); } @@ -261,7 +261,7 @@ export function toHaveCSS( options?: { timeout?: number }, ) { return toMatchText.call(this, 'toHaveCSS', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected]); + const expectedText = serializeExpectedTextValues([expected]); return await locator._expect('to.have.css', { expressionArg: name, expectedText, isNot, timeout }); }, expected, options); } @@ -273,7 +273,7 @@ export function toHaveId( options?: { timeout?: number }, ) { return toMatchText.call(this, 'toHaveId', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected]); + const expectedText = serializeExpectedTextValues([expected]); return await locator._expect('to.have.id', { expectedText, isNot, timeout }); }, expected, options); } @@ -299,7 +299,7 @@ export function toHaveRole( if (!isString(expected)) throw new Error(`"role" argument in toHaveRole must be a string`); return toMatchText.call(this, 'toHaveRole', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected]); + const expectedText = serializeExpectedTextValues([expected]); return await locator._expect('to.have.role', { expectedText, isNot, timeout }); }, expected, options); } @@ -312,12 +312,12 @@ export function toHaveText( ) { if (Array.isArray(expected)) { return toEqual.call(this, 'toHaveText', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues(expected, { normalizeWhiteSpace: true, ignoreCase: options.ignoreCase }); + const expectedText = serializeExpectedTextValues(expected, { normalizeWhiteSpace: true, ignoreCase: options.ignoreCase }); return await locator._expect('to.have.text.array', { expectedText, isNot, useInnerText: options?.useInnerText, timeout }); }, expected, options); } else { return toMatchText.call(this, 'toHaveText', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected], { normalizeWhiteSpace: true, ignoreCase: options.ignoreCase }); + const expectedText = serializeExpectedTextValues([expected], { normalizeWhiteSpace: true, ignoreCase: options.ignoreCase }); return await locator._expect('to.have.text', { expectedText, isNot, useInnerText: options?.useInnerText, timeout }); }, expected, options); } @@ -330,7 +330,7 @@ export function toHaveValue( options?: { timeout?: number }, ) { return toMatchText.call(this, 'toHaveValue', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected]); + const expectedText = serializeExpectedTextValues([expected]); return await locator._expect('to.have.value', { expectedText, isNot, timeout }); }, expected, options); } @@ -342,7 +342,7 @@ export function toHaveValues( options?: { timeout?: number }, ) { return toEqual.call(this, 'toHaveValues', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues(expected); + const expectedText = serializeExpectedTextValues(expected); return await locator._expect('to.have.values', { expectedText, isNot, timeout }); }, expected, options); } @@ -355,7 +355,7 @@ export function toHaveTitle( ) { const locator = page.locator(':root') as LocatorEx; return toMatchText.call(this, 'toHaveTitle', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected], { normalizeWhiteSpace: true }); + const expectedText = serializeExpectedTextValues([expected], { normalizeWhiteSpace: true }); return await locator._expect('to.have.title', { expectedText, isNot, timeout }); }, expected, options); } @@ -370,7 +370,7 @@ export function toHaveURL( expected = typeof expected === 'string' ? constructURLBasedOnBaseURL(baseURL, expected) : expected; const locator = page.locator(':root') as LocatorEx; return toMatchText.call(this, 'toHaveURL', locator, 'Locator', async (isNot, timeout) => { - const expectedText = toExpectedTextValues([expected], { ignoreCase: options?.ignoreCase }); + const expectedText = serializeExpectedTextValues([expected], { ignoreCase: options?.ignoreCase }); return await locator._expect('to.have.url', { expectedText, isNot, timeout }); }, expected, options); } diff --git a/packages/playwright/src/matchers/toMatchText.ts b/packages/playwright/src/matchers/toMatchText.ts index 790b402d2f..ebac8f8028 100644 --- a/packages/playwright/src/matchers/toMatchText.ts +++ b/packages/playwright/src/matchers/toMatchText.ts @@ -15,8 +15,6 @@ */ -import type { ExpectedTextValue } from '@protocol/channels'; -import { isRegExp, isString } from 'playwright-core/lib/utils'; import { expectTypes, callLogText } from '../util'; import { printReceivedStringContainExpectedResult, @@ -95,14 +93,3 @@ export async function toMatchText( timeout: timedOut ? timeout : undefined, }; } - -export function toExpectedTextValues(items: (string | RegExp)[], options: { matchSubstring?: boolean, normalizeWhiteSpace?: boolean, ignoreCase?: boolean } = {}): ExpectedTextValue[] { - return items.map(i => ({ - string: isString(i) ? i : undefined, - regexSource: isRegExp(i) ? i.source : undefined, - regexFlags: isRegExp(i) ? i.flags : undefined, - matchSubstring: options.matchSubstring, - ignoreCase: options.ignoreCase, - normalizeWhiteSpace: options.normalizeWhiteSpace, - })); -}