Filip/playwright
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

Revert "feat(test runner): file scope fixtures (#7969)" (#8081)

Browse source 
This reverts commit 1bbf86d060,
leaving small improvements around.
This commit is contained in:
Dmitry Gozman 2021-08-09 12:33:16 -07:00 committed by GitHub
parent 91394b257c
commit 41949e559e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 23 additions and 178 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
src/test/fixtures.ts
View file

@ -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<string, FixtureRegistration>;
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);
}

3
src/test/project.ts
View file

@ -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;

15
src/test/runner.ts
View file

@ -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<string, number>();
const requireFileToOrdinal = new Map<string, number>();
const fileFixturesToOrdinal = new Map<string, number>();
const groupById = new Map<string, TestGroup>();
const groupById = new Map<number, TestGroup>();
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 = {

1
src/test/test.ts
View file

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

8
src/test/workerRunner.ts
View file

@ -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();

46
tests/playwright-test/fixture-errors.spec.ts
View file

@ -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);
});

88
tests/playwright-test/fixtures.spec.ts
View file

@ -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);
});

9
tests/playwright-test/types.spec.ts
View file

@ -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);

2
types/test.d.ts vendored
View file

@ -2143,7 +2143,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 W]?: [WorkerFixtureValue<W[K], W & PW>, { scope: 'worker' | 'file', auto?: boolean }];
[K in keyof W]?: [WorkerFixtureValue<W[K], W & PW>, { scope: 'worker', auto?: boolean }];
} & {
[K in keyof T]?: TestFixtureValue<T[K], T & W & PT & PW> | [TestFixtureValue<T[K], T & W & PT & PW>, { scope?: 'test', auto?: boolean }];
};

2
utils/generate_types/overrides-test.d.ts vendored
View file

@ -266,7 +266,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 W]?: [WorkerFixtureValue<W[K], W & PW>, { scope: 'worker' | 'file', auto?: boolean }];
[K in keyof W]?: [WorkerFixtureValue<W[K], W & PW>, { scope: 'worker', auto?: boolean }];
} & {
[K in keyof T]?: TestFixtureValue<T[K], T & W & PT & PW> | [TestFixtureValue<T[K], T & W & PT & PW>, { scope?: 'test', auto?: boolean }];
};