diff --git a/.github/workflows/create_test_report.yml b/.github/workflows/create_test_report.yml index 4ddaaa8bdfb05..be985b4350344 100644 --- a/.github/workflows/create_test_report.yml +++ b/.github/workflows/create_test_report.yml @@ -190,7 +190,7 @@ jobs: if: needs.triage.outputs.has_draft == 'true' runs-on: ubuntu-latest permissions: - issues: write + pull-requests: write env: PR_NUMBER: ${{ needs.triage.outputs.pr_number }} GH_TOKEN: ${{ github.token }} @@ -206,4 +206,3 @@ jobs: run: | printf '\n\nTriaged by the Playwright bot - [agent run](%s)\n\n' "$WORKFLOW_URL" >> output/triage.md gh issue comment "$PR_NUMBER" --repo "${{ github.repository }}" --body-file output/triage.md - diff --git a/.github/workflows/fix-flakes-prompt.md b/.github/workflows/fix-flakes-prompt.md index c56b9dfa02fbd..ba43719927477 100644 --- a/.github/workflows/fix-flakes-prompt.md +++ b/.github/workflows/fix-flakes-prompt.md @@ -4,6 +4,9 @@ Turn CI test-results data into **one** concrete fix: pick a high-impact flaky-or confirm nobody's on it, fix the root cause *or* scope a skip, pick a reviewer, and hand off a single commit that becomes the PR. Fully autonomous — no approval stops. +The GitHub CLI (`gh`) is not authenticated in this job. Do not use it for GitHub API operations; +use GitHub MCP tools instead. + ## 1. Pick one target Query the DB following the patterns in `.claude/skills/playwright-test-results/SKILL.md`. diff --git a/.github/workflows/fix-flakes.yml b/.github/workflows/fix-flakes.yml index 464df570a6faf..740a857bdf70e 100644 --- a/.github/workflows/fix-flakes.yml +++ b/.github/workflows/fix-flakes.yml @@ -58,6 +58,9 @@ jobs: an expensive runner to fix a flaky or red test is most worthwhile right now, and hand off only that runner label. Do NOT fix anything. + The GitHub CLI is not authenticated in this job. Do not run gh; use GitHub MCP tools + for all GitHub reads. + Optimise for the OS with the highest-impact actionable flakiness/reds that isn't already being worked on OR already fixed. A candidate is dead if a fix PR touches it (open, or recently merged/closed) — the DB window still holds the failing runs from diff --git a/.github/workflows/triage.yml b/.github/workflows/triage.yml index 62fdbf209555d..7e3a766fef6c4 100644 --- a/.github/workflows/triage.yml +++ b/.github/workflows/triage.yml @@ -46,6 +46,9 @@ jobs: PROMPT=$(cat < i async waitForEvent(event: string, optionsOrPredicate: types.WaitForEventOptions = {}): Promise { return await this._wrapApiCall(async () => { - const { timeout, signal } = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); + const timeoutOptions = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); const predicate = typeof optionsOrPredicate === 'function' ? optionsOrPredicate : optionsOrPredicate.predicate; const waiter = Waiter.createForEvent(this, event); - waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded while waiting for event "${event}"`); - waiter.rejectOnSignal(signal); + waiter.rejectOnTimeout(timeoutOptions, `Timeout ${timeoutOptions.timeout}ms exceeded while waiting for event "${event}"`); if (event !== Events.AndroidDevice.Close) waiter.rejectOnEvent(this, Events.AndroidDevice.Close, () => new TargetClosedError()); const result = await waiter.waitForEvent(this, event, predicate as any); diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index 89c160f78928d..82445b69f0941 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -449,11 +449,10 @@ export class BrowserContext extends ChannelOwner async waitForEvent(event: string, optionsOrPredicate: WaitForEventOptions = {}): Promise { return await this._wrapApiCall(async () => { - const { timeout, signal } = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); + const timeoutOptions = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); const predicate = typeof optionsOrPredicate === 'function' ? optionsOrPredicate : optionsOrPredicate.predicate; const waiter = Waiter.createForEvent(this, event); - waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded while waiting for event "${event}"`); - waiter.rejectOnSignal(signal); + waiter.rejectOnTimeout(timeoutOptions, `Timeout ${timeoutOptions.timeout}ms exceeded while waiting for event "${event}"`); if (event !== Events.BrowserContext.Close) waiter.rejectOnEvent(this, Events.BrowserContext.Close, () => new TargetClosedError(this._effectiveCloseReason())); const result = await waiter.waitForEvent(this, event, predicate as any); diff --git a/packages/playwright-core/src/client/electron.ts b/packages/playwright-core/src/client/electron.ts index c773ac36f6c8e..d433427c57ba2 100644 --- a/packages/playwright-core/src/client/electron.ts +++ b/packages/playwright-core/src/client/electron.ts @@ -139,11 +139,10 @@ export class ElectronApplication extends ChannelOwner { return await this._wrapApiCall(async () => { - const { timeout, signal } = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); + const timeoutOptions = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); const predicate = typeof optionsOrPredicate === 'function' ? optionsOrPredicate : optionsOrPredicate.predicate; const waiter = Waiter.createForEvent(this, event); - waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded while waiting for event "${event}"`); - waiter.rejectOnSignal(signal); + waiter.rejectOnTimeout(timeoutOptions, `Timeout ${timeoutOptions.timeout}ms exceeded while waiting for event "${event}"`); if (event !== Events.ElectronApplication.Close) waiter.rejectOnEvent(this, Events.ElectronApplication.Close, () => new TargetClosedError()); const result = await waiter.waitForEvent(this, event, predicate as any); diff --git a/packages/playwright-core/src/client/frame.ts b/packages/playwright-core/src/client/frame.ts index 6c9de8ecdcfdb..ad25c267dd8e3 100644 --- a/packages/playwright-core/src/client/frame.ts +++ b/packages/playwright-core/src/client/frame.ts @@ -129,9 +129,8 @@ export class Frame extends ChannelOwner implements api.Fr waiter.rejectOnEvent(this._page!, Events.Page.Close, () => this._page!._closeErrorWithReason()); waiter.rejectOnEvent(this._page!, Events.Page.Crash, new Error('Navigation failed because page crashed!')); waiter.rejectOnEvent(this._page!, Events.Page.FrameDetached, new Error('Navigating frame was detached!'), frame => frame === this); - const { timeout } = this._page!._timeoutSettings.navigationTimeout(options); - waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded.`); - waiter.rejectOnSignal(options.signal); + const timeoutOptions = this._page!._timeoutSettings.navigationTimeout(options); + waiter.rejectOnTimeout(timeoutOptions, `Timeout ${timeoutOptions.timeout}ms exceeded.`); return waiter; } @@ -496,8 +495,8 @@ export class Frame extends ChannelOwner implements api.Fr return (await this._channel.title({}, kNoTimeout)).value; } - async _expect(expression: string, options: Omit & { timeout: number }, signal: AbortSignal | undefined): Promise { - const { timeout, ...rest } = options; + async _expect(expression: string, options: Omit & { timeout: number, signal?: AbortSignal }): Promise { + const { timeout, signal, ...rest } = options; const params: channels.FrameExpectParams = { expression, ...rest, isNot: !!rest.isNot }; params.expectedValue = serializeArgument(rest.expectedValue); try { diff --git a/packages/playwright-core/src/client/locator.ts b/packages/playwright-core/src/client/locator.ts index 8794dd09399e7..342799550bc40 100644 --- a/packages/playwright-core/src/client/locator.ts +++ b/packages/playwright-core/src/client/locator.ts @@ -404,11 +404,11 @@ export class Locator implements api.Locator { } - async _expect(expression: string, options: FrameExpectParams, signal: AbortSignal | undefined): Promise { + async _expect(expression: string, options: FrameExpectParams): Promise { return this._frame._expect(expression, { ...options, selector: this._selector, - }, signal); + }); } private _inspect() { diff --git a/packages/playwright-core/src/client/network.ts b/packages/playwright-core/src/client/network.ts index 7c4fb67cbffa6..4d36f5dd0a8b1 100644 --- a/packages/playwright-core/src/client/network.ts +++ b/packages/playwright-core/src/client/network.ts @@ -804,11 +804,10 @@ export class WebSocket extends ChannelOwner implement async waitForEvent(event: string, optionsOrPredicate: WaitForEventOptions = {}): Promise { return await this._wrapApiCall(async () => { - const { timeout, signal } = this._page._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); + const timeoutOptions = this._page._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); const predicate = typeof optionsOrPredicate === 'function' ? optionsOrPredicate : optionsOrPredicate.predicate; const waiter = Waiter.createForEvent(this, event); - waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded while waiting for event "${event}"`); - waiter.rejectOnSignal(signal); + waiter.rejectOnTimeout(timeoutOptions, `Timeout ${timeoutOptions.timeout}ms exceeded while waiting for event "${event}"`); if (event !== Events.WebSocket.Error) waiter.rejectOnEvent(this, Events.WebSocket.Error, new Error('Socket error')); if (event !== Events.WebSocket.Close) diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index 1fedf84e23ad9..f7f2ba405427b 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -19,6 +19,7 @@ import fs from 'fs'; import * as inspector from 'inspector'; import path from 'path'; +import { assertionAbortedMessage } from '@isomorphic/abortSignal'; import { assert } from '@isomorphic/assert'; import { headersObjectToArray } from '@isomorphic/headers'; import { trimStringWithEllipsis } from '@isomorphic/stringUtils'; @@ -32,7 +33,7 @@ import { Coverage } from './coverage'; import { DisposableObject, DisposableStub } from './disposable'; import { Download } from './download'; import { ElementHandle, determineScreenshotType } from './elementHandle'; -import { PlaywrightError, TargetClosedError, isTargetClosedError, parseError, serializeError } from './errors'; +import { AbortError, PlaywrightError, TargetClosedError, isTargetClosedError, parseError, serializeError } from './errors'; import { Events } from './events'; import { FileChooser } from './fileChooser'; import { Frame, verifyLoadState } from './frame'; @@ -77,6 +78,7 @@ export type ExpectScreenshotOptions = Omit implements api.Page private async _waitForEvent(event: string, optionsOrPredicate: WaitForEventOptions, logLine?: string): Promise { return await this._wrapApiCall(async () => { - const { timeout, signal } = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); + const timeoutOptions = this._timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); const predicate = typeof optionsOrPredicate === 'function' ? optionsOrPredicate : optionsOrPredicate.predicate; const waiter = Waiter.createForEvent(this, event); if (logLine) waiter.log(logLine); - waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded while waiting for event "${event}"`); - waiter.rejectOnSignal(signal); + waiter.rejectOnTimeout(timeoutOptions, `Timeout ${timeoutOptions.timeout}ms exceeded while waiting for event "${event}"`); if (event !== Events.Page.Crash) waiter.rejectOnEvent(this, Events.Page.Crash, new Error('Page crashed')); if (event !== Events.Page.Close) @@ -622,7 +623,7 @@ export class Page extends ChannelOwner implements api.Page } async _expectScreenshot(options: ExpectScreenshotOptions): Promise<{ actual?: Buffer, previous?: Buffer, diff?: Buffer, errorMessage?: string, log?: string[], timedOut?: boolean}> { - const { timeout, ...optionsWithoutTimeout } = options; + const { timeout, signal, ...optionsWithoutTimeout } = options; const mask = options?.mask ? options?.mask.map(locator => ({ frame: (locator as Locator)._frame._channel, selector: (locator as Locator)._selector, @@ -637,9 +638,11 @@ export class Page extends ChannelOwner implements api.Page isNot: !!options.isNot, locator, mask, - }, { signal: undefined, timeout }); + }, { timeout, signal }); return { actual: result.actual }; } catch (e) { + if (e instanceof AbortError) + return { errorMessage: 'Error: ' + assertionAbortedMessage(e.cause) }; if (!(e instanceof PlaywrightError)) throw e; const details = e.details as channels.PageExpectScreenshotErrorDetails; diff --git a/packages/playwright-core/src/client/types.ts b/packages/playwright-core/src/client/types.ts index dfa07bc012723..f340521105e1a 100644 --- a/packages/playwright-core/src/client/types.ts +++ b/packages/playwright-core/src/client/types.ts @@ -147,4 +147,4 @@ export type AnnotateOptions = { duration?: number, position?: AnnotatePosition, export type RemoteAddr = channels.RemoteAddr; export type SecurityDetails = channels.SecurityDetails; -export type FrameExpectParams = Omit & { expectedValue?: any, timeout: number }; +export type FrameExpectParams = Omit & { expectedValue?: any, timeout: number, signal?: AbortSignal }; diff --git a/packages/playwright-core/src/client/waiter.ts b/packages/playwright-core/src/client/waiter.ts index a20ae8dc61142..d5a5706deb8cc 100644 --- a/packages/playwright-core/src/client/waiter.ts +++ b/packages/playwright-core/src/client/waiter.ts @@ -80,25 +80,21 @@ export class Waiter { this._rejectOn(promise.then(() => { throw (typeof error === 'function' ? error() : error); }), dispose); } - rejectOnTimeout(timeout: number, message: string) { - if (!timeout) - return; - const { promise, dispose } = waitForTimeout(timeout); - this._rejectOn(promise.then(() => { throw new TimeoutError(message); }), dispose); - } + rejectOnTimeout({ timeout, signal }: channels.TimeoutOptions, timeoutMessage: string) { + if (signal) { + if (signal.aborted) + return this.rejectImmediately(new AbortError(undefined, { cause: signal.reason })); + let rejectPromise: (e: any) => void; + const promise = new Promise((_, reject) => { rejectPromise = reject; }); + const listener = () => rejectPromise!(new AbortError(undefined, { cause: signal.reason })); + signal.addEventListener('abort', listener, { once: true }); + this._rejectOn(promise, () => signal.removeEventListener('abort', listener)); + } - rejectOnSignal(signal: AbortSignal | undefined) { - if (!signal) - return; - if (signal.aborted) { - this.rejectImmediately(new AbortError(undefined, { cause: signal.reason })); - return; + if (timeout) { + const { promise, dispose } = waitForTimeout(timeout); + this._rejectOn(promise.then(() => { throw new TimeoutError(timeoutMessage); }), dispose); } - let rejectPromise: (e: any) => void; - const promise = new Promise((_, reject) => { rejectPromise = reject; }); - const listener = () => rejectPromise!(new AbortError(undefined, { cause: signal.reason })); - signal.addEventListener('abort', listener, { once: true }); - this._rejectOn(promise, () => signal.removeEventListener('abort', listener)); } rejectImmediately(error: Error) { diff --git a/packages/playwright-core/src/client/worker.ts b/packages/playwright-core/src/client/worker.ts index 5b193d6c5bf8e..90987e03a822c 100644 --- a/packages/playwright-core/src/client/worker.ts +++ b/packages/playwright-core/src/client/worker.ts @@ -89,11 +89,10 @@ export class Worker extends ChannelOwner implements api. async waitForEvent(event: string, optionsOrPredicate: WaitForEventOptions = {}): Promise { return await this._wrapApiCall(async () => { const timeoutSettings = this._page?._timeoutSettings ?? this._context?._timeoutSettings ?? new TimeoutSettings(); - const { timeout, signal } = timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); + const timeoutOptions = timeoutSettings.timeout(typeof optionsOrPredicate === 'function' ? {} : optionsOrPredicate); const predicate = typeof optionsOrPredicate === 'function' ? optionsOrPredicate : optionsOrPredicate.predicate; const waiter = Waiter.createForEvent(this, event); - waiter.rejectOnTimeout(timeout, `Timeout ${timeout}ms exceeded while waiting for event "${event}"`); - waiter.rejectOnSignal(signal); + waiter.rejectOnTimeout(timeoutOptions, `Timeout ${timeoutOptions.timeout}ms exceeded while waiting for event "${event}"`); if (event !== Events.Worker.Close) waiter.rejectOnEvent(this, Events.Worker.Close, () => this._closeErrorWithReason()); const result = await waiter.waitForEvent(this, event, predicate as any); diff --git a/packages/playwright-core/src/server/dispatchers/dispatcher.ts b/packages/playwright-core/src/server/dispatchers/dispatcher.ts index 9215be6d89070..172a58368b278 100644 --- a/packages/playwright-core/src/server/dispatchers/dispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/dispatcher.ts @@ -356,13 +356,10 @@ export class DispatcherConnection { type: dispatcher._type, method, params: params || {}, + timeout: validMetadata.timeout, log: [], }; - // TODO(skn0tt): promote to top-level metadata instead of smuggling through params. - if (validMetadata.timeout) - callMetadata.params = { ...callMetadata.params, timeout: validMetadata.timeout }; - const controller = dispatcher.createProgressController(callMetadata); this._activeProgressControllers.set(callMetadata.id, controller); diff --git a/packages/playwright-core/src/server/ebml.ts b/packages/playwright-core/src/server/ebml.ts index ce84b2ec3672a..cf58b55007b1d 100644 --- a/packages/playwright-core/src/server/ebml.ts +++ b/packages/playwright-core/src/server/ebml.ts @@ -90,7 +90,7 @@ function element(id: Buffer, payload: Buffer): Buffer { // Emits the Matroska header: EBML head, an unknown-size (streaming) Segment, stream Info with a // 1ms timestamp scale, and a single MJPEG video track. Frames follow as Clusters via writeClusterHeader. -export function writeHeader(width: number, height: number): Buffer { +export function writeHeader(): Buffer { const ebml = element(kEBML, Buffer.concat([ element(kEBMLVersion, uint(1)), element(kEBMLReadVersion, uint(1)), @@ -112,11 +112,12 @@ export function writeHeader(width: number, height: number): Buffer { element(kTrackType, uint(1)), // 1 = video. element(kFlagLacing, uint(0)), element(kCodecID, Buffer.from('V_MJPEG')), - // PixelWidth/PixelHeight are advisory: ffmpeg's mjpeg decoder uses the dimensions encoded in - // each JPEG frame, and the output video filters normalize to the requested size. + // PixelWidth/PixelHeight are mandatory, but the actual JPEG dimensions are not known yet. + // A larger placeholder can make ffmpeg mistake a short progressive JPEG for an interlaced + // field, so use 1x1 and let the MJPEG decoder read the real dimensions from each frame. element(kVideo, Buffer.concat([ - element(kPixelWidth, uint(width)), - element(kPixelHeight, uint(height)), + element(kPixelWidth, uint(1)), + element(kPixelHeight, uint(1)), ])), ])); const tracks = element(kTracks, track); diff --git a/packages/playwright-core/src/server/instrumentation.ts b/packages/playwright-core/src/server/instrumentation.ts index c5730e81a84fc..449782e268418 100644 --- a/packages/playwright-core/src/server/instrumentation.ts +++ b/packages/playwright-core/src/server/instrumentation.ts @@ -90,6 +90,7 @@ export type CallMetadata = { type: string; method: string; params: any; + timeout?: number; title?: string; // Client is making an internal call that should not show up in // the inspector or trace. diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts index 90a7018dac724..5fc90c8560c7d 100644 --- a/packages/playwright-core/src/server/trace/recorder/tracing.ts +++ b/packages/playwright-core/src/server/trace/recorder/tracing.ts @@ -738,7 +738,7 @@ function createBeforeActionTraceEvent(metadata: CallMetadata, parentId?: string) title: metadata.title, class: metadata.type, method: metadata.method, - params: metadata.params, + params: metadata.timeout ? { ...metadata.params, timeout: metadata.timeout } : metadata.params, stepId: metadata.stepId, pageId: metadata.pageId, }; diff --git a/packages/playwright-core/src/server/videoRecorder.ts b/packages/playwright-core/src/server/videoRecorder.ts index 292d6d84bef2f..65f47b30fedc3 100644 --- a/packages/playwright-core/src/server/videoRecorder.ts +++ b/packages/playwright-core/src/server/videoRecorder.ts @@ -189,7 +189,7 @@ class FfmpegVideoRecorder { }); this._process = launchedProcess; this._gracefullyClose = gracefullyClose; - launchedProcess.stdin!.write(writeHeader(w, h)); + launchedProcess.stdin!.write(writeHeader()); } writeFrame(frame: Buffer, timestamp: number) { diff --git a/packages/playwright/src/matchers/matchers.ts b/packages/playwright/src/matchers/matchers.ts index 72d86657d73bd..0bdf1b3ac7710 100644 --- a/packages/playwright/src/matchers/matchers.ts +++ b/packages/playwright/src/matchers/matchers.ts @@ -66,11 +66,11 @@ function serializeExpectedTextValues(items: (string | RegExp)[], options: { matc export interface LocatorEx extends Locator { _selector: string; - _expect(expression: string, options: FrameExpectParams, signal: AbortSignal | undefined): Promise; + _expect(expression: string, options: FrameExpectParams): Promise; } export interface FrameEx extends Frame { - _expect(expression: string, options: FrameExpectParams, signal: AbortSignal | undefined): Promise; + _expect(expression: string, options: FrameExpectParams): Promise; } interface APIResponseEx extends APIResponse { @@ -86,7 +86,7 @@ export function toBeAttached( const expected = attached ? 'attached' : 'detached'; const arg = attached ? '' : '{ attached: false }'; return toBeTruthy.call(this, 'toBeAttached', locator, 'Locator', expected, arg, async (isNot, timeout, signal) => { - return await locator._expect(attached ? 'to.be.attached' : 'to.be.detached', { isNot, timeout }, signal); + return await locator._expect(attached ? 'to.be.attached' : 'to.be.detached', { isNot, timeout, signal }); }, options); } @@ -111,7 +111,7 @@ export function toBeChecked( arg = options?.checked === false ? `{ checked: false }` : ''; } return toBeTruthy.call(this, 'toBeChecked', locator, 'Locator', expected, arg, async (isNot, timeout, signal) => { - return await locator._expect('to.be.checked', { isNot, timeout, expectedValue }, signal); + return await locator._expect('to.be.checked', { isNot, timeout, expectedValue, signal }); }, options); } @@ -121,7 +121,7 @@ export function toBeDisabled( options?: { timeout?: number, signal?: AbortSignal }, ) { return toBeTruthy.call(this, 'toBeDisabled', locator, 'Locator', 'disabled', '', async (isNot, timeout, signal) => { - return await locator._expect('to.be.disabled', { isNot, timeout }, signal); + return await locator._expect('to.be.disabled', { isNot, timeout, signal }); }, options); } @@ -134,7 +134,7 @@ export function toBeEditable( const expected = editable ? 'editable' : 'readOnly'; const arg = editable ? '' : '{ editable: false }'; return toBeTruthy.call(this, 'toBeEditable', locator, 'Locator', expected, arg, async (isNot, timeout, signal) => { - return await locator._expect(editable ? 'to.be.editable' : 'to.be.readonly', { isNot, timeout }, signal); + return await locator._expect(editable ? 'to.be.editable' : 'to.be.readonly', { isNot, timeout, signal }); }, options); } @@ -144,7 +144,7 @@ export function toBeEmpty( options?: { timeout?: number, signal?: AbortSignal }, ) { return toBeTruthy.call(this, 'toBeEmpty', locator, 'Locator', 'empty', '', async (isNot, timeout, signal) => { - return await locator._expect('to.be.empty', { isNot, timeout }, signal); + return await locator._expect('to.be.empty', { isNot, timeout, signal }); }, options); } @@ -157,7 +157,7 @@ export function toBeEnabled( const expected = enabled ? 'enabled' : 'disabled'; const arg = enabled ? '' : '{ enabled: false }'; return toBeTruthy.call(this, 'toBeEnabled', locator, 'Locator', expected, arg, async (isNot, timeout, signal) => { - return await locator._expect(enabled ? 'to.be.enabled' : 'to.be.disabled', { isNot, timeout }, signal); + return await locator._expect(enabled ? 'to.be.enabled' : 'to.be.disabled', { isNot, timeout, signal }); }, options); } @@ -167,7 +167,7 @@ export function toBeFocused( options?: { timeout?: number, signal?: AbortSignal }, ) { return toBeTruthy.call(this, 'toBeFocused', locator, 'Locator', 'focused', '', async (isNot, timeout, signal) => { - return await locator._expect('to.be.focused', { isNot, timeout }, signal); + return await locator._expect('to.be.focused', { isNot, timeout, signal }); }, options); } @@ -177,7 +177,7 @@ export function toBeHidden( options?: { timeout?: number, signal?: AbortSignal }, ) { return toBeTruthy.call(this, 'toBeHidden', locator, 'Locator', 'hidden', '', async (isNot, timeout, signal) => { - return await locator._expect('to.be.hidden', { isNot, timeout }, signal); + return await locator._expect('to.be.hidden', { isNot, timeout, signal }); }, options); } @@ -190,7 +190,7 @@ export function toBeVisible( const expected = visible ? 'visible' : 'hidden'; const arg = visible ? '' : '{ visible: false }'; return toBeTruthy.call(this, 'toBeVisible', locator, 'Locator', expected, arg, async (isNot, timeout, signal) => { - return await locator._expect(visible ? 'to.be.visible' : 'to.be.hidden', { isNot, timeout }, signal); + return await locator._expect(visible ? 'to.be.visible' : 'to.be.hidden', { isNot, timeout, signal }); }, options); } @@ -200,7 +200,7 @@ export function toBeInViewport( options?: { timeout?: number, ratio?: number, signal?: AbortSignal }, ) { return toBeTruthy.call(this, 'toBeInViewport', locator, 'Locator', 'in viewport', '', async (isNot, timeout, signal) => { - return await locator._expect('to.be.in.viewport', { isNot, expectedNumber: options?.ratio, timeout }, signal); + return await locator._expect('to.be.in.viewport', { isNot, expectedNumber: options?.ratio, timeout, signal }); }, options); } @@ -213,12 +213,12 @@ export function toContainText( if (Array.isArray(expected)) { return toEqual.call(this, 'toContainText', locator, 'Locator', async (isNot, timeout, signal) => { 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 }, signal); + return await locator._expect('to.contain.text.array', { expectedText, isNot, useInnerText: options.useInnerText, timeout, signal }); }, expected, { ...options, contains: true }); } else { return toMatchText.call(this, 'toContainText', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected], { matchSubstring: true, normalizeWhiteSpace: true, ignoreCase: options.ignoreCase }); - return await locator._expect('to.have.text', { expectedText, isNot, useInnerText: options.useInnerText, timeout }, signal); + return await locator._expect('to.have.text', { expectedText, isNot, useInnerText: options.useInnerText, timeout, signal }); }, expected, { ...options, matchSubstring: true }); } } @@ -231,7 +231,7 @@ export function toHaveAccessibleDescription( ) { return toMatchText.call(this, 'toHaveAccessibleDescription', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected], { ignoreCase: options?.ignoreCase, normalizeWhiteSpace: true }); - return await locator._expect('to.have.accessible.description', { expectedText, isNot, timeout }, signal); + return await locator._expect('to.have.accessible.description', { expectedText, isNot, timeout, signal }); }, expected, options); } @@ -243,7 +243,7 @@ export function toHaveAccessibleName( ) { return toMatchText.call(this, 'toHaveAccessibleName', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected], { ignoreCase: options?.ignoreCase, normalizeWhiteSpace: true }); - return await locator._expect('to.have.accessible.name', { expectedText, isNot, timeout }, signal); + return await locator._expect('to.have.accessible.name', { expectedText, isNot, timeout, signal }); }, expected, options); } @@ -255,7 +255,7 @@ export function toHaveAccessibleErrorMessage( ) { return toMatchText.call(this, 'toHaveAccessibleErrorMessage', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected], { ignoreCase: options?.ignoreCase, normalizeWhiteSpace: true }); - return await locator._expect('to.have.accessible.error.message', { expectedText: expectedText, isNot, timeout }, signal); + return await locator._expect('to.have.accessible.error.message', { expectedText: expectedText, isNot, timeout, signal }); }, expected, options); } @@ -275,12 +275,12 @@ export function toHaveAttribute( } if (expected === undefined) { return toBeTruthy.call(this, 'toHaveAttribute', locator, 'Locator', 'have attribute', '', async (isNot, timeout, signal) => { - return await locator._expect('to.have.attribute', { expressionArg: name, isNot, timeout }, signal); + return await locator._expect('to.have.attribute', { expressionArg: name, isNot, timeout, signal }); }, options); } return toMatchText.call(this, 'toHaveAttribute', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected as (string | RegExp)], { ignoreCase: options?.ignoreCase }); - return await locator._expect('to.have.attribute.value', { expressionArg: name, expectedText, isNot, timeout }, signal); + return await locator._expect('to.have.attribute.value', { expressionArg: name, expectedText, isNot, timeout, signal }); }, expected as (string | RegExp), options); } @@ -293,12 +293,12 @@ export function toHaveClass( if (Array.isArray(expected)) { return toEqual.call(this, 'toHaveClass', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues(expected); - return await locator._expect('to.have.class.array', { expectedText, isNot, timeout }, signal); + return await locator._expect('to.have.class.array', { expectedText, isNot, timeout, signal }); }, expected, options); } else { return toMatchText.call(this, 'toHaveClass', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected]); - return await locator._expect('to.have.class', { expectedText, isNot, timeout }, signal); + return await locator._expect('to.have.class', { expectedText, isNot, timeout, signal }); }, expected, options); } } @@ -314,14 +314,14 @@ export function toContainClass( throw new Error(`"expected" argument in toContainClass cannot contain RegExp values`); return toEqual.call(this, 'toContainClass', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues(expected); - return await locator._expect('to.contain.class.array', { expectedText, isNot, timeout }, signal); + return await locator._expect('to.contain.class.array', { expectedText, isNot, timeout, signal }); }, expected, options); } else { if (isRegExp(expected)) throw new Error(`"expected" argument in toContainClass cannot be a RegExp value`); return toMatchText.call(this, 'toContainClass', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected]); - return await locator._expect('to.contain.class', { expectedText, isNot, timeout }, signal); + return await locator._expect('to.contain.class', { expectedText, isNot, timeout, signal }); }, expected, options); } } @@ -333,7 +333,7 @@ export function toHaveCount( options?: { timeout?: number, signal?: AbortSignal }, ) { return toEqual.call(this, 'toHaveCount', locator, 'Locator', async (isNot, timeout, signal) => { - return await locator._expect('to.have.count', { expectedNumber: expected, isNot, timeout }, signal); + return await locator._expect('to.have.count', { expectedNumber: expected, isNot, timeout, signal }); }, expected, options); } @@ -347,7 +347,7 @@ export function toHaveCSS( ) { return toMatchText.call(this, 'toHaveCSS', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected]); - return await locator._expect('to.have.css', { expressionArg: name, expectedText, isNot, pseudo: options?.pseudo, timeout }, signal); + return await locator._expect('to.have.css', { expressionArg: name, expectedText, isNot, pseudo: options?.pseudo, timeout, signal }); }, expected, options); } @@ -359,7 +359,7 @@ export function toHaveId( ) { return toMatchText.call(this, 'toHaveId', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected]); - return await locator._expect('to.have.id', { expectedText, isNot, timeout }, signal); + return await locator._expect('to.have.id', { expectedText, isNot, timeout, signal }); }, expected, options); } @@ -371,7 +371,7 @@ export function toHaveJSProperty( options?: { timeout?: number, signal?: AbortSignal }, ) { return toEqual.call(this, 'toHaveJSProperty', locator, 'Locator', async (isNot, timeout, signal) => { - return await locator._expect('to.have.property', { expressionArg: name, expectedValue: expected, isNot, timeout }, signal); + return await locator._expect('to.have.property', { expressionArg: name, expectedValue: expected, isNot, timeout, signal }); }, expected, options); } @@ -385,7 +385,7 @@ export function toHaveRole( throw new Error(`"role" argument in toHaveRole must be a string`); return toMatchText.call(this, 'toHaveRole', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected]); - return await locator._expect('to.have.role', { expectedText, isNot, timeout }, signal); + return await locator._expect('to.have.role', { expectedText, isNot, timeout, signal }); }, expected, options); } @@ -398,12 +398,12 @@ export function toHaveText( if (Array.isArray(expected)) { return toEqual.call(this, 'toHaveText', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues(expected, { normalizeWhiteSpace: true, ignoreCase: options.ignoreCase }); - return await locator._expect('to.have.text.array', { expectedText, isNot, useInnerText: options?.useInnerText, timeout }, signal); + return await locator._expect('to.have.text.array', { expectedText, isNot, useInnerText: options?.useInnerText, timeout, signal }); }, expected, options); } else { return toMatchText.call(this, 'toHaveText', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected], { normalizeWhiteSpace: true, ignoreCase: options.ignoreCase }); - return await locator._expect('to.have.text', { expectedText, isNot, useInnerText: options?.useInnerText, timeout }, signal); + return await locator._expect('to.have.text', { expectedText, isNot, useInnerText: options?.useInnerText, timeout, signal }); }, expected, options); } } @@ -416,7 +416,7 @@ export function toHaveValue( ) { return toMatchText.call(this, 'toHaveValue', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected]); - return await locator._expect('to.have.value', { expectedText, isNot, timeout }, signal); + return await locator._expect('to.have.value', { expectedText, isNot, timeout, signal }); }, expected, options); } @@ -428,7 +428,7 @@ export function toHaveValues( ) { return toEqual.call(this, 'toHaveValues', locator, 'Locator', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues(expected); - return await locator._expect('to.have.values', { expectedText, isNot, timeout }, signal); + return await locator._expect('to.have.values', { expectedText, isNot, timeout, signal }); }, expected, options); } @@ -440,7 +440,7 @@ export function toHaveTitle( ) { return toMatchText.call(this, 'toHaveTitle', page, 'Page', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected], { normalizeWhiteSpace: true }); - return await (page.mainFrame() as FrameEx)._expect('to.have.title', { expectedText, isNot, timeout }, signal); + return await (page.mainFrame() as FrameEx)._expect('to.have.title', { expectedText, isNot, timeout, signal }); }, expected, options); } @@ -461,7 +461,7 @@ export function toHaveURL( expected = typeof expected === 'string' ? constructURLBasedOnBaseURL(baseURL, expected) : expected; return toMatchText.call(this, 'toHaveURL', page, 'Page', async (isNot, timeout, signal) => { const expectedText = serializeExpectedTextValues([expected], { ignoreCase: options?.ignoreCase }); - return await (page.mainFrame() as FrameEx)._expect('to.have.url', { expectedText, isNot, timeout }, signal); + return await (page.mainFrame() as FrameEx)._expect('to.have.url', { expectedText, isNot, timeout, signal }); }, expected, options); } diff --git a/packages/playwright/src/matchers/toMatchAriaSnapshot.ts b/packages/playwright/src/matchers/toMatchAriaSnapshot.ts index b7d89048793a9..ab5b13ea0ee9e 100644 --- a/packages/playwright/src/matchers/toMatchAriaSnapshot.ts +++ b/packages/playwright/src/matchers/toMatchAriaSnapshot.ts @@ -91,10 +91,10 @@ export async function toMatchAriaSnapshot( if (globalChildren && !expected.match(/^- \/children:/m)) expected = `- /children: ${globalChildren}\n` + expected; - const expectParams = { expectedValue: expected, isNot: this.isNot, timeout }; + const expectParams = { expectedValue: expected, isNot: this.isNot, timeout, signal: options.signal }; const { matches: pass, received, log, timedOut, errorMessage } = locator ? - await (locator as LocatorEx)._expect('to.match.aria', expectParams, options.signal) : - await ((receiver as Page).mainFrame() as FrameEx)._expect('to.match.aria', expectParams, options.signal); + await (locator as LocatorEx)._expect('to.match.aria', expectParams) : + await ((receiver as Page).mainFrame() as FrameEx)._expect('to.match.aria', expectParams); const typedReceived = received?.value as MatcherReceived; const message = () => { diff --git a/packages/playwright/src/matchers/toMatchSnapshot.ts b/packages/playwright/src/matchers/toMatchSnapshot.ts index 2b7a088c95c8d..2b79dec0674c4 100644 --- a/packages/playwright/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright/src/matchers/toMatchSnapshot.ts @@ -55,6 +55,7 @@ type ToHaveScreenshotOptions = ToHaveScreenshotConfigOptions & { mask?: Array; maskColor?: string; omitBackground?: boolean; + signal?: AbortSignal; }; // Keep in sync with above (begin). @@ -64,6 +65,7 @@ const NonConfigProperties: (keyof ToHaveScreenshotOptions)[] = [ 'mask', 'maskColor', 'omitBackground', + 'signal', ]; // Keep in sync with above (end). @@ -362,6 +364,7 @@ export async function toHaveScreenshot( style, isNot: !!this.isNot, timeout, + signal: helper.options.signal, type: screenshotType, comparator: helper.options.comparator, maxDiffPixels: helper.options.maxDiffPixels, diff --git a/packages/playwright/types/test.d.ts b/packages/playwright/types/test.d.ts index d30c7f240de59..093da901d0804 100644 --- a/packages/playwright/types/test.d.ts +++ b/packages/playwright/types/test.d.ts @@ -9618,6 +9618,13 @@ interface LocatorAssertions { */ scale?: "css"|"device"; + /** + * An optional [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) that can cancel the + * assertion. Aborting the signal fails the assertion like a timeout: if the signal is aborted while the assertion is + * retrying, or is already aborted before the assertion starts, the assertion fails without retrying further. + */ + signal?: AbortSignal; + /** * File name containing the stylesheet to apply while making the screenshot. This is where you can hide dynamic * elements, make elements invisible or change their properties to help you creating repeatable screenshots. This @@ -9714,6 +9721,13 @@ interface LocatorAssertions { */ scale?: "css"|"device"; + /** + * An optional [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) that can cancel the + * assertion. Aborting the signal fails the assertion like a timeout: if the signal is aborted while the assertion is + * retrying, or is already aborted before the assertion starts, the assertion fails without retrying further. + */ + signal?: AbortSignal; + /** * File name containing the stylesheet to apply while making the screenshot. This is where you can hide dynamic * elements, make elements invisible or change their properties to help you creating repeatable screenshots. This @@ -10605,6 +10619,13 @@ export interface PageAssertionsToHaveScreenshotOptions { */ scale?: "css"|"device"; + /** + * An optional [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) that can cancel the + * assertion. Aborting the signal fails the assertion like a timeout: if the signal is aborted while the assertion is + * retrying, or is already aborted before the assertion starts, the assertion fails without retrying further. + */ + signal?: AbortSignal; + /** * File name containing the stylesheet to apply while making the screenshot. This is where you can hide dynamic * elements, make elements invisible or change their properties to help you creating repeatable screenshots. This diff --git a/tests/library/video.spec.ts b/tests/library/video.spec.ts index 3c1cff83e442f..df431af1e858a 100644 --- a/tests/library/video.spec.ts +++ b/tests/library/video.spec.ts @@ -130,6 +130,25 @@ it.describe('screencast', () => { expectRedFrames(videoFile, size); }); + it('should pad a short frame to the video size', async ({ browser }, testInfo) => { + const videoSize = { width: 800, height: 600 }; + const context = await browser.newContext({ + recordVideo: { + dir: testInfo.outputPath(''), + size: videoSize, + }, + viewport: { width: 800, height: 396 }, + }); + const page = await context.newPage(); + await ensureSomeFrames(page); + await context.close(); + + const videoFile = await page.video().path(); + const videoPlayer = new VideoPlayer(videoFile); + expect(videoPlayer.videoWidth).toBe(videoSize.width); + expect(videoPlayer.videoHeight).toBe(videoSize.height); + }); + it('should continue recording main page after popup closes', async ({ browser, browserName }, testInfo) => { it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/30837' }); // Firefox does not have a mobile variant and has a large minimum size (500 on windows and 450 elsewhere). diff --git a/tests/playwright-test/to-have-screenshot.spec.ts b/tests/playwright-test/to-have-screenshot.spec.ts index 79f33f51825cd..22551fa95d1c8 100644 --- a/tests/playwright-test/to-have-screenshot.spec.ts +++ b/tests/playwright-test/to-have-screenshot.spec.ts @@ -60,6 +60,43 @@ test('should fail to screenshot a page with infinite animation', async ({ runInl expect(fs.existsSync(testInfo.outputPath('a.spec.js-snapshots', 'is-a-test-1.png'))).toBe(false); }); +test('should fail like a timeout when aborted', async ({ runInlineTest }) => { + const infiniteAnimationURL = pathToFileURL(path.join(__dirname, '../assets/rotate-z.html')); + const result = await runInlineTest({ + ...playwrightConfig({}), + 'a.spec.js': ` + const { test, expect } = require('@playwright/test'); + test('is a test', async ({ page }) => { + await page.goto('${infiniteAnimationURL}'); + const controller = new AbortController(); + const promise = expect(page).toHaveScreenshot({ animations: 'allow', timeout: 5000, signal: controller.signal }); + await page.waitForTimeout(500); + controller.abort(new Error('stop it')); + await promise; + }); + ` + }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain(`operation was aborted: stop it`); + expect(result.output).not.toContain(`Timeout 5000ms exceeded`); +}); + +test('should fail when already aborted', async ({ runInlineTest }) => { + const result = await runInlineTest({ + ...playwrightConfig({}), + 'a.spec.js': ` + const { test, expect } = require('@playwright/test'); + test('is a test', async ({ page }) => { + const controller = new AbortController(); + controller.abort(new Error('already aborted')); + await expect(page).toHaveScreenshot({ signal: controller.signal }); + }); + ` + }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain(`Error: The assertion was aborted: already aborted`); +}); + test('should disable animations by default', async ({ runInlineTest }, testInfo) => { const cssTransitionURL = pathToFileURL(path.join(__dirname, '../assets/css-transition.html')); const result = await runInlineTest({