diff --git a/packages/playwright-test/src/runner/loadUtils.ts b/packages/playwright-test/src/runner/loadUtils.ts index 9185be1fee..67a737337c 100644 --- a/packages/playwright-test/src/runner/loadUtils.ts +++ b/packages/playwright-test/src/runner/loadUtils.ts @@ -27,10 +27,16 @@ import type { Matcher, TestFileFilter } from '../util'; import { buildProjectsClosure, collectFilesForProject, filterProjects } from './projectUtils'; import { requireOrImport } from '../common/transform'; import { buildFileSuiteForProject, filterByFocusedLine, filterByTestIds, filterOnly, filterTestsRemoveEmptySuites } from '../common/suiteUtils'; -import { filterForShard } from './testGroups'; +import { createTestGroups, filterForShard, type TestGroup } from './testGroups'; import { dependenciesForTestFile } from '../common/compilationCache'; -export async function loadAllTests(mode: 'out-of-process' | 'in-process', config: FullConfigInternal, projectsToIgnore: Set, additionalFileMatcher: Matcher | undefined, errors: TestError[], shouldFilterOnly: boolean): Promise { +export type ProjectWithTestGroups = { + project: FullProjectInternal; + projectSuite: Suite; + testGroups: TestGroup[]; +}; + +export async function loadAllTests(mode: 'out-of-process' | 'in-process', config: FullConfigInternal, projectsToIgnore: Set, additionalFileMatcher: Matcher | undefined, errors: TestError[], shouldFilterOnly: boolean): Promise<{ rootSuite: Suite, projectsWithTestGroups: ProjectWithTestGroups[] }> { const projects = filterProjects(config.projects, config._internal.cliProjectFilter); // Interpret cli parameters. @@ -40,7 +46,7 @@ export async function loadAllTests(mode: 'out-of-process' | 'in-process', config const cliTitleMatcher = (title: string) => !grepInvertMatcher(title) && grepMatcher(title); const cliFileMatcher = config._internal.cliArgs.length ? createFileMatcherFromArguments(config._internal.cliArgs) : null; - let filesToRunByProject = new Map(); + const filesToRunByProject = new Map(); let topLevelProjects: FullProjectInternal[]; let dependencyProjects: FullProjectInternal[]; // Collect files, categorize top level and dependency projects. @@ -74,24 +80,12 @@ export async function loadAllTests(mode: 'out-of-process' | 'in-process', config } const projectClosure = buildProjectsClosure([...filesToRunByProject.keys()]); - // Remove files for dependency projects, they'll be added back later. - for (const project of projectClosure.filter(p => p._internal.type === 'dependency')) - filesToRunByProject.delete(project); - - // Shard only the top-level projects. - if (config.shard) - filesToRunByProject = filterForShard(config.shard, filesToRunByProject); - - // Re-build the closure, project set might have changed. - const filteredProjectClosure = buildProjectsClosure([...filesToRunByProject.keys()]); - topLevelProjects = filteredProjectClosure.filter(p => p._internal.type === 'top-level'); - dependencyProjects = filteredProjectClosure.filter(p => p._internal.type === 'dependency'); - - topLevelProjects = topLevelProjects.filter(p => !projectsToIgnore.has(p)); - dependencyProjects = dependencyProjects.filter(p => !projectsToIgnore.has(p)); + topLevelProjects = projectClosure.filter(p => p._internal.type === 'top-level' && !projectsToIgnore.has(p)); + dependencyProjects = projectClosure.filter(p => p._internal.type === 'dependency' && !projectsToIgnore.has(p)); // (Re-)add all files for dependent projects, disregard filters. for (const project of dependencyProjects) { + filesToRunByProject.delete(project); const files = allFilesForProject.get(project) || await collectFilesForProject(project, fsCache); filesToRunByProject.set(project, files); } @@ -132,12 +126,9 @@ export async function loadAllTests(mode: 'out-of-process' | 'in-process', config // Create root suites with clones for the projects. const rootSuite = new Suite('', 'root'); - // First iterate leaf projects to focus only, then add all other projects. - for (const project of topLevelProjects) { - const projectSuite = await createProjectSuite(fileSuites, project, { cliFileFilters, cliTitleMatcher, testIdMatcher: config._internal.testIdMatcher }, filesToRunByProject.get(project)!); - if (projectSuite) - rootSuite._addSuite(projectSuite); - } + // First iterate top-level projects to focus only, then add all other projects. + for (const project of topLevelProjects) + rootSuite._addSuite(await createProjectSuite(fileSuites, project, { cliFileFilters, cliTitleMatcher, testIdMatcher: config._internal.testIdMatcher }, filesToRunByProject.get(project)!)); // Complain about only. if (config.forbidOnly) { @@ -146,23 +137,60 @@ export async function loadAllTests(mode: 'out-of-process' | 'in-process', config errors.push(...createForbidOnlyErrors(onlyTestsAndSuites)); } - // Filter only for leaf projects. + // Filter only for top-level projects. 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); + + const shardedTests = new Set(); + for (const group of shardedTestGroups) { + for (const test of group.tests) + shardedTests.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); + + // Re-build the closure, project set might have changed. + const shardedProjectClosure = buildProjectsClosure(projectsWithTestGroups.map(p => p.project)); + topLevelProjects = shardedProjectClosure.filter(p => p._internal.type === 'top-level' && !projectsToIgnore.has(p)); + dependencyProjects = shardedProjectClosure.filter(p => p._internal.type === 'dependency' && !projectsToIgnore.has(p)); + } + // Prepend the projects that are dependencies. for (const project of dependencyProjects) { const projectSuite = await createProjectSuite(fileSuites, project, { cliFileFilters: [], cliTitleMatcher: undefined }, filesToRunByProject.get(project)!); - if (projectSuite) - rootSuite._prependSuite(projectSuite); + rootSuite._prependSuite(projectSuite); + const testGroups = createTestGroups(projectSuite, config.workers); + projectsWithTestGroups.push({ project, projectSuite, testGroups }); } - return rootSuite; + return { rootSuite, projectsWithTestGroups }; } -async function createProjectSuite(fileSuits: Suite[], project: FullProjectInternal, options: { cliFileFilters: TestFileFilter[], cliTitleMatcher?: Matcher, testIdMatcher?: Matcher }, files: string[]): Promise { +async function createProjectSuite(fileSuites: Suite[], project: FullProjectInternal, options: { cliFileFilters: TestFileFilter[], cliTitleMatcher?: Matcher, testIdMatcher?: Matcher }, files: string[]): Promise { const fileSuitesMap = new Map(); - for (const fileSuite of fileSuits) + for (const fileSuite of fileSuites) fileSuitesMap.set(fileSuite._requireFile, fileSuite); const projectSuite = new Suite(project.name, 'project'); diff --git a/packages/playwright-test/src/runner/tasks.ts b/packages/playwright-test/src/runner/tasks.ts index d181d50eae..09af08584f 100644 --- a/packages/playwright-test/src/runner/tasks.ts +++ b/packages/playwright-test/src/runner/tasks.ts @@ -22,27 +22,21 @@ import { Dispatcher } from './dispatcher'; import type { TestRunnerPluginRegistration } from '../plugins'; import type { Multiplexer } from '../reporters/multiplexer'; import type { TestGroup } from '../runner/testGroups'; -import { createTestGroups } 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 { loadAllTests, loadGlobalHook } from './loadUtils'; +import { loadAllTests, loadGlobalHook, type ProjectWithTestGroups } from './loadUtils'; import type { Matcher } from '../util'; const removeFolderAsync = promisify(rimraf); const readDirAsync = promisify(fs.readdir); -type ProjectWithTestGroups = { - project: FullProjectInternal; - projectSuite: Suite; - testGroups: TestGroup[]; -}; - export type TaskRunnerState = { reporter: Multiplexer; config: FullConfigInternal; rootSuite?: Suite; + projectsWithTestGroups?: ProjectWithTestGroups[]; phases: { dispatcher: Dispatcher, projects: ProjectWithTestGroups[] @@ -79,7 +73,7 @@ function addGlobalSetupTasks(taskRunner: TaskRunner, config: Fu } function addRunTasks(taskRunner: TaskRunner, config: FullConfigInternal) { - taskRunner.addTask('create tasks', createTestGroupsTask()); + taskRunner.addTask('create phases', createPhasesTask()); taskRunner.addTask('report begin', async ({ reporter, rootSuite }) => { reporter.onBegin?.(config, rootSuite!); return () => reporter.onEnd(); @@ -157,33 +151,40 @@ function createRemoveOutputDirsTask(): Task { function createLoadTask(mode: 'out-of-process' | 'in-process', shouldFilterOnly: boolean, projectsToIgnore = new Set(), additionalFileMatcher?: Matcher): Task { return async (context, errors) => { const { config } = context; - context.rootSuite = await loadAllTests(mode, config, projectsToIgnore, additionalFileMatcher, errors, shouldFilterOnly); + const loaded = await loadAllTests(mode, config, projectsToIgnore, additionalFileMatcher, errors, shouldFilterOnly); + context.rootSuite = loaded.rootSuite; + context.projectsWithTestGroups = loaded.projectsWithTestGroups; // Fail when no tests. if (!context.rootSuite.allTests().length && !config._internal.passWithNoTests && !config.shard) throw new Error(`No tests found`); }; } -function createTestGroupsTask(): Task { +function createPhasesTask(): Task { return async context => { - const { config, rootSuite, reporter } = context; context.config._internal.maxConcurrentTestGroups = 0; - for (const phase of buildPhases(rootSuite!.suites)) { - // Go over the phases, for each phase create list of task groups. - const projects: ProjectWithTestGroups[] = []; - for (const projectSuite of phase) { - const testGroups = createTestGroups(projectSuite, config.workers); - projects.push({ - project: projectSuite._projectConfig!, - projectSuite, - testGroups, - }); + + const processed = new Set(); + for (let i = 0; i < context.projectsWithTestGroups!.length; 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)) + continue; + if (projectWithTestGroups.project._internal.deps.find(p => !processed.has(p))) + continue; + phase.push(projectWithTestGroups); } - const testGroupsInPhase = projects.reduce((acc, project) => acc + project.testGroups.length, 0); - debug('pw:test:task')(`running phase with ${projects.map(p => p.project.name).sort()} projects, ${testGroupsInPhase} testGroups`); - context.phases.push({ dispatcher: new Dispatcher(config, reporter), projects }); - context.config._internal.maxConcurrentTestGroups = Math.max(context.config._internal.maxConcurrentTestGroups, testGroupsInPhase); + // 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 }); + context.config._internal.maxConcurrentTestGroups = Math.max(context.config._internal.maxConcurrentTestGroups, testGroupsInPhase); + } } }; } @@ -236,23 +237,3 @@ function createRunTestsTask(): Task { } }; } - -function buildPhases(projectSuites: Suite[]): Suite[][] { - const phases: Suite[][] = []; - const processed = new Set(); - for (let i = 0; i < projectSuites.length; i++) { - const phase: Suite[] = []; - for (const projectSuite of projectSuites) { - if (processed.has(projectSuite._projectConfig!)) - continue; - if (projectSuite._projectConfig!._internal.deps.find(p => !processed.has(p))) - continue; - phase.push(projectSuite); - } - for (const projectSuite of phase) - processed.add(projectSuite._projectConfig!); - if (phase.length) - phases.push(phase); - } - return phases; -} diff --git a/packages/playwright-test/src/runner/testGroups.ts b/packages/playwright-test/src/runner/testGroups.ts index ceb60a7310..60ea108997 100644 --- a/packages/playwright-test/src/runner/testGroups.ts +++ b/packages/playwright-test/src/runner/testGroups.ts @@ -15,7 +15,6 @@ */ import type { Suite, TestCase } from '../common/test'; -import type { FullProjectInternal } from '../common/types'; export type TestGroup = { workerHash: string; @@ -131,10 +130,10 @@ export function createTestGroups(projectSuite: Suite, workers: number): TestGrou return result; } -export function filterForShard(shard: { total: number, current: number }, filesByProject: Map): Map { +export function filterForShard(shard: { total: number, current: number }, testGroups: TestGroup[]): Set { let shardableTotal = 0; - for (const files of filesByProject.values()) - shardableTotal += files.length; + for (const group of testGroups) + shardableTotal += group.tests.length; // Each shard gets some tests. const shardSize = Math.floor(shardableTotal / shard.total); @@ -144,17 +143,15 @@ export function filterForShard(shard: { total: number, current: number }, filesB const currentShard = shard.current - 1; // Make it zero-based for calculations. const from = shardSize * currentShard + Math.min(extraOne, currentShard); const to = from + shardSize + (currentShard < extraOne ? 1 : 0); + let current = 0; - const result = new Map(); - for (const [project, files] of filesByProject) { - const shardFiles: string[] = []; - for (const file of files) { - if (current >= from && current < to) - shardFiles.push(file); - ++current; - } - if (shardFiles.length) - result.set(project, shardFiles); + const result = new Set(); + for (const group of testGroups) { + // Any test group goes to the shard that contains the first test of this group. + // So, this shard gets any group that starts at [from; to) + if (current >= from && current < to) + result.add(group); + current += group.tests.length; } return result; } diff --git a/tests/playwright-test/deps.spec.ts b/tests/playwright-test/deps.spec.ts index eefc4b422a..0f53316b29 100644 --- a/tests/playwright-test/deps.spec.ts +++ b/tests/playwright-test/deps.spec.ts @@ -187,7 +187,7 @@ test('should not filter dependency by only', async ({ runInlineTest }) => { expect(result.outputLines).toEqual(['setup in setup', 'setup 2 in setup', 'test in browser']); }); -test('should not filter dependency by only 2', async ({ runInlineTest }) => { +test('should filter dependency by only when running explicitly', async ({ runInlineTest }) => { const result = await runInlineTest({ 'playwright.config.ts': ` module.exports = { projects: [ diff --git a/tests/playwright-test/shard.spec.ts b/tests/playwright-test/shard.spec.ts index d0b7d076f3..70095abcca 100644 --- a/tests/playwright-test/shard.spec.ts +++ b/tests/playwright-test/shard.spec.ts @@ -17,69 +17,135 @@ import { test, expect } from './playwright-test-fixtures'; const tests = { - 'helper.ts': ` - import { test as base, expect } from '@playwright/test'; - export const headlessTest = base.extend({ headless: false }); - export const headedTest = base.extend({ headless: true }); - `, 'a1.spec.ts': ` - import { headlessTest, headedTest } from './helper'; - headlessTest('test1', async () => { - console.log('test1-done'); + import { test } from '@playwright/test'; + test('test1', async () => { + console.log('\\n%%a1-test1-done'); + }); + test('test2', async () => { + console.log('\\n%%a1-test2-done'); + }); + test('test3', async () => { + console.log('\\n%%a1-test3-done'); + }); + test('test4', async () => { + console.log('\\n%%a1-test4-done'); }); `, 'a2.spec.ts': ` - import { headlessTest, headedTest } from './helper'; - headedTest('test2', async () => { - console.log('test2-done'); + import { test } from '@playwright/test'; + test.describe.configure({ mode: 'parallel' }); + test('test1', async () => { + console.log('\\n%%a2-test1-done'); + }); + test('test2', async () => { + console.log('\\n%%a2-test2-done'); }); `, 'a3.spec.ts': ` - import { headlessTest, headedTest } from './helper'; - headlessTest('test3', async () => { - console.log('test3-done'); + import { test } from '@playwright/test'; + test.describe.configure({ mode: 'parallel' }); + test('test1', async () => { + console.log('\\n%%a3-test1-done'); + }); + test('test2', async () => { + console.log('\\n%%a3-test2-done'); }); `, - 'b1.spec.ts': ` - import { headlessTest, headedTest } from './helper'; - headlessTest('test4', async () => { - console.log('test4-done'); + 'a4.spec.ts': ` + import { test } from '@playwright/test'; + test('test1', async () => { + console.log('\\n%%a4-test1-done'); }); - `, - 'b2.spec.ts': ` - import { headlessTest, headedTest } from './helper'; - headedTest('test5', async () => { - console.log('test5-done'); + test('test2', async () => { + console.log('\\n%%a4-test2-done'); }); `, }; test('should respect shard=1/2', async ({ runInlineTest }) => { - const result = await runInlineTest(tests, { shard: '1/2' }); + const result = await runInlineTest(tests, { shard: '1/2', workers: 1 }); expect(result.exitCode).toBe(0); - expect(result.passed).toBe(3); + expect(result.passed).toBe(5); expect(result.skipped).toBe(0); - expect(result.output).toContain('test1-done'); - expect(result.output).toContain('test2-done'); - expect(result.output).toContain('test3-done'); + expect(result.outputLines).toEqual([ + 'a1-test1-done', + 'a1-test2-done', + 'a1-test3-done', + 'a1-test4-done', + 'a2-test1-done', + ]); }); test('should respect shard=2/2', async ({ runInlineTest }) => { - const result = await runInlineTest(tests, { shard: '2/2' }); + const result = await runInlineTest(tests, { shard: '2/2', workers: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(5); + expect(result.skipped).toBe(0); + expect(result.outputLines).toEqual([ + 'a2-test2-done', + 'a3-test1-done', + 'a3-test2-done', + 'a4-test1-done', + 'a4-test2-done', + ]); +}); + +test('should respect shard=1/3', async ({ runInlineTest }) => { + const result = await runInlineTest(tests, { shard: '1/3', workers: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(4); + expect(result.skipped).toBe(0); + expect(result.outputLines).toEqual([ + 'a1-test1-done', + 'a1-test2-done', + 'a1-test3-done', + 'a1-test4-done', + ]); +}); + +test('should respect shard=2/3', async ({ runInlineTest }) => { + const result = await runInlineTest(tests, { shard: '2/3', workers: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(3); + expect(result.skipped).toBe(0); + expect(result.outputLines).toEqual([ + 'a2-test1-done', + 'a2-test2-done', + 'a3-test1-done', + ]); +}); + +test('should respect shard=3/3', async ({ runInlineTest }) => { + const result = await runInlineTest(tests, { shard: '3/3', workers: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(3); + expect(result.skipped).toBe(0); + expect(result.outputLines).toEqual([ + 'a3-test2-done', + 'a4-test1-done', + 'a4-test2-done', + ]); +}); + +test('should respect shard=3/4', async ({ runInlineTest }) => { + const result = await runInlineTest(tests, { shard: '3/4', workers: 1 }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(2); expect(result.skipped).toBe(0); - expect(result.output).toContain('test4-done'); - expect(result.output).toContain('test5-done'); + expect(result.outputLines).toEqual([ + 'a3-test1-done', + 'a3-test2-done', + ]); }); test('should not produce skipped tests for zero-sized shards', async ({ runInlineTest }) => { - const result = await runInlineTest(tests, { shard: '10/10' }); + const result = await runInlineTest(tests, { shard: '10/10', workers: 1 }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(0); expect(result.failed).toBe(0); expect(result.skipped).toBe(0); - expect(result.output).not.toContain('-done'); + expect(result.outputLines).toEqual([]); }); test('should respect shard=1/2 in config', async ({ runInlineTest }) => { @@ -88,13 +154,17 @@ test('should respect shard=1/2 in config', async ({ runInlineTest }) => { 'playwright.config.js': ` module.exports = { shard: { current: 1, total: 2 } }; `, - }); + }, { workers: 1 }); expect(result.exitCode).toBe(0); - expect(result.passed).toBe(3); + expect(result.passed).toBe(5); expect(result.skipped).toBe(0); - expect(result.output).toContain('test1-done'); - expect(result.output).toContain('test2-done'); - expect(result.output).toContain('test3-done'); + expect(result.outputLines).toEqual([ + 'a1-test1-done', + 'a1-test2-done', + 'a1-test3-done', + 'a1-test4-done', + 'a2-test1-done', + ]); }); test('should work with workers=1 and --fully-parallel', async ({ runInlineTest }) => { @@ -119,3 +189,39 @@ test('should work with workers=1 and --fully-parallel', async ({ runInlineTest } expect(result.passed).toBe(1); expect(result.skipped).toBe(1); }); + +test('should skip dependency when project is sharded out', async ({ runInlineTest }) => { + const tests = { + 'playwright.config.ts': ` + module.exports = { + projects: [ + { name: 'setup1', testMatch: /setup.ts/ }, + { name: 'tests1', dependencies: ['setup1'] }, + { name: 'setup2', testMatch: /setup.ts/ }, + { name: 'tests2', dependencies: ['setup2'] }, + ], + }; + `, + 'test.spec.ts': ` + import { test } from '@playwright/test'; + test('test', async ({}) => { + console.log('\\n%%test in ' + test.info().project.name); + }); + `, + 'setup.ts': ` + import { test } from '@playwright/test'; + test('setup', async ({}) => { + console.log('\\n%%setup in ' + test.info().project.name); + }); + `, + }; + + const result = await runInlineTest(tests, { shard: '2/2', workers: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(2); + expect(result.skipped).toBe(0); + expect(result.outputLines).toEqual([ + 'setup in setup2', + 'test in tests2', + ]); +});