diff --git a/src/test/fixtures.ts b/src/test/fixtures.ts index 9b81582a83..f963a993a4 100644 --- a/src/test/fixtures.ts +++ b/src/test/fixtures.ts @@ -18,9 +18,7 @@ import { formatLocation, wrapInPromise } from './util'; import * as crypto from 'crypto'; import { FixturesWithLocation, Location, WorkerInfo, TestInfo } from './types'; -type FixtureScope = 'test' | 'worker' | 'file'; -const scopeOrder: FixtureScope[] = ['worker', 'file', 'test']; - +type FixtureScope = 'test' | 'worker'; type FixtureRegistration = { location: Location; name: string; @@ -104,7 +102,7 @@ class Fixture { } export class FixturePool { - readonly digest: { worker: string, file: string }; + readonly digest: string; readonly registrations: Map; constructor(fixturesList: FixturesWithLocation[], parentPool?: FixturePool) { @@ -136,7 +134,7 @@ export class FixturePool { options = { auto: false, scope: 'test' }; } - if (options.scope !== 'test' && options.scope !== 'worker' && options.scope !== 'file') + if (options.scope !== 'test' && options.scope !== 'worker') throw errorWithLocations(`Fixture "${name}" has unknown { scope: '${options.scope}' }.`, { location, name }); const deps = fixtureParameterNames(fn, location); @@ -163,8 +161,8 @@ export class FixturePool { else throw errorWithLocations(`Fixture "${registration.name}" has unknown parameter "${name}".`, registration); } - if (scopeOrder.indexOf(registration.scope) < scopeOrder.indexOf(dep.scope)) - throw errorWithLocations(`${registration.scope} fixture "${registration.name}" cannot depend on a ${dep.scope} fixture "${name}".`, registration, dep); + if (registration.scope === 'worker' && dep.scope === 'test') + throw errorWithLocations(`Worker fixture "${registration.name}" cannot depend on a test fixture "${name}".`, registration, dep); if (!markers.has(dep)) { visit(dep); } else if (markers.get(dep) === 'visiting') { @@ -178,18 +176,15 @@ export class FixturePool { stack.pop(); }; - const hashWorker = crypto.createHash('sha1'); - const hashFile = crypto.createHash('sha1'); + const hash = crypto.createHash('sha1'); const names = Array.from(this.registrations.keys()).sort(); for (const name of names) { const registration = this.registrations.get(name)!; visit(registration); if (registration.scope === 'worker') - hashWorker.update(registration.id + ';'); - else if (registration.scope === 'file') - hashFile.update(registration.id + ';'); + hash.update(registration.id + ';'); } - return { worker: hashWorker.digest('hex'), file: hashFile.digest('hex') }; + return hash.digest('hex'); } validateFunction(fn: Function, prefix: string, allowTestFixtures: boolean, location: Location) { @@ -222,7 +217,7 @@ export class FixtureRunner { setPool(pool: FixturePool) { if (!this.testScopeClean) throw new Error('Did not teardown test scope'); - if (this.pool && pool.digest.worker !== this.pool.digest.worker) + if (this.pool && pool.digest !== this.pool.digest) throw new Error('Digests do not match'); this.pool = pool; } @@ -239,8 +234,8 @@ export class FixtureRunner { async resolveParametersAndRunHookOrTest(fn: Function, scope: FixtureScope, info: WorkerInfo|TestInfo) { // Install all automatic fixtures. for (const registration of this.pool!.registrations.values()) { - const shouldRun = scopeOrder.indexOf(registration.scope) <= scopeOrder.indexOf(scope); - if (registration.auto && shouldRun) + const shouldSkip = scope === 'worker' && registration.scope === 'test'; + if (registration.auto && !shouldSkip) await this.setupFixtureForRegistration(registration, info); } diff --git a/src/test/project.ts b/src/test/project.ts index 0a5f3a31c2..db67fb39e7 100644 --- a/src/test/project.ts +++ b/src/test/project.ts @@ -92,8 +92,7 @@ export class ProjectImpl { const test = entry._clone(); test.projectName = this.config.name; test.retries = this.config.retries; - test._workerHash = `run${this.index}-${pool.digest.worker}-repeat${repeatEachIndex}`; - test._fileFixturesHash = pool.digest.file; + test._workerHash = `run${this.index}-${pool.digest}-repeat${repeatEachIndex}`; test._id = `${entry._ordinalInFile}@${entry._requireFile}#run${this.index}-repeat${repeatEachIndex}`; test._pool = pool; test._repeatEachIndex = repeatEachIndex; diff --git a/src/test/runner.ts b/src/test/runner.ts index dde0d446e6..2a5116edef 100644 --- a/src/test/runner.ts +++ b/src/test/runner.ts @@ -454,17 +454,14 @@ function createTestGroups(rootSuite: Suite): TestGroup[] { // - They belong to different projects - requires different workers. // - They have a different repeatEachIndex - requires different workers. // - They have a different set of worker fixtures in the pool - requires different workers. - // - They have a different set of file fixtures in the pool - reuses the worker, - // but resets the 'file' scope for each group. // - They have a different requireFile - reuses the worker, but runs each requireFile separately. // We try to preserve the order of tests when they require different workers // by ordering different worker hashes sequentially. const workerHashToOrdinal = new Map(); const requireFileToOrdinal = new Map(); - const fileFixturesToOrdinal = new Map(); - const groupById = new Map(); + const groupById = new Map(); for (const projectSuite of rootSuite.suites) { for (const test of projectSuite.allTests()) { let workerHashOrdinal = workerHashToOrdinal.get(test._workerHash); @@ -479,15 +476,7 @@ function createTestGroups(rootSuite: Suite): TestGroup[] { requireFileToOrdinal.set(test._requireFile, requireFileOrdinal); } - let fileFixturesOrdinal = fileFixturesToOrdinal.get(test._fileFixturesHash); - if (!fileFixturesOrdinal) { - fileFixturesOrdinal = fileFixturesToOrdinal.size + 1; - fileFixturesToOrdinal.set(test._fileFixturesHash, fileFixturesOrdinal); - } - - const id = String(workerHashOrdinal).padStart(10, '0') - + '::' + String(requireFileOrdinal).padStart(10, '0') - + '::' + String(fileFixturesOrdinal).padStart(10, '0'); + const id = workerHashOrdinal * 10000 + requireFileOrdinal; let group = groupById.get(id); if (!group) { group = { diff --git a/src/test/test.ts b/src/test/test.ts index 7048a1c2db..ca7e8cadd1 100644 --- a/src/test/test.ts +++ b/src/test/test.ts @@ -128,7 +128,6 @@ export class TestCase extends Base implements reporterTypes.TestCase { _testType: TestTypeImpl; _id = ''; _workerHash = ''; - _fileFixturesHash = ''; _pool: FixturePool | undefined; _repeatEachIndex = 0; _projectIndex = 0; diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index 4108fa3a00..9e26ab4362 100644 --- a/src/test/workerRunner.ts +++ b/src/test/workerRunner.ts @@ -139,10 +139,6 @@ export class WorkerRunner extends EventEmitter { if (this._isStopped) return; - const result = await raceAgainstDeadline(this._fixtureRunner.teardownScope('file'), this._deadline()); - if (result.timedOut) - this._fatalError = serializeError(new Error(`Timeout of ${this._project.config.timeout}ms exceeded while shutting down environment`)); - this._reportDone(); } @@ -173,7 +169,7 @@ export class WorkerRunner extends EventEmitter { if (this._isStopped) return; // TODO: separate timeout for beforeAll? - const result = await raceAgainstDeadline(this._fixtureRunner.resolveParametersAndRunHookOrTest(hook.fn, 'file', this._workerInfo), this._deadline()); + const result = await raceAgainstDeadline(this._fixtureRunner.resolveParametersAndRunHookOrTest(hook.fn, 'worker', this._workerInfo), this._deadline()); if (result.timedOut) { this._fatalError = serializeError(new Error(`Timeout of ${this._project.config.timeout}ms exceeded while running beforeAll hook`)); this._reportDoneAndStop(); @@ -191,7 +187,7 @@ export class WorkerRunner extends EventEmitter { if (this._isStopped) return; // TODO: separate timeout for afterAll? - const result = await raceAgainstDeadline(this._fixtureRunner.resolveParametersAndRunHookOrTest(hook.fn, 'file', this._workerInfo), this._deadline()); + const result = await raceAgainstDeadline(this._fixtureRunner.resolveParametersAndRunHookOrTest(hook.fn, 'worker', this._workerInfo), this._deadline()); if (result.timedOut) { this._fatalError = serializeError(new Error(`Timeout of ${this._project.config.timeout}ms exceeded while running afterAll hook`)); this._reportDoneAndStop(); diff --git a/tests/playwright-test/fixture-errors.spec.ts b/tests/playwright-test/fixture-errors.spec.ts index 070e15f1e5..7e01ee2b52 100644 --- a/tests/playwright-test/fixture-errors.spec.ts +++ b/tests/playwright-test/fixture-errors.spec.ts @@ -174,49 +174,7 @@ test('should throw when worker fixture depends on a test fixture', async ({ runI test('works', async ({bar}) => {}); `, }); - expect(result.output).toContain('worker fixture "bar" cannot depend on a test fixture "foo".'); - expect(result.output).toContain(`f.spec.ts:5`); - expect(result.exitCode).toBe(1); -}); - -test('should throw when worker fixture depends on a file fixture', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'f.spec.ts': ` - const test = pwt.test.extend({ - foo: [async ({}, runTest) => { - await runTest(); - }, { scope: 'file' }], - - bar: [async ({ foo }, runTest) => { - await runTest(); - }, { scope: 'worker' }], - }); - - test('works', async ({bar}) => {}); - `, - }); - expect(result.output).toContain('worker fixture "bar" cannot depend on a file fixture "foo".'); - expect(result.output).toContain(`f.spec.ts:5`); - expect(result.exitCode).toBe(1); -}); - -test('should throw when file fixture depends on a test fixture', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'f.spec.ts': ` - const test = pwt.test.extend({ - foo: [async ({}, runTest) => { - await runTest(); - }, { scope: 'test' }], - - bar: [async ({ foo }, runTest) => { - await runTest(); - }, { scope: 'file' }], - }); - - test('works', async ({bar}) => {}); - `, - }); - expect(result.output).toContain('file fixture "bar" cannot depend on a test fixture "foo".'); + expect(result.output).toContain('Worker fixture "bar" cannot depend on a test fixture "foo".'); expect(result.output).toContain(`f.spec.ts:5`); expect(result.exitCode).toBe(1); }); @@ -362,7 +320,7 @@ test('should throw when overridden worker fixture depends on a test fixture', as test2('works', async ({bar}) => {}); `, }); - expect(result.output).toContain('worker fixture "bar" cannot depend on a test fixture "foo".'); + expect(result.output).toContain('Worker fixture "bar" cannot depend on a test fixture "foo".'); expect(result.exitCode).toBe(1); }); diff --git a/tests/playwright-test/fixtures.spec.ts b/tests/playwright-test/fixtures.spec.ts index 5c66443931..130a0554e5 100644 --- a/tests/playwright-test/fixtures.spec.ts +++ b/tests/playwright-test/fixtures.spec.ts @@ -628,91 +628,3 @@ test('should run tests in order', async ({ runInlineTest }) => { '%%test3', ]); }); - -test('should reset file scope but not create a new worker', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'helper.ts': ` - export const test = pwt.test.extend({ - foo: [async ({}, use) => { - await use({ value: 'default' }); - }, { scope: 'file' }] - }); - `, - 'a.test.ts': ` - import { test } from './helper'; - - test('default1', async ({foo}, testInfo) => { - expect(testInfo.workerIndex).toBe(0); - expect(foo.value).toBe('default'); - expect(foo.marker).toBe(undefined); - console.log('\\n%%default1'); - foo.marker = 42; - }); - - test.describe('suite1', () => { - test.use({ foo: { value: 'bar' } }); - test('bar', async ({foo}, testInfo) => { - expect(testInfo.workerIndex).toBe(0); - expect(foo.value).toBe('bar'); - expect(foo.marker).toBe(undefined); - console.log('\\n%%bar'); - }); - }); - - test('default2', async ({foo}, testInfo) => { - expect(testInfo.workerIndex).toBe(0); - expect(foo.value).toBe('default'); - expect(foo.marker).toBe(42); - console.log('\\n%%default2'); - }); - `, - 'b.test.ts': ` - import { test } from './helper'; - - test.use({ foo: { value: 'baz' } }); - test.beforeAll(async ({foo}) => { - expect(foo.value).toBe('baz'); - expect(foo.marker).toBe(undefined); - console.log('\\n%%beforeBaz'); - }); - test('baz', async ({foo}, testInfo) => { - expect(testInfo.workerIndex).toBe(0); - expect(foo.value).toBe('baz'); - expect(foo.marker).toBe(undefined); - console.log('\\n%%baz'); - }); - `, - }, { workers: 1 }); - expect(result.exitCode).toBe(0); - expect(result.passed).toBe(4); - expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ - '%%default1', - '%%default2', - '%%bar', - '%%beforeBaz', - '%%baz', - ]); -}); - -test('should auto-start file scope', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'a.test.ts': ` - const test = pwt.test.extend({ - foo: [async ({}, use) => { - global.fooRun = true; - await use('foo'); - }, { scope: 'file', auto: true }] - }); - - test.beforeAll(async () => { - expect(global.fooRun).toBe(true); - }); - - test('test', async () => { - expect(global.fooRun).toBe(true); - }); - `, - }); - expect(result.exitCode).toBe(0); - expect(result.passed).toBe(1); -}); diff --git a/tests/playwright-test/types.spec.ts b/tests/playwright-test/types.spec.ts index aba07443b8..4970c7bfe9 100644 --- a/tests/playwright-test/types.spec.ts +++ b/tests/playwright-test/types.spec.ts @@ -16,7 +16,7 @@ import { test, expect } from './playwright-test-fixtures'; -test('basics', async ({runTSC}) => { +test('sanity', async ({runTSC}) => { const result = await runTSC({ 'a.spec.ts': ` const { test } = pwt; @@ -31,10 +31,9 @@ test('should check types of fixtures', async ({runTSC}) => { const result = await runTSC({ 'helper.ts': ` export type MyOptions = { foo: string, bar: number }; - export const test = pwt.test.extend<{ foo: string }, { bar: number, file: string }>({ + export const test = pwt.test.extend<{ foo: string }, { bar: number }>({ foo: 'foo', bar: [ 42, { scope: 'worker' } ], - file: [ 'hey', { scope: 'file' } ], }); const good1 = test.extend<{}>({ foo: async ({ bar }, run) => run('foo') }); @@ -102,16 +101,14 @@ test('should check types of fixtures', async ({runTSC}) => { import { test } from './helper'; test.use({ foo: 'foo' }); test.use({}); - test.use({ file: 'hi' }); // @ts-expect-error test.use({ foo: 42 }); // @ts-expect-error test.use({ baz: 'baz' }); - test('my test', async ({ foo, bar, file }) => { + test('my test', async ({ foo, bar }) => { bar += parseInt(foo); - bar = foo.indexOf(file); }); test('my test', ({ foo, bar }) => { bar += parseInt(foo); diff --git a/types/test.d.ts b/types/test.d.ts index 89a71791e2..e1bcec7b49 100644 --- a/types/test.d.ts +++ b/types/test.d.ts @@ -2143,7 +2143,7 @@ export type Fixtures; } & { - [K in keyof W]?: [WorkerFixtureValue, { scope: 'worker' | 'file', auto?: boolean }]; + [K in keyof W]?: [WorkerFixtureValue, { scope: 'worker', auto?: boolean }]; } & { [K in keyof T]?: TestFixtureValue | [TestFixtureValue, { scope?: 'test', auto?: boolean }]; }; diff --git a/utils/generate_types/overrides-test.d.ts b/utils/generate_types/overrides-test.d.ts index 40073c70d6..049aa6014d 100644 --- a/utils/generate_types/overrides-test.d.ts +++ b/utils/generate_types/overrides-test.d.ts @@ -266,7 +266,7 @@ export type Fixtures; } & { - [K in keyof W]?: [WorkerFixtureValue, { scope: 'worker' | 'file', auto?: boolean }]; + [K in keyof W]?: [WorkerFixtureValue, { scope: 'worker', auto?: boolean }]; } & { [K in keyof T]?: TestFixtureValue | [TestFixtureValue, { scope?: 'test', auto?: boolean }]; };