fix(test runner): ignore undefined values in fixtures definitions (#15119)

These mean "I don't want to specify this fixture/option"
instead of "I want the value of undefined", aligned with how TypeScript works.
We already do similar things in the config.

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, mergeObjects, errorWithFile } from './util';
+import { getPackageJsonPath, filterUndefinedFixtures, 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 = mergeObjects(config.use, this._configCLIOverrides.use);
+    config.use = { ...filterUndefinedFixtures(config.use), ...filterUndefinedFixtures(this._configCLIOverrides.use) };
     for (const project of config.projects || [])
       this._applyCLIOverridesToProject(project);
 
@@ -224,7 +224,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 = mergeObjects(projectConfig.use, this._configCLIOverrides.use);
+    projectConfig.use = { ...filterUndefinedFixtures(projectConfig.use), ...filterUndefinedFixtures(this._configCLIOverrides.use) };
   }
 
   private _resolveProject(config: Config, fullConfig: FullConfigInternal, projectConfig: Project, throwawayArtifactsPath: string): FullProjectInternal {
@@ -263,7 +263,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: mergeObjects(config.use, projectConfig.use),
+      use: { ...filterUndefinedFixtures(config.use), ...filterUndefinedFixtures(projectConfig.use) },
     };
   }
 

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, serializeError } from './util';
+import { errorWithLocation, filterUndefinedFixtures, serializeError } from './util';
 
 const testTypeSymbol = Symbol('testType');
 
@@ -196,7 +196,7 @@ export class TestTypeImpl {
 
   private _use(location: Location, fixtures: Fixtures) {
     const suite = this._ensureCurrentSuite(location, `test.use()`);
-    suite._use.push({ fixtures, location });
+    suite._use.push({ fixtures: filterUndefinedFixtures(fixtures) , location });
   }
 
   private async _step(location: Location, title: string, body: () => Promise<void>): Promise<void> {
@@ -222,7 +222,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, location };
+    const fixturesWithLocation: FixturesWithLocation = { fixtures: filterUndefinedFixtures(fixtures), location };
     return new TestTypeImpl([...this.fixtures, fixturesWithLocation]).test;
   }
 

packages/playwright-test/src/util.ts

@@ -158,15 +158,11 @@ export function createTitleMatcher(patterns: RegExp | RegExp[]): Matcher {
   };
 }
 
-export function mergeObjects<A extends object, B extends object>(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 filterUndefinedFixtures<T extends object>(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 forceRegExp(pattern: string): RegExp {

tests/playwright-test/test-extend.spec.ts

@@ -225,3 +225,43 @@ test('test._extendTest should print nice message when used as extend', async ({
   expect(result.passed).toBe(0);
   expect(result.output).toContain('Did you mean to call test.extend() with fixtures instead?');
 });
+
+test('fixture options should ignore undefined value', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'playwright.config.ts': `
+      module.exports = {
+        use: { option: undefined },
+      };
+    `,
+    'a.test.js': `
+      const test = pwt.test.extend({ option: [ 'default', { option: true } ] });
+      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 }) => {
+            console.log('test3-' + option);
+          });
+        });
+      });
+
+      test.extend({ option: undefined })('test4', async ({ option }) => {
+        console.log('test4-' + option);
+      });
+    `,
+  });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(4);
+  expect(result.output).toContain('test1-default');
+  expect(result.output).toContain('test2-foo');
+  expect(result.output).toContain('test3-foo');
+  expect(result.output).toContain('test4-default');
+});