diff --git a/src/test/dispatcher.ts b/src/test/dispatcher.ts index f5bf8a6ad9..069303664d 100644 --- a/src/test/dispatcher.ts +++ b/src/test/dispatcher.ts @@ -19,9 +19,10 @@ import path from 'path'; import { EventEmitter } from 'events'; import { RunPayload, TestBeginPayload, TestEndPayload, DonePayload, TestOutputPayload, WorkerInitParams } from './ipc'; import type { TestResult, Reporter, TestStatus } from '../../types/testReporter'; -import { Suite, TestCase } from './test'; +import { TestCase } from './test'; import { Loader } from './loader'; +// TODO: use TestGroup instead of DispatcherEntry type DispatcherEntry = { runPayload: RunPayload; hash: string; @@ -29,6 +30,14 @@ type DispatcherEntry = { projectIndex: number; }; +export type TestGroup = { + workerHash: string; + requireFile: string; + repeatEachIndex: number; + projectIndex: number; + tests: TestCase[]; +}; + export class Dispatcher { private _workers = new Set(); private _freeWorkers: Worker[] = []; @@ -38,82 +47,36 @@ export class Dispatcher { private _queue: DispatcherEntry[] = []; private _stopCallback = () => {}; readonly _loader: Loader; - private _suite: Suite; private _reporter: Reporter; private _hasWorkerErrors = false; private _isStopped = false; private _failureCount = 0; - constructor(loader: Loader, suite: Suite, reporter: Reporter) { + constructor(loader: Loader, testGroups: TestGroup[], reporter: Reporter) { this._loader = loader; this._reporter = reporter; - this._suite = suite; - for (const suite of this._suite.suites) { - for (const test of suite.allTests()) - this._testById.set(test._id, { test, result: test._appendTestResult() }); - } - - this._queue = this._filesSortedByWorkerHash(); - - // Shard tests. - const shard = this._loader.fullConfig().shard; - if (shard) { - let total = this._suite.allTests().length; - const shardSize = Math.ceil(total / shard.total); - const from = shardSize * (shard.current - 1); - const to = shardSize * shard.current; - let current = 0; - total = 0; - const filteredQueue: DispatcherEntry[] = []; - for (const entry of this._queue) { - if (current >= from && current < to) { - filteredQueue.push(entry); - total += entry.runPayload.entries.length; - } - current += entry.runPayload.entries.length; - } - this._queue = filteredQueue; - } - } - - _filesSortedByWorkerHash(): DispatcherEntry[] { - const entriesByWorkerHashAndFile = new Map>(); - for (const projectSuite of this._suite.suites) { - for (const test of projectSuite.allTests()) { - let entriesByFile = entriesByWorkerHashAndFile.get(test._workerHash); - if (!entriesByFile) { - entriesByFile = new Map(); - entriesByWorkerHashAndFile.set(test._workerHash, entriesByFile); - } - const file = test._requireFile; - let entry = entriesByFile.get(file); - if (!entry) { - entry = { - runPayload: { - entries: [], - file, - }, - repeatEachIndex: test._repeatEachIndex, - projectIndex: test._projectIndex, - hash: test._workerHash, - }; - entriesByFile.set(file, entry); - } + this._queue = []; + for (const group of testGroups) { + const entry: DispatcherEntry = { + runPayload: { + file: group.requireFile, + entries: [] + }, + hash: group.workerHash, + repeatEachIndex: group.repeatEachIndex, + projectIndex: group.projectIndex, + }; + for (const test of group.tests) { + const result = test._appendTestResult(); + this._testById.set(test._id, { test, result }); entry.runPayload.entries.push({ - retry: this._testById.get(test._id)!.result.retry, + retry: result.retry, testId: test._id, }); } + this._queue.push(entry); } - - const result: DispatcherEntry[] = []; - for (const entriesByFile of entriesByWorkerHashAndFile.values()) { - for (const entry of entriesByFile.values()) - result.push(entry); - } - result.sort((a, b) => a.hash < b.hash ? -1 : (a.hash === b.hash ? 0 : 1)); - return result; } async run() { diff --git a/src/test/runner.ts b/src/test/runner.ts index fbe24bc602..60f874a96b 100644 --- a/src/test/runner.ts +++ b/src/test/runner.ts @@ -20,7 +20,7 @@ import rimraf from 'rimraf'; import * as fs from 'fs'; import * as path from 'path'; import { promisify } from 'util'; -import { Dispatcher } from './dispatcher'; +import { Dispatcher, TestGroup } from './dispatcher'; import { createMatcher, FilePatternFilter, monotonicTime, raceAgainstDeadline } from './util'; import { TestCase, Suite } from './test'; import { Loader } from './loader'; @@ -224,12 +224,51 @@ export class Runner { outputDirs.add(project.config.outputDir); } - const total = rootSuite.allTests().length; + let total = rootSuite.allTests().length; if (!total) return { status: 'no-tests' }; await Promise.all(Array.from(outputDirs).map(outputDir => removeFolderAsync(outputDir).catch(e => {}))); + let testGroups = createTestGroups(rootSuite); + + const shard = config.shard; + if (shard) { + 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 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) { + shardGroups.push(group); + for (const test of group.tests) + shardTests.add(test); + } + current += group.tests.length; + } + + testGroups = shardGroups; + filterSuite(rootSuite, () => false, test => shardTests.has(test)); + total = rootSuite.allTests().length; + } + + if (process.stdout.isTTY) { + console.log(); + const jobs = Math.min(config.workers, testGroups.length); + const shardDetails = shard ? `, shard ${shard.current} of ${shard.total}` : ''; + console.log(`Running ${total} test${total > 1 ? 's' : ''} using ${jobs} worker${jobs > 1 ? 's' : ''}${shardDetails}`); + } + let sigint = false; let sigintCallback: () => void; const sigIntPromise = new Promise(f => sigintCallback = f); @@ -244,23 +283,11 @@ export class Runner { }; process.on('SIGINT', sigintHandler); - if (process.stdout.isTTY) { - const workers = new Set(); - rootSuite.allTests().forEach(test => { - workers.add(test._requireFile + test._workerHash); - }); - console.log(); - const jobs = Math.min(config.workers, workers.size); - const shard = config.shard; - const shardDetails = shard ? `, shard ${shard.current} of ${shard.total}` : ''; - console.log(`Running ${total} test${total > 1 ? 's' : ''} using ${jobs} worker${jobs > 1 ? 's' : ''}${shardDetails}`); - } - this._reporter.onBegin?.(config, rootSuite); this._didBegin = true; let hasWorkerErrors = false; if (!list) { - const dispatcher = new Dispatcher(this._loader, rootSuite, this._reporter); + const dispatcher = new Dispatcher(this._loader, testGroups, this._reporter); await Promise.race([dispatcher.run(), sigIntPromise]); await dispatcher.stop(); hasWorkerErrors = dispatcher.hasWorkerErrors(); @@ -411,6 +438,30 @@ function buildItemLocation(rootDir: string, testOrSuite: Suite | TestCase) { return `${path.relative(rootDir, testOrSuite.location.file)}:${testOrSuite.location.line}`; } +function createTestGroups(rootSuite: Suite): TestGroup[] { + const groupById = new Map(); + for (const projectSuite of rootSuite.suites) { + for (const test of projectSuite.allTests()) { + const id = test._workerHash + '::' + test._requireFile; + let group = groupById.get(id); + if (!group) { + group = { + workerHash: test._workerHash, + requireFile: test._requireFile, + repeatEachIndex: test._repeatEachIndex, + projectIndex: test._projectIndex, + tests: [], + }; + groupById.set(id, group); + } + group.tests.push(test); + } + } + const groups = Array.from(groupById.values()); + groups.sort((a, b) => a.workerHash.localeCompare(b.workerHash)); + return groups; +} + class ListModeReporter implements Reporter { onBegin(config: FullConfig, suite: Suite): void { console.log(`Listing tests:`); diff --git a/tests/playwright-test/fixtures.spec.ts b/tests/playwright-test/fixtures.spec.ts index 75eb1b03f7..130a0554e5 100644 --- a/tests/playwright-test/fixtures.spec.ts +++ b/tests/playwright-test/fixtures.spec.ts @@ -557,7 +557,7 @@ test('should create a new worker for worker fixtures', async ({ runInlineTest }) 'a.test.ts': ` const { test } = pwt; test('base test', async ({}, testInfo) => { - expect(testInfo.workerIndex).toBe(0); + console.log('\\n%%base-' + testInfo.workerIndex); }); const test2 = test.extend({ @@ -567,7 +567,7 @@ test('should create a new worker for worker fixtures', async ({ runInlineTest }) }, { scope: 'worker' }], }); test2('a test', async ({ foo }, testInfo) => { - expect(testInfo.workerIndex).toBe(1); + console.log('\\n%%a-' + testInfo.workerIndex); }); `, 'b.test.ts': ` @@ -579,12 +579,16 @@ test('should create a new worker for worker fixtures', async ({ runInlineTest }) }, }); test2('b test', async ({ bar }, testInfo) => { - expect(testInfo.workerIndex).toBe(0); + console.log('\\n%%b-' + testInfo.workerIndex); }); `, }, { workers: 1 }); expect(result.output).toContain('foo-a'); expect(result.output).toContain('bar-b'); + const baseWorker = +result.output.match(/%%base-(\d)/)[1]; + expect(result.output).toContain(`%%base-${baseWorker}`); + expect(result.output).toContain(`%%a-${1 - baseWorker}`); + expect(result.output).toContain(`%%b-${baseWorker}`); expect(result.passed).toBe(3); }); diff --git a/tests/playwright-test/json-reporter.spec.ts b/tests/playwright-test/json-reporter.spec.ts index 90064c2f61..15c59afebd 100644 --- a/tests/playwright-test/json-reporter.spec.ts +++ b/tests/playwright-test/json-reporter.spec.ts @@ -34,7 +34,7 @@ test('should support spec.ok', async ({ runInlineTest }) => { expect(result.report.suites[0].specs[1].ok).toBe(false); }); -test('should report skipped due to sharding', async ({ runInlineTest }) => { +test('should not report skipped due to sharding', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.test.js': ` const { test } = pwt; @@ -56,11 +56,10 @@ test('should report skipped due to sharding', async ({ runInlineTest }) => { `, }, { shard: '1/3', reporter: 'json' }); expect(result.exitCode).toBe(0); + expect(result.report.suites.length).toBe(1); + expect(result.report.suites[0].specs.length).toBe(2); expect(result.report.suites[0].specs[0].tests[0].status).toBe('expected'); expect(result.report.suites[0].specs[1].tests[0].status).toBe('skipped'); - expect(result.report.suites[1].specs[0].tests[0].status).toBe('skipped'); - expect(result.report.suites[1].specs[1].tests[0].status).toBe('skipped'); - expect(result.report.suites[1].specs[2].tests[0].status).toBe('skipped'); }); test('should report projects', async ({ runInlineTest }, testInfo) => { diff --git a/tests/playwright-test/junit-reporter.spec.ts b/tests/playwright-test/junit-reporter.spec.ts index 11a9ca3bc0..42f264e7f4 100644 --- a/tests/playwright-test/junit-reporter.spec.ts +++ b/tests/playwright-test/junit-reporter.spec.ts @@ -179,13 +179,10 @@ test('should report skipped due to sharding', async ({ runInlineTest }) => { `, }, { shard: '1/3', reporter: 'junit' }); const xml = parseXML(result.output); + expect(xml['testsuites']['testsuite'].length).toBe(1); expect(xml['testsuites']['testsuite'][0]['$']['tests']).toBe('2'); expect(xml['testsuites']['testsuite'][0]['$']['failures']).toBe('0'); expect(xml['testsuites']['testsuite'][0]['$']['skipped']).toBe('1'); - - expect(xml['testsuites']['testsuite'][1]['$']['tests']).toBe('3'); - expect(xml['testsuites']['testsuite'][1]['$']['failures']).toBe('0'); - expect(xml['testsuites']['testsuite'][1]['$']['skipped']).toBe('3'); expect(result.exitCode).toBe(0); }); diff --git a/tests/playwright-test/shard.spec.ts b/tests/playwright-test/shard.spec.ts index 1246afd1cf..a75d26353e 100644 --- a/tests/playwright-test/shard.spec.ts +++ b/tests/playwright-test/shard.spec.ts @@ -18,22 +18,29 @@ import { test, expect } from './playwright-test-fixtures'; const tests = { 'a.spec.ts': ` - const { test } = pwt; + const { test } = pwt; + test.use({ headless: false }); test('test1', async () => { console.log('test1-done'); }); - test('test2', async () => { - console.log('test2-done'); + test.describe('suite', () => { + test.use({ headless: true }); + test('test2', async () => { + console.log('test2-done'); + }); }); test('test3', async () => { console.log('test3-done'); }); `, 'b.spec.ts': ` - const { test } = pwt; + const { test } = pwt; test('test4', async () => { console.log('test4-done'); }); + test('test5', async () => { + console.log('test5-done'); + }); `, }; @@ -41,18 +48,19 @@ test('should respect shard=1/2', async ({ runInlineTest }) => { const result = await runInlineTest(tests, { shard: '1/2' }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(3); - expect(result.skipped).toBe(1); - expect(result.output).toContain('test1-done'); + expect(result.skipped).toBe(0); expect(result.output).toContain('test2-done'); - expect(result.output).toContain('test3-done'); + expect(result.output).toContain('test4-done'); + expect(result.output).toContain('test5-done'); }); test('should respect shard=2/2', async ({ runInlineTest }) => { const result = await runInlineTest(tests, { shard: '2/2' }); expect(result.exitCode).toBe(0); - expect(result.passed).toBe(1); - expect(result.skipped).toBe(3); - expect(result.output).toContain('test4-done'); + expect(result.passed).toBe(2); + expect(result.skipped).toBe(0); + expect(result.output).toContain('test1-done'); + expect(result.output).toContain('test3-done'); }); test('should respect shard=1/2 in config', async ({ runInlineTest }) => { @@ -64,8 +72,8 @@ test('should respect shard=1/2 in config', async ({ runInlineTest }) => { }, { shard: '1/2' }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(3); - expect(result.skipped).toBe(1); - expect(result.output).toContain('test1-done'); + expect(result.skipped).toBe(0); expect(result.output).toContain('test2-done'); - expect(result.output).toContain('test3-done'); + expect(result.output).toContain('test4-done'); + expect(result.output).toContain('test5-done'); });