From 3f850d27e938d7fe9e470a13ca400cfa3ab9bf83 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 24 Oct 2022 18:01:48 -0400 Subject: [PATCH] fix(highlight): fix the testing harness to be real (#18294) --- .../src/server/injected/highlight.ts | 7 +- .../src/server/injected/recorder.ts | 17 ++-- tests/library/inspector/cli-codegen-1.spec.ts | 79 +++++++++---------- tests/library/inspector/cli-codegen-2.spec.ts | 17 ++-- tests/library/inspector/cli-codegen-3.spec.ts | 28 +++---- tests/library/inspector/inspectorTest.ts | 25 +----- tests/page/locator-highlight.spec.ts | 18 ++--- 7 files changed, 78 insertions(+), 113 deletions(-) diff --git a/packages/playwright-core/src/server/injected/highlight.ts b/packages/playwright-core/src/server/injected/highlight.ts index e90d1fc52b..a584adfd6e 100644 --- a/packages/playwright-core/src/server/injected/highlight.ts +++ b/packages/playwright-core/src/server/injected/highlight.ts @@ -54,10 +54,7 @@ export class Highlight { this._actionPointElement = document.createElement('x-pw-action-point'); this._actionPointElement.setAttribute('hidden', 'true'); - - // NOTE: do not use an open shadow root, event for test. - // Closed shadow root prevents selectors matching our internal previews. - this._glassPaneShadow = this._glassPaneElement.attachShadow({ mode: 'closed' }); + this._glassPaneShadow = this._glassPaneElement.attachShadow({ mode: this._isUnderTest ? 'open' : 'closed' }); this._glassPaneShadow.appendChild(this._actionPointElement); const styleElement = document.createElement('style'); styleElement.textContent = ` @@ -182,8 +179,6 @@ export class Highlight { tooltipElement.style.top = '0'; tooltipElement.style.left = '0'; tooltipElement.style.display = 'flex'; - if (this._isUnderTest) - console.error('Highlight text for test: ' + JSON.stringify(tooltipElement.textContent)); // eslint-disable-line no-console } this._highlightEntries.push({ targetElement: elements[i], tooltipElement, highlightElement }); } diff --git a/packages/playwright-core/src/server/injected/recorder.ts b/packages/playwright-core/src/server/injected/recorder.ts index ae974af105..8b6ea24270 100644 --- a/packages/playwright-core/src/server/injected/recorder.ts +++ b/packages/playwright-core/src/server/injected/recorder.ts @@ -74,7 +74,7 @@ class Recorder { addEventListener(document, 'mouseup', event => this._onMouseUp(event as MouseEvent), true), addEventListener(document, 'mousemove', event => this._onMouseMove(event as MouseEvent), true), addEventListener(document, 'mouseleave', event => this._onMouseLeave(event as MouseEvent), true), - addEventListener(document, 'focus', () => this._onFocus(), true), + addEventListener(document, 'focus', () => this._onFocus(true), true), addEventListener(document, 'scroll', () => { this._hoveredModel = null; this._highlight.hideActionPoint(); @@ -234,12 +234,13 @@ class Recorder { } } - private _onFocus() { + private _onFocus(userGesture: boolean) { const activeElement = this._deepActiveElement(document); const result = activeElement ? generateSelector(this._injectedScript, activeElement, true) : null; this._activeModel = result && result.selector ? result : null; - if (this._injectedScript.isUnderTest) - console.error('Highlight updated for test: ' + (result ? result.selector : null)); // eslint-disable-line no-console + if (userGesture) + this._hoveredElement = activeElement as HTMLElement | null; + this._updateModelForHoveredElement(); } private _updateModelForHoveredElement() { @@ -250,12 +251,10 @@ class Recorder { } const hoveredElement = this._hoveredElement; const { selector, elements } = generateSelector(this._injectedScript, hoveredElement, true); - if ((this._hoveredModel && this._hoveredModel.selector === selector) || this._hoveredElement !== hoveredElement) + if ((this._hoveredModel && this._hoveredModel.selector === selector)) return; this._hoveredModel = selector ? { selector, elements } : null; this._updateHighlight(); - if (this._injectedScript.isUnderTest) - console.error('Highlight updated for test: ' + selector); // eslint-disable-line no-console } private _updateHighlight() { @@ -392,10 +391,8 @@ class Recorder { await globalThis.__pw_recorderPerformAction(action).catch(() => {}); this._performingAction = false; - // Action could have changed DOM, update hovered model selectors. - this._updateModelForHoveredElement(); // If that was a keyboard action, it similarly requires new selectors for active model. - this._onFocus(); + this._onFocus(false); if (this._injectedScript.isUnderTest) { // Serialize all to string as we cannot attribute console message to isolated world diff --git a/tests/library/inspector/cli-codegen-1.spec.ts b/tests/library/inspector/cli-codegen-1.spec.ts index 8b63d7817e..08a9c982ba 100644 --- a/tests/library/inspector/cli-codegen-1.spec.ts +++ b/tests/library/inspector/cli-codegen-1.spec.ts @@ -25,8 +25,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.hoverOverElement('button'); - expect(selector).toBe('internal:role=button[name=\"Submit\"]'); + const locator = await recorder.hoverOverElement('button'); + expect(locator).toBe(`getByRole('button', { name: 'Submit' })`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -68,8 +68,8 @@ test.describe('cli codegen', () => { // the second unnecessary copy of the recorder script. await page.waitForTimeout(1000); - const selector = await recorder.hoverOverElement('button'); - expect(selector).toBe('internal:role=button[name=\"Submit\"]'); + const locator = await recorder.hoverOverElement('button'); + expect(locator).toBe(`getByRole('button', { name: 'Submit' })`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -95,10 +95,10 @@ test.describe('cli codegen', () => { `); - const selector = await recorder.waitForHighlight(() => recorder.page.hover('canvas', { + const locator = await recorder.waitForHighlight(() => recorder.page.hover('canvas', { position: { x: 250, y: 250 }, })); - expect(selector).toBe('canvas'); + expect(locator).toBe(`locator('canvas')`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), recorder.waitForOutput('JavaScript', 'click'), @@ -148,8 +148,8 @@ test.describe('cli codegen', () => { `); - const selector = await recorder.hoverOverElement('button'); - expect(selector).toBe('internal:role=button[name=\"Submit\"]'); + const locator = await recorder.hoverOverElement('button'); + expect(locator).toBe(`getByRole('button', { name: 'Submit' })`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -191,11 +191,10 @@ test.describe('cli codegen', () => { document.documentElement.appendChild(div); }); - const selector = await recorder.hoverOverElement('div'); - expect(selector).toBe('internal:text="Some long text here"i'); + const locator = await recorder.hoverOverElement('div'); + expect(locator).toBe(`getByText('Some long text here')`); - // Sanity check that selector does not match our highlight. - const divContents = await page.$eval(selector, div => div.outerHTML); + const divContents = await page.$eval('div', div => div.outerHTML); expect(divContents.replace(/\s__playwright_target__="[^"]+"/, '')).toBe(`
Some long text here
`); const [message, sources] = await Promise.all([ @@ -212,8 +211,8 @@ test.describe('cli codegen', () => { const recorder = await openRecorder(); await recorder.setContentAndWait(``); - const selector = await recorder.focusElement('input'); - expect(selector).toBe('input[name="name"]'); + const locator = await recorder.focusElement('input'); + expect(locator).toBe(`locator('input[name="name"]')`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -243,16 +242,16 @@ test.describe('cli codegen', () => { // In Japanese, "てすと" or "テスト" means "test". await recorder.setContentAndWait(``); - const selector = await recorder.focusElement('input'); - expect(selector).toBe('input[name="name"]'); + const locator = await recorder.focusElement('input'); + expect(locator).toBe(`locator('input[name="name"]')`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), recorder.waitForOutput('JavaScript', 'fill'), (async () => { - await recorder.page.dispatchEvent(selector, 'keydown', { key: 'Process' }); + await recorder.page.dispatchEvent('input', 'keydown', { key: 'Process' }); await recorder.page.keyboard.insertText('てすと'); - await recorder.page.dispatchEvent(selector, 'keyup', { key: 'Process' }); + await recorder.page.dispatchEvent('input', 'keyup', { key: 'Process' }); })() ]); expect(sources.get('JavaScript').text).toContain(` @@ -276,8 +275,8 @@ test.describe('cli codegen', () => { const recorder = await openRecorder(); await recorder.setContentAndWait(``); - const selector = await recorder.focusElement('textarea'); - expect(selector).toBe('textarea[name="name"]'); + const locator = await recorder.focusElement('textarea'); + expect(locator).toBe(`locator('textarea[name="name"]')`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -294,8 +293,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.focusElement('input'); - expect(selector).toBe('input[name="name"]'); + const locator = await recorder.focusElement('input'); + expect(locator).toBe(`locator('input[name="name"]')`); const messages: any[] = []; page.on('console', message => messages.push(message)); @@ -357,8 +356,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.focusElement('input'); - expect(selector).toBe('input[name="name"]'); + const locator = await recorder.focusElement('input'); + expect(locator).toBe(`locator('input[name="name"]')`); const messages: any[] = []; page.on('console', message => { @@ -379,8 +378,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.focusElement('input'); - expect(selector).toBe('input[name="name"]'); + const locator = await recorder.focusElement('input'); + expect(locator).toBe(`locator('input[name="name"]')`); const messages: any[] = []; page.on('console', message => { @@ -404,8 +403,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.focusElement('input'); - expect(selector).toBe('input[name="accept"]'); + const locator = await recorder.focusElement('input'); + expect(locator).toBe(`locator('input[name="accept"]')`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -436,8 +435,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.focusElement('input'); - expect(selector).toBe('input[name="accept"]'); + const locator = await recorder.focusElement('input'); + expect(locator).toBe(`locator('input[name="accept"]')`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -455,8 +454,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.focusElement('input'); - expect(selector).toBe('input[name="accept"]'); + const locator = await recorder.focusElement('input'); + expect(locator).toBe(`locator('input[name="accept"]')`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -474,8 +473,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.focusElement('input'); - expect(selector).toBe('input[name="accept"]'); + const locator = await recorder.focusElement('input'); + expect(locator).toBe(`locator('input[name="accept"]')`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -506,8 +505,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(''); - const selector = await recorder.hoverOverElement('select'); - expect(selector).toBe('select'); + const locator = await recorder.hoverOverElement('select'); + expect(locator).toBe(`locator('select')`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -539,8 +538,8 @@ test.describe('cli codegen', () => { const recorder = await openRecorder(); await recorder.setContentAndWait('link'); - const selector = await recorder.hoverOverElement('a'); - expect(selector).toBe('internal:role=link[name=\"link\"]'); + const locator = await recorder.hoverOverElement('a'); + expect(locator).toBe(`getByRole('link', { name: 'link' })`); const [popup, sources] = await Promise.all([ page.context().waitForEvent('page'), @@ -583,8 +582,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(`link`); - const selector = await recorder.hoverOverElement('a'); - expect(selector).toBe('internal:text="link"i'); + const locator = await recorder.hoverOverElement('a'); + expect(locator).toBe(`getByText('link')`); const [, sources] = await Promise.all([ page.waitForNavigation(), recorder.waitForOutput('JavaScript', '.click()'), diff --git a/tests/library/inspector/cli-codegen-2.spec.ts b/tests/library/inspector/cli-codegen-2.spec.ts index 9b62a12468..7167f16a80 100644 --- a/tests/library/inspector/cli-codegen-2.spec.ts +++ b/tests/library/inspector/cli-codegen-2.spec.ts @@ -98,8 +98,7 @@ test.describe('cli codegen', () => { const errors: any[] = []; recorder.page.on('pageerror', e => errors.push(e)); await recorder.page.evaluate(() => document.querySelector('body').remove()); - const selector = await recorder.hoverOverElement('html'); - expect(selector).toBe('html'); + await page.dispatchEvent('html', 'mousemove', { detail: 1 }); await recorder.page.close(); await recorder.waitForOutput('JavaScript', 'page.close();'); expect(errors.length).toBe(0); @@ -219,10 +218,10 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(` Download `, server.PREFIX); - await recorder.hoverOverElement('text=Download'); + await recorder.hoverOverElement('a'); await Promise.all([ page.waitForEvent('download'), - page.click('text=Download') + page.click('a') ]); const sources = await recorder.waitForOutput('JavaScript', 'waitForEvent'); @@ -274,7 +273,7 @@ test.describe('cli codegen', () => { page.once('dialog', async dialog => { await dialog.dismiss(); }); - await page.click('text=click me'); + await page.click('button'); const sources = await recorder.waitForOutput('JavaScript', 'once'); @@ -332,8 +331,8 @@ test.describe('cli codegen', () => { const recorder = await openRecorder(); await recorder.setContentAndWait(`link`); - const selector = await recorder.hoverOverElement('a'); - expect(selector).toBe('internal:role=link[name=\"link\"]'); + const locator = await recorder.hoverOverElement('a'); + expect(locator).toBe(`getByRole('link', { name: 'link' })`); await page.click('a', { modifiers: [platform === 'darwin' ? 'Meta' : 'Control'] }); const sources = await recorder.waitForOutput('JavaScript', 'page1'); @@ -502,8 +501,8 @@ test.describe('cli codegen', () => { const recorder = await openRecorder(); await recorder.setContentAndWait(``); - const selector = await recorder.focusElement('textarea'); - expect(selector).toBe('textarea[name="name"]'); + const locator = await recorder.focusElement('textarea'); + expect(locator).toBe(`locator('textarea[name="name"]')`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), diff --git a/tests/library/inspector/cli-codegen-3.spec.ts b/tests/library/inspector/cli-codegen-3.spec.ts index e04720f508..4546708a13 100644 --- a/tests/library/inspector/cli-codegen-3.spec.ts +++ b/tests/library/inspector/cli-codegen-3.spec.ts @@ -27,8 +27,8 @@ test.describe('cli codegen', () => { `); - const selector = await recorder.hoverOverElement('button'); - expect(selector).toBe('internal:role=button[name=\"Submit\"] >> nth=0'); + const locator = await recorder.hoverOverElement('button'); + expect(locator).toBe(`getByRole('button', { name: 'Submit' }).first()`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -62,8 +62,8 @@ test.describe('cli codegen', () => { `); - const selector = await recorder.hoverOverElement('button >> nth=1'); - expect(selector).toBe('internal:role=button[name=\"Submit\"] >> nth=1'); + const locator = await recorder.hoverOverElement('button >> nth=1'); + expect(locator).toBe(`getByRole('button', { name: 'Submit' }).nth(1)`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -234,8 +234,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(`
Submit
`); - const selector = await recorder.hoverOverElement('div'); - expect(selector).toBe('internal:attr=[data-testid="testid"]'); + const locator = await recorder.hoverOverElement('div'); + expect(locator).toBe(`getByTestId('testid')`); const [message, sources] = await Promise.all([ page.waitForEvent('console', msg => msg.type() !== 'error'), @@ -266,8 +266,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.hoverOverElement('input'); - expect(selector).toBe('internal:attr=[placeholder="Country"i]'); + const locator = await recorder.hoverOverElement('input'); + expect(locator).toBe(`getByPlaceholder('Country')`); const [sources] = await Promise.all([ recorder.waitForOutput('JavaScript', 'click'), @@ -295,8 +295,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.hoverOverElement('input'); - expect(selector).toBe('internal:attr=[alt="Country"i]'); + const locator = await recorder.hoverOverElement('input'); + expect(locator).toBe(`getByAltText('Country')`); const [sources] = await Promise.all([ recorder.waitForOutput('JavaScript', 'click'), @@ -324,8 +324,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.hoverOverElement('input'); - expect(selector).toBe('internal:label="Country"i'); + const locator = await recorder.hoverOverElement('input'); + expect(locator).toBe(`getByLabel('Country')`); const [sources] = await Promise.all([ recorder.waitForOutput('JavaScript', 'click'), @@ -353,8 +353,8 @@ test.describe('cli codegen', () => { await recorder.setContentAndWait(``); - const selector = await recorder.hoverOverElement('input'); - expect(selector).toBe('internal:label="Coun\\\"try"i'); + const locator = await recorder.hoverOverElement('input'); + expect(locator).toBe(`getByLabel('Coun"try')`); const [sources] = await Promise.all([ recorder.waitForOutput('JavaScript', 'click'), diff --git a/tests/library/inspector/inspectorTest.ts b/tests/library/inspector/inspectorTest.ts index d3c08c3465..588952522e 100644 --- a/tests/library/inspector/inspectorTest.ts +++ b/tests/library/inspector/inspectorTest.ts @@ -15,7 +15,7 @@ */ import { contextTest } from '../../config/browserTest'; -import type { ConsoleMessage, Page } from 'playwright-core'; +import type { Page } from 'playwright-core'; import * as path from 'path'; import type { Source } from '../../../packages/recorder/src/recorderTypes'; import type { CommonFixtures, TestChildProcess } from '../../config/commonFixtures'; @@ -149,27 +149,10 @@ class Recorder { } async waitForHighlight(action: () => Promise): Promise { - // We get the last highlighted selector, because Firefox sometimes issues multiple - // focus events. - let generatedSelector: string | undefined; - let callback: Function | undefined; - - const listener = async (msg: ConsoleMessage) => { - const prefix = 'Highlight updated for test: '; - if (msg.text().startsWith(prefix)) { - generatedSelector = msg.text().substr(prefix.length); - if (callback) { - this.page.off('console', listener); - callback(generatedSelector); - } - } - }; - this.page.on('console', listener); - await action(); - if (generatedSelector) - return generatedSelector; - return await new Promise(f => callback = f); + await this.page.locator('x-pw-highlight').waitFor(); + await this.page.locator('x-pw-tooltip').waitFor(); + return this.page.locator('x-pw-tooltip').textContent(); } async waitForActionPerformed(): Promise<{ hovered: string | null, active: string | null }> { diff --git a/tests/page/locator-highlight.spec.ts b/tests/page/locator-highlight.spec.ts index bb69968284..8bc5c75085 100644 --- a/tests/page/locator-highlight.spec.ts +++ b/tests/page/locator-highlight.spec.ts @@ -15,27 +15,19 @@ */ import { test as it, expect } from './pageTest'; -import { waitForTestLog } from '../config/utils'; import type { Locator } from 'playwright-core'; type BoundingBox = Awaited>; it.skip(({ mode }) => mode !== 'default', 'Highlight element has a closed shadow-root on != default'); -it('should highlight locator', async ({ page, isAndroid }) => { +it('should highlight locator', async ({ page }) => { await page.setContent(``); - const textPromise = waitForTestLog(page, 'Highlight text for test: '); - const boxPromise = waitForTestLog<{ x: number, y: number, width: number, height: number }>(page, 'Highlight box for test: '); await page.locator('input').highlight(); - expect(await textPromise).toBe('locator(\'input\')'); - let box1 = await page.locator('input').boundingBox(); - let box2 = await boxPromise; - - if (isAndroid) { - box1 = roundBox(box1); - box2 = roundBox(box2); - } - + await expect(page.locator('x-pw-tooltip')).toHaveText('locator(\'input\')'); + await expect(page.locator('x-pw-highlight')).toBeVisible(); + const box1 = roundBox(await page.locator('input').boundingBox()); + const box2 = roundBox(await page.locator('x-pw-highlight').boundingBox()); expect(box1).toEqual(box2); });