From 9491f6652d6b7835dade43c5c8cefdc5a8451024 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 13 Dec 2021 13:32:53 -0800 Subject: [PATCH] fix(steps): do not show unnecessary steps for internal waitForEventInfo (#10889) --- .../playwright-core/src/client/android.ts | 2 +- .../src/client/browserContext.ts | 2 +- .../playwright-core/src/client/electron.ts | 2 +- packages/playwright-core/src/client/frame.ts | 2 +- .../playwright-core/src/client/network.ts | 2 +- packages/playwright-core/src/client/page.ts | 10 ++++----- packages/playwright-core/src/client/waiter.ts | 22 +++++++++++-------- tests/playwright-test/reporter-html.spec.ts | 2 +- tests/playwright-test/reporter.spec.ts | 11 +++++++--- 9 files changed, 32 insertions(+), 23 deletions(-) diff --git a/packages/playwright-core/src/client/android.ts b/packages/playwright-core/src/client/android.ts index 9c1eea0c45..8dae7ccb82 100644 --- a/packages/playwright-core/src/client/android.ts +++ b/packages/playwright-core/src/client/android.ts @@ -200,7 +200,7 @@ export class AndroidDevice extends ChannelOwner i return this._wrapApiCall(async () => { const timeout = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); const predicate = typeof optionsOrPredicate === 'function' ? optionsOrPredicate : optionsOrPredicate.predicate; - const waiter = Waiter.createForEvent(this._channel, event); + const waiter = Waiter.createForEvent(this, event); waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded while waiting for event "${event}"`); if (event !== Events.AndroidDevice.Close) waiter.rejectOnEvent(this, Events.AndroidDevice.Close, new Error('Device closed')); diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index 3b2cd0a809..e243675545 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -268,7 +268,7 @@ export class BrowserContext extends ChannelOwner return this._wrapApiCall(async () => { const timeout = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); const predicate = typeof optionsOrPredicate === 'function' ? optionsOrPredicate : optionsOrPredicate.predicate; - const waiter = Waiter.createForEvent(this._channel, event); + const waiter = Waiter.createForEvent(this, event); waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded while waiting for event "${event}"`); if (event !== Events.BrowserContext.Close) waiter.rejectOnEvent(this, Events.BrowserContext.Close, new Error('Context closed')); diff --git a/packages/playwright-core/src/client/electron.ts b/packages/playwright-core/src/client/electron.ts index 630942a5ef..5321a9cf78 100644 --- a/packages/playwright-core/src/client/electron.ts +++ b/packages/playwright-core/src/client/electron.ts @@ -104,7 +104,7 @@ export class ElectronApplication extends ChannelOwner { const timeout = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); const predicate = typeof optionsOrPredicate === 'function' ? optionsOrPredicate : optionsOrPredicate.predicate; - const waiter = Waiter.createForEvent(this._channel, event); + const waiter = Waiter.createForEvent(this, event); waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded while waiting for event "${event}"`); if (event !== Events.ElectronApplication.Close) waiter.rejectOnEvent(this, Events.ElectronApplication.Close, new Error('Electron application closed')); diff --git a/packages/playwright-core/src/client/frame.ts b/packages/playwright-core/src/client/frame.ts index d559fe8e05..b8dcc0e101 100644 --- a/packages/playwright-core/src/client/frame.ts +++ b/packages/playwright-core/src/client/frame.ts @@ -93,7 +93,7 @@ export class Frame extends ChannelOwner implements api.Fr } private _setupNavigationWaiter(options: { timeout?: number }): Waiter { - const waiter = new Waiter(this._page!._channel, ''); + const waiter = new Waiter(this._page!, ''); if (this._page!.isClosed()) waiter.rejectImmediately(new Error('Navigation failed because page was closed!')); waiter.rejectOnEvent(this._page!, Events.Page.Close, new Error('Navigation failed because page was closed!')); diff --git a/packages/playwright-core/src/client/network.ts b/packages/playwright-core/src/client/network.ts index b84ce0d654..29df56be44 100644 --- a/packages/playwright-core/src/client/network.ts +++ b/packages/playwright-core/src/client/network.ts @@ -474,7 +474,7 @@ export class WebSocket extends ChannelOwner implement return this._wrapApiCall(async () => { const timeout = this._page._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); const predicate = typeof optionsOrPredicate === 'function' ? optionsOrPredicate : optionsOrPredicate.predicate; - const waiter = Waiter.createForEvent(this._channel, event); + const waiter = Waiter.createForEvent(this, event); waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded while waiting for event "${event}"`); if (event !== Events.WebSocket.Error) waiter.rejectOnEvent(this, Events.WebSocket.Error, new Error('Socket error')); diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index b4eac5d8f4..bd404fdf67 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -359,7 +359,7 @@ export class Page extends ChannelOwner implements api.Page }; const trimmedUrl = trimUrl(urlOrPredicate); const logLine = trimmedUrl ? `waiting for request ${trimmedUrl}` : undefined; - return this._waitForEvent(this._channel, Events.Page.Request, { predicate, timeout: options.timeout }, logLine); + return this._waitForEvent(Events.Page.Request, { predicate, timeout: options.timeout }, logLine); } async waitForResponse(urlOrPredicate: string | RegExp | ((r: Response) => boolean | Promise), options: { timeout?: number } = {}): Promise { @@ -370,17 +370,17 @@ export class Page extends ChannelOwner implements api.Page }; const trimmedUrl = trimUrl(urlOrPredicate); const logLine = trimmedUrl ? `waiting for response ${trimmedUrl}` : undefined; - return this._waitForEvent(this._channel, Events.Page.Response, { predicate, timeout: options.timeout }, logLine); + return this._waitForEvent(Events.Page.Response, { predicate, timeout: options.timeout }, logLine); } async waitForEvent(event: string, optionsOrPredicate: WaitForEventOptions = {}): Promise { - return this._waitForEvent(this._channel, event, optionsOrPredicate, `waiting for event "${event}"`); + return this._waitForEvent(event, optionsOrPredicate, `waiting for event "${event}"`); } - private async _waitForEvent(channel: channels.EventTargetChannel, event: string, optionsOrPredicate: WaitForEventOptions, logLine?: string): Promise { + private async _waitForEvent(event: string, optionsOrPredicate: WaitForEventOptions, logLine?: string): Promise { const timeout = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); const predicate = typeof optionsOrPredicate === 'function' ? optionsOrPredicate : optionsOrPredicate.predicate; - const waiter = Waiter.createForEvent(channel, event); + const waiter = Waiter.createForEvent(this, event); if (logLine) waiter.log(logLine); waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded while waiting for event "${event}"`); diff --git a/packages/playwright-core/src/client/waiter.ts b/packages/playwright-core/src/client/waiter.ts index b066c30af6..a2e07edf61 100644 --- a/packages/playwright-core/src/client/waiter.ts +++ b/packages/playwright-core/src/client/waiter.ts @@ -19,28 +19,30 @@ import { rewriteErrorMessage } from '../utils/stackTrace'; import { TimeoutError } from '../utils/errors'; import { createGuid } from '../utils/utils'; import * as channels from '../protocol/channels'; +import { ChannelOwner } from './channelOwner'; export class Waiter { private _dispose: (() => void)[]; private _failures: Promise[] = []; private _immediateError?: Error; - // TODO: can/should we move these logs into wrapApiCall? private _logs: string[] = []; - private _channel: channels.EventTargetChannel; + private _channelOwner: ChannelOwner; private _waitId: string; private _error: string | undefined; - constructor(channel: channels.EventTargetChannel, event: string) { + constructor(channelOwner: ChannelOwner, event: string) { this._waitId = createGuid(); - this._channel = channel; - this._channel.waitForEventInfo({ info: { waitId: this._waitId, phase: 'before', event } }).catch(() => {}); + this._channelOwner = channelOwner; + this._channelOwner._channel.waitForEventInfo({ info: { waitId: this._waitId, phase: 'before', event } }).catch(() => {}); this._dispose = [ - () => this._channel.waitForEventInfo({ info: { waitId: this._waitId, phase: 'after', error: this._error } }).catch(() => {}) + () => this._channelOwner._wrapApiCall(async () => { + await this._channelOwner._channel.waitForEventInfo({ info: { waitId: this._waitId, phase: 'after', error: this._error } }); + }, true).catch(() => {}), ]; } - static createForEvent(channel: channels.EventTargetChannel, event: string) { - return new Waiter(channel, event); + static createForEvent(channelOwner: ChannelOwner, event: string) { + return new Waiter(channelOwner, event); } async waitForEvent(emitter: EventEmitter, event: string, predicate?: (arg: T) => boolean | Promise): Promise { @@ -89,7 +91,9 @@ export class Waiter { log(s: string) { this._logs.push(s); - this._channel.waitForEventInfo({ info: { waitId: this._waitId, phase: 'log', message: s } }).catch(() => {}); + this._channelOwner._wrapApiCall(async () => { + await this._channelOwner._channel.waitForEventInfo({ info: { waitId: this._waitId, phase: 'log', message: s } }).catch(() => {}); + }, true); } private _rejectOn(promise: Promise, dispose?: () => void) { diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts index 92fc248425..67600dab7f 100644 --- a/tests/playwright-test/reporter-html.spec.ts +++ b/tests/playwright-test/reporter-html.spec.ts @@ -252,7 +252,7 @@ test('should show trace title', async ({ runInlineTest, page, showReport }) => { test('should show timed out steps', async ({ runInlineTest, page, showReport }) => { const result = await runInlineTest({ 'playwright.config.js': ` - module.exports = { timeout: 500 }; + module.exports = { timeout: 3000 }; `, 'a.test.js': ` const { test } = pwt; diff --git a/tests/playwright-test/reporter.spec.ts b/tests/playwright-test/reporter.spec.ts index fbdf7132e5..1d95ffb8fe 100644 --- a/tests/playwright-test/reporter.spec.ts +++ b/tests/playwright-test/reporter.spec.ts @@ -259,7 +259,10 @@ test('should report api steps', async ({ runInlineTest }) => { 'a.test.ts': ` const { test } = pwt; test('pass', async ({ page }) => { - await page.setContent(''); + await Promise.all([ + page.waitForNavigation(), + page.goto('data:text/html,'), + ]); await page.click('button'); }); @@ -290,8 +293,10 @@ test('should report api steps', async ({ runInlineTest }) => { `%% begin {\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"Before Hooks\",\"category\":\"hook\",\"steps\":[{\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"}]}`, - `%% begin {\"title\":\"page.setContent\",\"category\":\"pw:api\"}`, - `%% end {\"title\":\"page.setContent\",\"category\":\"pw:api\"}`, + `%% begin {\"title\":\"page.waitForNavigation\",\"category\":\"pw:api\"}`, + `%% begin {\"title\":\"page.goto(data:text/html,)\",\"category\":\"pw:api\"}`, + `%% end {\"title\":\"page.waitForNavigation\",\"category\":\"pw:api\"}`, + `%% end {\"title\":\"page.goto(data:text/html,)\",\"category\":\"pw:api\"}`, `%% begin {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"page.click(button)\",\"category\":\"pw:api\"}`, `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`,