From 122818c62c129455862246a2f6cac57e8fb39321 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 24 Jun 2024 21:43:43 -0700 Subject: [PATCH] feat: allow boxing and titling fixtures, simulate context fixture deps (#31423) Fixes https://github.com/microsoft/playwright/issues/31411 --- docs/src/test-fixtures-js.md | 464 ++---------------- packages/playwright/src/common/fixtures.ts | 68 ++- packages/playwright/src/index.ts | 18 +- .../playwright/src/worker/fixtureRunner.ts | 6 +- packages/playwright/types/test.d.ts | 8 +- tests/playwright-test/fixture-errors.spec.ts | 35 ++ tests/playwright-test/timeout.spec.ts | 4 +- utils/generate_types/overrides-test.d.ts | 8 +- 8 files changed, 146 insertions(+), 465 deletions(-) diff --git a/docs/src/test-fixtures-js.md b/docs/src/test-fixtures-js.md index 0971922d35..57334aa2ef 100644 --- a/docs/src/test-fixtures-js.md +++ b/docs/src/test-fixtures-js.md @@ -43,48 +43,7 @@ Here is how typical test environment setup differs between traditional test styl Click to expand the code for the TodoPage
-```js tab=js-js title="todo-page.js" -export class TodoPage { - /** - * @param {import('@playwright/test').Page} page - */ - constructor(page) { - this.page = page; - this.inputBox = this.page.locator('input.new-todo'); - this.todoItems = this.page.getByTestId('todo-item'); - } - - async goto() { - await this.page.goto('https://demo.playwright.dev/todomvc/'); - } - - /** - * @param {string} text - */ - async addToDo(text) { - await this.inputBox.fill(text); - await this.inputBox.press('Enter'); - } - - /** - * @param {string} text - */ - async remove(text) { - const todo = this.todoItems.filter({ hasText: text }); - await todo.hover(); - await todo.getByLabel('Delete').click(); - } - - async removeAll() { - while ((await this.todoItems.count()) > 0) { - await this.todoItems.first().hover(); - await this.todoItems.getByLabel('Delete').first().click(); - } - } -} -``` - -```js tab=js-ts title="todo-page.ts" +```js title="todo-page.ts" import type { Page, Locator } from '@playwright/test'; export class TodoPage { @@ -167,48 +126,7 @@ Fixtures have a number of advantages over before/after hooks: Click to expand the code for the TodoPage
-```js tab=js-js title="todo-page.js" -export class TodoPage { - /** - * @param {import('@playwright/test').Page} page - */ - constructor(page) { - this.page = page; - this.inputBox = this.page.locator('input.new-todo'); - this.todoItems = this.page.getByTestId('todo-item'); - } - - async goto() { - await this.page.goto('https://demo.playwright.dev/todomvc/'); - } - - /** - * @param {string} text - */ - async addToDo(text) { - await this.inputBox.fill(text); - await this.inputBox.press('Enter'); - } - - /** - * @param {string} text - */ - async remove(text) { - const todo = this.todoItems.filter({ hasText: text }); - await todo.hover(); - await todo.getByLabel('Delete').click(); - } - - async removeAll() { - while ((await this.todoItems.count()) > 0) { - await this.todoItems.first().hover(); - await this.todoItems.getByLabel('Delete').first().click(); - } - } -} -``` - -```js tab=js-ts title="todo-page.ts" +```js title="todo-page.ts" import type { Page, Locator } from '@playwright/test'; export class TodoPage { @@ -246,34 +164,7 @@ export class TodoPage {
-```js tab=js-js title="todo.spec.js" -const base = require('@playwright/test'); -const { TodoPage } = require('./todo-page'); - -// Extend basic test by providing a "todoPage" fixture. -const test = base.test.extend({ - todoPage: async ({ page }, use) => { - const todoPage = new TodoPage(page); - await todoPage.goto(); - await todoPage.addToDo('item1'); - await todoPage.addToDo('item2'); - await use(todoPage); - await todoPage.removeAll(); - }, -}); - -test('should add an item', async ({ todoPage }) => { - await todoPage.addToDo('my item'); - // ... -}); - -test('should remove an item', async ({ todoPage }) => { - await todoPage.remove('item1'); - // ... -}); -``` - -```js tab=js-ts title="example.spec.ts" +```js title="example.spec.ts" import { test as base } from '@playwright/test'; import { TodoPage } from './todo-page'; @@ -309,48 +200,8 @@ Below we create two fixtures `todoPage` and `settingsPage` that follow the [Page
Click to expand the code for the TodoPage and SettingsPage
-```js tab=js-js title="todo-page.js" -export class TodoPage { - /** - * @param {import('@playwright/test').Page} page - */ - constructor(page) { - this.page = page; - this.inputBox = this.page.locator('input.new-todo'); - this.todoItems = this.page.getByTestId('todo-item'); - } - async goto() { - await this.page.goto('https://demo.playwright.dev/todomvc/'); - } - - /** - * @param {string} text - */ - async addToDo(text) { - await this.inputBox.fill(text); - await this.inputBox.press('Enter'); - } - - /** - * @param {string} text - */ - async remove(text) { - const todo = this.todoItems.filter({ hasText: text }); - await todo.hover(); - await todo.getByLabel('Delete').click(); - } - - async removeAll() { - while ((await this.todoItems.count()) > 0) { - await this.todoItems.first().hover(); - await this.todoItems.getByLabel('Delete').first().click(); - } - } -} -``` - -```js tab=js-ts title="todo-page.ts" +```js title="todo-page.ts" import type { Page, Locator } from '@playwright/test'; export class TodoPage { @@ -388,22 +239,7 @@ export class TodoPage { SettingsPage is similar: -```js tab=js-js title="settings-page.js" -export class SettingsPage { - /** - * @param {import('@playwright/test').Page} page - */ - constructor(page) { - this.page = page; - } - - async switchToDarkMode() { - // ... - } -} -``` - -```js tab=js-ts title="settings-page.ts" +```js title="settings-page.ts" import type { Page } from '@playwright/test'; export class SettingsPage { @@ -419,36 +255,7 @@ export class SettingsPage {
-```js tab=js-js title="my-test.js" -const base = require('@playwright/test'); -const { TodoPage } = require('./todo-page'); -const { SettingsPage } = require('./settings-page'); - -// Extend base test by providing "todoPage" and "settingsPage". -// This new "test" can be used in multiple test files, and each of them will get the fixtures. -exports.test = base.test.extend({ - todoPage: async ({ page }, use) => { - // Set up the fixture. - const todoPage = new TodoPage(page); - await todoPage.goto(); - await todoPage.addToDo('item1'); - await todoPage.addToDo('item2'); - - // Use the fixture value in the test. - await use(todoPage); - - // Clean up the fixture. - await todoPage.removeAll(); - }, - - settingsPage: async ({ page }, use) => { - await use(new SettingsPage(page)); - }, -}); -exports.expect = base.expect; -``` - -```js tab=js-ts title="my-test.ts" +```js title="my-test.ts" import { test as base } from '@playwright/test'; import { TodoPage } from './todo-page'; import { SettingsPage } from './settings-page'; @@ -493,20 +300,7 @@ Just mention fixture in your test function argument, and test runner will take c Below we use the `todoPage` and `settingsPage` fixtures defined above. -```js tab=js-js -const { test, expect } = require('./my-test'); - -test.beforeEach(async ({ settingsPage }) => { - await settingsPage.switchToDarkMode(); -}); - -test('basic test', async ({ todoPage, page }) => { - await todoPage.addToDo('something nice'); - await expect(page.getByTestId('todo-title')).toContainText(['something nice']); -}); -``` - -```js tab=js-ts +```js import { test, expect } from './my-test'; test.beforeEach(async ({ settingsPage }) => { @@ -560,47 +354,7 @@ Playwright Test uses [worker processes](./test-parallel.md) to run test files. S Below we'll create an `account` fixture that will be shared by all tests in the same worker, and override the `page` fixture to login into this account for each test. To generate unique accounts, we'll use the [`property: WorkerInfo.workerIndex`] that is available to any test or fixture. Note the tuple-like syntax for the worker fixture - we have to pass `{scope: 'worker'}` so that test runner sets up this fixture once per worker. -```js tab=js-js title="my-test.js" -const base = require('@playwright/test'); - -exports.test = base.test.extend({ - account: [async ({ browser }, use, workerInfo) => { - // Unique username. - const username = 'user' + workerInfo.workerIndex; - const password = 'verysecure'; - - // Create the account with Playwright. - const page = await browser.newPage(); - await page.goto('/signup'); - await page.getByLabel('User Name').fill(username); - await page.getByLabel('Password').fill(password); - await page.getByText('Sign up').click(); - // Make sure everything is ok. - await expect(page.locator('#result')).toHaveText('Success'); - // Do not forget to cleanup. - await page.close(); - - // Use the account value. - await use({ username, password }); - }, { scope: 'worker' }], - - page: async ({ page, account }, use) => { - // Sign in with our account. - const { username, password } = account; - await page.goto('/signin'); - await page.getByLabel('User Name').fill(username); - await page.getByLabel('Password').fill(password); - await page.getByText('Sign in').click(); - await expect(page.getByTestId('userinfo')).toHaveText(username); - - // Use signed-in page in the test. - await use(page); - }, -}); -exports.expect = base.expect; -``` - -```js tab=js-ts title="my-test.ts" +```js title="my-test.ts" import { test as base } from '@playwright/test'; type Account = { @@ -652,32 +406,7 @@ Automatic fixtures are set up for each test/worker, even when the test does not Here is an example fixture that automatically attaches debug logs when the test fails, so we can later review the logs in the reporter. Note how it uses [TestInfo] object that is available in each test/fixture to retrieve metadata about the test being run. -```js tab=js-js title="my-test.js" -const debug = require('debug'); -const fs = require('fs'); -const base = require('@playwright/test'); - -exports.test = base.test.extend({ - saveLogs: [async ({}, use, testInfo) => { - // Collecting logs during the test. - const logs = []; - debug.log = (...args) => logs.push(args.map(String).join('')); - debug.enable('myserver'); - - await use(); - - // After the test we can check whether the test passed or failed. - if (testInfo.status !== testInfo.expectedStatus) { - // outputPath() API guarantees a unique file name. - const logFile = testInfo.outputPath('logs.txt'); - await fs.promises.writeFile(logFile, logs.join('\n'), 'utf8'); - testInfo.attachments.push({ name: 'logs', contentType: 'text/plain', path: logFile }); - } - }, { auto: true }], -}); -``` - -```js tab=js-ts title="my-test.ts" +```js title="my-test.ts" import * as debug from 'debug'; import * as fs from 'fs'; import { test as base } from '@playwright/test'; @@ -707,22 +436,7 @@ export { expect } from '@playwright/test'; By default, fixture shares timeout with the test. However, for slow fixtures, especially [worker-scoped](#worker-scoped-fixtures) ones, it is convenient to have a separate timeout. This way you can keep the overall test timeout small, and give the slow fixture more time. -```js tab=js-js -const { test: base, expect } = require('@playwright/test'); - -const test = base.extend({ - slowFixture: [async ({}, use) => { - // ... perform a slow operation ... - await use('hello'); - }, { timeout: 60000 }] -}); - -test('example test', async ({ slowFixture }) => { - // ... -}); -``` - -```js tab=js-ts +```js import { test as base, expect } from '@playwright/test'; const test = base.extend<{ slowFixture: string }>({ @@ -752,48 +466,7 @@ Below we'll create a `defaultItem` option in addition to the `todoPage` fixture Click to expand the code for the TodoPage
-```js tab=js-js title="todo-page.js" -export class TodoPage { - /** - * @param {import('@playwright/test').Page} page - */ - constructor(page) { - this.page = page; - this.inputBox = this.page.locator('input.new-todo'); - this.todoItems = this.page.getByTestId('todo-item'); - } - - async goto() { - await this.page.goto('https://demo.playwright.dev/todomvc/'); - } - - /** - * @param {string} text - */ - async addToDo(text) { - await this.inputBox.fill(text); - await this.inputBox.press('Enter'); - } - - /** - * @param {string} text - */ - async remove(text) { - const todo = this.todoItems.filter({ hasText: text }); - await todo.hover(); - await todo.getByLabel('Delete').click(); - } - - async removeAll() { - while ((await this.todoItems.count()) > 0) { - await this.todoItems.first().hover(); - await this.todoItems.getByLabel('Delete').first().click(); - } - } -} -``` - -```js tab=js-ts title="todo-page.ts" +```js title="todo-page.ts" import type { Page, Locator } from '@playwright/test'; export class TodoPage { @@ -832,28 +505,7 @@ export class TodoPage {
-```js tab=js-js title="my-test.js" -const base = require('@playwright/test'); -const { TodoPage } = require('./todo-page'); - -exports.test = base.test.extend({ - // Define an option and provide a default value. - // We can later override it in the config. - defaultItem: ['Something nice', { option: true }], - - // Our "todoPage" fixture depends on the option. - todoPage: async ({ page, defaultItem }, use) => { - const todoPage = new TodoPage(page); - await todoPage.goto(); - await todoPage.addToDo(defaultItem); - await use(todoPage); - await todoPage.removeAll(); - }, -}); -exports.expect = base.expect; -``` - -```js tab=js-ts title="my-test.ts" +```js title="my-test.ts" import { test as base } from '@playwright/test'; import { TodoPage } from './todo-page'; @@ -885,25 +537,7 @@ export { expect } from '@playwright/test'; We can now use `todoPage` fixture as usual, and set the `defaultItem` option in the config file. -```js tab=js-js title="playwright.config.ts" -// @ts-check - -const { defineConfig } = require('@playwright/test'); -module.exports = defineConfig({ - projects: [ - { - name: 'shopping', - use: { defaultItem: 'Buy milk' }, - }, - { - name: 'wellbeing', - use: { defaultItem: 'Exercise!' }, - }, - ] -}); -``` - -```js tab=js-ts title="playwright.config.ts" +```js title="playwright.config.ts" import { defineConfig } from '@playwright/test'; import type { MyOptions } from './my-test'; @@ -932,50 +566,7 @@ Fixtures follow these rules to determine the execution order: Consider the following example: -```js tab=js-js -const { test: base } = require('@playwright/test'); - -const test = base.extend({ - workerFixture: [async ({ browser }) => { - // workerFixture setup... - await use('workerFixture'); - // workerFixture teardown... - }, { scope: 'worker' }], - - autoWorkerFixture: [async ({ browser }) => { - // autoWorkerFixture setup... - await use('autoWorkerFixture'); - // autoWorkerFixture teardown... - }, { scope: 'worker', auto: true }], - - testFixture: [async ({ page, workerFixture }) => { - // testFixture setup... - await use('testFixture'); - // testFixture teardown... - }, { scope: 'test' }], - - autoTestFixture: [async () => { - // autoTestFixture setup... - await use('autoTestFixture'); - // autoTestFixture teardown... - }, { scope: 'test', auto: true }], - - unusedFixture: [async ({ page }) => { - // unusedFixture setup... - await use('unusedFixture'); - // unusedFixture teardown... - }, { scope: 'test' }], -}); - -test.beforeAll(async () => { /* ... */ }); -test.beforeEach(async ({ page }) => { /* ... */ }); -test('first test', async ({ page }) => { /* ... */ }); -test('second test', async ({ testFixture }) => { /* ... */ }); -test.afterEach(async () => { /* ... */ }); -test.afterAll(async () => { /* ... */ }); -``` - -```js tab=js-ts +```js import { test as base } from '@playwright/test'; const test = base.extend<{ @@ -1081,3 +672,30 @@ test('passes', async ({ database, page, a11y }) => { // use database and a11y fixtures. }); ``` + +## Box fixtures + +You can minimize the fixture exposure to the reporters UI and error messages via boxing it: + +```js +import { test as base } from '@playwright/test'; + +export const test = base.extend({ + _helperFixture: [async ({}, use, testInfo) => { + }, { box: true }], +}); +``` + +## Custom fixture title + +You can assign a custom title to a fixture to be used in error messages and in the +reporters UI: + +```js +import { test as base } from '@playwright/test'; + +export const test = base.extend({ + _innerFixture: [async ({}, use, testInfo) => { + }, { title: 'my fixture' }], +}); +``` diff --git a/packages/playwright/src/common/fixtures.ts b/packages/playwright/src/common/fixtures.ts index aa3886718c..6b50947ab5 100644 --- a/packages/playwright/src/common/fixtures.ts +++ b/packages/playwright/src/common/fixtures.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { formatLocation } from '../util'; +import { filterStackFile, formatLocation } from '../util'; import * as crypto from 'crypto'; import type { Fixtures } from '../../types/test'; import type { Location } from '../../types/testReporter'; @@ -23,7 +23,7 @@ import type { FixturesWithLocation } from './config'; export type FixtureScope = 'test' | 'worker'; type FixtureAuto = boolean | 'all-hooks-included'; const kScopeOrder: FixtureScope[] = ['test', 'worker']; -type FixtureOptions = { auto?: FixtureAuto, scope?: FixtureScope, option?: boolean, timeout?: number | undefined }; +type FixtureOptions = { auto?: FixtureAuto, scope?: FixtureScope, option?: boolean, timeout?: number | undefined, title?: string, box?: boolean }; type FixtureTuple = [ value: any, options: FixtureOptions ]; export type FixtureRegistration = { // Fixture registration location. @@ -49,8 +49,8 @@ export type FixtureRegistration = { super?: FixtureRegistration; // Whether this fixture is an option override value set from the config. optionOverride?: boolean; - // Do not generate the step for this fixture. - hideStep?: boolean; + // Do not generate the step for this fixture, consider it internal. + box?: boolean; }; export type LoadError = { message: string; @@ -63,7 +63,7 @@ type OptionOverrides = { }; function isFixtureTuple(value: any): value is FixtureTuple { - return Array.isArray(value) && typeof value[1] === 'object' && ('scope' in value[1] || 'auto' in value[1] || 'option' in value[1] || 'timeout' in value[1]); + return Array.isArray(value) && typeof value[1] === 'object'; } function isFixtureOption(value: any): value is FixtureTuple { @@ -103,15 +103,15 @@ export class FixturePool { for (const entry of Object.entries(fixtures)) { const name = entry[0]; let value = entry[1]; - let options: { auto: FixtureAuto, scope: FixtureScope, option: boolean, timeout: number | undefined, customTitle: string | undefined, hideStep: boolean | undefined } | undefined; + let options: { auto: FixtureAuto, scope: FixtureScope, option: boolean, timeout: number | undefined, customTitle?: string, box?: boolean } | undefined; if (isFixtureTuple(value)) { options = { auto: value[1].auto ?? false, scope: value[1].scope || 'test', option: !!value[1].option, timeout: value[1].timeout, - customTitle: (value[1] as any)._title, - hideStep: (value[1] as any)._hideStep, + customTitle: value[1].title, + box: value[1].box, }; value = value[0]; } @@ -128,9 +128,9 @@ export class FixturePool { continue; } } else if (previous) { - options = { auto: previous.auto, scope: previous.scope, option: previous.option, timeout: previous.timeout, customTitle: previous.customTitle, hideStep: undefined }; + options = { auto: previous.auto, scope: previous.scope, option: previous.option, timeout: previous.timeout, customTitle: previous.customTitle, box: previous.box }; } else if (!options) { - options = { auto: false, scope: 'test', option: false, timeout: undefined, customTitle: undefined, hideStep: undefined }; + options = { auto: false, scope: 'test', option: false, timeout: undefined }; } if (!kScopeOrder.includes(options.scope)) { @@ -152,7 +152,7 @@ export class FixturePool { } const deps = fixtureParameterNames(fn, location, e => this._onLoadError(e)); - const registration: FixtureRegistration = { id: '', name, location, scope: options.scope, fn, auto: options.auto, option: options.option, timeout: options.timeout, customTitle: options.customTitle, hideStep: options.hideStep, deps, super: previous, optionOverride: isOptionsOverride }; + const registration: FixtureRegistration = { id: '', name, location, scope: options.scope, fn, auto: options.auto, option: options.option, timeout: options.timeout, customTitle: options.customTitle, box: options.box, deps, super: previous, optionOverride: isOptionsOverride }; registrationId(registration); this._registrations.set(name, registration); } @@ -161,29 +161,36 @@ export class FixturePool { private validate() { const markers = new Map(); const stack: FixtureRegistration[] = []; - const visit = (registration: FixtureRegistration) => { + let hasDependencyErrors = false; + const addDependencyError = (message: string, location: Location) => { + hasDependencyErrors = true; + this._addLoadError(message, location); + }; + const visit = (registration: FixtureRegistration, boxedOnly: boolean) => { markers.set(registration, 'visiting'); stack.push(registration); for (const name of registration.deps) { const dep = this.resolve(name, registration); if (!dep) { if (name === registration.name) - this._addLoadError(`Fixture "${registration.name}" references itself, but does not have a base implementation.`, registration.location); + addDependencyError(`Fixture "${registration.name}" references itself, but does not have a base implementation.`, registration.location); else - this._addLoadError(`Fixture "${registration.name}" has unknown parameter "${name}".`, registration.location); + addDependencyError(`Fixture "${registration.name}" has unknown parameter "${name}".`, registration.location); continue; } if (kScopeOrder.indexOf(registration.scope) > kScopeOrder.indexOf(dep.scope)) { - this._addLoadError(`${registration.scope} fixture "${registration.name}" cannot depend on a ${dep.scope} fixture "${name}" defined in ${formatLocation(dep.location)}.`, registration.location); + addDependencyError(`${registration.scope} fixture "${registration.name}" cannot depend on a ${dep.scope} fixture "${name}" defined in ${formatPotentiallyInternalLocation(dep.location)}.`, registration.location); continue; } if (!markers.has(dep)) { - visit(dep); + visit(dep, boxedOnly); } else if (markers.get(dep) === 'visiting') { const index = stack.indexOf(dep); - const regs = stack.slice(index, stack.length); + const allRegs = stack.slice(index, stack.length); + const filteredRegs = allRegs.filter(r => !r.box); + const regs = boxedOnly ? filteredRegs : allRegs; const names = regs.map(r => `"${r.name}"`); - this._addLoadError(`Fixtures ${names.join(' -> ')} -> "${dep.name}" form a dependency cycle: ${regs.map(r => formatLocation(r.location)).join(' -> ')}`, dep.location); + addDependencyError(`Fixtures ${names.join(' -> ')} -> "${dep.name}" form a dependency cycle: ${regs.map(r => formatPotentiallyInternalLocation(r.location)).join(' -> ')} -> ${formatPotentiallyInternalLocation(dep.location)}`, dep.location); continue; } } @@ -191,11 +198,27 @@ export class FixturePool { stack.pop(); }; - const hash = crypto.createHash('sha1'); const names = Array.from(this._registrations.keys()).sort(); + + // First iterate over non-boxed fixtures to provide clear error messages. + for (const name of names) { + const registration = this._registrations.get(name)!; + if (!registration.box) + visit(registration, true); + } + + // If no errors found, iterate over boxed fixtures + if (!hasDependencyErrors) { + for (const name of names) { + const registration = this._registrations.get(name)!; + if (registration.box) + visit(registration, false); + } + } + + const hash = crypto.createHash('sha1'); for (const name of names) { const registration = this._registrations.get(name)!; - visit(registration); if (registration.scope === 'worker') hash.update(registration.id + ';'); } @@ -227,6 +250,11 @@ export class FixturePool { const signatureSymbol = Symbol('signature'); +export function formatPotentiallyInternalLocation(location: Location): string { + const isUserFixture = location && filterStackFile(location.file); + return isUserFixture ? formatLocation(location) : ''; +} + export function fixtureParameterNames(fn: Function | any, location: Location, onError: LoadErrorSink): string[] { if (typeof fn !== 'function') return []; diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index 3fbaf33585..2e1236d8ea 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -59,13 +59,13 @@ 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' }], + _playwrightImpl: [({}, use) => use(require('playwright-core')), { scope: 'worker', box: true }], playwright: [async ({ _playwrightImpl, screenshot }, use) => { await connector.setPlaywright(_playwrightImpl, screenshot); await use(_playwrightImpl); await connector.setPlaywright(undefined, screenshot); - }, { scope: 'worker', _hideStep: true } as any], + }, { scope: 'worker', box: true }], headless: [({ launchOptions }, use) => use(launchOptions.headless ?? true), { scope: 'worker', option: true }], channel: [({ launchOptions }, use) => use(launchOptions.channel), { scope: 'worker', option: true }], @@ -93,7 +93,7 @@ const playwrightFixtures: Fixtures = ({ await use(options); for (const browserType of [playwright.chromium, playwright.firefox, playwright.webkit]) (browserType as any)._defaultLaunchOptions = undefined; - }, { scope: 'worker', auto: true }], + }, { scope: 'worker', auto: true, box: true }], browser: [async ({ playwright, browserName, _browserOptions, connectOptions, _reuseContext }, use, testInfo) => { if (!['chromium', 'firefox', 'webkit'].includes(browserName)) @@ -152,7 +152,7 @@ const playwrightFixtures: Fixtures = ({ serviceWorkers: [({ contextOptions }, use) => use(contextOptions.serviceWorkers ?? 'allow'), { option: true }], contextOptions: [{}, { option: true }], - _combinedContextOptions: async ({ + _combinedContextOptions: [async ({ acceptDownloads, bypassCSP, colorScheme, @@ -223,7 +223,7 @@ const playwrightFixtures: Fixtures = ({ ...contextOptions, ...options, }); - }, + }, { box: true }], _setupContextOptions: [async ({ playwright, _combinedContextOptions, actionTimeout, navigationTimeout, testIdAttribute }, use, testInfo) => { if (testIdAttribute) @@ -246,9 +246,9 @@ const playwrightFixtures: Fixtures = ({ (browserType as any)._defaultContextTimeout = undefined; (browserType as any)._defaultContextNavigationTimeout = undefined; } - }, { auto: 'all-hooks-included', _title: 'context configuration' } as any], + }, { auto: 'all-hooks-included', title: 'context configuration', box: true } as any], - _contextFactory: [async ({ browser, video, _reuseContext }, use, testInfo) => { + _contextFactory: [async ({ browser, video, _reuseContext, _combinedContextOptions /** mitigate dep-via-auto lack of traceability */ }, use, testInfo) => { const testInfoImpl = testInfo as TestInfoImpl; const videoMode = normalizeVideoMode(video); const captureVideo = shouldCaptureVideo(videoMode, testInfo) && !_reuseContext; @@ -301,7 +301,7 @@ const playwrightFixtures: Fixtures = ({ } })); - }, { scope: 'test', _title: 'context' } as any], + }, { scope: 'test', title: 'context', box: true }], _optionContextReuseMode: ['none', { scope: 'worker', option: true }], _optionConnectOptions: [undefined, { scope: 'worker', option: true }], @@ -312,7 +312,7 @@ const playwrightFixtures: Fixtures = ({ mode = 'when-possible'; const reuse = mode === 'when-possible' && normalizeVideoMode(video) === 'off'; await use(reuse); - }, { scope: 'worker', _title: 'context' } as any], + }, { scope: 'worker', title: 'context', box: true }], context: async ({ playwright, browser, _reuseContext, _contextFactory }, use, testInfo) => { attachConnectedHeaderIfNeeded(testInfo, browser); diff --git a/packages/playwright/src/worker/fixtureRunner.ts b/packages/playwright/src/worker/fixtureRunner.ts index 8832acf8e2..0d94f40631 100644 --- a/packages/playwright/src/worker/fixtureRunner.ts +++ b/packages/playwright/src/worker/fixtureRunner.ts @@ -40,10 +40,10 @@ class Fixture { this.runner = runner; this.registration = registration; this.value = null; - const shouldGenerateStep = !this.registration.hideStep && !this.registration.name.startsWith('_') && !this.registration.option; - const isInternalFixture = this.registration.location && filterStackFile(this.registration.location.file); + const shouldGenerateStep = !this.registration.box && !this.registration.option; + const isUserFixture = this.registration.location && filterStackFile(this.registration.location.file); const title = this.registration.customTitle || this.registration.name; - const location = isInternalFixture ? this.registration.location : undefined; + const location = isUserFixture ? this.registration.location : undefined; this._stepInfo = shouldGenerateStep ? { category: 'fixture', location } : undefined; this._setupDescription = { title, diff --git a/packages/playwright/types/test.d.ts b/packages/playwright/types/test.d.ts index 2fd171e3bd..5a18eded3f 100644 --- a/packages/playwright/types/test.d.ts +++ b/packages/playwright/types/test.d.ts @@ -4811,13 +4811,13 @@ export type WorkerFixture = (args: Args, use: (r: R) = type TestFixtureValue = Exclude | TestFixture; type WorkerFixtureValue = Exclude | WorkerFixture; export type Fixtures = { - [K in keyof PW]?: WorkerFixtureValue | [WorkerFixtureValue, { scope: 'worker', timeout?: number | undefined }]; + [K in keyof PW]?: WorkerFixtureValue | [WorkerFixtureValue, { scope: 'worker', timeout?: number | undefined, title?: string, box?: boolean }]; } & { - [K in keyof PT]?: TestFixtureValue | [TestFixtureValue, { scope: 'test', timeout?: number | undefined }]; + [K in keyof PT]?: TestFixtureValue | [TestFixtureValue, { scope: 'test', timeout?: number | undefined, title?: string, box?: boolean }]; } & { - [K in keyof W]?: [WorkerFixtureValue, { scope: 'worker', auto?: boolean, option?: boolean, timeout?: number | undefined }]; + [K in keyof W]?: [WorkerFixtureValue, { scope: 'worker', auto?: boolean, option?: boolean, timeout?: number | undefined, title?: string, box?: boolean }]; } & { - [K in keyof T]?: TestFixtureValue | [TestFixtureValue, { scope?: 'test', auto?: boolean, option?: boolean, timeout?: number | undefined }]; + [K in keyof T]?: TestFixtureValue | [TestFixtureValue, { scope?: 'test', auto?: boolean, option?: boolean, timeout?: number | undefined, title?: string, box?: boolean }]; }; type BrowserName = 'chromium' | 'firefox' | 'webkit'; diff --git a/tests/playwright-test/fixture-errors.spec.ts b/tests/playwright-test/fixture-errors.spec.ts index c17db65bce..51c928f156 100644 --- a/tests/playwright-test/fixture-errors.spec.ts +++ b/tests/playwright-test/fixture-errors.spec.ts @@ -256,6 +256,41 @@ test('should detect fixture dependency cycle', async ({ runInlineTest }) => { expect(result.exitCode).toBe(1); }); +test('should hide boxed fixtures in dependency cycle', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'x.spec.ts': ` + import { test as base } from '@playwright/test'; + const test = base.extend({ + storageState: async ({ context, storageState }, use) => { + await use(storageState); + } + }); + test('failed', async ({ page }) => {}); + `, + }); + expect(result.output).toContain('Fixtures "context" -> "storageState" -> "context" form a dependency cycle: -> x.spec.ts:3:25 -> '); + expect(result.exitCode).toBe(1); +}); + +test('should show boxed fixtures in dependency cycle if there are no public fixtures', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'x.spec.ts': ` + import { test as base } from '@playwright/test'; + const test = base.extend({ + f1: [async ({ f2 }, use) => { + await use(f2); + }, { box: true }], + f2: [async ({ f1 }, use) => { + await use(f1); + }, { box: true }], + }); + test('failed', async ({ f1, f2 }) => {}); + `, + }); + expect(result.output).toContain('Fixtures "f1" -> "f2" -> "f1" form a dependency cycle: x.spec.ts:3:25 -> x.spec.ts:3:25 -> x.spec.ts:3:25'); + expect(result.exitCode).toBe(1); +}); + test('should not reuse fixtures from one file in another one', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.spec.ts': ` diff --git a/tests/playwright-test/timeout.spec.ts b/tests/playwright-test/timeout.spec.ts index 50069b3280..a079c3f70b 100644 --- a/tests/playwright-test/timeout.spec.ts +++ b/tests/playwright-test/timeout.spec.ts @@ -182,7 +182,7 @@ test('should respect fixture timeout', async ({ runInlineTest }) => { slowSetup: [async ({}, use) => { await new Promise(f => setTimeout(f, 2000)); await use('hey'); - }, { timeout: 500, _title: 'custom title' }], + }, { timeout: 500, title: 'custom title' }], slowTeardown: [async ({}, use) => { await use('hey'); await new Promise(f => setTimeout(f, 2000)); @@ -227,7 +227,7 @@ test('should respect test.setTimeout in the worker fixture', async ({ runInlineT slowTeardown: [async ({}, use) => { await use('hey'); await new Promise(f => setTimeout(f, 2000)); - }, { scope: 'worker', timeout: 400, _title: 'custom title' }], + }, { scope: 'worker', timeout: 400, title: 'custom title' }], }); test('test ok', async ({ fixture, noTimeout }) => { await new Promise(f => setTimeout(f, 1000)); diff --git a/utils/generate_types/overrides-test.d.ts b/utils/generate_types/overrides-test.d.ts index 0367f3259c..80880f71a7 100644 --- a/utils/generate_types/overrides-test.d.ts +++ b/utils/generate_types/overrides-test.d.ts @@ -140,13 +140,13 @@ export type WorkerFixture = (args: Args, use: (r: R) = type TestFixtureValue = Exclude | TestFixture; type WorkerFixtureValue = Exclude | WorkerFixture; export type Fixtures = { - [K in keyof PW]?: WorkerFixtureValue | [WorkerFixtureValue, { scope: 'worker', timeout?: number | undefined }]; + [K in keyof PW]?: WorkerFixtureValue | [WorkerFixtureValue, { scope: 'worker', timeout?: number | undefined, title?: string, box?: boolean }]; } & { - [K in keyof PT]?: TestFixtureValue | [TestFixtureValue, { scope: 'test', timeout?: number | undefined }]; + [K in keyof PT]?: TestFixtureValue | [TestFixtureValue, { scope: 'test', timeout?: number | undefined, title?: string, box?: boolean }]; } & { - [K in keyof W]?: [WorkerFixtureValue, { scope: 'worker', auto?: boolean, option?: boolean, timeout?: number | undefined }]; + [K in keyof W]?: [WorkerFixtureValue, { scope: 'worker', auto?: boolean, option?: boolean, timeout?: number | undefined, title?: string, box?: boolean }]; } & { - [K in keyof T]?: TestFixtureValue | [TestFixtureValue, { scope?: 'test', auto?: boolean, option?: boolean, timeout?: number | undefined }]; + [K in keyof T]?: TestFixtureValue | [TestFixtureValue, { scope?: 'test', auto?: boolean, option?: boolean, timeout?: number | undefined, title?: string, box?: boolean }]; }; type BrowserName = 'chromium' | 'firefox' | 'webkit';