From 4f5fbea26ffdf9261d508a1f40cb2d61cb70266e Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 3 May 2022 13:36:24 +0100 Subject: [PATCH] chore: get rid of ProjectImpl (#13894) --- packages/playwright-test/src/loader.ts | 116 +++++++++++++++++- packages/playwright-test/src/project.ts | 121 ------------------- packages/playwright-test/src/runner.ts | 8 +- packages/playwright-test/src/testInfo.ts | 9 +- packages/playwright-test/src/workerRunner.ts | 21 ++-- 5 files changed, 128 insertions(+), 147 deletions(-) delete mode 100644 packages/playwright-test/src/project.ts diff --git a/packages/playwright-test/src/loader.ts b/packages/playwright-test/src/loader.ts index 3eecf0a6d0..3a445f645e 100644 --- a/packages/playwright-test/src/loader.ts +++ b/packages/playwright-test/src/loader.ts @@ -15,11 +15,10 @@ */ import { installTransform, setCurrentlyLoadingTestFile } from './transform'; -import type { Config, Project, ReporterDescription, FullProjectInternal } from './types'; -import type { FullConfigInternal } from './types'; +import type { Config, Project, ReporterDescription, FullProjectInternal, FullConfigInternal, Fixtures, FixturesWithLocation } from './types'; import { getPackageJsonPath, mergeObjects, errorWithFile } from './util'; import { setCurrentlyLoadingFileSuite } from './globals'; -import { Suite } from './test'; +import { Suite, type TestCase } from './test'; import type { SerializedLoaderData } from './ipc'; import * as path from 'path'; import * as url from 'url'; @@ -28,10 +27,12 @@ import * as os from 'os'; import type { BuiltInReporter, ConfigCLIOverrides } from './runner'; import type { Reporter } from '../types/testReporter'; import { builtInReporters } from './runner'; -import { isRegExp } from 'playwright-core/lib/utils'; +import { isRegExp, calculateSha1 } from 'playwright-core/lib/utils'; import { serializeError } from './util'; import { _legacyWebServer } from './plugins/webServerPlugin'; import { hostPlatform } from 'playwright-core/lib/utils/hostPlatform'; +import { FixturePool, isFixtureOption } from './fixtures'; +import type { TestTypeImpl } from './testType'; export const defaultTimeout = 30000; @@ -44,6 +45,7 @@ export class Loader { private _fullConfig: FullConfigInternal; private _configDir: string = ''; private _configFile: string | undefined; + private _projectSuiteBuilders = new Map(); constructor(configCLIOverrides?: ConfigCLIOverrides) { this._configCLIOverrides = configCLIOverrides || {}; @@ -233,6 +235,13 @@ export class Loader { return this._fullConfig; } + buildFileSuiteForProject(project: FullProjectInternal, suite: Suite, repeatEachIndex: number, filter: (test: TestCase) => boolean): Suite | undefined { + if (!this._projectSuiteBuilders.has(project)) + this._projectSuiteBuilders.set(project, new ProjectSuiteBuilder(project, this._fullConfig.projects.indexOf(project))); + const builder = this._projectSuiteBuilders.get(project)!; + return builder.cloneFileSuite(suite, repeatEachIndex, filter); + } + serialize(): SerializedLoaderData { const result: SerializedLoaderData = { configFile: this._configFile, @@ -324,6 +333,105 @@ ${'='.repeat(80)}\n`); } } +class ProjectSuiteBuilder { + private _config: FullProjectInternal; + private _index: number; + private _testTypePools = new Map(); + private _testPools = new Map(); + + constructor(project: FullProjectInternal, index: number) { + this._config = project; + this._index = index; + } + + private _buildTestTypePool(testType: TestTypeImpl): FixturePool { + if (!this._testTypePools.has(testType)) { + const fixtures = this._applyConfigUseOptions(testType, this._config.use); + const pool = new FixturePool(fixtures); + this._testTypePools.set(testType, pool); + } + return this._testTypePools.get(testType)!; + } + + // TODO: we can optimize this function by building the pool inline in cloneSuite + private _buildPool(test: TestCase): FixturePool { + if (!this._testPools.has(test)) { + 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._isDescribe); + 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); + this._testPools.set(test, pool); + } + return this._testPools.get(test)!; + } + + private _cloneEntries(from: Suite, to: Suite, repeatEachIndex: number, filter: (test: TestCase) => boolean, relativeTitlePath: string): boolean { + for (const entry of from._entries) { + if (entry instanceof Suite) { + const suite = entry._clone(); + to._addSuite(suite); + if (!this._cloneEntries(entry, suite, repeatEachIndex, filter, relativeTitlePath + ' ' + suite.title)) { + to._entries.pop(); + to.suites.pop(); + } + } else { + const test = entry._clone(); + test.retries = this._config.retries; + // We rely upon relative paths being unique. + // See `getClashingTestsPerSuite()` in `runner.ts`. + test._id = `${calculateSha1(relativeTitlePath + ' ' + entry.title)}@${entry._requireFile}#run${this._index}-repeat${repeatEachIndex}`; + test.repeatEachIndex = repeatEachIndex; + test._projectIndex = this._index; + to._addTest(test); + if (!filter(test)) { + to._entries.pop(); + to.tests.pop(); + } else { + const pool = this._buildPool(entry); + test._workerHash = `run${this._index}-${pool.digest}-repeat${repeatEachIndex}`; + test._pool = pool; + } + } + } + if (!to._entries.length) + return false; + return true; + } + + cloneFileSuite(suite: Suite, repeatEachIndex: number, filter: (test: TestCase) => boolean): Suite | undefined { + const result = suite._clone(); + return this._cloneEntries(suite, result, repeatEachIndex, filter, '') ? result : undefined; + } + + private _applyConfigUseOptions(testType: TestTypeImpl, configUse: Fixtures): FixturesWithLocation[] { + return testType.fixtures.map(f => { + const configKeys = new Set(Object.keys(configUse || {})); + const resolved = { ...f.fixtures }; + for (const [key, value] of Object.entries(resolved)) { + if (!isFixtureOption(value) || !configKeys.has(key)) + continue; + // Apply override from config file. + const override = (configUse as any)[key]; + (resolved as any)[key] = [override, value[1]]; + } + return { fixtures: resolved, location: f.location }; + }); + } +} + function takeFirst(...args: (T | undefined)[]): T { for (const arg of args) { if (arg !== undefined) diff --git a/packages/playwright-test/src/project.ts b/packages/playwright-test/src/project.ts deleted file mode 100644 index 90b43816cb..0000000000 --- a/packages/playwright-test/src/project.ts +++ /dev/null @@ -1,121 +0,0 @@ -/** - * 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 type { Fixtures, FixturesWithLocation, FullProjectInternal } from './types'; -import type { TestCase } from './test'; -import { Suite } from './test'; -import { FixturePool, isFixtureOption } from './fixtures'; -import type { TestTypeImpl } from './testType'; -import { calculateSha1 } from 'playwright-core/lib/utils'; - -export class ProjectImpl { - config: FullProjectInternal; - private index: number; - private testTypePools = new Map(); - private testPools = new Map(); - - constructor(project: FullProjectInternal, index: number) { - this.config = project; - this.index = index; - } - - private _buildTestTypePool(testType: TestTypeImpl): FixturePool { - if (!this.testTypePools.has(testType)) { - const fixtures = this._resolveFixtures(testType, this.config.use); - const pool = new FixturePool(fixtures); - this.testTypePools.set(testType, pool); - } - return this.testTypePools.get(testType)!; - } - - // TODO: we can optimize this function by building the pool inline in cloneSuite - private _buildPool(test: TestCase): FixturePool { - if (!this.testPools.has(test)) { - 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._isDescribe); - 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); - this.testPools.set(test, pool); - } - return this.testPools.get(test)!; - } - - private _cloneEntries(from: Suite, to: Suite, repeatEachIndex: number, filter: (test: TestCase) => boolean, relativeTitlePath: string): boolean { - for (const entry of from._entries) { - if (entry instanceof Suite) { - const suite = entry._clone(); - to._addSuite(suite); - if (!this._cloneEntries(entry, suite, repeatEachIndex, filter, relativeTitlePath + ' ' + suite.title)) { - to._entries.pop(); - to.suites.pop(); - } - } else { - const test = entry._clone(); - test.retries = this.config.retries; - // We rely upon relative paths being unique. - // See `getClashingTestsPerSuite()` in `runner.ts`. - test._id = `${calculateSha1(relativeTitlePath + ' ' + entry.title)}@${entry._requireFile}#run${this.index}-repeat${repeatEachIndex}`; - test.repeatEachIndex = repeatEachIndex; - test._projectIndex = this.index; - to._addTest(test); - if (!filter(test)) { - to._entries.pop(); - to.tests.pop(); - } else { - const pool = this._buildPool(entry); - test._workerHash = `run${this.index}-${pool.digest}-repeat${repeatEachIndex}`; - test._pool = pool; - } - } - } - if (!to._entries.length) - return false; - return true; - } - - cloneFileSuite(suite: Suite, repeatEachIndex: number, filter: (test: TestCase) => boolean): Suite | undefined { - const result = suite._clone(); - return this._cloneEntries(suite, result, repeatEachIndex, filter, '') ? result : undefined; - } - - private _resolveFixtures(testType: TestTypeImpl, configUse: Fixtures): FixturesWithLocation[] { - return testType.fixtures.map(f => { - const configKeys = new Set(Object.keys(configUse || {})); - const resolved = { ...f.fixtures }; - for (const [key, value] of Object.entries(resolved)) { - if (!isFixtureOption(value) || !configKeys.has(key)) - continue; - // Apply override from config file. - const override = (configUse as any)[key]; - (resolved as any)[key] = [override, value[1]]; - } - return { fixtures: resolved, location: f.location }; - }); - } -} diff --git a/packages/playwright-test/src/runner.ts b/packages/playwright-test/src/runner.ts index b4d13efb96..e2895cb329 100644 --- a/packages/playwright-test/src/runner.ts +++ b/packages/playwright-test/src/runner.ts @@ -37,7 +37,6 @@ import JSONReporter from './reporters/json'; import JUnitReporter from './reporters/junit'; import EmptyReporter from './reporters/empty'; import HtmlReporter from './reporters/html'; -import { ProjectImpl } from './project'; import type { Config, FullProjectInternal } from './types'; import type { FullConfigInternal } from './types'; import { raceAgainstTimeout } from 'playwright-core/lib/utils/timeoutRunner'; @@ -291,7 +290,6 @@ export class Runner { const outputDirs = new Set(); const rootSuite = new Suite(''); for (const [project, files] of filesByProject) { - const projectImpl = new ProjectImpl(project, config.projects.indexOf(project)); const grepMatcher = createTitleMatcher(project.grep); const grepInvertMatcher = project.grepInvert ? createTitleMatcher(project.grepInvert) : null; const projectSuite = new Suite(project.name); @@ -304,14 +302,14 @@ export class Runner { if (!fileSuite) continue; for (let repeatEachIndex = 0; repeatEachIndex < project.repeatEach; repeatEachIndex++) { - const cloned = projectImpl.cloneFileSuite(fileSuite, repeatEachIndex, test => { + const builtSuite = this._loader.buildFileSuiteForProject(project, fileSuite, repeatEachIndex, test => { const grepTitle = test.titlePath().join(' '); if (grepInvertMatcher?.(grepTitle)) return false; return grepMatcher(grepTitle); }); - if (cloned) - projectSuite._addSuite(cloned); + if (builtSuite) + projectSuite._addSuite(builtSuite); } } outputDirs.add(project.outputDir); diff --git a/packages/playwright-test/src/testInfo.ts b/packages/playwright-test/src/testInfo.ts index 7a06c06920..d1ea1fb3a7 100644 --- a/packages/playwright-test/src/testInfo.ts +++ b/packages/playwright-test/src/testInfo.ts @@ -20,14 +20,12 @@ import type { TestError, TestInfo, TestStatus } from '../types/test'; import type { FullConfigInternal, FullProjectInternal } from './types'; import type { WorkerInitParams } from './ipc'; import type { Loader } from './loader'; -import type { ProjectImpl } from './project'; import type { TestCase } from './test'; import { TimeoutManager } from './timeoutManager'; import type { Annotation, TestStepInternal } from './types'; import { addSuffixToFilePath, getContainedPath, monotonicTime, normalizeAndSaveAttachment, sanitizeForFilePath, serializeError, trimLongString } from './util'; export class TestInfoImpl implements TestInfo { - private _projectImpl: ProjectImpl; private _addStepImpl: (data: Omit) => TestStepInternal; readonly _test: TestCase; readonly _timeoutManager: TimeoutManager; @@ -85,13 +83,12 @@ export class TestInfoImpl implements TestInfo { constructor( loader: Loader, - projectImpl: ProjectImpl, + project: FullProjectInternal, workerParams: WorkerInitParams, test: TestCase, retry: number, addStepImpl: (data: Omit) => TestStepInternal, ) { - this._projectImpl = projectImpl; this._test = test; this._addStepImpl = addStepImpl; this._startTime = monotonicTime(); @@ -101,7 +98,7 @@ export class TestInfoImpl implements TestInfo { this.retry = retry; this.workerIndex = workerParams.workerIndex; this.parallelIndex = workerParams.parallelIndex; - this.project = this._projectImpl.config; + this.project = project; this.config = loader.fullConfig(); this.title = test.title; this.titlePath = test.titlePath(); @@ -117,7 +114,7 @@ export class TestInfoImpl implements TestInfo { const sameName = loader.fullConfig().projects.filter(project => project.name === this.project.name); let uniqueProjectNamePathSegment: string; if (sameName.length > 1) - uniqueProjectNamePathSegment = this.project.name + (sameName.indexOf(this._projectImpl.config) + 1); + uniqueProjectNamePathSegment = this.project.name + (sameName.indexOf(this.project) + 1); else uniqueProjectNamePathSegment = this.project.name; diff --git a/packages/playwright-test/src/workerRunner.ts b/packages/playwright-test/src/workerRunner.ts index 89cf9375e9..6d9da0bd34 100644 --- a/packages/playwright-test/src/workerRunner.ts +++ b/packages/playwright-test/src/workerRunner.ts @@ -22,8 +22,7 @@ import type { TestBeginPayload, TestEndPayload, RunPayload, DonePayload, WorkerI import { setCurrentTestInfo } from './globals'; import { Loader } from './loader'; import type { Suite, TestCase } from './test'; -import type { Annotation, TestError, TestStepInternal } from './types'; -import { ProjectImpl } from './project'; +import type { Annotation, FullProjectInternal, TestError, TestStepInternal } from './types'; import { FixtureRunner } from './fixtures'; import { ManualPromise } from 'playwright-core/lib/utils/manualPromise'; import { TestInfoImpl } from './testInfo'; @@ -35,7 +34,7 @@ const removeFolderAsync = util.promisify(rimraf); export class WorkerRunner extends EventEmitter { private _params: WorkerInitParams; private _loader!: Loader; - private _project!: ProjectImpl; + private _project!: FullProjectInternal; private _fixtureRunner: FixtureRunner; // Accumulated fatal errors that cannot be attributed to a test. @@ -111,7 +110,7 @@ export class WorkerRunner extends EventEmitter { private async _teardownScopes() { // TODO: separate timeout for teardown? - const timeoutManager = new TimeoutManager(this._project.config.timeout); + const timeoutManager = new TimeoutManager(this._project.timeout); timeoutManager.setCurrentRunnable({ type: 'teardown' }); const timeoutError = await timeoutManager.runWithTimeout(async () => { await this._fixtureRunner.teardownScope('test', timeoutManager); @@ -151,7 +150,7 @@ export class WorkerRunner extends EventEmitter { return; this._loader = await Loader.deserialize(this._params.loader); - this._project = new ProjectImpl(this._loader.fullConfig().projects[this._params.projectIndex], this._params.projectIndex); + this._project = this._loader.fullConfig().projects[this._params.projectIndex]; } async runTestGroup(runPayload: RunPayload) { @@ -161,7 +160,7 @@ export class WorkerRunner extends EventEmitter { try { await this._loadIfNeeded(); const fileSuite = await this._loader.loadTestFile(runPayload.file, 'worker'); - const suite = this._project.cloneFileSuite(fileSuite, this._params.repeatEachIndex, test => { + const suite = this._loader.buildFileSuiteForProject(this._project, fileSuite, this._params.repeatEachIndex, test => { if (!entries.has(test._id)) return false; return true; @@ -327,7 +326,7 @@ export class WorkerRunner extends EventEmitter { this._extraSuiteAnnotations.set(suite, extraAnnotations); didFailBeforeAllForSuite = suite; // Assume failure, unless reset below. // Separate timeout for each "beforeAll" modifier. - const timeSlot = { timeout: this._project.config.timeout, elapsed: 0 }; + const timeSlot = { timeout: this._project.timeout, elapsed: 0 }; await this._runModifiersForSuite(suite, testInfo, 'worker', timeSlot, extraAnnotations); } @@ -379,7 +378,7 @@ export class WorkerRunner extends EventEmitter { let afterHooksSlot: TimeSlot | undefined; if (testInfo.status === 'timedOut') { // A timed-out test gets a full additional timeout to run after hooks. - afterHooksSlot = { timeout: this._project.config.timeout, elapsed: 0 }; + afterHooksSlot = { timeout: this._project.timeout, elapsed: 0 }; } await testInfo._runWithTimeout(async () => { // Note: do not wrap all teardown steps together, because failure in any of them @@ -421,7 +420,7 @@ export class WorkerRunner extends EventEmitter { const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo); firstAfterHooksError = firstAfterHooksError || afterAllError; } - const teardownSlot = { timeout: this._project.config.timeout, elapsed: 0 }; + const teardownSlot = { timeout: this._project.timeout, elapsed: 0 }; testInfo._timeoutManager.setCurrentRunnable({ type: 'teardown', slot: teardownSlot }); const testScopeError = await testInfo._runFn(() => this._fixtureRunner.teardownScope('test', testInfo._timeoutManager)); firstAfterHooksError = firstAfterHooksError || testScopeError; @@ -470,7 +469,7 @@ export class WorkerRunner extends EventEmitter { continue; try { // Separate time slot for each "beforeAll" hook. - const timeSlot = { timeout: this._project.config.timeout, elapsed: 0 }; + const timeSlot = { timeout: this._project.timeout, elapsed: 0 }; testInfo._timeoutManager.setCurrentRunnable({ type: 'beforeAll', location: hook.location, slot: timeSlot }); await testInfo._runAsStep(() => this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo), { category: 'hook', @@ -498,7 +497,7 @@ export class WorkerRunner extends EventEmitter { continue; const afterAllError = await testInfo._runFn(async () => { // Separate time slot for each "afterAll" hook. - const timeSlot = { timeout: this._project.config.timeout, elapsed: 0 }; + const timeSlot = { timeout: this._project.timeout, elapsed: 0 }; testInfo._timeoutManager.setCurrentRunnable({ type: 'afterAll', location: hook.location, slot: timeSlot }); await testInfo._runAsStep(() => this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo), { category: 'hook',