diff --git a/packages/playwright-core/src/client/tracing.ts b/packages/playwright-core/src/client/tracing.ts index 187e1980b3887..7330cd9f269bd 100644 --- a/packages/playwright-core/src/client/tracing.ts +++ b/packages/playwright-core/src/client/tracing.ts @@ -34,8 +34,8 @@ export class Tracing extends ChannelOwner implements ap } async start(options: { name?: string, title?: string, snapshots?: boolean, screenshots?: boolean, sources?: boolean, _live?: boolean } = {}) { - await this._wrapApiCall(async () => { - this._includeSources = !!options.sources; + this._includeSources = !!options.sources; + const traceName = await this._wrapApiCall(async () => { await this._channel.tracingStart({ name: options.name, snapshots: options.snapshots, @@ -43,15 +43,14 @@ export class Tracing extends ChannelOwner implements ap live: options._live, }); const response = await this._channel.tracingStartChunk({ name: options.name, title: options.title }); - await this._startCollectingStacks(response.traceName); + return response.traceName; }, true); + await this._startCollectingStacks(traceName); } async startChunk(options: { name?: string, title?: string } = {}) { - await this._wrapApiCall(async () => { - const { traceName } = await this._channel.tracingStartChunk(options); - await this._startCollectingStacks(traceName); - }, true); + const { traceName } = await this._channel.tracingStartChunk(options); + await this._startCollectingStacks(traceName); } private async _startCollectingStacks(traceName: string) { diff --git a/packages/playwright/src/common/globals.ts b/packages/playwright/src/common/globals.ts index 5b7abef1a94d4..746e1ec847874 100644 --- a/packages/playwright/src/common/globals.ts +++ b/packages/playwright/src/common/globals.ts @@ -42,19 +42,3 @@ export function setIsWorkerProcess() { export function isWorkerProcess() { return _isWorkerProcess; } - -export interface TestLifecycleInstrumentation { - onTestBegin?(): Promise; - onTestFunctionEnd?(): Promise; - onTestEnd?(): Promise; -} - -let _testLifecycleInstrumentation: TestLifecycleInstrumentation | undefined; - -export function setTestLifecycleInstrumentation(instrumentation: TestLifecycleInstrumentation | undefined) { - _testLifecycleInstrumentation = instrumentation; -} - -export function testLifecycleInstrumentation() { - return _testLifecycleInstrumentation; -} diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index baeb3f9c4c629..0fb3134eaceef 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -16,14 +16,15 @@ import * as fs from 'fs'; import * as path from 'path'; -import type { APIRequestContext, BrowserContext, Browser, BrowserContextOptions, LaunchOptions, Page, Tracing, Video, PageScreenshotOptions } from 'playwright-core'; +import type { APIRequestContext, BrowserContext, Browser, BrowserContextOptions, LaunchOptions, Page, Tracing, Video } from 'playwright-core'; +import * as playwrightLibrary from 'playwright-core'; import { createGuid, debugMode, addInternalStackPrefix, isString, asLocator, jsonStringifyForceASCII } from 'playwright-core/lib/utils'; import type { Fixtures, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, ScreenshotMode, TestInfo, TestType, VideoMode } from '../types/test'; import type { TestInfoImpl } from './worker/testInfo'; import { rootTestType } from './common/testType'; import type { ContextReuseMode } from './common/config'; import type { ClientInstrumentation, ClientInstrumentationListener } from '../../playwright-core/src/client/clientInstrumentation'; -import { currentTestInfo, setTestLifecycleInstrumentation, type TestLifecycleInstrumentation } from './common/globals'; +import { currentTestInfo } from './common/globals'; export { expect } from './matchers/expect'; export const _baseTest: TestType<{}, {}> = rootTestType.test; @@ -44,12 +45,11 @@ if ((process as any)['__pw_initiator__']) { type TestFixtures = PlaywrightTestArgs & PlaywrightTestOptions & { _combinedContextOptions: BrowserContextOptions, _setupContextOptions: void; + _setupArtifacts: void; _contextFactory: (options?: BrowserContextOptions) => Promise; }; type WorkerFixtures = PlaywrightWorkerArgs & PlaywrightWorkerOptions & { - // Same as "playwright", but exposed so that our internal tests can override it. - _playwrightImpl: PlaywrightWorkerArgs['playwright']; _browserOptions: LaunchOptions; _optionContextReuseMode: ContextReuseMode, _optionConnectOptions: PlaywrightWorkerOptions['connectOptions'], @@ -59,14 +59,9 @@ type WorkerFixtures = PlaywrightWorkerArgs & PlaywrightWorkerOptions & { const playwrightFixtures: Fixtures = ({ defaultBrowserType: ['chromium', { scope: 'worker', option: true }], browserName: [({ defaultBrowserType }, use) => use(defaultBrowserType), { scope: 'worker', option: true }], - _playwrightImpl: [({}, use) => use(require('playwright-core')), { scope: 'worker' }], - - playwright: [async ({ _playwrightImpl, screenshot }, use) => { - await connector.setPlaywright(_playwrightImpl, screenshot); - await use(_playwrightImpl); - await connector.setPlaywright(undefined, screenshot); + playwright: [async ({}, use) => { + await use(require('playwright-core')); }, { scope: 'worker', _hideStep: true } as any], - headless: [({ launchOptions }, use) => use(launchOptions.headless ?? true), { scope: 'worker', option: true }], channel: [({ launchOptions }, use) => use(launchOptions.channel), { scope: 'worker', option: true }], launchOptions: [{}, { scope: 'worker', option: true }], @@ -227,7 +222,7 @@ const playwrightFixtures: Fixtures = ({ _setupContextOptions: [async ({ playwright, _combinedContextOptions, actionTimeout, navigationTimeout, testIdAttribute }, use, testInfo) => { if (testIdAttribute) - playwright.selectors.setTestIdAttribute(testIdAttribute); + playwrightLibrary.selectors.setTestIdAttribute(testIdAttribute); testInfo.snapshotSuffix = process.platform; if (debugMode()) testInfo.setTimeout(0); @@ -248,6 +243,58 @@ const playwrightFixtures: Fixtures = ({ } }, { auto: 'all-hooks-included', _title: 'context configuration' } as any], + _setupArtifacts: [async ({ playwright, screenshot }, use, testInfo) => { + const artifactsRecorder = new ArtifactsRecorder(playwright, tracing().artifactsDir(), screenshot); + await artifactsRecorder.willStartTest(testInfo as TestInfoImpl); + const csiListener: ClientInstrumentationListener = { + onApiCallBegin: (apiName: string, params: Record, frames: StackFrame[], userData: any, out: { stepId?: string }) => { + const testInfo = currentTestInfo(); + if (!testInfo || apiName.includes('setTestIdAttribute')) + return { userObject: null }; + const step = testInfo._addStep({ + location: frames[0] as any, + category: 'pw:api', + title: renderApiCall(apiName, params), + apiName, + params, + }); + userData.userObject = step; + out.stepId = step.stepId; + }, + onApiCallEnd: (userData: any, error?: Error) => { + const step = userData.userObject; + step?.complete({ error }); + }, + onWillPause: () => { + currentTestInfo()?.setTimeout(0); + }, + runAfterCreateBrowserContext: async (context: BrowserContext) => { + await artifactsRecorder?.didCreateBrowserContext(context); + const testInfo = currentTestInfo(); + if (testInfo) + attachConnectedHeaderIfNeeded(testInfo, context.browser()); + }, + runAfterCreateRequestContext: async (context: APIRequestContext) => { + await artifactsRecorder?.didCreateRequestContext(context); + }, + runBeforeCloseBrowserContext: async (context: BrowserContext) => { + await artifactsRecorder?.willCloseBrowserContext(context); + }, + runBeforeCloseRequestContext: async (context: APIRequestContext) => { + await artifactsRecorder?.willCloseRequestContext(context); + }, + }; + + const clientInstrumentation = (playwright as any)._instrumentation as ClientInstrumentation; + clientInstrumentation.addListener(csiListener); + + await use(); + + clientInstrumentation.removeListener(csiListener); + await artifactsRecorder.didFinishTest(); + + }, { auto: 'all-hooks-included', _title: 'trace recording' } as any], + _contextFactory: [async ({ browser, video, _reuseContext }, use, testInfo) => { const testInfoImpl = testInfo as TestInfoImpl; const videoMode = normalizeVideoMode(video); @@ -424,7 +471,7 @@ class ArtifactsRecorder { private _playwright: Playwright; private _artifactsDir: string; private _screenshotMode: ScreenshotMode; - private _screenshotOptions: { mode: ScreenshotMode } & Pick | undefined; + private _screenshotOptions: { mode: ScreenshotMode } & Pick | undefined; private _temporaryScreenshots: string[] = []; private _temporaryArtifacts: string[] = []; private _reusedContexts = new Set(); @@ -449,6 +496,7 @@ class ArtifactsRecorder { async willStartTest(testInfo: TestInfoImpl) { this._testInfo = testInfo; + testInfo._onDidFinishTestFunction = () => this.didFinishTestFunction(); // Since beforeAll(s), test and afterAll(s) reuse the same TestInfo, make sure we do not // overwrite previous screenshots. @@ -630,101 +678,6 @@ function tracing() { return (test.info() as TestInfoImpl)._tracing; } -class InstrumentationConnector implements TestLifecycleInstrumentation, ClientInstrumentationListener { - private _playwright: PlaywrightWorkerArgs['playwright'] | undefined; - private _screenshot: ScreenshotOption = 'off'; - private _artifactsRecorder: ArtifactsRecorder | undefined; - private _testIsRunning = false; - - constructor() { - setTestLifecycleInstrumentation(this); - } - - async setPlaywright(playwright: PlaywrightWorkerArgs['playwright'] | undefined, screenshot: ScreenshotOption) { - if (this._playwright) { - if (this._testIsRunning) { - // When "playwright" is destroyed during a test, collect artifacts immediately. - await this.onTestEnd(); - } - const clientInstrumentation = (this._playwright as any)._instrumentation as ClientInstrumentation; - clientInstrumentation.removeListener(this); - } - this._playwright = playwright; - this._screenshot = screenshot; - if (this._playwright) { - const clientInstrumentation = (this._playwright as any)._instrumentation as ClientInstrumentation; - clientInstrumentation.addListener(this); - if (this._testIsRunning) { - // When "playwright" is created during a test, wire it up immediately. - await this.onTestBegin(); - } - } - } - - async onTestBegin() { - this._testIsRunning = true; - if (this._playwright) { - this._artifactsRecorder = new ArtifactsRecorder(this._playwright, tracing().artifactsDir(), this._screenshot); - await this._artifactsRecorder.willStartTest(currentTestInfo() as TestInfoImpl); - } - } - - async onTestFunctionEnd() { - await this._artifactsRecorder?.didFinishTestFunction(); - } - - async onTestEnd() { - await this._artifactsRecorder?.didFinishTest(); - this._artifactsRecorder = undefined; - this._testIsRunning = false; - } - - onApiCallBegin(apiName: string, params: Record, frames: StackFrame[], userData: any, out: { stepId?: string }) { - const testInfo = currentTestInfo(); - if (!testInfo || apiName.includes('setTestIdAttribute')) - return { userObject: null }; - const step = testInfo._addStep({ - location: frames[0] as any, - category: 'pw:api', - title: renderApiCall(apiName, params), - apiName, - params, - }); - userData.userObject = step; - out.stepId = step.stepId; - } - - onApiCallEnd(userData: any, error?: Error) { - const step = userData.userObject; - step?.complete({ error }); - } - - onWillPause() { - currentTestInfo()?.setTimeout(0); - } - - async runAfterCreateBrowserContext(context: BrowserContext) { - await this._artifactsRecorder?.didCreateBrowserContext(context); - const testInfo = currentTestInfo(); - if (testInfo) - attachConnectedHeaderIfNeeded(testInfo, context.browser()); - } - - async runAfterCreateRequestContext(context: APIRequestContext) { - await this._artifactsRecorder?.didCreateRequestContext(context); - } - - async runBeforeCloseBrowserContext(context: BrowserContext) { - await this._artifactsRecorder?.willCloseBrowserContext(context); - } - - async runBeforeCloseRequestContext(context: APIRequestContext) { - await this._artifactsRecorder?.willCloseRequestContext(context); - } -} - -const connector = new InstrumentationConnector(); - export const test = _baseTest.extend(playwrightFixtures); export { defineConfig } from './common/configLoader'; diff --git a/packages/playwright/src/worker/testInfo.ts b/packages/playwright/src/worker/testInfo.ts index b6e57ee36eccc..1e9e1f9c46ee1 100644 --- a/packages/playwright/src/worker/testInfo.ts +++ b/packages/playwright/src/worker/testInfo.ts @@ -68,6 +68,7 @@ export class TestInfoImpl implements TestInfo { readonly _projectInternal: FullProjectInternal; readonly _configInternal: FullConfigInternal; private readonly _steps: TestStepInternal[] = []; + _onDidFinishTestFunction: (() => Promise) | undefined; private readonly _stages: TestStage[] = []; _hasNonRetriableError = false; _hasUnhandledError = false; diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index 366af91c97c81..4b0d19daacffd 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -17,7 +17,7 @@ import { colors } from 'playwright-core/lib/utilsBundle'; import { debugTest, relativeFilePath, serializeError } from '../util'; import { type TestBeginPayload, type TestEndPayload, type RunPayload, type DonePayload, type WorkerInitParams, type TeardownErrorsPayload, stdioChunkToParams } from '../common/ipc'; -import { setCurrentTestInfo, setIsWorkerProcess, testLifecycleInstrumentation } from '../common/globals'; +import { setCurrentTestInfo, setIsWorkerProcess } from '../common/globals'; import { deserializeConfig } from '../common/configLoader'; import type { Suite, TestCase } from '../common/test'; import type { Annotation, FullConfigInternal, FullProjectInternal } from '../common/config'; @@ -304,11 +304,10 @@ export class WorkerMain extends ProcessRunner { if (this._lastRunningTests.length > 10) this._lastRunningTests.shift(); let shouldRunAfterEachHooks = false; - const tracingSlot = { timeout: this._project.project.timeout, elapsed: 0 }; testInfo._allowSkips = true; await testInfo._runAsStage({ title: 'setup and test' }, async () => { - await testInfo._runAsStage({ title: 'start tracing', runnable: { type: 'test', slot: tracingSlot } }, async () => { + await testInfo._runAsStage({ title: 'start tracing', runnable: { type: 'test' } }, async () => { // Ideally, "trace" would be an config-level option belonging to the // test runner instead of a fixture belonging to Playwright. // However, for backwards compatibility, we have to read it from a fixture today. @@ -319,7 +318,6 @@ export class WorkerMain extends ProcessRunner { if (typeof traceFixtureRegistration.fn === 'function') throw new Error(`"trace" option cannot be a function`); await testInfo._tracing.startIfNeeded(traceFixtureRegistration.fn); - await testLifecycleInstrumentation()?.onTestBegin?.(); }); if (this._isStopped || isSkipped) { @@ -374,10 +372,10 @@ export class WorkerMain extends ProcessRunner { try { // Run "immediately upon test function finish" callback. - await testInfo._runAsStage({ title: 'on-test-function-finish', runnable: { type: 'test', slot: tracingSlot } }, async () => { - await testLifecycleInstrumentation()?.onTestFunctionEnd?.(); - }); + await testInfo._runAsStage({ title: 'on-test-function-finish', runnable: { type: 'test', slot: afterHooksSlot } }, async () => testInfo._onDidFinishTestFunction?.()); } catch (error) { + if (error instanceof TimeoutManagerError) + didTimeoutInAfterHooks = true; firstAfterHooksError = firstAfterHooksError ?? error; } @@ -460,8 +458,8 @@ export class WorkerMain extends ProcessRunner { }).catch(() => {}); // Ignore the top-level error, it is already inside TestInfo.errors. } + const tracingSlot = { timeout: this._project.project.timeout, elapsed: 0 }; await testInfo._runAsStage({ title: 'stop tracing', runnable: { type: 'test', slot: tracingSlot } }, async () => { - await testLifecycleInstrumentation()?.onTestEnd?.(); await testInfo._tracing.stopIfNeeded(); }).catch(() => {}); // Ignore the top-level error, it is already inside TestInfo.errors. diff --git a/tests/config/testModeFixtures.ts b/tests/config/testModeFixtures.ts index 40b199671957c..6b6feff7c2913 100644 --- a/tests/config/testModeFixtures.ts +++ b/tests/config/testModeFixtures.ts @@ -30,12 +30,11 @@ export type TestModeTestFixtures = { export type TestModeWorkerFixtures = { toImplInWorkerScope: (rpcObject?: any) => any; playwright: typeof import('@playwright/test'); - _playwrightImpl: typeof import('@playwright/test'); }; export const testModeTest = test.extend({ mode: ['default', { scope: 'worker', option: true }], - _playwrightImpl: [async ({ mode }, run) => { + playwright: [async ({ mode }, run) => { const testMode = { 'default': new DefaultTestMode(), 'service': new DefaultTestMode(), diff --git a/tests/playwright-test/playwright.artifacts.spec.ts b/tests/playwright-test/playwright.artifacts.spec.ts index a7c0699c9d306..b666aa4b70e6d 100644 --- a/tests/playwright-test/playwright.artifacts.spec.ts +++ b/tests/playwright-test/playwright.artifacts.spec.ts @@ -151,8 +151,10 @@ test('should work with screenshot: on', async ({ runInlineTest }, testInfo) => { ' test-finished-1.png', 'artifacts-shared-shared-failing', ' test-failed-1.png', + ' test-failed-2.png', 'artifacts-shared-shared-passing', ' test-finished-1.png', + ' test-finished-2.png', 'artifacts-two-contexts', ' test-finished-1.png', ' test-finished-2.png', @@ -183,6 +185,7 @@ test('should work with screenshot: only-on-failure', async ({ runInlineTest }, t ' test-failed-1.png', 'artifacts-shared-shared-failing', ' test-failed-1.png', + ' test-failed-2.png', 'artifacts-two-contexts-failing', ' test-failed-1.png', ' test-failed-2.png', diff --git a/tests/playwright-test/playwright.trace.spec.ts b/tests/playwright-test/playwright.trace.spec.ts index 13af444edffb1..3e441fbf19ce6 100644 --- a/tests/playwright-test/playwright.trace.spec.ts +++ b/tests/playwright-test/playwright.trace.spec.ts @@ -569,19 +569,14 @@ test('should opt out of attachments', async ({ runInlineTest, server }, testInfo expect([...trace.resources.keys()].filter(f => f.startsWith('resources/'))).toHaveLength(0); }); -test('should record with custom page fixture that closes the context', async ({ runInlineTest }, testInfo) => { - // Note that original issue did not close the context, but we do not support such usecase. - test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23220' }); - +test('should record with custom page fixture', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ 'a.spec.ts': ` import { test as base, expect } from '@playwright/test'; const test = base.extend({ myPage: async ({ browser }, use) => { - const page = await browser.newPage(); - await use(page); - await page.close(); + await use(await browser.newPage()); }, }); @@ -1118,120 +1113,35 @@ test('trace:retain-on-first-failure should create trace if request context is di expect(result.failed).toBe(1); }); -test('should record trace in workerStorageState', async ({ runInlineTest }) => { - test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/30287' }); +test('should record trace for manually created context in a failed test', async ({ runInlineTest }) => { + test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/31541' }); const result = await runInlineTest({ 'a.spec.ts': ` - import { test as base, expect } from '@playwright/test'; - const test = base.extend({ - storageState: ({ workerStorageState }, use) => use(workerStorageState), - workerStorageState: [async ({ browser }, use) => { - const page = await browser.newPage({ storageState: undefined }); - await page.setContent('
hello
'); - await page.close(); - await use(undefined); - }, { scope: 'worker' }], - }) - test('pass', async ({ page }) => { - await page.goto('data:text/html,
hi
'); + import { test, expect } from '@playwright/test'; + test('fail', async ({ browser }) => { + const page = await browser.newPage(); + await page.setContent(''); + expect(1).toBe(2); }); `, }, { trace: 'on' }); - expect(result.exitCode).toBe(0); - expect(result.passed).toBe(1); - - const tracePath = test.info().outputPath('test-results', 'a-pass', 'trace.zip'); - const trace = await parseTrace(tracePath); - expect(trace.actionTree).toEqual([ - 'Before Hooks', - ' fixture: browser', - ' browserType.launch', - ' fixture: workerStorageState', - ' browser.newPage', - ' page.setContent', - ' page.close', - ' fixture: context', - ' browser.newContext', - ' fixture: page', - ' browserContext.newPage', - 'page.goto', - 'After Hooks', - ' fixture: page', - ' fixture: context', - ]); -}); - -test('should record trace after fixture teardown timeout', async ({ runInlineTest }) => { - test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/30718' }); - - const result = await runInlineTest({ - 'a.spec.ts': ` - import { test as base, expect } from '@playwright/test'; - const test = base.extend({ - fixture: async ({}, use) => { - await use('foo'); - await new Promise(() => {}); - }, - }) - test('fails', async ({ fixture, page }) => { - await page.evaluate(() => console.log('from the page')); - }); - `, - }, { trace: 'on', timeout: '4000' }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); - const tracePath = test.info().outputPath('test-results', 'a-fails', 'trace.zip'); + const tracePath = test.info().outputPath('test-results', 'a-fail', 'trace.zip'); const trace = await parseTrace(tracePath); expect(trace.actionTree).toEqual([ 'Before Hooks', - ' fixture: fixture', ' fixture: browser', ' browserType.launch', - ' fixture: context', - ' browser.newContext', - ' fixture: page', - ' browserContext.newPage', - 'page.evaluate', + 'browser.newPage', + 'page.setContent', + 'expect.toBe', 'After Hooks', - ' fixture: page', - ' fixture: context', - ' fixture: fixture', 'Worker Cleanup', ' fixture: browser', ]); // Check console events to make sure that library trace is recorded. expect(trace.events).toContainEqual(expect.objectContaining({ type: 'console', text: 'from the page' })); }); - -test('should take a screenshot-on-failure in workerStorageState', async ({ runInlineTest }) => { - test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/30959' }); - - const result = await runInlineTest({ - 'playwright.config.ts': ` - export default { - use: { - screenshot: 'only-on-failure', - }, - }; - `, - 'a.spec.ts': ` - import { test as base, expect } from '@playwright/test'; - const test = base.extend({ - storageState: ({ workerStorageState }, use) => use(workerStorageState), - workerStorageState: [async ({ browser }, use) => { - const page = await browser.newPage({ storageState: undefined }); - await page.setContent('hello world!'); - throw new Error('Failed!'); - await use(undefined); - }, { scope: 'worker' }], - }) - test('fail', async ({ page }) => { - }); - `, - }); - expect(result.exitCode).toBe(1); - expect(result.failed).toBe(1); - expect(fs.existsSync(test.info().outputPath('test-results', 'a-fail', 'test-failed-1.png'))).toBeTruthy(); -});