fix(shards): shard test groups instead of files (#21638)

Fixes #21497.
This commit is contained in:
Dmitry Gozman 2023-03-14 10:41:37 -07:00 committed by GitHub
parent a2c96494e0
commit 455e11c1d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 241 additions and 129 deletions

View File

@ -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<FullProjectInternal>, additionalFileMatcher: Matcher | undefined, errors: TestError[], shouldFilterOnly: boolean): Promise<Suite> {
export type ProjectWithTestGroups = {
project: FullProjectInternal;
projectSuite: Suite;
testGroups: TestGroup[];
};
export async function loadAllTests(mode: 'out-of-process' | 'in-process', config: FullConfigInternal, projectsToIgnore: Set<FullProjectInternal>, 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<FullProjectInternal, string[]>();
const filesToRunByProject = new Map<FullProjectInternal, string[]>();
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<TestCase>();
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<Suite | null> {
async function createProjectSuite(fileSuites: Suite[], project: FullProjectInternal, options: { cliFileFilters: TestFileFilter[], cliTitleMatcher?: Matcher, testIdMatcher?: Matcher }, files: string[]): Promise<Suite> {
const fileSuitesMap = new Map<string, Suite>();
for (const fileSuite of fileSuits)
for (const fileSuite of fileSuites)
fileSuitesMap.set(fileSuite._requireFile, fileSuite);
const projectSuite = new Suite(project.name, 'project');

View File

@ -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<TaskRunnerState>, config: Fu
}
function addRunTasks(taskRunner: TaskRunner<TaskRunnerState>, 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<TaskRunnerState> {
function createLoadTask(mode: 'out-of-process' | 'in-process', shouldFilterOnly: boolean, projectsToIgnore = new Set<FullProjectInternal>(), additionalFileMatcher?: Matcher): Task<TaskRunnerState> {
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<TaskRunnerState> {
function createPhasesTask(): Task<TaskRunnerState> {
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<FullProjectInternal>();
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<TaskRunnerState> {
}
};
}
function buildPhases(projectSuites: Suite[]): Suite[][] {
const phases: Suite[][] = [];
const processed = new Set<FullProjectInternal>();
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;
}

View File

@ -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<FullProjectInternal, string[]>): Map<FullProjectInternal, string[]> {
export function filterForShard(shard: { total: number, current: number }, testGroups: TestGroup[]): Set<TestGroup> {
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<FullProjectInternal, string[]>();
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<TestGroup>();
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;
}

View File

@ -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: [

View File

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