feat(test runner): file scope fixtures (#7969)

These are reset after running tests from a single file.
This commit is contained in:
Dmitry Gozman 2021-08-04 21:11:02 -07:00 committed by GitHub
parent 450cdd8966
commit 1bbf86d060
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 214 additions and 32 deletions

View file

@ -18,7 +18,9 @@ import { formatLocation, wrapInPromise } from './util';
import * as crypto from 'crypto'; import * as crypto from 'crypto';
import { FixturesWithLocation, Location, WorkerInfo, TestInfo } from './types'; import { FixturesWithLocation, Location, WorkerInfo, TestInfo } from './types';
type FixtureScope = 'test' | 'worker'; type FixtureScope = 'test' | 'worker' | 'file';
const scopeOrder: FixtureScope[] = ['worker', 'file', 'test'];
type FixtureRegistration = { type FixtureRegistration = {
location: Location; location: Location;
name: string; name: string;
@ -102,7 +104,7 @@ class Fixture {
} }
export class FixturePool { export class FixturePool {
readonly digest: string; readonly digest: { worker: string, file: string };
readonly registrations: Map<string, FixtureRegistration>; readonly registrations: Map<string, FixtureRegistration>;
constructor(fixturesList: FixturesWithLocation[], parentPool?: FixturePool) { constructor(fixturesList: FixturesWithLocation[], parentPool?: FixturePool) {
@ -134,6 +136,9 @@ export class FixturePool {
options = { auto: false, scope: 'test' }; options = { auto: false, scope: 'test' };
} }
if (options.scope !== 'test' && options.scope !== 'worker' && options.scope !== 'file')
throw errorWithLocations(`Fixture "${name}" has unknown { scope: '${options.scope}' }.`, { location, name });
const deps = fixtureParameterNames(fn, location); const deps = fixtureParameterNames(fn, location);
const registration: FixtureRegistration = { id: '', name, location, scope: options.scope, fn, auto: options.auto, deps, super: previous }; const registration: FixtureRegistration = { id: '', name, location, scope: options.scope, fn, auto: options.auto, deps, super: previous };
registrationId(registration); registrationId(registration);
@ -158,8 +163,8 @@ export class FixturePool {
else else
throw errorWithLocations(`Fixture "${registration.name}" has unknown parameter "${name}".`, registration); throw errorWithLocations(`Fixture "${registration.name}" has unknown parameter "${name}".`, registration);
} }
if (registration.scope === 'worker' && dep.scope === 'test') if (scopeOrder.indexOf(registration.scope) < scopeOrder.indexOf(dep.scope))
throw errorWithLocations(`Worker fixture "${registration.name}" cannot depend on a test fixture "${name}".`, registration, dep); throw errorWithLocations(`${registration.scope} fixture "${registration.name}" cannot depend on a ${dep.scope} fixture "${name}".`, registration, dep);
if (!markers.has(dep)) { if (!markers.has(dep)) {
visit(dep); visit(dep);
} else if (markers.get(dep) === 'visiting') { } else if (markers.get(dep) === 'visiting') {
@ -173,15 +178,18 @@ export class FixturePool {
stack.pop(); stack.pop();
}; };
const hash = crypto.createHash('sha1'); const hashWorker = crypto.createHash('sha1');
const hashFile = crypto.createHash('sha1');
const names = Array.from(this.registrations.keys()).sort(); const names = Array.from(this.registrations.keys()).sort();
for (const name of names) { for (const name of names) {
const registration = this.registrations.get(name)!; const registration = this.registrations.get(name)!;
visit(registration); visit(registration);
if (registration.scope === 'worker') if (registration.scope === 'worker')
hash.update(registration.id + ';'); hashWorker.update(registration.id + ';');
else if (registration.scope === 'file')
hashFile.update(registration.id + ';');
} }
return hash.digest('hex'); return { worker: hashWorker.digest('hex'), file: hashFile.digest('hex') };
} }
validateFunction(fn: Function, prefix: string, allowTestFixtures: boolean, location: Location) { validateFunction(fn: Function, prefix: string, allowTestFixtures: boolean, location: Location) {
@ -214,7 +222,7 @@ export class FixtureRunner {
setPool(pool: FixturePool) { setPool(pool: FixturePool) {
if (!this.testScopeClean) if (!this.testScopeClean)
throw new Error('Did not teardown test scope'); throw new Error('Did not teardown test scope');
if (this.pool && pool.digest !== this.pool.digest) if (this.pool && pool.digest.worker !== this.pool.digest.worker)
throw new Error('Digests do not match'); throw new Error('Digests do not match');
this.pool = pool; this.pool = pool;
} }
@ -231,8 +239,8 @@ export class FixtureRunner {
async resolveParametersAndRunHookOrTest(fn: Function, scope: FixtureScope, info: WorkerInfo|TestInfo) { async resolveParametersAndRunHookOrTest(fn: Function, scope: FixtureScope, info: WorkerInfo|TestInfo) {
// Install all automatic fixtures. // Install all automatic fixtures.
for (const registration of this.pool!.registrations.values()) { for (const registration of this.pool!.registrations.values()) {
const shouldSkip = scope === 'worker' && registration.scope === 'test'; const shouldRun = scopeOrder.indexOf(registration.scope) <= scopeOrder.indexOf(scope);
if (registration.auto && !shouldSkip) if (registration.auto && shouldRun)
await this.setupFixtureForRegistration(registration, info); await this.setupFixtureForRegistration(registration, info);
} }

View file

@ -92,7 +92,8 @@ export class ProjectImpl {
const test = entry._clone(); const test = entry._clone();
test.projectName = this.config.name; test.projectName = this.config.name;
test.retries = this.config.retries; test.retries = this.config.retries;
test._workerHash = `run${this.index}-${pool.digest}-repeat${repeatEachIndex}`; test._workerHash = `run${this.index}-${pool.digest.worker}-repeat${repeatEachIndex}`;
test._fileFixturesHash = pool.digest.file;
test._id = `${entry._ordinalInFile}@${entry._requireFile}#run${this.index}-repeat${repeatEachIndex}`; test._id = `${entry._ordinalInFile}@${entry._requireFile}#run${this.index}-repeat${repeatEachIndex}`;
test._pool = pool; test._pool = pool;
test._repeatEachIndex = repeatEachIndex; test._repeatEachIndex = repeatEachIndex;

View file

@ -449,12 +449,22 @@ function buildItemLocation(rootDir: string, testOrSuite: Suite | TestCase) {
} }
function createTestGroups(rootSuite: Suite): TestGroup[] { function createTestGroups(rootSuite: Suite): TestGroup[] {
// This function groups tests that can be run together.
// Tests cannot be run together when:
// - 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 // We try to preserve the order of tests when they require different workers
// by ordering different worker hashes sequentially. // by ordering different worker hashes sequentially.
const workerHashToOrdinal = new Map<string, number>(); const workerHashToOrdinal = new Map<string, number>();
const requireFileToOrdinal = new Map<string, number>(); const requireFileToOrdinal = new Map<string, number>();
const fileFixturesToOrdinal = new Map<string, number>();
const groupById = new Map<number, TestGroup>(); const groupById = new Map<string, TestGroup>();
for (const projectSuite of rootSuite.suites) { for (const projectSuite of rootSuite.suites) {
for (const test of projectSuite.allTests()) { for (const test of projectSuite.allTests()) {
let workerHashOrdinal = workerHashToOrdinal.get(test._workerHash); let workerHashOrdinal = workerHashToOrdinal.get(test._workerHash);
@ -469,7 +479,15 @@ function createTestGroups(rootSuite: Suite): TestGroup[] {
requireFileToOrdinal.set(test._requireFile, requireFileOrdinal); requireFileToOrdinal.set(test._requireFile, requireFileOrdinal);
} }
const id = workerHashOrdinal * 10000 + 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');
let group = groupById.get(id); let group = groupById.get(id);
if (!group) { if (!group) {
group = { group = {

View file

@ -128,6 +128,7 @@ export class TestCase extends Base implements reporterTypes.TestCase {
_testType: TestTypeImpl; _testType: TestTypeImpl;
_id = ''; _id = '';
_workerHash = ''; _workerHash = '';
_fileFixturesHash = '';
_pool: FixturePool | undefined; _pool: FixturePool | undefined;
_repeatEachIndex = 0; _repeatEachIndex = 0;
_projectIndex = 0; _projectIndex = 0;

View file

@ -139,6 +139,10 @@ export class WorkerRunner extends EventEmitter {
if (this._isStopped) if (this._isStopped)
return; 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(); this._reportDone();
} }
@ -169,7 +173,7 @@ export class WorkerRunner extends EventEmitter {
if (this._isStopped) if (this._isStopped)
return; return;
// TODO: separate timeout for beforeAll? // TODO: separate timeout for beforeAll?
const result = await raceAgainstDeadline(this._fixtureRunner.resolveParametersAndRunHookOrTest(hook.fn, 'worker', this._workerInfo), this._deadline()); const result = await raceAgainstDeadline(this._fixtureRunner.resolveParametersAndRunHookOrTest(hook.fn, 'file', this._workerInfo), this._deadline());
if (result.timedOut) { if (result.timedOut) {
this._fatalError = serializeError(new Error(`Timeout of ${this._project.config.timeout}ms exceeded while running beforeAll hook`)); this._fatalError = serializeError(new Error(`Timeout of ${this._project.config.timeout}ms exceeded while running beforeAll hook`));
this._reportDoneAndStop(); this._reportDoneAndStop();
@ -187,7 +191,7 @@ export class WorkerRunner extends EventEmitter {
if (this._isStopped) if (this._isStopped)
return; return;
// TODO: separate timeout for afterAll? // TODO: separate timeout for afterAll?
const result = await raceAgainstDeadline(this._fixtureRunner.resolveParametersAndRunHookOrTest(hook.fn, 'worker', this._workerInfo), this._deadline()); const result = await raceAgainstDeadline(this._fixtureRunner.resolveParametersAndRunHookOrTest(hook.fn, 'file', this._workerInfo), this._deadline());
if (result.timedOut) { if (result.timedOut) {
this._fatalError = serializeError(new Error(`Timeout of ${this._project.config.timeout}ms exceeded while running afterAll hook`)); this._fatalError = serializeError(new Error(`Timeout of ${this._project.config.timeout}ms exceeded while running afterAll hook`));
this._reportDoneAndStop(); this._reportDoneAndStop();

View file

@ -174,7 +174,49 @@ test('should throw when worker fixture depends on a test fixture', async ({ runI
test('works', async ({bar}) => {}); test('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.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(`f.spec.ts:5`); expect(result.output).toContain(`f.spec.ts:5`);
expect(result.exitCode).toBe(1); expect(result.exitCode).toBe(1);
}); });
@ -320,7 +362,7 @@ test('should throw when overridden worker fixture depends on a test fixture', as
test2('works', async ({bar}) => {}); 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); expect(result.exitCode).toBe(1);
}); });
@ -390,3 +432,19 @@ test('should exit with timeout when fixture causes an exception in the test', as
expect(result.failed).toBe(1); expect(result.failed).toBe(1);
expect(result.output).toContain('Timeout of 500ms exceeded'); expect(result.output).toContain('Timeout of 500ms exceeded');
}); });
test('should error for unsupported scope', async ({ runInlineTest }) => {
const result = await runInlineTest({
'a.spec.ts': `
const test = pwt.test.extend({
failure: [async ({}, use) => {
await use();
}, { scope: 'foo' }]
});
test('skipped', async ({failure}) => {
});
`
});
expect(result.exitCode).toBe(1);
expect(result.output).toContain(`Error: Fixture "failure" has unknown { scope: 'foo' }`);
});

View file

@ -628,3 +628,91 @@ test('should run tests in order', async ({ runInlineTest }) => {
'%%test3', '%%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);
});

View file

@ -241,7 +241,7 @@ export const test = base.extend<Fixtures>({
return runResult; return runResult;
}); });
if (testInfo.status !== testInfo.expectedStatus && runResult && !process.env.PW_RUNNER_DEBUG) if (testInfo.status !== testInfo.expectedStatus && runResult && !process.env.PW_RUNNER_DEBUG)
console.log(runResult.output); console.log('\n' + runResult.output + '\n');
}, },
runTSC: async ({}, use, testInfo) => { runTSC: async ({}, use, testInfo) => {
@ -252,7 +252,7 @@ export const test = base.extend<Fixtures>({
return tscResult; return tscResult;
}); });
if (testInfo.status !== testInfo.expectedStatus && tscResult && !process.env.PW_RUNNER_DEBUG) if (testInfo.status !== testInfo.expectedStatus && tscResult && !process.env.PW_RUNNER_DEBUG)
console.log(tscResult.output); console.log('\n' + tscResult.output + '\n');
}, },
}); });

View file

@ -85,7 +85,7 @@ test('sigint should stop workers', async ({ runInlineTest }) => {
const { test } = pwt; const { test } = pwt;
test('interrupted1', async () => { test('interrupted1', async () => {
console.log('\\n%%SEND-SIGINT%%1'); console.log('\\n%%SEND-SIGINT%%1');
await new Promise(f => setTimeout(f, 1000)); await new Promise(f => setTimeout(f, 3000));
}); });
test('skipped1', async () => { test('skipped1', async () => {
console.log('\\n%%skipped1'); console.log('\\n%%skipped1');
@ -95,7 +95,7 @@ test('sigint should stop workers', async ({ runInlineTest }) => {
const { test } = pwt; const { test } = pwt;
test('interrupted2', async () => { test('interrupted2', async () => {
console.log('\\n%%SEND-SIGINT%%2'); console.log('\\n%%SEND-SIGINT%%2');
await new Promise(f => setTimeout(f, 1000)); await new Promise(f => setTimeout(f, 3000));
}); });
test('skipped2', async () => { test('skipped2', async () => {
console.log('\\n%%skipped2'); console.log('\\n%%skipped2');

View file

@ -28,13 +28,14 @@ test('should get top level stdio', async ({runInlineTest}) => {
}); });
` `
}); });
expect(result.output.split('\n').filter(x => x.startsWith('%%'))).toEqual([ // top level logs appear twice, because the file is required twice
'%% top level stdout', expect(result.output.split('\n').filter(x => x.startsWith('%%')).sort()).toEqual([
'%% top level stderr', '%% stderr in a test',
'%% top level stdout', // top level logs appear twice, because the file is required twice
'%% top level stderr',
'%% stdout in a test', '%% stdout in a test',
'%% stderr in a test' '%% top level stderr',
'%% top level stderr',
'%% top level stdout',
'%% top level stdout',
]); ]);
}); });

View file

@ -16,7 +16,7 @@
import { test, expect } from './playwright-test-fixtures'; import { test, expect } from './playwright-test-fixtures';
test('sanity', async ({runTSC}) => { test('basics', async ({runTSC}) => {
const result = await runTSC({ const result = await runTSC({
'a.spec.ts': ` 'a.spec.ts': `
const { test } = pwt; const { test } = pwt;
@ -31,9 +31,10 @@ test('should check types of fixtures', async ({runTSC}) => {
const result = await runTSC({ const result = await runTSC({
'helper.ts': ` 'helper.ts': `
export type MyOptions = { foo: string, bar: number }; export type MyOptions = { foo: string, bar: number };
export const test = pwt.test.extend<{ foo: string }, { bar: number }>({ export const test = pwt.test.extend<{ foo: string }, { bar: number, file: string }>({
foo: 'foo', foo: 'foo',
bar: [ 42, { scope: 'worker' } ], bar: [ 42, { scope: 'worker' } ],
file: [ 'hey', { scope: 'file' } ],
}); });
const good1 = test.extend<{}>({ foo: async ({ bar }, run) => run('foo') }); const good1 = test.extend<{}>({ foo: async ({ bar }, run) => run('foo') });
@ -101,14 +102,16 @@ test('should check types of fixtures', async ({runTSC}) => {
import { test } from './helper'; import { test } from './helper';
test.use({ foo: 'foo' }); test.use({ foo: 'foo' });
test.use({}); test.use({});
test.use({ file: 'hi' });
// @ts-expect-error // @ts-expect-error
test.use({ foo: 42 }); test.use({ foo: 42 });
// @ts-expect-error // @ts-expect-error
test.use({ baz: 'baz' }); test.use({ baz: 'baz' });
test('my test', async ({ foo, bar }) => { test('my test', async ({ foo, bar, file }) => {
bar += parseInt(foo); bar += parseInt(foo);
bar = foo.indexOf(file);
}); });
test('my test', ({ foo, bar }) => { test('my test', ({ foo, bar }) => {
bar += parseInt(foo); bar += parseInt(foo);

2
types/test.d.ts vendored
View file

@ -2141,7 +2141,7 @@ export type Fixtures<T extends KeyValue = {}, W extends KeyValue = {}, PT extend
} & { } & {
[K in keyof PT]?: TestFixtureValue<PT[K], T & W & PT & PW>; [K in keyof PT]?: TestFixtureValue<PT[K], T & W & PT & PW>;
} & { } & {
[K in keyof W]?: [WorkerFixtureValue<W[K], W & PW>, { scope: 'worker', auto?: boolean }]; [K in keyof W]?: [WorkerFixtureValue<W[K], W & PW>, { scope: 'worker' | 'file', auto?: boolean }];
} & { } & {
[K in keyof T]?: TestFixtureValue<T[K], T & W & PT & PW> | [TestFixtureValue<T[K], T & W & PT & PW>, { scope?: 'test', auto?: boolean }]; [K in keyof T]?: TestFixtureValue<T[K], T & W & PT & PW> | [TestFixtureValue<T[K], T & W & PT & PW>, { scope?: 'test', auto?: boolean }];
}; };

View file

@ -264,7 +264,7 @@ export type Fixtures<T extends KeyValue = {}, W extends KeyValue = {}, PT extend
} & { } & {
[K in keyof PT]?: TestFixtureValue<PT[K], T & W & PT & PW>; [K in keyof PT]?: TestFixtureValue<PT[K], T & W & PT & PW>;
} & { } & {
[K in keyof W]?: [WorkerFixtureValue<W[K], W & PW>, { scope: 'worker', auto?: boolean }]; [K in keyof W]?: [WorkerFixtureValue<W[K], W & PW>, { scope: 'worker' | 'file', auto?: boolean }];
} & { } & {
[K in keyof T]?: TestFixtureValue<T[K], T & W & PT & PW> | [TestFixtureValue<T[K], T & W & PT & PW>, { scope?: 'test', auto?: boolean }]; [K in keyof T]?: TestFixtureValue<T[K], T & W & PT & PW> | [TestFixtureValue<T[K], T & W & PT & PW>, { scope?: 'test', auto?: boolean }];
}; };