diff --git a/packages/isomorphic/abortSignal.ts b/packages/isomorphic/abortSignal.ts index c2d3e176f5446..f8df1295b0849 100644 --- a/packages/isomorphic/abortSignal.ts +++ b/packages/isomorphic/abortSignal.ts @@ -18,3 +18,32 @@ export function assertionAbortedMessage(reason: unknown): string { const detail = reason instanceof Error ? reason.message : reason === undefined || reason === null ? '' : String(reason); return 'The assertion was aborted' + (detail ? `: ${detail}` : ''); } + +export function combineSignals(a?: AbortSignal, b?: AbortSignal): { signal: AbortSignal | undefined, cleanup: () => void } { + const noop = () => {}; + if (!a) + return { signal: b, cleanup: noop }; + if (!b) + return { signal: a, cleanup: noop }; + if (a.aborted) + return { signal: a, cleanup: noop }; + if (b.aborted) + return { signal: b, cleanup: noop }; + + const controller = new AbortController(); + const onA = () => onAbort(a); + const onB = () => onAbort(b); + const cleanup = () => { + a.removeEventListener('abort', onA); + b.removeEventListener('abort', onB); + }; + const onAbort = (source: AbortSignal) => { + controller.abort(source.reason); + cleanup(); + }; + a.addEventListener('abort', onA, { once: true }); + b.addEventListener('abort', onB, { once: true }); + return { signal: controller.signal, cleanup }; +} + +export class TestEndedError extends Error {} diff --git a/packages/isomorphic/index.ts b/packages/isomorphic/index.ts index 4a623a16c3e12..19cc95861becc 100644 --- a/packages/isomorphic/index.ts +++ b/packages/isomorphic/index.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +export * from './abortSignal'; export * from './ariaSnapshot'; export * from './assert'; export * from './base64'; diff --git a/packages/playwright-core/src/client/channelOwner.ts b/packages/playwright-core/src/client/channelOwner.ts index 18d1e7ddc793c..70c542404aeb6 100644 --- a/packages/playwright-core/src/client/channelOwner.ts +++ b/packages/playwright-core/src/client/channelOwner.ts @@ -15,6 +15,7 @@ */ import { getMetainfo } from '@isomorphic/protocolMetainfo'; +import { combineSignals } from '@isomorphic/abortSignal'; import { showInternalStackFrames, stringifyStackFrames } from '@utils/stackTrace'; import { isUnderTest } from '@utils/debug'; import { debugLogger } from '@utils/debugLogger'; @@ -154,16 +155,22 @@ export abstract class ChannelOwner { return await this._wrapApiCall(async apiZone => { const validatedParams = validator(params, '', this._validatorToWireContext()); - if (!apiZone.internal && !apiZone.reported) { - // Reporting/tracing/logging this api call for the first time. - apiZone.reported = true; - this._instrumentation.onApiCallBegin(apiZone, { type: this._type, method: prop, params }); - logApiCall(this._logger, `=> ${apiZone.apiName} started`); - return await this._connection.sendMessageToServer(this, prop, validatedParams, { ...apiZone, signal }); + const def = this._connection._defaultOperationSignal; + const { signal: effectiveSignal, cleanup } = combineSignals(signal, def?.aborted ? undefined : def); + try { + if (!apiZone.internal && !apiZone.reported) { + // Reporting/tracing/logging this api call for the first time. + apiZone.reported = true; + this._instrumentation.onApiCallBegin(apiZone, { type: this._type, method: prop, params }); + logApiCall(this._logger, `=> ${apiZone.apiName} started`); + return await this._connection.sendMessageToServer(this, prop, validatedParams, { ...apiZone, signal: effectiveSignal }); + } + // Since this api call is either internal, or has already been reported/traced once, + // passing as internal. + return await this._connection.sendMessageToServer(this, prop, validatedParams, { internal: true, signal: effectiveSignal }); + } finally { + cleanup(); } - // Since this api call is either internal, or has already been reported/traced once, - // passing as internal. - return await this._connection.sendMessageToServer(this, prop, validatedParams, { internal: true, signal }); }, { internal }); }; } diff --git a/packages/playwright-core/src/client/connection.ts b/packages/playwright-core/src/client/connection.ts index db9de249a081c..cdb711be08c5b 100644 --- a/packages/playwright-core/src/client/connection.ts +++ b/packages/playwright-core/src/client/connection.ts @@ -15,6 +15,7 @@ */ import colors from 'colors/safe'; +import { TestEndedError } from '@isomorphic/abortSignal'; import { rewriteErrorMessage } from '@utils/stackTrace'; import { isUnderTest } from '@utils/debug'; import { debugLogger } from '@utils/debugLogger'; @@ -87,6 +88,8 @@ export class Connection extends EventEmitter { readonly headers: HeadersArray; private _objectFactories = new Map(); + _defaultOperationSignal: AbortSignal | undefined; + constructor(localUtils?: LocalUtils, instrumentation?: ClientInstrumentation, headers: HeadersArray = []) { super(); this._instrumentation = instrumentation || createInstrumentation(); @@ -246,6 +249,9 @@ export class Connection extends EventEmitter { const parsedError = parseError(error); if (callback.signal?.aborted && parsedError instanceof AbortError) parsedError.cause = callback.signal.reason; + // hack to preserve error message strings from @playwright/test + if (parsedError.cause instanceof TestEndedError) + rewriteErrorMessage(parsedError, parsedError.cause.message); parsedError.log = log || []; rewriteErrorMessage(parsedError, parsedError.message + formatCallLog(log)); const detailsValidator = maybeFindValidator(callback.type, callback.method, 'ErrorDetails'); diff --git a/packages/playwright-core/src/client/playwright.ts b/packages/playwright-core/src/client/playwright.ts index f2bd2773f4dca..6caefcbac06ac 100644 --- a/packages/playwright-core/src/client/playwright.ts +++ b/packages/playwright-core/src/client/playwright.ts @@ -42,6 +42,14 @@ export class Playwright extends ChannelOwner { _defaultContextTimeout?: number; _defaultContextNavigationTimeout?: number; + get _defaultOperationSignal(): AbortSignal | undefined { + return this._connection._defaultOperationSignal; + } + + set _defaultOperationSignal(signal: AbortSignal | undefined) { + this._connection._defaultOperationSignal = signal; + } + constructor(parent: ChannelOwner, type: string, guid: string, initializer: channels.PlaywrightInitializer) { super(parent, type, guid, initializer); this.request = new APIRequest(this); diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index 34f9d6dedaba3..08cb44a24e296 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -384,9 +384,11 @@ const playwrightFixtures: Fixtures { @@ -420,8 +422,7 @@ const playwrightFixtures: Fixtures(); + readonly _operationAbortController = new AbortController(); _lastStepId = 0; private readonly _requireFile: string; readonly _projectInternal: commonConfig.FullProjectInternal; @@ -404,6 +406,11 @@ export class TestInfoImpl implements TestInfo { throw new TestAbortError('Test aborted' + (message ? ': ' + message : '')); } + _abortOperations(reason: string) { + if (!this._operationAbortController.signal.aborted) + this._operationAbortController.abort(new TestEndedError(reason)); + } + _interrupt() { // Mark as interrupted so we can ignore TimeoutError thrown by interrupt() call. this._interruptedPromise.resolve(); diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index efb16b9d71d7e..b188b0c0ae36b 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -452,6 +452,9 @@ export class WorkerMain extends ProcessRunner { testInfo._tracing.didFinishTestFunctionAndAfterEachHooks(); + const abortReason = testInfo.status === 'timedOut' ? 'Test timeout of ' + testInfo.timeout + 'ms exceeded.' : 'Test ended.'; + testInfo._abortOperations(abortReason); + try { // Teardown test-scoped fixtures. Attribute to 'test' so that users understand // they should probably increase the test timeout to fix this issue. diff --git a/tests/components/ct-react-vite/tests/route.spec.tsx b/tests/components/ct-react-vite/tests/route.spec.tsx index 4368a4d41e17b..e287f3a4dee19 100644 --- a/tests/components/ct-react-vite/tests/route.spec.tsx +++ b/tests/components/ct-react-vite/tests/route.spec.tsx @@ -50,8 +50,10 @@ test.describe('request handlers', () => { respond(); await expect(component.getByTestId('name')).toHaveText('John Doe'); + const postResponse = page.waitForResponse('**/post'); await component.getByRole('button', { name: 'Post it' }).click(); expect(await postBody).toBe('hello from the page'); + await postResponse; }); test('should add dynamically', async ({ page, mount, router }) => { @@ -179,4 +181,3 @@ test.describe('request handlers', () => { }); }); - diff --git a/tests/library/page-close.spec.ts b/tests/library/page-close.spec.ts index 5569dfbcad11e..e54d2d18a841a 100644 --- a/tests/library/page-close.spec.ts +++ b/tests/library/page-close.spec.ts @@ -211,11 +211,15 @@ test('should not throw when continuing while page is closing', async ({ page, se test('should not throw when continuing after page is closed', async ({ page, server }) => { let done; + let routeHandledCallback = () => {}; + const routeHandled = new Promise(f => routeHandledCallback = f); await page.route('**/*', async route => { await page.close(); done = route.continue(); + routeHandledCallback(); }); const error = await page.goto(server.EMPTY_PAGE).catch(e => e); + await routeHandled; await done; expect(error).toBeInstanceOf(Error); }); diff --git a/tests/page/interception.spec.ts b/tests/page/interception.spec.ts index 0d3fbdd02e074..89ec0efb243ca 100644 --- a/tests/page/interception.spec.ts +++ b/tests/page/interception.spec.ts @@ -23,12 +23,14 @@ const { globToRegexPattern, urlMatches } = iso; it('should work with navigation @smoke', async ({ page, server }) => { const requests = new Map(); + const routeContinuations: Promise[] = []; await page.route('**/*', route => { requests.set(route.request().url().split('/').pop(), route.request()); - void route.continue(); + routeContinuations.push(route.continue()); }); server.setRedirect('/rrredirect', '/frames/one-frame.html'); await page.goto(server.PREFIX + '/rrredirect'); + await Promise.all(routeContinuations); expect(requests.get('rrredirect').isNavigationRequest()).toBe(true); expect(requests.get('frame.html').isNavigationRequest()).toBe(true); expect(requests.get('script.js').isNavigationRequest()).toBe(false); diff --git a/tests/playwright-test/expect.spec.ts b/tests/playwright-test/expect.spec.ts index 25bf26e86b382..42b538761f0b4 100644 --- a/tests/playwright-test/expect.spec.ts +++ b/tests/playwright-test/expect.spec.ts @@ -565,7 +565,7 @@ test('should print expected/received before timeout', async ({ runInlineTest }) expect(result.failed).toBe(1); expect(result.output).toContain('Test timeout of 2000ms exceeded.'); expect(result.output).toContain('Expected: "Text 2"'); - expect(result.output).toContain('Received: "Text content"'); + expect(result.output).toContain('Error: The assertion was aborted: Test timeout of 2000ms exceeded.'); }); test('should print pending operations for toHaveText', async ({ runInlineTest }) => { @@ -584,7 +584,7 @@ test('should print pending operations for toHaveText', async ({ runInlineTest }) const output = result.output; expect(output).toContain(`expect(locator).toHaveText(expected)`); expect(output).toContain('Expected: "Text"'); - expect(output).toContain('Error: element(s) not found'); + expect(output).toContain('Error: The assertion was aborted: Test timeout of 2000ms exceeded.'); expect(output).toContain('waiting for locator(\'no-such-thing\')'); }); @@ -640,7 +640,7 @@ test('should print expected/received on Ctrl+C', async ({ interactWithTestRunner expect(result.passed).toBe(0); expect(result.interrupted).toBe(1); expect(result.output).toContain('Expected: "Text 2"'); - expect(result.output).toContain('Received: "Text content"'); + expect(result.output).toContain('Error: The assertion was aborted: Test ended.'); }); test('should not print timed out error message when test times out', async ({ runInlineTest }) => { diff --git a/tests/playwright-test/test-step.spec.ts b/tests/playwright-test/test-step.spec.ts index 0a994c6d8576f..df27dd58d1850 100644 --- a/tests/playwright-test/test-step.spec.ts +++ b/tests/playwright-test/test-step.spec.ts @@ -630,14 +630,13 @@ pw:api | Launch browser pw:api | Create page @ a.test.ts:6 pw:api |Set content @ a.test.ts:15 pw:api |Click locator('div') @ a.test.ts:16 -pw:api |↪ error: Error: page.click: Target page, context or browser has been closed hook |After Hooks hook | afterAll hook @ a.test.ts:9 pw:api | Close context @ a.test.ts:10 hook |Worker Cleanup fixture | Fixture "browser" |Test timeout of 2000ms exceeded. - |Error: page.click: Target page, context or browser has been closed + |AbortError: page.click: Test timeout of 2000ms exceeded. `); });