From 65ce4cd213acac18bd5e21b5b8efd0a297865fd0 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 3 Oct 2023 13:26:30 -0700 Subject: [PATCH] feat: expose `composedTest()` instead of `test._extendTest()` (#27414) --- packages/playwright/src/common/testType.ts | 25 ++++++++------- packages/playwright/src/index.ts | 3 ++ packages/playwright/test.js | 2 -- packages/playwright/types/test.d.ts | 9 ++++++ tests/config/baseTest.ts | 15 ++------- .../playwright-test-plugin-types.ts | 4 +-- .../fixture-scripts/plugin.spec.ts | 4 +-- tests/playwright-test/test-extend.spec.ts | 16 +++++----- tests/playwright-test/types-2.spec.ts | 32 ++++++++++++++----- utils/generate_types/overrides-test.d.ts | 9 ++++++ 10 files changed, 74 insertions(+), 45 deletions(-) diff --git a/packages/playwright/src/common/testType.ts b/packages/playwright/src/common/testType.ts index 7d32cf7ba2..5af6df000e 100644 --- a/packages/playwright/src/common/testType.ts +++ b/packages/playwright/src/common/testType.ts @@ -57,7 +57,6 @@ export class TestTypeImpl { test.step = this._step.bind(this); test.use = wrapFunctionWithLocation(this._use.bind(this)); test.extend = wrapFunctionWithLocation(this._extend.bind(this)); - test._extendTest = wrapFunctionWithLocation(this._extendTest.bind(this)); test.info = () => { const result = currentTestInfo(); if (!result) @@ -231,19 +230,10 @@ 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()?`); + throw new Error(`test.extend() accepts fixtures object, not a test object.\nDid you mean to call composedTest()?`); const fixturesWithLocation: FixturesWithLocation = { fixtures, location }; return new TestTypeImpl([...this.fixtures, fixturesWithLocation]).test; } - - private _extendTest(location: Location, test: TestType) { - const testTypeImpl = (test as any)[testTypeSymbol] as TestTypeImpl; - if (!testTypeImpl) - throw new Error(`test._extendTest() accepts a single "test" parameter.\nDid you mean to call test.extend() with fixtures instead?`); - // Filter out common ancestor fixtures. - const newFixtures = testTypeImpl.fixtures.filter(theirs => !this.fixtures.find(ours => ours.fixtures === theirs.fixtures)); - return new TestTypeImpl([...this.fixtures, ...newFixtures]).test; - } } function throwIfRunningInsideJest() { @@ -258,3 +248,16 @@ function throwIfRunningInsideJest() { } export const rootTestType = new TestTypeImpl([]); + +export function composedTest(...tests: TestType[]) { + let result = rootTestType; + for (const t of tests) { + const testTypeImpl = (t as any)[testTypeSymbol] as TestTypeImpl; + if (!testTypeImpl) + throw new Error(`composedTest() accepts "test" functions as parameters.\nDid you mean to call test.extend() with fixtures instead?`); + // Filter out common ancestor fixtures. + const newFixtures = testTypeImpl.fixtures.filter(theirs => !result.fixtures.find(ours => ours.fixtures === theirs.fixtures)); + result = new TestTypeImpl([...result.fixtures, ...newFixtures]); + } + return result.test; +} diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index 0fcdb8f345..d1b1f8b35d 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -764,4 +764,7 @@ function renderApiCall(apiName: string, params: any) { export const test = _baseTest.extend(playwrightFixtures); +export { defineConfig } from './common/configLoader'; +export { composedTest } from './common/testType'; + export default test; diff --git a/packages/playwright/test.js b/packages/playwright/test.js index c066192a74..30128d505c 100644 --- a/packages/playwright/test.js +++ b/packages/playwright/test.js @@ -15,12 +15,10 @@ */ const pwt = require('./lib/index'); -const { defineConfig } = require('./lib/common/configLoader'); const playwright = require('./index'); const combinedExports = { ...playwright, ...pwt, - defineConfig, }; Object.defineProperty(combinedExports, '__esModule', { value: true }); diff --git a/packages/playwright/types/test.d.ts b/packages/playwright/types/test.d.ts index 5c477d3768..3582e5505f 100644 --- a/packages/playwright/types/test.d.ts +++ b/packages/playwright/types/test.d.ts @@ -5198,6 +5198,15 @@ export function defineConfig(config: PlaywrightTestConfig, ...configs: Playwrigh export function defineConfig(config: PlaywrightTestConfig, ...configs: PlaywrightTestConfig[]): PlaywrightTestConfig; export function defineConfig(config: PlaywrightTestConfig, ...configs: PlaywrightTestConfig[]): PlaywrightTestConfig; +type MergedT = List extends [TestType, ...(infer Rest)] ? T & MergedT : {}; +type MergedW = List extends [TestType, ...(infer Rest)] ? W & MergedW : {}; +type MergedTestType = TestType, MergedW>; + +/** + * Merges fixtures + */ +export function composedTest(...tests: List): MergedTestType; + // This is required to not export everything by default. See https://github.com/Microsoft/TypeScript/issues/19545#issuecomment-340490459 export {}; diff --git a/tests/config/baseTest.ts b/tests/config/baseTest.ts index a74bee9cf0..a89c5ff7cc 100644 --- a/tests/config/baseTest.ts +++ b/tests/config/baseTest.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import type { TestType, Fixtures } from '@playwright/test'; +import { composedTest } from '@playwright/test'; import { test } from '@playwright/test'; import type { CommonFixtures, CommonWorkerFixtures } from './commonFixtures'; import { commonFixtures } from './commonFixtures'; @@ -24,18 +24,9 @@ import { coverageTest } from './coverageFixtures'; import { platformTest } from './platformFixtures'; import { testModeTest } from './testModeFixtures'; -interface TestTypeEx extends TestType { - extend(fixtures: Fixtures): TestTypeEx; - _extendTest(other: TestType): TestTypeEx; -} -type BaseT = (typeof test) extends TestType ? T : never; // eslint-disable-line -type BaseW = (typeof test) extends TestType ? W : never; // eslint-disable-line -export const base = test as TestTypeEx; +export const base = test; -export const baseTest = base - ._extendTest(coverageTest) - ._extendTest(platformTest) - ._extendTest(testModeTest) +export const baseTest = composedTest(base, coverageTest, platformTest, testModeTest) .extend(commonFixtures) .extend(serverFixtures); diff --git a/tests/installation/fixture-scripts/playwright-test-plugin-types.ts b/tests/installation/fixture-scripts/playwright-test-plugin-types.ts index c18826a078..1b8057af5b 100644 --- a/tests/installation/fixture-scripts/playwright-test-plugin-types.ts +++ b/tests/installation/fixture-scripts/playwright-test-plugin-types.ts @@ -1,8 +1,8 @@ -import { test as test1 } from '@playwright/test'; +import { test as test1, composedTest } from '@playwright/test'; import type { Page } from '@playwright/test'; import { test as test2 } from 'playwright-test-plugin'; -const test = (test1 as any)._extendTest(test2); +const test = composedTest(test1, test2); test('sample test', async ({ page, plugin }) => { type IsPage = (typeof page) extends Page ? true : never; diff --git a/tests/installation/fixture-scripts/plugin.spec.ts b/tests/installation/fixture-scripts/plugin.spec.ts index fdf1f42b27..23b0bfddca 100644 --- a/tests/installation/fixture-scripts/plugin.spec.ts +++ b/tests/installation/fixture-scripts/plugin.spec.ts @@ -1,7 +1,7 @@ -import { test as test1, expect } from '@playwright/test'; +import { test as test1, expect, composedTest } from '@playwright/test'; import { test as test2 } from 'playwright-test-plugin'; -const test = (test1 as any)._extendTest(test2); +const test = composedTest(test1, test2); test('sample test', async ({ page, plugin }) => { await page.setContent(`
hello
world`); diff --git a/tests/playwright-test/test-extend.spec.ts b/tests/playwright-test/test-extend.spec.ts index 7bbc0df20d..b4584b1d48 100644 --- a/tests/playwright-test/test-extend.spec.ts +++ b/tests/playwright-test/test-extend.spec.ts @@ -160,7 +160,7 @@ test('config should override options but not fixtures', async ({ runInlineTest } expect(result.output).toContain('fixture-config-fixture'); }); -test('test.extend should be able to merge', async ({ runInlineTest }) => { +test('composedTest should be able to merge', async ({ runInlineTest }) => { const result = await runInlineTest({ 'playwright.config.ts': ` module.exports = { @@ -168,7 +168,7 @@ test('test.extend should be able to merge', async ({ runInlineTest }) => { }; `, 'a.test.ts': ` - import { test, expect } from '@playwright/test'; + import { test, expect, composedTest } from '@playwright/test'; const base = test.extend({ myFixture: 'abc', }); @@ -184,7 +184,7 @@ test('test.extend should be able to merge', async ({ runInlineTest }) => { fixture2: ({}, use) => use('fixture2'), }); - const test3 = test1._extendTest(test2); + const test3 = composedTest(test1, test2); test3('merged', async ({ param, fixture1, myFixture, fixture2 }) => { console.log('param-' + param); @@ -202,7 +202,7 @@ test('test.extend should be able to merge', async ({ runInlineTest }) => { expect(result.output).toContain('fixture2-fixture2'); }); -test('test.extend should print nice message when used as _extendTest', async ({ runInlineTest }) => { +test('test.extend should print nice message when used as composedTest', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.ts': ` import { test as base, expect } from '@playwright/test'; @@ -215,14 +215,14 @@ test('test.extend should print nice message when used as _extendTest', async ({ }); expect(result.exitCode).toBe(1); expect(result.passed).toBe(0); - expect(result.output).toContain('Did you mean to call test._extendTest()?'); + expect(result.output).toContain('Did you mean to call composedTest()?'); }); -test('test._extendTest should print nice message when used as extend', async ({ runInlineTest }) => { +test('composedTest should print nice message when used as extend', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.ts': ` - import { test as base, expect } from '@playwright/test'; - const test3 = base._extendTest({}); + import { test as base, expect, composedTest } from '@playwright/test'; + const test3 = composedTest(base, {}); test3('test', () => {}); `, }); diff --git a/tests/playwright-test/types-2.spec.ts b/tests/playwright-test/types-2.spec.ts index 25fa0f1e0c..699e241ec5 100644 --- a/tests/playwright-test/types-2.spec.ts +++ b/tests/playwright-test/types-2.spec.ts @@ -84,11 +84,11 @@ test('can return anything from hooks', async ({ runTSC }) => { test('test.extend options should check types', async ({ runTSC }) => { const result = await runTSC({ 'helper.ts': ` - import { test as base, expect } from '@playwright/test'; + import { test as base, expect, composedTest } from '@playwright/test'; export type Params = { foo: string }; export const test = base; export const test1 = test.extend({ foo: [ 'foo', { option: true } ] }); - export const test1b = test.extend<{ bar: string }>({ bar: [ 'bar', { option: true } ] }); + export const testW = test.extend<{}, { bar: string }>({ bar: ['bar', { scope: 'worker' }] }); export const testerror = test.extend<{ foo: string }>({ // @ts-expect-error foo: 123 @@ -100,9 +100,21 @@ test('test.extend options should check types', async ({ runTSC }) => { // @ts-expect-error bar: async ({ baz }, run) => { await run(42); } }); - // TODO: enable when _extendTest is out of experiment. - // export const test4 = test1._extendTest(test1b); - export const test4 = test1; + export const test4 = composedTest(test1, testW); + const test5 = test4.extend<{}, { hey: string, hey2: string }>({ + // @ts-expect-error + hey: [async ({ foo }, use) => { + await use(foo); + }, { scope: 'worker' }], + hey2: [async ({ bar }, use) => { + await use(bar); + }, { scope: 'worker' }], + }); + export const test6 = test4.extend<{ hey: string }>({ + hey: async ({ foo }, use) => { + await use(foo); + }, + }); `, 'playwright.config.ts': ` import { Params } from './helper'; @@ -127,7 +139,7 @@ test('test.extend options should check types', async ({ runTSC }) => { module.exports = configs; `, 'a.spec.ts': ` - import { test, test1, test2, test3, test4 } from './helper'; + import { test, test1, test2, test3, test4, test6 } from './helper'; // @ts-expect-error test('my test', async ({ foo }) => {}); test1('my test', async ({ foo }) => {}); @@ -136,8 +148,12 @@ test('test.extend options should check types', async ({ runTSC }) => { test2('my test', async ({ foo, bar }) => {}); // @ts-expect-error test2('my test', async ({ foo, baz }) => {}); - // TODO: enable when _extendTest is out of experiment. - // test4('my test', async ({ foo, bar }) => {}); + test4('my test', async ({ foo, bar }) => {}); + // @ts-expect-error + test4('my test', async ({ foo, qux }) => {}); + test6('my test', async ({ bar, hey }) => {}); + // @ts-expect-error + test6('my test', async ({ qux }) => {}); ` }); expect(result.exitCode).toBe(0); diff --git a/utils/generate_types/overrides-test.d.ts b/utils/generate_types/overrides-test.d.ts index 3e4a5bdee7..b8c4c7e72b 100644 --- a/utils/generate_types/overrides-test.d.ts +++ b/utils/generate_types/overrides-test.d.ts @@ -455,5 +455,14 @@ export function defineConfig(config: PlaywrightTestConfig, ...configs: Playwrigh export function defineConfig(config: PlaywrightTestConfig, ...configs: PlaywrightTestConfig[]): PlaywrightTestConfig; export function defineConfig(config: PlaywrightTestConfig, ...configs: PlaywrightTestConfig[]): PlaywrightTestConfig; +type MergedT = List extends [TestType, ...(infer Rest)] ? T & MergedT : {}; +type MergedW = List extends [TestType, ...(infer Rest)] ? W & MergedW : {}; +type MergedTestType = TestType, MergedW>; + +/** + * Merges fixtures + */ +export function composedTest(...tests: List): MergedTestType; + // This is required to not export everything by default. See https://github.com/Microsoft/TypeScript/issues/19545#issuecomment-340490459 export {};