diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index 2887577da1d6d..156dc1c1c2874 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -19,13 +19,12 @@ import * as path from 'path'; 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, TraceMode, VideoMode } from '../types/test'; +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 } from './common/globals'; -import { mergeTraceFiles } from './worker/testTracing'; export { expect } from './matchers/expect'; export { store as _store } from './store'; export const _baseTest: TestType<{}, {}> = rootTestType.test; @@ -54,7 +53,6 @@ type TestFixtures = PlaywrightTestArgs & PlaywrightTestOptions & { }; type WorkerFixtures = PlaywrightWorkerArgs & PlaywrightWorkerOptions & { _browserOptions: LaunchOptions; - _artifactsDir: string; }; const playwrightFixtures: Fixtures = ({ @@ -73,11 +71,7 @@ const playwrightFixtures: Fixtures = ({ video: ['off', { scope: 'worker', option: true }], trace: ['off', { scope: 'worker', option: true }], - _artifactsDir: [async ({}, use) => { - await use(process.env.TEST_ARTIFACTS_DIR!); - }, { scope: 'worker', _title: 'playwright configuration' } as any], - - _browserOptions: [async ({ playwright, headless, channel, launchOptions, _artifactsDir }, use) => { + _browserOptions: [async ({ playwright, headless, channel, launchOptions }, use) => { const options: LaunchOptions = { handleSIGINT: false, ...launchOptions, @@ -86,7 +80,7 @@ const playwrightFixtures: Fixtures = ({ options.headless = headless; if (channel !== undefined) options.channel = channel; - options.tracesDir = path.join(_artifactsDir, 'traces'); + options.tracesDir = tracing().tracesDir(); for (const browserType of [playwright.chromium, playwright.firefox, playwright.webkit]) (browserType as any)._defaultLaunchOptions = options; @@ -225,7 +219,7 @@ const playwrightFixtures: Fixtures = ({ }); }, - _setupContextOptions: [async ({ playwright, _combinedContextOptions, _artifactsDir, actionTimeout, navigationTimeout, testIdAttribute }, use, testInfo) => { + _setupContextOptions: [async ({ playwright, _combinedContextOptions, actionTimeout, navigationTimeout, testIdAttribute }, use, testInfo) => { if (testIdAttribute) playwrightLibrary.selectors.setTestIdAttribute(testIdAttribute); testInfo.snapshotSuffix = process.platform; @@ -237,7 +231,7 @@ const playwrightFixtures: Fixtures = ({ (browserType as any)._defaultContextNavigationTimeout = navigationTimeout || 0; } (playwright.request as any)._defaultContextOptions = { ..._combinedContextOptions }; - (playwright.request as any)._defaultContextOptions.tracesDir = path.join(_artifactsDir, 'traces'); + (playwright.request as any)._defaultContextOptions.tracesDir = tracing().tracesDir(); (playwright.request as any)._defaultContextOptions.timeout = actionTimeout || 0; await use(); (playwright.request as any)._defaultContextOptions = undefined; @@ -248,8 +242,8 @@ const playwrightFixtures: Fixtures = ({ } }, { auto: 'all-hooks-included', _title: 'context configuration' } as any], - _setupArtifacts: [async ({ playwright, _artifactsDir, trace, screenshot }, use, testInfo) => { - const artifactsRecorder = new ArtifactsRecorder(playwright, _artifactsDir, trace, screenshot); + _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[], wallTime: number, userData: any) => { @@ -297,11 +291,11 @@ const playwrightFixtures: Fixtures = ({ await use(); clientInstrumentation.removeListener(csiListener); - await artifactsRecorder?.didFinishTest(); + await artifactsRecorder.didFinishTest(); }, { auto: 'all-hooks-included', _title: 'trace recording' } as any], - _contextFactory: [async ({ browser, video, _artifactsDir, _reuseContext }, use, testInfo) => { + _contextFactory: [async ({ browser, video, _reuseContext }, use, testInfo) => { const testInfoImpl = testInfo as TestInfoImpl; const videoMode = normalizeVideoMode(video); const captureVideo = shouldCaptureVideo(videoMode, testInfo) && !_reuseContext; @@ -318,7 +312,7 @@ const playwrightFixtures: Fixtures = ({ } const videoOptions: BrowserContextOptions = captureVideo ? { recordVideo: { - dir: _artifactsDir, + dir: tracing().artifactsDir(), size: typeof video === 'string' ? undefined : video.size, } } : {}; @@ -333,7 +327,6 @@ const playwrightFixtures: Fixtures = ({ let counter = 0; const closeReason = testInfo.status === 'timedOut' ? 'Test timeout of ' + testInfo.timeout + 'ms exceeded.' : 'Test ended.'; await Promise.all([...contexts.keys()].map(async context => { - (context as any)[kStartedContextTearDown] = true; await (context as any)._wrapApiCall(async () => { await context.close({ reason: closeReason }); }, true); @@ -395,7 +388,6 @@ const playwrightFixtures: Fixtures = ({ request: async ({ playwright }, use) => { const request = await playwright.request.newContext(); await use(request); - (request as any)[kStartedContextTearDown] = true; await request.dispose(); }, }); @@ -414,7 +406,6 @@ type StackFrame = { }; type ScreenshotOption = PlaywrightWorkerOptions['screenshot'] | undefined; -type TraceOption = PlaywrightWorkerOptions['trace'] | undefined; type Playwright = PlaywrightWorkerArgs['playwright']; function normalizeVideoMode(video: VideoMode | 'retry-with-video' | { mode: VideoMode } | undefined): VideoMode { @@ -430,19 +421,6 @@ function shouldCaptureVideo(videoMode: VideoMode, testInfo: TestInfo) { return (videoMode === 'on' || videoMode === 'retain-on-failure' || (videoMode === 'on-first-retry' && testInfo.retry === 1)); } -function normalizeTraceMode(trace: TraceOption): TraceMode { - if (!trace) - return 'off'; - let traceMode = typeof trace === 'string' ? trace : trace.mode; - if (traceMode === 'retry-with-trace') - traceMode = 'on-first-retry'; - return traceMode; -} - -function shouldCaptureTrace(traceMode: TraceMode, testInfo: TestInfo) { - return traceMode === 'on' || traceMode === 'retain-on-failure' || (traceMode === 'on-first-retry' && testInfo.retry === 1) || (traceMode === 'on-all-retries' && testInfo.retry > 0); -} - function normalizeScreenshotMode(screenshot: ScreenshotOption): ScreenshotMode { if (!screenshot) return 'off'; @@ -467,8 +445,6 @@ function attachConnectedHeaderIfNeeded(testInfo: TestInfo, browser: Browser | nu const kTracingStarted = Symbol('kTracingStarted'); const kIsReusedContext = Symbol('kReusedContext'); -const kStartedContextTearDown = Symbol('kStartedContextTearDown'); -let traceOrdinal = 0; function connectOptionsFromEnv() { const wsEndpoint = process.env.PW_TEST_CONNECT_WS_ENDPOINT; @@ -487,11 +463,7 @@ class ArtifactsRecorder { private _playwright: Playwright; private _artifactsDir: string; private _screenshotMode: ScreenshotMode; - private _traceMode: TraceMode; - private _captureTrace = false; private _screenshotOptions: { mode: ScreenshotMode } & Pick | undefined; - private _traceOptions: { screenshots: boolean, snapshots: boolean, sources: boolean, attachments: boolean, _live: boolean, mode?: TraceMode }; - private _temporaryTraceFiles: string[] = []; private _temporaryScreenshots: string[] = []; private _temporaryArtifacts: string[] = []; private _reusedContexts = new Set(); @@ -499,14 +471,11 @@ class ArtifactsRecorder { private _screenshottedSymbol: symbol; private _startedCollectingArtifacts: symbol; - constructor(playwright: Playwright, artifactsDir: string, trace: TraceOption, screenshot: ScreenshotOption) { + constructor(playwright: Playwright, artifactsDir: string, screenshot: ScreenshotOption) { this._playwright = playwright; this._artifactsDir = artifactsDir; this._screenshotMode = normalizeScreenshotMode(screenshot); this._screenshotOptions = typeof screenshot === 'string' ? undefined : screenshot; - this._traceMode = normalizeTraceMode(trace); - const defaultTraceOptions = { screenshots: true, snapshots: true, sources: true, attachments: true, _live: false }; - this._traceOptions = typeof trace === 'string' ? defaultTraceOptions : { ...defaultTraceOptions, ...trace, mode: undefined }; this._screenshottedSymbol = Symbol('screenshotted'); this._startedCollectingArtifacts = Symbol('startedCollectingArtifacts'); } @@ -520,9 +489,6 @@ class ArtifactsRecorder { async willStartTest(testInfo: TestInfoImpl) { this._testInfo = testInfo; testInfo._onDidFinishTestFunction = () => this.didFinishTestFunction(); - this._captureTrace = shouldCaptureTrace(this._traceMode, testInfo) && !process.env.PW_TEST_DISABLE_TRACING; - if (this._captureTrace) - this._testInfo._tracing.start(this._createTemporaryArtifact('traces', `${this._testInfo.testId}-test.trace`), this._traceOptions); // Since beforeAll(s), test and afterAll(s) reuse the same TestInfo, make sure we do not // overwrite previous screenshots. @@ -555,7 +521,7 @@ class ArtifactsRecorder { // Do not record empty traces and useless screenshots for them. if (this._reusedContexts.has(context)) return; - await this._stopTracing(context.tracing, (context as any)[kStartedContextTearDown]); + await this._stopTracing(context.tracing); if (this._screenshotMode === 'on' || this._screenshotMode === 'only-on-failure') { // Capture screenshot for now. We'll know whether we have to preserve them // after the test finishes. @@ -570,7 +536,7 @@ class ArtifactsRecorder { async willCloseRequestContext(context: APIRequestContext) { const tracing = (context as any)._tracing as Tracing; - await this._stopTracing(tracing, (context as any)[kStartedContextTearDown]); + await this._stopTracing(tracing); } async didFinishTestFunction() { @@ -591,10 +557,10 @@ class ArtifactsRecorder { // Collect traces/screenshots for remaining contexts. await Promise.all(leftoverContexts.map(async context => { - await this._stopTracing(context.tracing, true); + await this._stopTracing(context.tracing); }).concat(leftoverApiRequests.map(async context => { const tracing = (context as any)._tracing as Tracing; - await this._stopTracing(tracing, true); + await this._stopTracing(tracing); }))); // Attach temporary screenshots for contexts closed before collecting the test trace. @@ -608,32 +574,6 @@ class ArtifactsRecorder { } } } - - // Collect test trace. - if (this._preserveTrace()) { - const tracePath = this._createTemporaryArtifact(createGuid() + '.zip'); - this._temporaryTraceFiles.push(tracePath); - await this._testInfo._tracing.stop(tracePath); - } - - // Either remove or attach temporary traces for contexts closed before the - // test has finished. - if (this._preserveTrace() && this._temporaryTraceFiles.length) { - const tracePath = this._testInfo.outputPath(`trace.zip`); - // This could be: beforeHooks, or beforeHooks + test, etc. - const beforeHooksHadTrace = fs.existsSync(tracePath); - if (beforeHooksHadTrace) { - await fs.promises.rename(tracePath, tracePath + '.tmp'); - this._temporaryTraceFiles.unshift(tracePath + '.tmp'); - } - await mergeTraceFiles(tracePath, this._temporaryTraceFiles); - // Do not add attachment twice. - if (!beforeHooksHadTrace) - this._testInfo.attachments.push({ name: 'trace', path: tracePath, contentType: 'application/zip' }); - } - - for (const file of this._temporaryArtifacts) - await fs.promises.unlink(file).catch(() => {}); } private _createScreenshotAttachmentPath() { @@ -675,15 +615,12 @@ class ArtifactsRecorder { } private async _startTraceChunkOnContextCreation(tracing: Tracing) { - if (this._captureTrace) { - const title = [path.relative(this._testInfo.project.testDir, this._testInfo.file) + ':' + this._testInfo.line, ...this._testInfo.titlePath.slice(1)].join(' › '); - const ordinalSuffix = traceOrdinal ? `-context${traceOrdinal}` : ''; - ++traceOrdinal; - const retrySuffix = this._testInfo.retry ? `-retry${this._testInfo.retry}` : ''; - // Note that trace name must start with testId for live tracing to work. - const name = `${this._testInfo.testId}${retrySuffix}${ordinalSuffix}`; + const options = this._testInfo._tracing.traceOptions(); + if (options) { + const title = this._testInfo._tracing.traceTitle(); + const name = this._testInfo._tracing.generateNextTraceRecordingName(); if (!(tracing as any)[kTracingStarted]) { - await tracing.start({ ...this._traceOptions, title, name }); + await tracing.start({ ...options, title, name }); (tracing as any)[kTracingStarted] = true; } else { await tracing.startChunk({ title, name }); @@ -696,26 +633,12 @@ class ArtifactsRecorder { } } - private _preserveTrace() { - const testFailed = this._testInfo.status !== this._testInfo.expectedStatus; - return this._captureTrace && (this._traceMode === 'on' || (testFailed && this._traceMode === 'retain-on-failure') || (this._traceMode === 'on-first-retry' && this._testInfo.retry === 1) || (this._traceMode === 'on-all-retries' && this._testInfo.retry > 0)); - } - - private async _stopTracing(tracing: Tracing, contextTearDownStarted: boolean) { + private async _stopTracing(tracing: Tracing) { if ((tracing as any)[this._startedCollectingArtifacts]) return; (tracing as any)[this._startedCollectingArtifacts] = true; - if (this._captureTrace) { - let tracePath; - // Create a trace file if we know that: - // - it is's going to be used due to the config setting and the test status or - // - we are inside a test or afterEach and the user manually closed the context. - if (this._preserveTrace() || !contextTearDownStarted) { - tracePath = this._createTemporaryArtifact(createGuid() + '.zip'); - this._temporaryTraceFiles.push(tracePath); - } - await tracing.stopChunk({ path: tracePath }); - } + if (this._testInfo._tracing.traceOptions()) + await tracing.stopChunk({ path: this._testInfo._tracing.generateNextTraceRecordingPath() }); } } @@ -743,6 +666,10 @@ function renderApiCall(apiName: string, params: any) { return apiName + paramsText; } +function tracing() { + return (test.info() as TestInfoImpl)._tracing; +} + 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 37a523c0abaa9..5a940061cc241 100644 --- a/packages/playwright/src/worker/testInfo.ts +++ b/packages/playwright/src/worker/testInfo.ts @@ -57,7 +57,7 @@ export class TestInfoImpl implements TestInfo { readonly _startTime: number; readonly _startWallTime: number; private _hasHardError: boolean = false; - readonly _tracing = new TestTracing(); + readonly _tracing: TestTracing; _didTimeout = false; _wasInterrupted = false; @@ -187,6 +187,8 @@ export class TestInfoImpl implements TestInfo { this._attach(a.name, a); return this.attachments.length; }; + + this._tracing = new TestTracing(this, workerParams.artifactsDir); } private _modifier(type: 'skip' | 'fail' | 'fixme' | 'slow', modifierArgs: [arg?: any, description?: string]) { @@ -223,6 +225,7 @@ export class TestInfoImpl implements TestInfo { if (!this._wasInterrupted && timeoutError && !this._didTimeout) { this._didTimeout = true; this.errors.push(timeoutError); + this._tracing.appendForError(timeoutError); // Do not overwrite existing failure upon hook/teardown timeout. if (this.status === 'passed' || this.status === 'skipped') this.status = 'timedOut'; @@ -359,6 +362,7 @@ export class TestInfoImpl implements TestInfo { if (step && step.boxedStack) serialized.stack = `${error.name}: ${error.message}\n${stringifyStackFrames(step.boxedStack).join('\n')}`; this.errors.push(serialized); + this._tracing.appendForError(serialized); } async _runAsStepWithRunnable( diff --git a/packages/playwright/src/worker/testTracing.ts b/packages/playwright/src/worker/testTracing.ts index 89a6e99d44108..579bf8bc87db9 100644 --- a/packages/playwright/src/worker/testTracing.ts +++ b/packages/playwright/src/worker/testTracing.ts @@ -19,31 +19,104 @@ import type * as trace from '@trace/trace'; import type EventEmitter from 'events'; import fs from 'fs'; import path from 'path'; -import { ManualPromise, calculateSha1, monotonicTime } from 'playwright-core/lib/utils'; +import { ManualPromise, calculateSha1, monotonicTime, createGuid } from 'playwright-core/lib/utils'; import { yauzl, yazl } from 'playwright-core/lib/zipBundle'; import type { TestInfo, TestInfoError } from '../../types/test'; import { filteredStackTrace } from '../util'; - +import type { TraceMode, PlaywrightWorkerOptions } from '../../types/test'; +import type { TestInfoImpl } from './testInfo'; export type Attachment = TestInfo['attachments'][0]; export const testTraceEntryName = 'test.trace'; +let traceOrdinal = 0; + +type TraceFixtureValue = PlaywrightWorkerOptions['trace'] | undefined; +type TraceOptions = { screenshots: boolean, snapshots: boolean, sources: boolean, attachments: boolean, _live: boolean, mode: TraceMode }; export class TestTracing { + private _testInfo: TestInfoImpl; + private _options: TraceOptions | undefined; private _liveTraceFile: string | undefined; private _traceEvents: trace.TraceEvent[] = []; - private _options: { sources: boolean; attachments: boolean; _live: boolean; } | undefined; + private _temporaryTraceFiles: string[] = []; + private _artifactsDir: string; + private _tracesDir: string; + + constructor(testInfo: TestInfoImpl, artifactsDir: string) { + this._testInfo = testInfo; + this._artifactsDir = artifactsDir; + this._tracesDir = path.join(this._artifactsDir, 'traces'); + } + + async startIfNeeded(value: TraceFixtureValue) { + const defaultTraceOptions: TraceOptions = { screenshots: true, snapshots: true, sources: true, attachments: true, _live: false, mode: 'off' }; + if (!value) { + this._options = defaultTraceOptions; + } else if (typeof value === 'string') { + this._options = { ...defaultTraceOptions, mode: value === 'retry-with-trace' ? 'on-first-retry' : value as TraceMode }; + } else { + const mode = value.mode || 'off'; + this._options = { ...defaultTraceOptions, ...value, mode: (mode as string) === 'retry-with-trace' ? 'on-first-retry' : mode }; + } - start(liveFileName: string, options: { sources: boolean, attachments: boolean, _live: boolean }) { - this._options = options; - if (!this._liveTraceFile && options._live) { - this._liveTraceFile = liveFileName; - fs.mkdirSync(path.dirname(this._liveTraceFile), { recursive: true }); + let shouldCaptureTrace = this._options.mode === 'on' || this._options.mode === 'retain-on-failure' || (this._options.mode === 'on-first-retry' && this._testInfo.retry === 1) || (this._options.mode === 'on-all-retries' && this._testInfo.retry > 0); + shouldCaptureTrace = shouldCaptureTrace && !process.env.PW_TEST_DISABLE_TRACING; + if (!shouldCaptureTrace) { + this._options = undefined; + return; + } + + if (!this._liveTraceFile && this._options._live) { + // Note that trace name must start with testId for live tracing to work. + this._liveTraceFile = path.join(this._tracesDir, `${this._testInfo.testId}-test.trace`); + await fs.promises.mkdir(path.dirname(this._liveTraceFile), { recursive: true }); const data = this._traceEvents.map(e => JSON.stringify(e)).join('\n') + '\n'; - fs.writeFileSync(this._liveTraceFile, data); + await fs.promises.writeFile(this._liveTraceFile, data); } } - async stop(fileName: string) { + artifactsDir() { + return this._artifactsDir; + } + + tracesDir() { + return this._tracesDir; + } + + traceTitle() { + return [path.relative(this._testInfo.project.testDir, this._testInfo.file) + ':' + this._testInfo.line, ...this._testInfo.titlePath.slice(1)].join(' › '); + } + + generateNextTraceRecordingName() { + const ordinalSuffix = traceOrdinal ? `-recording${traceOrdinal}` : ''; + ++traceOrdinal; + const retrySuffix = this._testInfo.retry ? `-retry${this._testInfo.retry}` : ''; + // Note that trace name must start with testId for live tracing to work. + return `${this._testInfo.testId}${retrySuffix}${ordinalSuffix}`; + } + + generateNextTraceRecordingPath() { + const file = path.join(this._artifactsDir, createGuid() + '.zip'); + this._temporaryTraceFiles.push(file); + return file; + } + + traceOptions() { + return this._options; + } + + async stopIfNeeded() { + if (!this._options) + return; + + const testFailed = this._testInfo.status !== this._testInfo.expectedStatus; + const shouldAbandonTrace = !testFailed && this._options.mode === 'retain-on-failure'; + if (shouldAbandonTrace) { + for (const file of this._temporaryTraceFiles) + await fs.promises.unlink(file).catch(() => {}); + return; + } + const zipFile = new yazl.ZipFile(); if (!this._options?.attachments) { @@ -97,9 +170,13 @@ export class TestTracing { await new Promise(f => { zipFile.end(undefined, () => { - zipFile.outputStream.pipe(fs.createWriteStream(fileName)).on('close', f); + zipFile.outputStream.pipe(fs.createWriteStream(this.generateNextTraceRecordingPath())).on('close', f); }); }); + + const tracePath = this._testInfo.outputPath('trace.zip'); + await mergeTraceFiles(tracePath, this._temporaryTraceFiles); + this._testInfo.attachments.push({ name: 'trace', path: tracePath, contentType: 'application/zip' }); } appendForError(error: TestInfoError) { @@ -185,7 +262,7 @@ function generatePreview(value: any, visited = new Set()): string { return String(value); } -export async function mergeTraceFiles(fileName: string, temporaryTraceFiles: string[]) { +async function mergeTraceFiles(fileName: string, temporaryTraceFiles: string[]) { if (temporaryTraceFiles.length === 1) { await fs.promises.rename(temporaryTraceFiles[0], fileName); return; diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index bd529bbfd8f2b..8c76f4091a09b 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -62,7 +62,6 @@ export class WorkerMain extends ProcessRunner { super(); process.env.TEST_WORKER_INDEX = String(params.workerIndex); process.env.TEST_PARALLEL_INDEX = String(params.parallelIndex); - process.env.TEST_ARTIFACTS_DIR = params.artifactsDir; setIsWorkerProcess(); this._params = params; @@ -313,6 +312,21 @@ export class WorkerMain extends ProcessRunner { let shouldRunAfterEachHooks = false; await testInfo._runWithTimeout(async () => { + const traceError = await testInfo._runAndFailOnError(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. + // We decided to not introduce the config-level option just yet. + const traceFixtureRegistration = test._pool!.registrations.get('trace'); + if (!traceFixtureRegistration) + return; + if (typeof traceFixtureRegistration.fn === 'function') + throw new Error(`"trace" option cannot be a function`); + await testInfo._tracing.startIfNeeded(traceFixtureRegistration.fn); + }); + if (traceError) + return; + if (this._isStopped || isSkipped) { // Two reasons to get here: // - Last test is skipped, so we should not run the test, but run the cleanup. @@ -379,9 +393,6 @@ export class WorkerMain extends ProcessRunner { await fn(testFunctionParams, testInfo); debugTest(`test function finished`); }, 'allowSkips'); - - for (const error of testInfo.errors) - testInfo._tracing.appendForError(error); }); if (didFailBeforeAllForSuite) { @@ -474,6 +485,10 @@ export class WorkerMain extends ProcessRunner { step.complete({ error: firstAfterHooksError }); }); + await testInfo._runAndFailOnError(async () => { + await testInfo._tracing.stopIfNeeded(); + }); + this._currentTest = null; setCurrentTestInfo(null); this.dispatchEvent('testEnd', buildTestEndPayload(testInfo)); diff --git a/tests/playwright-test/playwright.trace.spec.ts b/tests/playwright-test/playwright.trace.spec.ts index c62e7c98ca160..9587315e653bc 100644 --- a/tests/playwright-test/playwright.trace.spec.ts +++ b/tests/playwright-test/playwright.trace.spec.ts @@ -326,13 +326,13 @@ test('should not override trace file in afterAll', async ({ runInlineTest, serve 'After Hooks', ' fixture: page', ' fixture: context', - ' attach \"trace\"', ' afterAll hook', ' fixture: request', ' apiRequest.newContext', ' apiRequestContext.get', ' fixture: request', ' apiRequestContext.dispose', + ' fixture: browser', ]); expect(trace1.errors).toEqual([`'oh no!'`]); @@ -407,18 +407,16 @@ for (const mode of ['off', 'retain-on-failure', 'on-first-retry', 'on-all-retrie 'a.spec.ts': ` import { test as base, expect } from '@playwright/test'; import fs from 'fs'; - const test = base.extend<{ - locale: string | undefined, - _artifactsDir: () => string, - }>({ - // Override locale fixture to check in teardown that no temporary trace zip was created. - locale: [async ({ locale, _artifactsDir }, use) => { - await use(locale); - const entries = fs.readdirSync(_artifactsDir); + let artifactsDir; + const test = base.extend({ + workerAuto: [async ({}, use) => { + await use(); + const entries = fs.readdirSync(artifactsDir); expect(entries.filter(e => e.endsWith('.zip'))).toEqual([]); - }, { option: true }], + }, { scope: 'worker', auto: true }], }); test('passing test', async ({ page }) => { + artifactsDir = test.info()._tracing.artifactsDir(); await page.goto('about:blank'); }); `, @@ -432,18 +430,16 @@ for (const mode of ['off', 'retain-on-failure', 'on-first-retry', 'on-all-retrie 'a.spec.ts': ` import { test as base, expect } from '@playwright/test'; import fs from 'fs'; - const test = base.extend<{ - locale: string | undefined, - _artifactsDir: () => string, - }>({ - // Override locale fixture to check in teardown that no temporary trace zip was created. - locale: [async ({ locale, _artifactsDir }, use) => { - await use(locale); - const entries = fs.readdirSync(_artifactsDir); + let artifactsDir; + const test = base.extend({ + workerAuto: [async ({}, use) => { + await use(); + const entries = fs.readdirSync(artifactsDir); expect(entries.filter(e => e.endsWith('.zip'))).toEqual([]); - }, { option: true }], + }, { scope: 'worker', auto: true }], }); test('passing test', async ({ request }) => { + artifactsDir = test.info()._tracing.artifactsDir(); expect(await request.get('${server.EMPTY_PAGE}')).toBeOK(); }); `, @@ -671,10 +667,51 @@ test('should show non-expect error in trace', async ({ runInlineTest }, testInfo 'After Hooks', ' fixture: page', ' fixture: context', + ' fixture: browser', ]); expect(trace.errors).toEqual(['ReferenceError: undefinedVariable1 is not defined']); }); +test('should show error from beforeAll in trace', async ({ runInlineTest }, testInfo) => { + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { use: { trace: { mode: 'on' } } }; + `, + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test.beforeAll(async () => { + throw new Error('Oh my!'); + }) + test('fail', async ({ page }) => { + }); + `, + }, { workers: 1 }); + + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + const trace = await parseTrace(testInfo.outputPath('test-results', 'a-fail', 'trace.zip')); + expect(trace.errors).toEqual(['Error: Oh my!']); +}); + +test('should throw when trace fixture is a function', async ({ runInlineTest }, testInfo) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test.use({ + trace: async ({}, use) => { + await use('on'); + }, + }); + test('skipped', async ({ page }) => { + }); + `, + }, { workers: 1 }); + + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + expect(result.output).toContain('Error: "trace" option cannot be a function'); +}); + test('should not throw when attachment is missing', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ 'playwright.config.ts': ` diff --git a/tests/playwright-test/ui-mode-test-progress.spec.ts b/tests/playwright-test/ui-mode-test-progress.spec.ts index 38091dcd93ff9..f87eaa8fbcd68 100644 --- a/tests/playwright-test/ui-mode-test-progress.spec.ts +++ b/tests/playwright-test/ui-mode-test-progress.spec.ts @@ -288,3 +288,63 @@ test('should show live trace for serial', async ({ runUITest, server, createLatc /expect.not.toBeCheckedlocator\('input'\)[\d.]/, ]); }); + +test('should show live trace from hooks', async ({ runUITest, createLatch }) => { + const latch1 = createLatch(); + const latch2 = createLatch(); + + const { page } = await runUITest({ + 'a.test.ts': ` + import { test, expect } from '@playwright/test'; + test.beforeAll(async ({ browser }) => { + const page = await browser.newPage(); + ${latch1.blockingCode} + await page.close(); + }); + test.beforeEach(async ({ browser }) => { + const page = await browser.newPage(); + ${latch2.blockingCode} + await page.close(); + }); + test('test one', async ({ page }) => { + await page.setContent('Page content'); + }); + `, + }); + + await expect.poll(dumpTestTree(page)).toBe(` + ▼ ◯ a.test.ts + ◯ test one + `); + await page.getByText('test one').dblclick(); + + const listItem = page.getByTestId('actions-tree').getByRole('listitem'); + await expect( + listItem, + 'action list' + ).toHaveText([ + /Before Hooks/, + /beforeAll hook/, + /fixture: browser/, + /browser.newPage/, + ]); + latch1.open(); + await expect( + listItem, + 'action list' + ).toHaveText([ + /Before Hooks/, + /beforeAll hook/, + /beforeEach hook/, + /browser.newPage/, + ]); + latch2.open(); + await expect( + listItem, + 'action list' + ).toHaveText([ + /Before Hooks/, + /page.setContent/, + /After Hooks/, + ]); +});