Skip to content

Commit

Permalink
chore: extract pool builder, simplify project suite cloning (#20235)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman committed Jan 19, 2023
1 parent d950f5b commit fdd62f3
Show file tree
Hide file tree
Showing 10 changed files with 187 additions and 151 deletions.
8 changes: 4 additions & 4 deletions packages/playwright-test/src/configLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import * as os from 'os';
import * as path from 'path';
import { isRegExp } from 'playwright-core/lib/utils';
import type { Reporter } from '../types/testReporter';
import type { SerializedLoaderData } from './ipc';
import type { SerializedConfig } from './ipc';
import type { BuiltInReporter, ConfigCLIOverrides } from './runner';
import { builtInReporters } from './runner';
import { requireOrImport } from './transform';
Expand All @@ -39,7 +39,7 @@ export class ConfigLoader {
this._fullConfig = { ...baseFullConfig };
}

static async deserialize(data: SerializedLoaderData): Promise<ConfigLoader> {
static async deserialize(data: SerializedConfig): Promise<ConfigLoader> {
const loader = new ConfigLoader(data.configCLIOverrides);
if (data.configFile)
await loader.loadConfigFile(data.configFile);
Expand Down Expand Up @@ -182,8 +182,8 @@ export class ConfigLoader {
return this._fullConfig;
}

serialize(): SerializedLoaderData {
const result: SerializedLoaderData = {
serializedConfig(): SerializedConfig {
const result: SerializedConfig = {
configFile: this._configFile,
configDir: this._configDir,
configCLIOverrides: this._configCLIOverrides,
Expand Down
6 changes: 3 additions & 3 deletions packages/playwright-test/src/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import type { TestBeginPayload, TestEndPayload, DonePayload, TestOutputPayload, StepBeginPayload, StepEndPayload, TeardownErrorsPayload, RunPayload, SerializedLoaderData } from './ipc';
import type { TestBeginPayload, TestEndPayload, DonePayload, TestOutputPayload, StepBeginPayload, StepEndPayload, TeardownErrorsPayload, RunPayload, SerializedConfig } from './ipc';
import type { TestResult, Reporter, TestStep, TestError } from '../types/testReporter';
import type { Suite } from './test';
import type { ConfigLoader } from './configLoader';
Expand Down Expand Up @@ -105,7 +105,7 @@ export class Dispatcher {

// 2. Start the worker if it is down.
if (!worker) {
worker = this._createWorker(job, index, this._configLoader.serialize());
worker = this._createWorker(job, index, this._configLoader.serializedConfig());
this._workerSlots[index].worker = worker;
worker.on('exit', () => this._workerSlots[index].worker = undefined);
await worker.start();
Expand Down Expand Up @@ -426,7 +426,7 @@ export class Dispatcher {
return result;
}

_createWorker(testGroup: TestGroup, parallelIndex: number, loaderData: SerializedLoaderData) {
_createWorker(testGroup: TestGroup, parallelIndex: number, loaderData: SerializedConfig) {
const worker = new WorkerHost(testGroup, parallelIndex, loaderData);
const handleOutput = (params: TestOutputPayload) => {
const chunk = chunkFromParams(params);
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-test/src/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import type { ConfigCLIOverrides } from './runner';
import type { TestInfoError, TestStatus } from './types';

export type SerializedLoaderData = {
export type SerializedConfig = {
configFile: string | undefined;
configDir: string;
configCLIOverrides: ConfigCLIOverrides;
Expand All @@ -40,7 +40,7 @@ export type WorkerInitParams = {
parallelIndex: number;
repeatEachIndex: number;
projectId: string;
loader: SerializedLoaderData;
config: SerializedConfig;
};

export type TestBeginPayload = {
Expand Down
88 changes: 88 additions & 0 deletions packages/playwright-test/src/poolBuilder.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/**
* Copyright Microsoft Corporation. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { FixturePool, isFixtureOption } from './fixtures';
import type { Suite, TestCase } from './test';
import type { TestTypeImpl } from './testType';
import type { Fixtures, FixturesWithLocation, FullProjectInternal } from './types';

export class PoolBuilder {
private _project: FullProjectInternal;
private _testTypePools = new Map<TestTypeImpl, FixturePool>();

constructor(project: FullProjectInternal) {
this._project = project;
}

buildPools(suite: Suite, repeatEachIndex: number) {
suite.forEachTest(test => {
const pool = this._buildPoolForTest(test);
test._workerHash = `run${this._project._id}-${pool.digest}-repeat${repeatEachIndex}`;
test._pool = pool;
});
}

private _buildPoolForTest(test: TestCase): FixturePool {
let pool = this._buildTestTypePool(test._testType);

const parents: Suite[] = [];
for (let parent: Suite | undefined = test.parent; parent; parent = parent.parent)
parents.push(parent);
parents.reverse();

for (const parent of parents) {
if (parent._use.length)
pool = new FixturePool(parent._use, pool, parent._type === 'describe');
for (const hook of parent._hooks)
pool.validateFunction(hook.fn, hook.type + ' hook', hook.location);
for (const modifier of parent._modifiers)
pool.validateFunction(modifier.fn, modifier.type + ' modifier', modifier.location);
}

pool.validateFunction(test.fn, 'Test', test.location);
return pool;
}

private _buildTestTypePool(testType: TestTypeImpl): FixturePool {
if (!this._testTypePools.has(testType)) {
const fixtures = this._applyConfigUseOptions(testType, this._project.use || {});
const pool = new FixturePool(fixtures);
this._testTypePools.set(testType, pool);
}
return this._testTypePools.get(testType)!;
}

private _applyConfigUseOptions(testType: TestTypeImpl, configUse: Fixtures): FixturesWithLocation[] {
const configKeys = new Set(Object.keys(configUse));
if (!configKeys.size)
return testType.fixtures;
const result: FixturesWithLocation[] = [];
for (const f of testType.fixtures) {
result.push(f);
const optionsFromConfig: Fixtures = {};
for (const [key, value] of Object.entries(f.fixtures)) {
if (isFixtureOption(value) && configKeys.has(key))
(optionsFromConfig as any)[key] = [(configUse as any)[key], value[1]];
}
if (Object.entries(optionsFromConfig).length) {
// Add config options immediately after original option definition,
// so that any test.use() override it.
result.push({ fixtures: optionsFromConfig, location: { file: `project#${this._project._id}`, line: 1, column: 1 }, fromConfig: true });
}
}
return result;
}
}
11 changes: 8 additions & 3 deletions packages/playwright-test/src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ import { createFileMatcher, createFileMatcherFromFilters, createTitleMatcher, se
import type { Matcher, TestFileFilter } from './util';
import { setFatalErrorSink } from './globals';
import { TestLoader } from './testLoader';
import { buildFileSuiteForProject, filterTests } from './suiteUtils';
import { PoolBuilder } from './poolBuilder';

const removeFolderAsync = promisify(rimraf);
const readDirAsync = promisify(fs.readdir);
Expand Down Expand Up @@ -319,6 +321,7 @@ export class Runner {

const rootSuite = new Suite('', 'root');
for (const [project, files] of filesByProject) {
const poolBuilder = new PoolBuilder(project);
const grepMatcher = createTitleMatcher(project.grep);
const grepInvertMatcher = project.grepInvert ? createTitleMatcher(project.grepInvert) : null;

Expand All @@ -339,9 +342,11 @@ export class Runner {
if (!fileSuite)
continue;
for (let repeatEachIndex = 0; repeatEachIndex < project.repeatEach; repeatEachIndex++) {
const builtSuite = testLoader.buildFileSuiteForProject(project, fileSuite, repeatEachIndex, titleMatcher);
if (builtSuite)
projectSuite._addSuite(builtSuite);
const builtSuite = buildFileSuiteForProject(project, fileSuite, repeatEachIndex);
if (!filterTests(builtSuite, titleMatcher))
continue;
projectSuite._addSuite(builtSuite);
poolBuilder.buildPools(builtSuite, repeatEachIndex);
}
}
}
Expand Down
58 changes: 58 additions & 0 deletions packages/playwright-test/src/suiteUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* Copyright Microsoft Corporation. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import path from 'path';
import { calculateSha1 } from 'playwright-core/lib/utils';
import type { Suite, TestCase } from './test';
import type { FullProjectInternal } from './types';

export function filterTests(suite: Suite, filter: (test: TestCase) => boolean): boolean {
suite.suites = suite.suites.filter(child => filterTests(child, filter));
suite.tests = suite.tests.filter(filter);
const entries = new Set([...suite.suites, ...suite.tests]);
suite._entries = suite._entries.filter(e => entries.has(e)); // Preserve the order.
return !!suite._entries.length;
}

export function buildFileSuiteForProject(project: FullProjectInternal, suite: Suite, repeatEachIndex: number): Suite {
const relativeFile = path.relative(project.testDir, suite.location!.file).split(path.sep).join('/');
const fileId = calculateSha1(relativeFile).slice(0, 20);

// Clone suite.
const result = suite._deepClone();
result._fileId = fileId;

// Assign test properties with project-specific values.
result.forEachTest((test, suite) => {
suite._fileId = fileId;
const repeatEachIndexSuffix = repeatEachIndex ? ` (repeat:${repeatEachIndex})` : '';
// At the point of the query, suite is not yet attached to the project, so we only get file, describe and test titles.
const testIdExpression = `[project=${project._id}]${test.titlePath().join('\x1e')}${repeatEachIndexSuffix}`;
const testId = fileId + '-' + calculateSha1(testIdExpression).slice(0, 20);
test.id = testId;
test.repeatEachIndex = repeatEachIndex;
test._projectId = project._id;
test.retries = project.retries;
for (let parentSuite: Suite | undefined = suite; parentSuite; parentSuite = parentSuite.parent) {
if (parentSuite._retries !== undefined) {
test.retries = parentSuite._retries;
break;
}
}
});

return result;
}
9 changes: 9 additions & 0 deletions packages/playwright-test/src/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,15 @@ export class Suite extends Base implements reporterTypes.Suite {
return suite;
}

forEachTest(visitor: (test: TestCase, suite: Suite) => void) {
for (const entry of this._entries) {
if (entry instanceof Suite)
entry.forEachTest(visitor);
else
visitor(entry, this);
}
}

_clone(): Suite {
const suite = new Suite(this.title, this._type);
suite._only = this._only;
Expand Down

0 comments on commit fdd62f3

Please sign in to comment.