diff --git a/packages/playwright-test/src/loader.ts b/packages/playwright-test/src/loader.ts index d8f1a06a2b..f0e8cf5205 100644 --- a/packages/playwright-test/src/loader.ts +++ b/packages/playwright-test/src/loader.ts @@ -16,7 +16,7 @@ import { installTransform } from './transform'; import type { Config, Project, ReporterDescription, FullProjectInternal, FullConfigInternal, Fixtures, FixturesWithLocation } from './types'; -import { getPackageJsonPath, filterUndefinedFixtures, errorWithFile } from './util'; +import { getPackageJsonPath, mergeObjects, errorWithFile } from './util'; import { setCurrentlyLoadingFileSuite } from './globals'; import { Suite, type TestCase } from './test'; import type { SerializedLoaderData } from './ipc'; @@ -98,7 +98,7 @@ export class Loader { throw new Error(`Cannot use --browser option when configuration file defines projects. Specify browserName in the projects instead.`); config.projects = takeFirst(this._configCLIOverrides.projects, config.projects as any); config.workers = takeFirst(this._configCLIOverrides.workers, config.workers); - config.use = { ...filterUndefinedFixtures(config.use), ...filterUndefinedFixtures(this._configCLIOverrides.use) }; + config.use = mergeObjects(config.use, this._configCLIOverrides.use); for (const project of config.projects || []) this._applyCLIOverridesToProject(project); @@ -232,7 +232,7 @@ export class Loader { projectConfig.repeatEach = takeFirst(this._configCLIOverrides.repeatEach, projectConfig.repeatEach); projectConfig.retries = takeFirst(this._configCLIOverrides.retries, projectConfig.retries); projectConfig.timeout = takeFirst(this._configCLIOverrides.timeout, projectConfig.timeout); - projectConfig.use = { ...filterUndefinedFixtures(projectConfig.use), ...filterUndefinedFixtures(this._configCLIOverrides.use) }; + projectConfig.use = mergeObjects(projectConfig.use, this._configCLIOverrides.use); } private _resolveProject(config: Config, fullConfig: FullConfigInternal, projectConfig: Project, throwawayArtifactsPath: string): FullProjectInternal { @@ -271,7 +271,7 @@ export class Loader { testIgnore: takeFirst(projectConfig.testIgnore, config.testIgnore, []), testMatch: takeFirst(projectConfig.testMatch, config.testMatch, '**/?(*.)@(spec|test).*'), timeout: takeFirst(projectConfig.timeout, config.timeout, defaultTimeout), - use: { ...filterUndefinedFixtures(config.use), ...filterUndefinedFixtures(projectConfig.use) }, + use: mergeObjects(config.use, projectConfig.use), }; } diff --git a/packages/playwright-test/src/testType.ts b/packages/playwright-test/src/testType.ts index f840d44a1a..3c2a932e89 100644 --- a/packages/playwright-test/src/testType.ts +++ b/packages/playwright-test/src/testType.ts @@ -19,7 +19,7 @@ import { currentlyLoadingFileSuite, currentTestInfo, setCurrentlyLoadingFileSuit import { TestCase, Suite } from './test'; import { wrapFunctionWithLocation } from './transform'; import type { Fixtures, FixturesWithLocation, Location, TestType } from './types'; -import { errorWithLocation, filterUndefinedFixtures, serializeError } from './util'; +import { errorWithLocation, serializeError } from './util'; const testTypeSymbol = Symbol('testType'); @@ -197,7 +197,7 @@ export class TestTypeImpl { private _use(location: Location, fixtures: Fixtures) { const suite = this._ensureCurrentSuite(location, `test.use()`); - suite._use.push({ fixtures: filterUndefinedFixtures(fixtures) , location }); + suite._use.push({ fixtures, location }); } private async _step(location: Location, title: string, body: () => Promise): Promise { @@ -223,7 +223,7 @@ export class TestTypeImpl { private _extend(location: Location, fixtures: Fixtures) { if ((fixtures as any)[testTypeSymbol]) throw new Error(`test.extend() accepts fixtures object, not a test object.\nDid you mean to call test._extendTest()?`); - const fixturesWithLocation: FixturesWithLocation = { fixtures: filterUndefinedFixtures(fixtures), location }; + const fixturesWithLocation: FixturesWithLocation = { fixtures, location }; return new TestTypeImpl([...this.fixtures, fixturesWithLocation]).test; } diff --git a/packages/playwright-test/src/util.ts b/packages/playwright-test/src/util.ts index 7873fb657d..9fcd51208e 100644 --- a/packages/playwright-test/src/util.ts +++ b/packages/playwright-test/src/util.ts @@ -153,11 +153,15 @@ export function createTitleMatcher(patterns: RegExp | RegExp[]): Matcher { }; } -export function filterUndefinedFixtures(o: T | undefined): T { - // We don't want "undefined" values to actually mean "undefined", - // but rather "no opinion about this option", like in all other - // places in the config. - return Object.fromEntries(Object.entries(o || {}).filter(entry => !Object.is(entry[1], undefined))) as any as T; +export function mergeObjects(a: A | undefined | void, b: B | undefined | void): A & B { + const result = { ...a } as any; + if (!Object.is(b, undefined)) { + for (const [name, value] of Object.entries(b as B)) { + if (!Object.is(value, undefined)) + result[name] = value; + } + } + return result as any; } export function forceRegExp(pattern: string): RegExp { diff --git a/tests/playwright-test/test-extend.spec.ts b/tests/playwright-test/test-extend.spec.ts index 8028e04895..ab385337dd 100644 --- a/tests/playwright-test/test-extend.spec.ts +++ b/tests/playwright-test/test-extend.spec.ts @@ -226,11 +226,11 @@ test('test._extendTest should print nice message when used as extend', async ({ expect(result.output).toContain('Did you mean to call test.extend() with fixtures instead?'); }); -test('fixture options should ignore undefined value', async ({ runInlineTest }) => { +test('test.use() with undefined should not be ignored', async ({ runInlineTest }) => { const result = await runInlineTest({ 'playwright.config.ts': ` module.exports = { - use: { option: undefined }, + use: { option: 'config' }, }; `, 'a.test.js': ` @@ -238,13 +238,11 @@ test('fixture options should ignore undefined value', async ({ runInlineTest }) test('test1', async ({ option }) => { console.log('test1-' + option); }); - test.describe('', () => { test.use({ option: 'foo' }); test('test2', async ({ option }) => { console.log('test2-' + option); }); - test.describe('', () => { test.use({ option: undefined }); test('test3', async ({ option }) => { @@ -252,7 +250,6 @@ test('fixture options should ignore undefined value', async ({ runInlineTest }) }); }); }); - test.extend({ option: undefined })('test4', async ({ option }) => { console.log('test4-' + option); }); @@ -260,8 +257,8 @@ test('fixture options should ignore undefined value', async ({ runInlineTest }) }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(4); - expect(result.output).toContain('test1-default'); + expect(result.output).toContain('test1-config'); expect(result.output).toContain('test2-foo'); - expect(result.output).toContain('test3-foo'); - expect(result.output).toContain('test4-default'); + expect(result.output).toContain('test3-undefined'); + expect(result.output).toContain('test4-undefined'); });