From f37f38e5533cf79934261a74197731c706237da5 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 16 Mar 2023 17:11:15 -0700 Subject: [PATCH] chore: decouple test groups from root suite as much as possible (#21731) --- .../playwright-test/src/runner/loadUtils.ts | 54 ++++++------------ packages/playwright-test/src/runner/tasks.ts | 57 ++++++++++++------- 2 files changed, 51 insertions(+), 60 deletions(-) diff --git a/packages/playwright-test/src/runner/loadUtils.ts b/packages/playwright-test/src/runner/loadUtils.ts index 493aa9057f..1f6c774199 100644 --- a/packages/playwright-test/src/runner/loadUtils.ts +++ b/packages/playwright-test/src/runner/loadUtils.ts @@ -30,12 +30,6 @@ import { buildFileSuiteForProject, filterByFocusedLine, filterByTestIds, filterO import { createTestGroups, filterForShard, type TestGroup } from './testGroups'; import { dependenciesForTestFile } from '../common/compilationCache'; -export type ProjectWithTestGroups = { - project: FullProjectInternal; - projectSuite: Suite; - testGroups: TestGroup[]; -}; - export async function collectProjectsAndTestFiles(config: FullConfigInternal, projectsToIgnore: Set, additionalFileMatcher: Matcher | undefined) { const fsCache = new Map(); const sourceMapCache = new Map(); @@ -120,7 +114,7 @@ export async function loadFileSuites(mode: 'out-of-process' | 'in-process', conf return fileSuitesByProject; } -export async function createRootSuiteAndTestGroups(config: FullConfigInternal, fileSuitesByProject: Map, errors: TestError[], shouldFilterOnly: boolean): Promise<{ rootSuite: Suite, projectsWithTestGroups: ProjectWithTestGroups[] }> { +export async function createRootSuite(config: FullConfigInternal, fileSuitesByProject: Map, errors: TestError[], shouldFilterOnly: boolean): Promise { // Create root suite, where each child will be a project suite with cloned file suites inside it. const rootSuite = new Suite('', 'root'); @@ -150,55 +144,39 @@ export async function createRootSuiteAndTestGroups(config: FullConfigInternal, f if (shouldFilterOnly) filterOnly(rootSuite); - // Create test groups for top-level projects. - let projectsWithTestGroups: ProjectWithTestGroups[] = []; - for (const projectSuite of rootSuite.suites) { - const project = projectSuite.project() as FullProjectInternal; - const testGroups = createTestGroups(projectSuite, config.workers); - projectsWithTestGroups.push({ project, projectSuite, testGroups }); - } - // Shard only the top-level projects. if (config.shard) { - const allTestGroups: TestGroup[] = []; - for (const { testGroups } of projectsWithTestGroups) - allTestGroups.push(...testGroups); - const shardedTestGroups = filterForShard(config.shard, allTestGroups); + // Create test groups for top-level projects. + const testGroups: TestGroup[] = []; + for (const projectSuite of rootSuite.suites) + testGroups.push(...createTestGroups(projectSuite, config.workers)); - const shardedTests = new Set(); - for (const group of shardedTestGroups) { + // Shard test groups. + const testGroupsInThisShard = filterForShard(config.shard, testGroups); + const testsInThisShard = new Set(); + for (const group of testGroupsInThisShard) { for (const test of group.tests) - shardedTests.add(test); + testsInThisShard.add(test); } - // Update project suites and test groups. - for (const p of projectsWithTestGroups) { - p.testGroups = p.testGroups.filter(group => shardedTestGroups.has(group)); - filterTestsRemoveEmptySuites(p.projectSuite, test => shardedTests.has(test)); - } - - // Remove now-empty top-level projects. - projectsWithTestGroups = projectsWithTestGroups.filter(p => p.testGroups.length > 0); + // Update project suites, removing empty ones. + filterTestsRemoveEmptySuites(rootSuite, test => testsInThisShard.has(test)); } // Now prepend dependency projects. { // Filtering only and sharding might have reduced the number of top-level projects. // Build the project closure to only include dependencies that are still needed. - const projectClosure = new Set(buildProjectsClosure(projectsWithTestGroups.map(p => p.project))); + const projectClosure = new Set(buildProjectsClosure(rootSuite.suites.map(suite => suite.project() as FullProjectInternal))); // Clone file suites for dependency projects. for (const [project, fileSuites] of fileSuitesByProject) { - if (project._internal.type === 'dependency' && projectClosure.has(project)) { - const projectSuite = await createProjectSuite(fileSuites, project, { cliFileFilters: [], cliTitleMatcher: undefined }); - rootSuite._prependSuite(projectSuite); - const testGroups = createTestGroups(projectSuite, config.workers); - projectsWithTestGroups.push({ project, projectSuite, testGroups }); - } + if (project._internal.type === 'dependency' && projectClosure.has(project)) + rootSuite._prependSuite(await createProjectSuite(fileSuites, project, { cliFileFilters: [], cliTitleMatcher: undefined })); } } - return { rootSuite, projectsWithTestGroups }; + return rootSuite; } async function createProjectSuite(fileSuites: Suite[], project: FullProjectInternal, options: { cliFileFilters: TestFileFilter[], cliTitleMatcher?: Matcher, testIdMatcher?: Matcher }): Promise { diff --git a/packages/playwright-test/src/runner/tasks.ts b/packages/playwright-test/src/runner/tasks.ts index 6077c75166..c77862a3c8 100644 --- a/packages/playwright-test/src/runner/tasks.ts +++ b/packages/playwright-test/src/runner/tasks.ts @@ -21,26 +21,33 @@ import { debug, rimraf } from 'playwright-core/lib/utilsBundle'; import { Dispatcher } from './dispatcher'; import type { TestRunnerPluginRegistration } from '../plugins'; import type { Multiplexer } from '../reporters/multiplexer'; -import type { TestGroup } from '../runner/testGroups'; +import { createTestGroups, type TestGroup } from '../runner/testGroups'; import type { Task } from './taskRunner'; import { TaskRunner } from './taskRunner'; import type { Suite } from '../common/test'; import type { FullConfigInternal, FullProjectInternal } from '../common/types'; -import { collectProjectsAndTestFiles, createRootSuiteAndTestGroups, loadFileSuites, loadGlobalHook, type ProjectWithTestGroups } from './loadUtils'; +import { collectProjectsAndTestFiles, createRootSuite, loadFileSuites, loadGlobalHook } from './loadUtils'; import type { Matcher } from '../util'; const removeFolderAsync = promisify(rimraf); const readDirAsync = promisify(fs.readdir); +type ProjectWithTestGroups = { + project: FullProjectInternal; + projectSuite: Suite; + testGroups: TestGroup[]; +}; + +type Phase = { + dispatcher: Dispatcher, + projects: ProjectWithTestGroups[] +}; + export type TaskRunnerState = { reporter: Multiplexer; config: FullConfigInternal; rootSuite?: Suite; - projectsWithTestGroups?: ProjectWithTestGroups[]; - phases: { - dispatcher: Dispatcher, - projects: ProjectWithTestGroups[] - }[]; + phases: Phase[]; }; export function createTaskRunner(config: FullConfigInternal, reporter: Multiplexer): TaskRunner { @@ -153,9 +160,7 @@ function createLoadTask(mode: 'out-of-process' | 'in-process', shouldFilterOnly: const { config } = context; const filesToRunByProject = await collectProjectsAndTestFiles(config, projectsToIgnore, additionalFileMatcher); const fileSuitesByProject = await loadFileSuites(mode, config, filesToRunByProject, errors); - const loaded = await createRootSuiteAndTestGroups(config, fileSuitesByProject, errors, shouldFilterOnly); - context.rootSuite = loaded.rootSuite; - context.projectsWithTestGroups = loaded.projectsWithTestGroups; + context.rootSuite = await createRootSuite(config, fileSuitesByProject, errors, shouldFilterOnly); // Fail when no tests. if (!context.rootSuite.allTests().length && !config._internal.passWithNoTests && !config.shard) throw new Error(`No tests found`); @@ -167,24 +172,32 @@ function createPhasesTask(): Task { context.config._internal.maxConcurrentTestGroups = 0; const processed = new Set(); - for (let i = 0; i < context.projectsWithTestGroups!.length; i++) { + const projectToSuite = new Map(context.rootSuite!.suites.map(suite => [suite.project() as FullProjectInternal, suite])); + for (let i = 0; i < projectToSuite.size; i++) { // Find all projects that have all their dependencies processed by previous phases. - const phase: ProjectWithTestGroups[] = []; - for (const projectWithTestGroups of context.projectsWithTestGroups!) { - if (processed.has(projectWithTestGroups.project)) + const phaseProjects: FullProjectInternal[] = []; + for (const project of projectToSuite.keys()) { + if (processed.has(project)) continue; - if (projectWithTestGroups.project._internal.deps.find(p => !processed.has(p))) + if (project._internal.deps.find(p => !processed.has(p))) continue; - phase.push(projectWithTestGroups); + phaseProjects.push(project); } // Create a new phase. - for (const projectWithTestGroups of phase) - processed.add(projectWithTestGroups.project); - if (phase.length) { - const testGroupsInPhase = phase.reduce((acc, projectWithTestGroups) => acc + projectWithTestGroups.testGroups.length, 0); - debug('pw:test:task')(`created phase #${context.phases.length} with ${phase.map(p => p.project.name).sort()} projects, ${testGroupsInPhase} testGroups`); - context.phases.push({ dispatcher: new Dispatcher(context.config, context.reporter), projects: phase }); + for (const project of phaseProjects) + processed.add(project); + if (phaseProjects.length) { + let testGroupsInPhase = 0; + const phase: Phase = { dispatcher: new Dispatcher(context.config, context.reporter), projects: [] }; + context.phases.push(phase); + for (const project of phaseProjects) { + const projectSuite = projectToSuite.get(project)!; + const testGroups = createTestGroups(projectSuite, context.config.workers); + phase.projects.push({ project, projectSuite, testGroups }); + testGroupsInPhase += testGroups.length; + } + debug('pw:test:task')(`created phase #${context.phases.length} with ${phase.projects.map(p => p.project.name).sort()} projects, ${testGroupsInPhase} testGroups`); context.config._internal.maxConcurrentTestGroups = Math.max(context.config._internal.maxConcurrentTestGroups, testGroupsInPhase); } }