From 08a3a269cdf7bf28e13b643300af2e25ee5e7002 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 12 Oct 2022 14:34:22 -0700 Subject: [PATCH] feat(runner): project.canShard (#18037) --- docs/src/test-api/class-testproject.md | 6 + packages/playwright-test/src/dispatcher.ts | 1 + packages/playwright-test/src/loader.ts | 2 + packages/playwright-test/src/runner.ts | 92 +++++++++------ packages/playwright-test/types/test.d.ts | 11 ++ tests/playwright-test/stages.spec.ts | 130 +++++++++++++++++++++ utils/generate_types/overrides-test.d.ts | 1 + 7 files changed, 209 insertions(+), 34 deletions(-) diff --git a/docs/src/test-api/class-testproject.md b/docs/src/test-api/class-testproject.md index b34f6127b0..eb9c237b3a 100644 --- a/docs/src/test-api/class-testproject.md +++ b/docs/src/test-api/class-testproject.md @@ -105,6 +105,12 @@ const config: PlaywrightTestConfig = { export default config; ``` +## property: TestProject.canShard +* since: v1.28 +- type: ?<[boolean]> + +If set to false and the tests run with --shard command line option, all tests from this project will run in every shard. If not specified, the project can be split between several shards. + ## property: TestProject.expect * since: v1.10 - type: ?<[Object]> diff --git a/packages/playwright-test/src/dispatcher.ts b/packages/playwright-test/src/dispatcher.ts index 09e28c3ef5..3034811761 100644 --- a/packages/playwright-test/src/dispatcher.ts +++ b/packages/playwright-test/src/dispatcher.ts @@ -31,6 +31,7 @@ export type TestGroup = { repeatEachIndex: number; projectId: string; stopOnFailure: boolean; + canShard: boolean; tests: TestCase[]; watchMode: boolean; }; diff --git a/packages/playwright-test/src/loader.ts b/packages/playwright-test/src/loader.ts index 44d6767900..0235ca41bf 100644 --- a/packages/playwright-test/src/loader.ts +++ b/packages/playwright-test/src/loader.ts @@ -279,6 +279,7 @@ export class Loader { const name = takeFirst(projectConfig.name, config.name, ''); const stage = takeFirst(projectConfig.stage, 0); const stopOnFailure = takeFirst(projectConfig.stopOnFailure, false); + const canShard = takeFirst(projectConfig.canShard, true); let screenshotsDir = takeFirst((projectConfig as any).screenshotsDir, (config as any).screenshotsDir, path.join(testDir, '__screenshots__', process.platform, name)); if (process.env.PLAYWRIGHT_DOCKER) { @@ -300,6 +301,7 @@ export class Loader { testDir, stage, stopOnFailure, + canShard, _respectGitIgnore: respectGitIgnore, snapshotDir, _screenshotsDir: screenshotsDir, diff --git a/packages/playwright-test/src/runner.ts b/packages/playwright-test/src/runner.ts index 878d45b0d2..977db3a0e0 100644 --- a/packages/playwright-test/src/runner.ts +++ b/packages/playwright-test/src/runner.ts @@ -44,8 +44,8 @@ import { SigIntWatcher } from './sigIntWatcher'; import type { TestCase } from './test'; import { Suite } from './test'; import type { Config, FullConfigInternal, FullProjectInternal, ReporterInternal } from './types'; -import type { Matcher, TestFileFilter } from './util'; import { createFileMatcher, createTitleMatcher, serializeError } from './util'; +import type { Matcher, TestFileFilter } from './util'; const removeFolderAsync = promisify(rimraf); const readDirAsync = promisify(fs.readdir); @@ -341,6 +341,60 @@ export class Runner { return { rootSuite, concurrentTestGroups }; } + private _filterForCurrentShard(rootSuite: Suite, concurrentTestGroups: TestGroup[][]) { + const shard = this._loader.fullConfig().shard; + if (!shard) + return; + + // Each shard includes: + // - all non shardale tests and + // - its portion of the shardable ones. + let shardableTotal = 0; + for (const projectSuite of rootSuite.suites) { + if (projectSuite.project()!.canShard) + shardableTotal += projectSuite.allTests().length; + } + + const shardTests = new Set(); + + // Each shard gets some tests. + const shardSize = Math.floor(shardableTotal / shard.total); + // First few shards get one more test each. + const extraOne = shardableTotal - shardSize * shard.total; + + 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 shardConcurrentTestGroups = []; + for (const stage of concurrentTestGroups) { + const shardedStage: TestGroup[] = []; + for (const group of stage) { + let includeGroupInShard = false; + if (group.canShard) { + // 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) + includeGroupInShard = true; + current += group.tests.length; + } else { + includeGroupInShard = true; + } + if (includeGroupInShard) { + shardedStage.push(group); + for (const test of group.tests) + shardTests.add(test); + } + } + if (shardedStage.length) + shardConcurrentTestGroups.push(shardedStage); + } + concurrentTestGroups.length = 0; + concurrentTestGroups.push(...shardConcurrentTestGroups); + + filterSuiteWithOnlySemantics(rootSuite, () => false, test => shardTests.has(test)); + } + private async _run(options: RunOptions): Promise { const config = this._loader.fullConfig(); const fatalErrors: TestError[] = []; @@ -349,41 +403,10 @@ export class Runner { const { rootSuite, concurrentTestGroups } = await this._collectTestGroups(options, fatalErrors); // Fail when no tests. - let total = rootSuite.allTests().length; - if (!total && !options.passWithNoTests) + if (!rootSuite.allTests().length && !options.passWithNoTests) fatalErrors.push(createNoTestsError()); - // Compute shards. - const shard = config.shard; - if (shard) { - assert(concurrentTestGroups.length === 1); - const shardGroups: TestGroup[] = []; - const shardTests = new Set(); - - // Each shard gets some tests. - const shardSize = Math.floor(total / shard.total); - // First few shards get one more test each. - const extraOne = total - shardSize * shard.total; - - 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; - for (const group of concurrentTestGroups[0]) { - // 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) { - shardGroups.push(group); - for (const test of group.tests) - shardTests.add(test); - } - current += group.tests.length; - } - - concurrentTestGroups[0] = shardGroups; - filterSuiteWithOnlySemantics(rootSuite, () => false, test => shardTests.has(test)); - total = rootSuite.allTests().length; - } + this._filterForCurrentShard(rootSuite, concurrentTestGroups); config._maxConcurrentTestGroups = Math.max(...concurrentTestGroups.map(g => g.length)); @@ -749,6 +772,7 @@ function createTestGroups(projectSuites: Suite[], workers: number): TestGroup[] repeatEachIndex: test.repeatEachIndex, projectId: test._projectId, stopOnFailure: test.parent.project()!.stopOnFailure, + canShard: test.parent.project()!.canShard, tests: [], watchMode: false, }; diff --git a/packages/playwright-test/types/test.d.ts b/packages/playwright-test/types/test.d.ts index 54cfeeca69..9488bcf4bb 100644 --- a/packages/playwright-test/types/test.d.ts +++ b/packages/playwright-test/types/test.d.ts @@ -267,6 +267,11 @@ export interface FullProject { * skipped. */ stopOnFailure: boolean; + /** + * If set to false and the tests run with --shard command line option, all tests from this project will run in every shard. + * If not specified, the project can be split between several shards. + */ + canShard: boolean; /** * Directory that will be recursively scanned for test files. Defaults to the directory of the configuration file. * @@ -4298,6 +4303,12 @@ export interface TestError { * */ interface TestProject { + /** + * If set to false and the tests run with --shard command line option, all tests from this project will run in every shard. + * If not specified, the project can be split between several shards. + */ + canShard?: boolean; + /** * Configuration for the `expect` assertion library. * diff --git a/tests/playwright-test/stages.spec.ts b/tests/playwright-test/stages.spec.ts index 4632750753..8ad32eaa02 100644 --- a/tests/playwright-test/stages.spec.ts +++ b/tests/playwright-test/stages.spec.ts @@ -285,3 +285,133 @@ test('should support stopOnFailire', async ({ runGroups }, testInfo) => { expect(projectNames(timeline)).not.toContainEqual(['d', 'e']); }); +test('should split project if no canShard', async ({ runGroups }, testInfo) => { + const files = { + 'playwright.config.ts': ` + module.exports = { + projects: [ + { + stage: 10, + name: 'proj-1', + testMatch: /.*(a|b).test.ts/, + }, + { + stage: 20, + name: 'proj-2', + testMatch: /.*c.test.ts/, + }, + ] + };`, + 'a.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + test('test2', async () => { }); + test('test3', async () => { }); + test('test4', async () => { }); + `, + 'b.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + test('test2', async () => { }); + `, + 'c.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + test('test2', async () => { }); + `, + }; + + { // Shard 1/2 + const { exitCode, passed, output } = await runGroups(files, { shard: '1/2' }); + expect(output).toContain('Running 4 tests using 1 worker, shard 1 of 2'); + expect(output).toContain('[proj-1] › a.test.ts:6:7 › test1'); + expect(output).toContain('[proj-1] › a.test.ts:7:7 › test2'); + expect(output).toContain('[proj-1] › a.test.ts:8:7 › test3'); + expect(output).toContain('[proj-1] › a.test.ts:9:7 › test4'); + expect(output).not.toContain('[proj-2]'); + expect(output).not.toContain('b.test.ts'); + expect(output).not.toContain('c.test.ts'); + expect(exitCode).toBe(0); + expect(passed).toBe(4); + } + { // Shard 2/2 + const { exitCode, passed, output } = await runGroups(files, { shard: '2/2' }); + expect(output).toContain('Running 4 tests using 1 worker, shard 2 of 2'); + expect(output).toContain('[proj-1] › b.test.ts:6:7 › test1'); + expect(output).toContain('[proj-1] › b.test.ts:7:7 › test2'); + expect(output).toContain('[proj-2] › c.test.ts:6:7 › test1'); + expect(output).toContain('[proj-2] › c.test.ts:7:7 › test2'); + expect(output).not.toContain('a.test.ts'); + expect(exitCode).toBe(0); + expect(passed).toBe(4); + } +}); + +test('should not split project with canShard=false', async ({ runGroups }, testInfo) => { + const files = { + 'playwright.config.ts': ` + module.exports = { + projects: [ + { + stage: 10, + name: 'proj-1', + testMatch: /.*(a|b).test.ts/, + canShard: false, + }, + { + stage: 20, + name: 'proj-2', + testMatch: /.*(c|d).test.ts/, + }, + ] + };`, + 'a.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + test('test2', async () => { }); + `, + 'b.test.ts': ` + const { test } = pwt; + test('test2', async () => { }); + `, + 'c.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + test('test2', async () => { }); + test('test3', async () => { }); + `, + 'd.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + test('test2', async () => { }); + `, + }; + + { // Shard 1/2 + const { exitCode, passed, output } = await runGroups(files, { shard: '1/2' }); + expect(output).toContain('Running 6 tests using 2 workers, shard 1 of 2'); + // proj-1 is non shardable => a.test.ts and b.test.ts should run in both shards. + expect(output).toContain('[proj-1] › b.test.ts:6:7 › test2'); + expect(output).toContain('[proj-1] › a.test.ts:6:7 › test1'); + expect(output).toContain('[proj-1] › a.test.ts:7:7 › test2'); + expect(output).toContain('[proj-2] › c.test.ts:6:7 › test1'); + expect(output).toContain('[proj-2] › c.test.ts:7:7 › test2'); + expect(output).not.toContain('d.test.ts'); + expect(exitCode).toBe(0); + expect(passed).toBe(6); + } + { // Shard 1/2 + const { exitCode, passed, output } = await runGroups(files, { shard: '2/2' }); + expect(output).toContain('Running 5 tests using 2 workers, shard 2 of 2'); + // proj-1 is non shardable => a.test.ts and b.test.ts should run in both shards. + expect(output).toContain('[proj-1] › b.test.ts:6:7 › test2'); + expect(output).toContain('[proj-1] › a.test.ts:6:7 › test1'); + expect(output).toContain('[proj-1] › a.test.ts:7:7 › test2'); + expect(output).toContain('[proj-2] › d.test.ts:6:7 › test1'); + expect(output).toContain('[proj-2] › d.test.ts:7:7 › test2'); + expect(output).not.toContain('c.test.ts'); + expect(exitCode).toBe(0); + expect(passed).toBe(5); + } +}); + diff --git a/utils/generate_types/overrides-test.d.ts b/utils/generate_types/overrides-test.d.ts index c7957636fb..2e1f339aed 100644 --- a/utils/generate_types/overrides-test.d.ts +++ b/utils/generate_types/overrides-test.d.ts @@ -48,6 +48,7 @@ export interface FullProject { retries: number; stage: number; stopOnFailure: boolean; + canShard: boolean; testDir: string; testIgnore: string | RegExp | (string | RegExp)[]; testMatch: string | RegExp | (string | RegExp)[];