From 18be5f5319b248228b8231946891651082a150f7 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 16 Jul 2021 15:23:50 -0700 Subject: [PATCH] feat(test-runner): suite per project (#7688) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes our suite structure the following: ``` Root(title='') > Project(title=projectName) > File(title=relativeFilePath) > ...suites > test ``` Removed `fullTitle()` because it is not used directly by anyone. Default reporters now report each test as ``` [project-name] › relative/file/path.spec.ts:42:42 › suite subsuite test title ``` --- src/test/dispatcher.ts | 10 +++--- src/test/loader.ts | 2 +- src/test/project.ts | 35 ++++++++++++-------- src/test/reporter.ts | 3 -- src/test/reporters/base.ts | 12 ++++--- src/test/reporters/json.ts | 21 ++++++------ src/test/reporters/junit.ts | 9 +++-- src/test/runner.ts | 25 +++++++++----- src/test/test.ts | 19 +++-------- src/test/testType.ts | 2 -- src/test/workerRunner.ts | 2 +- tests/playwright-test/base-reporter.spec.ts | 16 ++++----- tests/playwright-test/junit-reporter.spec.ts | 6 ++-- tests/playwright-test/list-reporter.spec.ts | 8 ++--- tests/playwright-test/match-grep.spec.ts | 18 ---------- tests/playwright-test/reporter.spec.ts | 8 ++--- 16 files changed, 91 insertions(+), 105 deletions(-) diff --git a/src/test/dispatcher.ts b/src/test/dispatcher.ts index 10d02964e8..e7955fd855 100644 --- a/src/test/dispatcher.ts +++ b/src/test/dispatcher.ts @@ -79,14 +79,14 @@ export class Dispatcher { _filesSortedByWorkerHash(): DispatcherEntry[] { const entriesByWorkerHashAndFile = new Map>(); - for (const fileSuite of this._suite.suites) { - const file = fileSuite._requireFile; - for (const test of fileSuite.allTests()) { + 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 = { @@ -94,8 +94,8 @@ export class Dispatcher { entries: [], file, }, - repeatEachIndex: fileSuite._repeatEachIndex, - projectIndex: fileSuite._projectIndex, + repeatEachIndex: test._repeatEachIndex, + projectIndex: test._projectIndex, hash: test._workerHash, }; entriesByFile.set(file, entry); diff --git a/src/test/loader.ts b/src/test/loader.ts index 222b5461ac..f1d34829cd 100644 --- a/src/test/loader.ts +++ b/src/test/loader.ts @@ -110,7 +110,7 @@ export class Loader { if (this._fileSuites.has(file)) return this._fileSuites.get(file)!; try { - const suite = new Suite(''); + const suite = new Suite(path.relative(this._fullConfig.rootDir, file) || path.basename(file)); suite._requireFile = file; suite.location.file = file; setCurrentlyLoadingFileSuite(suite); diff --git a/src/test/project.ts b/src/test/project.ts index 50dc75e613..cb62975b07 100644 --- a/src/test/project.ts +++ b/src/test/project.ts @@ -78,15 +78,15 @@ export class ProjectImpl { return this.testPools.get(test)!; } - cloneSuite(suite: Suite, repeatEachIndex: number, filter: (test: Test) => boolean): Suite | undefined { - const result = suite._clone(); - result._repeatEachIndex = repeatEachIndex; - result._projectIndex = this.index; - for (const entry of suite._entries) { + private _cloneEntries(from: Suite, to: Suite, repeatEachIndex: number, filter: (test: Test) => boolean): boolean { + for (const entry of from._entries) { if (entry instanceof Suite) { - const cloned = this.cloneSuite(entry, repeatEachIndex, filter); - if (cloned) - result._addSuite(cloned); + const suite = entry._clone(); + to._addSuite(suite); + if (!this._cloneEntries(entry, suite, repeatEachIndex, filter)) { + to._entries.pop(); + to.suites.pop(); + } } else { const pool = this.buildPool(entry); const test = entry._clone(); @@ -95,14 +95,21 @@ export class ProjectImpl { test._workerHash = `run${this.index}-${pool.digest}-repeat${repeatEachIndex}`; test._id = `${entry._ordinalInFile}@${entry._requireFile}#run${this.index}-repeat${repeatEachIndex}`; test._pool = pool; - test._buildTitlePath(suite._titlePath); - if (!filter(test)) - continue; - result._addTest(test); + test._repeatEachIndex = repeatEachIndex; + test._projectIndex = this.index; + to._addTest(test); + if (!filter(test)) { + to._entries.pop(); + to.suites.pop(); + } } } - if (result._entries.length) - return result; + return to._entries.length > 0; + } + + cloneFileSuite(suite: Suite, repeatEachIndex: number, filter: (test: Test) => boolean): Suite | undefined { + const result = suite._clone(); + return this._cloneEntries(suite, result, repeatEachIndex, filter) ? result : undefined; } private resolveFixtures(testType: TestTypeImpl): FixturesWithLocation[] { diff --git a/src/test/reporter.ts b/src/test/reporter.ts index 3246b71eb0..19728cf14e 100644 --- a/src/test/reporter.ts +++ b/src/test/reporter.ts @@ -28,7 +28,6 @@ export interface Suite { suites: Suite[]; tests: Test[]; titlePath(): string[]; - fullTitle(): string; allTests(): Test[]; } export interface Test { @@ -38,10 +37,8 @@ export interface Test { expectedStatus: TestStatus; timeout: number; annotations: { type: string, description?: string }[]; - projectName: string; retries: number; titlePath(): string[]; - fullTitle(): string; status(): 'skipped' | 'expected' | 'unexpected' | 'flaky'; ok(): boolean; } diff --git a/src/test/reporters/base.ts b/src/test/reporters/base.ts index 20771a8093..c3cae17f1d 100644 --- a/src/test/reporters/base.ts +++ b/src/test/reporters/base.ts @@ -50,8 +50,9 @@ export class BaseReporter implements Reporter { } onTestEnd(test: Test, result: TestResult) { + const projectName = test.titlePath()[1]; const relativePath = relativeTestPath(this.config, test); - const fileAndProject = relativePath + (test.projectName ? ` [${test.projectName}]` : ''); + const fileAndProject = (projectName ? `[${projectName}] › ` : '') + relativePath; const duration = this.fileDurations.get(fileAndProject) || 0; this.fileDurations.set(fileAndProject, duration + result.duration); } @@ -156,10 +157,11 @@ function relativeTestPath(config: FullConfig, test: Test): string { } export function formatTestTitle(config: FullConfig, test: Test): string { - let relativePath = relativeTestPath(config, test); - relativePath += ':' + test.location.line + ':' + test.location.column; - const title = (test.projectName ? `[${test.projectName}] ` : '') + test.fullTitle(); - return `${relativePath} › ${title}`; + // root, project, file, ...describes, test + const [, projectName, , ...titles] = test.titlePath(); + const location = `${relativeTestPath(config, test)}:${test.location.line}:${test.location.column}`; + const projectTitle = projectName ? `[${projectName}] › ` : ''; + return `${projectTitle}${location} › ${titles.join(' ')}`; } function formatTestHeader(config: FullConfig, test: Test, indent: string, index?: number): string { diff --git a/src/test/reporters/json.ts b/src/test/reporters/json.ts index 7240f34b60..f1e7b0662c 100644 --- a/src/test/reporters/json.ts +++ b/src/test/reporters/json.ts @@ -123,18 +123,19 @@ class JSONReporter implements Reporter { } private _mergeSuites(suites: Suite[]): JSONReportSuite[] { - debugger; const fileSuites = new Map(); const result: JSONReportSuite[] = []; - for (const suite of suites) { - if (!fileSuites.has(suite.location.file)) { - const serialized = this._serializeSuite(suite); - if (serialized) { - fileSuites.set(suite.location.file, serialized); - result.push(serialized); + for (const projectSuite of suites) { + for (const fileSuite of projectSuite.suites) { + if (!fileSuites.has(fileSuite.location.file)) { + const serialized = this._serializeSuite(fileSuite); + if (serialized) { + fileSuites.set(fileSuite.location.file, serialized); + result.push(serialized); + } + } else { + this._mergeTestsFromSuite(fileSuites.get(fileSuite.location.file)!, fileSuite); } - } else { - this._mergeTestsFromSuite(fileSuites.get(suite.location.file)!, suite); } } return result; @@ -202,7 +203,7 @@ class JSONReporter implements Reporter { timeout: test.timeout, annotations: test.annotations, expectedStatus: test.expectedStatus, - projectName: test.projectName, + projectName: test.titlePath()[1], results: test.results.map(r => this._serializeTestResult(r)), status: test.status(), }; diff --git a/src/test/reporters/junit.ts b/src/test/reporters/junit.ts index a35c314ab8..b341a519c0 100644 --- a/src/test/reporters/junit.ts +++ b/src/test/reporters/junit.ts @@ -46,8 +46,10 @@ class JUnitReporter implements Reporter { async onEnd(result: FullResult) { const duration = monotonicTime() - this.startTime; const children: XMLEntry[] = []; - for (const suite of this.suite.suites) - children.push(this._buildTestSuite(suite)); + for (const projectSuite of this.suite.suites) { + for (const fileSuite of projectSuite.suites) + children.push(this._buildTestSuite(fileSuite)); + } const tokens: string[] = []; const self = this; @@ -119,7 +121,8 @@ class JUnitReporter implements Reporter { const entry = { name: 'testcase', attributes: { - name: test.fullTitle(), + // Skip root, project, file + name: test.titlePath().slice(3).join(' '), classname: formatTestTitle(this.config, test), time: (test.results.reduce((acc, value) => acc + value.duration, 0)) / 1000 }, diff --git a/src/test/runner.ts b/src/test/runner.ts index 22b22c5d74..fbccc47cba 100644 --- a/src/test/runner.ts +++ b/src/test/runner.ts @@ -180,8 +180,14 @@ export class Runner { preprocessRoot._addSuite(fileSuite); if (config.forbidOnly) { const onlyTestsAndSuites = preprocessRoot._getOnlyItems(); - if (onlyTestsAndSuites.length > 0) - return { status: 'forbid-only', locations: onlyTestsAndSuites.map(testOrSuite => `${buildItemLocation(config.rootDir, testOrSuite)} > ${testOrSuite.fullTitle()}`) }; + if (onlyTestsAndSuites.length > 0) { + const locations = onlyTestsAndSuites.map(testOrSuite => { + // Skip root and file. + const title = testOrSuite.titlePath().slice(2).join(' '); + return `${buildItemLocation(config.rootDir, testOrSuite)} > ${title}`; + }); + return { status: 'forbid-only', locations }; + } } const clashingTests = getClashingTestsPerSuite(preprocessRoot); if (clashingTests.size > 0) @@ -198,20 +204,21 @@ export class Runner { const grepInvertMatcher = config.grepInvert ? createMatcher(config.grepInvert) : null; const rootSuite = new Suite(''); for (const project of projects) { + const projectSuite = new Suite(project.config.name); + rootSuite._addSuite(projectSuite); for (const file of files.get(project)!) { const fileSuite = fileSuites.get(file); if (!fileSuite) continue; for (let repeatEachIndex = 0; repeatEachIndex < project.config.repeatEach; repeatEachIndex++) { - const cloned = project.cloneSuite(fileSuite, repeatEachIndex, test => { - const fullTitle = test.fullTitle(); - const titleWithProject = (test.projectName ? `[${test.projectName}] ` : '') + fullTitle; - if (grepInvertMatcher?.(titleWithProject)) + const cloned = project.cloneFileSuite(fileSuite, repeatEachIndex, test => { + const grepTitle = test.titlePath().join(' '); + if (grepInvertMatcher?.(grepTitle)) return false; - return grepMatcher(titleWithProject); + return grepMatcher(grepTitle); }); if (cloned) - rootSuite._addSuite(cloned); + projectSuite._addSuite(cloned); } } outputDirs.add(project.config.outputDir); @@ -380,7 +387,7 @@ function getClashingTestsPerSuite(rootSuite: Suite): Map { for (const childSuite of suite.suites) visit(childSuite, clashingTests); for (const test of suite.tests) { - const fullTitle = test.fullTitle(); + const fullTitle = test.titlePath().slice(2).join(' '); if (!clashingTests.has(fullTitle)) clashingTests.set(fullTitle, []); clashingTests.set(fullTitle, clashingTests.get(fullTitle)!.concat(test)); diff --git a/src/test/test.ts b/src/test/test.ts index 987906ea8b..692348020c 100644 --- a/src/test/test.ts +++ b/src/test/test.ts @@ -24,7 +24,6 @@ class Base { location: Location = { file: '', line: 0, column: 0 }; parent?: Suite; - _titlePath: string[] = []; _only = false; _requireFile: string = ''; @@ -32,18 +31,10 @@ class Base { this.title = title; } - _buildTitlePath(parentTitlePath: string[]) { - this._titlePath = [...parentTitlePath]; - if (this.title) - this._titlePath.push(this.title); - } - titlePath(): string[] { - return this._titlePath; - } - - fullTitle(): string { - return this._titlePath.join(' '); + const titlePath = this.parent ? this.parent.titlePath() : []; + titlePath.push(this.title); + return titlePath; } } @@ -67,8 +58,6 @@ export class Suite extends Base implements reporterTypes.Suite { _timeout: number | undefined; _annotations: Annotations = []; _modifiers: Modifier[] = []; - _repeatEachIndex = 0; - _projectIndex = 0; _addTest(test: Test) { test.parent = this; @@ -139,6 +128,8 @@ export class Test extends Base implements reporterTypes.Test { _id = ''; _workerHash = ''; _pool: FixturePool | undefined; + _repeatEachIndex = 0; + _projectIndex = 0; constructor(title: string, fn: Function, ordinalInFile: number, testType: TestTypeImpl) { super(title); diff --git a/src/test/testType.ts b/src/test/testType.ts index a16da74f44..c3af29a4c9 100644 --- a/src/test/testType.ts +++ b/src/test/testType.ts @@ -66,7 +66,6 @@ export class TestTypeImpl { test._requireFile = suite._requireFile; test.location = location; suite._addTest(test); - test._buildTitlePath(suite._titlePath); if (type === 'only') test._only = true; @@ -81,7 +80,6 @@ export class TestTypeImpl { child._requireFile = suite._requireFile; child.location = location; suite._addSuite(child); - child._buildTitlePath(suite._titlePath); if (type === 'only') child._only = true; diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index 7693f81f11..54f6f3515c 100644 --- a/src/test/workerRunner.ts +++ b/src/test/workerRunner.ts @@ -119,7 +119,7 @@ export class WorkerRunner extends EventEmitter { const fileSuite = await this._loader.loadTestFile(runPayload.file); let anyPool: FixturePool | undefined; - const suite = this._project.cloneSuite(fileSuite, this._params.repeatEachIndex, test => { + const suite = this._project.cloneFileSuite(fileSuite, this._params.repeatEachIndex, test => { if (!this._entries.has(test._id)) return false; anyPool = test._pool; diff --git a/tests/playwright-test/base-reporter.spec.ts b/tests/playwright-test/base-reporter.spec.ts index 2429f9a897..743a2bad60 100644 --- a/tests/playwright-test/base-reporter.spec.ts +++ b/tests/playwright-test/base-reporter.spec.ts @@ -123,14 +123,14 @@ test('should print slow tests', async ({ runInlineTest }) => { }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(8); - expect(stripAscii(result.output)).toContain(`Slow test: dir${path.sep}a.test.js [foo] (`); - expect(stripAscii(result.output)).toContain(`Slow test: dir${path.sep}a.test.js [bar] (`); - expect(stripAscii(result.output)).toContain(`Slow test: dir${path.sep}a.test.js [baz] (`); - expect(stripAscii(result.output)).toContain(`Slow test: dir${path.sep}a.test.js [qux] (`); - expect(stripAscii(result.output)).not.toContain(`Slow test: dir${path.sep}b.test.js [foo] (`); - expect(stripAscii(result.output)).not.toContain(`Slow test: dir${path.sep}b.test.js [bar] (`); - expect(stripAscii(result.output)).not.toContain(`Slow test: dir${path.sep}b.test.js [baz] (`); - expect(stripAscii(result.output)).not.toContain(`Slow test: dir${path.sep}b.test.js [qux] (`); + expect(stripAscii(result.output)).toContain(`Slow test: [foo] › dir${path.sep}a.test.js (`); + expect(stripAscii(result.output)).toContain(`Slow test: [bar] › dir${path.sep}a.test.js (`); + expect(stripAscii(result.output)).toContain(`Slow test: [baz] › dir${path.sep}a.test.js (`); + expect(stripAscii(result.output)).toContain(`Slow test: [qux] › dir${path.sep}a.test.js (`); + expect(stripAscii(result.output)).not.toContain(`Slow test: [foo] › dir${path.sep}b.test.js (`); + expect(stripAscii(result.output)).not.toContain(`Slow test: [bar] › dir${path.sep}b.test.js (`); + expect(stripAscii(result.output)).not.toContain(`Slow test: [baz] › dir${path.sep}b.test.js (`); + expect(stripAscii(result.output)).not.toContain(`Slow test: [qux] › dir${path.sep}b.test.js (`); }); test('should not print slow tests', async ({ runInlineTest }) => { diff --git a/tests/playwright-test/junit-reporter.spec.ts b/tests/playwright-test/junit-reporter.spec.ts index b975544e34..11a9ca3bc0 100644 --- a/tests/playwright-test/junit-reporter.spec.ts +++ b/tests/playwright-test/junit-reporter.spec.ts @@ -212,16 +212,14 @@ test('should render projects', async ({ runInlineTest }) => { expect(xml['testsuites']['testsuite'][0]['$']['failures']).toBe('0'); expect(xml['testsuites']['testsuite'][0]['$']['skipped']).toBe('0'); expect(xml['testsuites']['testsuite'][0]['testcase'][0]['$']['name']).toBe('one'); - expect(xml['testsuites']['testsuite'][0]['testcase'][0]['$']['classname']).toContain('[project1] one'); - expect(xml['testsuites']['testsuite'][0]['testcase'][0]['$']['classname']).toContain('a.test.js:6:7'); + expect(xml['testsuites']['testsuite'][0]['testcase'][0]['$']['classname']).toContain('[project1] › a.test.js:6:7 › one'); expect(xml['testsuites']['testsuite'][1]['$']['name']).toBe('a.test.js'); expect(xml['testsuites']['testsuite'][1]['$']['tests']).toBe('1'); expect(xml['testsuites']['testsuite'][1]['$']['failures']).toBe('0'); expect(xml['testsuites']['testsuite'][1]['$']['skipped']).toBe('0'); expect(xml['testsuites']['testsuite'][1]['testcase'][0]['$']['name']).toBe('one'); - expect(xml['testsuites']['testsuite'][1]['testcase'][0]['$']['classname']).toContain('[project2] one'); - expect(xml['testsuites']['testsuite'][1]['testcase'][0]['$']['classname']).toContain('a.test.js:6:7'); + expect(xml['testsuites']['testsuite'][1]['testcase'][0]['$']['classname']).toContain('[project2] › a.test.js:6:7 › one'); expect(result.exitCode).toBe(0); }); diff --git a/tests/playwright-test/list-reporter.spec.ts b/tests/playwright-test/list-reporter.spec.ts index 2fefa0414b..a9c34541eb 100644 --- a/tests/playwright-test/list-reporter.spec.ts +++ b/tests/playwright-test/list-reporter.spec.ts @@ -37,9 +37,9 @@ test('render each test with project name', async ({ runInlineTest }) => { const text = stripAscii(result.output); const positiveStatusMarkPrefix = process.platform === 'win32' ? 'ok' : '✓ '; const negativateStatusMarkPrefix = process.platform === 'win32' ? 'x ' : '✘ '; - expect(text).toContain(`${negativateStatusMarkPrefix} 1) a.test.ts:6:7 › [foo] fails`); - expect(text).toContain(`${negativateStatusMarkPrefix} 2) a.test.ts:6:7 › [bar] fails`); - expect(text).toContain(`${positiveStatusMarkPrefix} a.test.ts:9:7 › [foo] passes`); - expect(text).toContain(`${positiveStatusMarkPrefix} a.test.ts:9:7 › [bar] passes`); + expect(text).toContain(`${negativateStatusMarkPrefix} 1) [foo] › a.test.ts:6:7 › fails`); + expect(text).toContain(`${negativateStatusMarkPrefix} 2) [bar] › a.test.ts:6:7 › fails`); + expect(text).toContain(`${positiveStatusMarkPrefix} [foo] › a.test.ts:9:7 › passes`); + expect(text).toContain(`${positiveStatusMarkPrefix} [bar] › a.test.ts:9:7 › passes`); expect(result.exitCode).toBe(1); }); diff --git a/tests/playwright-test/match-grep.spec.ts b/tests/playwright-test/match-grep.spec.ts index c39c25acbc..218cbad512 100644 --- a/tests/playwright-test/match-grep.spec.ts +++ b/tests/playwright-test/match-grep.spec.ts @@ -81,24 +81,6 @@ test('should grep test name with regular expression and a space', async ({ runIn expect(result.exitCode).toBe(0); }); -test('should grep by project name', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'playwright.config.ts': ` - module.exports = { projects: [ - { name: 'foo' }, - { name: 'bar' }, - ]}; - `, - 'a.spec.ts': ` - pwt.test('should work', () => {}); - `, - }, { 'grep': 'foo]' }); - expect(result.passed).toBe(1); - expect(result.skipped).toBe(0); - expect(result.failed).toBe(0); - expect(result.exitCode).toBe(0); -}); - test('should grep invert test name', async ({ runInlineTest }) => { const result = await runInlineTest(files, { 'grep-invert': 'BB' }); expect(result.passed).toBe(6); diff --git a/tests/playwright-test/reporter.spec.ts b/tests/playwright-test/reporter.spec.ts index 53848fa3b4..3545c8b787 100644 --- a/tests/playwright-test/reporter.spec.ts +++ b/tests/playwright-test/reporter.spec.ts @@ -24,10 +24,10 @@ test('should work with custom reporter', async ({ runInlineTest }) => { this.options = options; } onBegin(config, suite) { - console.log('\\n%%reporter-begin-' + this.options.begin + '-' + suite.suites.length + '%%'); + console.log('\\n%%reporter-begin-' + this.options.begin + '%%'); } onTestBegin(test) { - console.log('\\n%%reporter-testbegin-' + test.title + '-' + test.projectName + '%%'); + console.log('\\n%%reporter-testbegin-' + test.title + '-' + test.titlePath()[1] + '%%'); } onStdOut() { console.log('\\n%%reporter-stdout%%'); @@ -36,7 +36,7 @@ test('should work with custom reporter', async ({ runInlineTest }) => { console.log('\\n%%reporter-stderr%%'); } onTestEnd(test) { - console.log('\\n%%reporter-testend-' + test.title + '-' + test.projectName + '%%'); + console.log('\\n%%reporter-testend-' + test.title + '-' + test.titlePath()[1] + '%%'); } onTimeout() { console.log('\\n%%reporter-timeout%%'); @@ -73,7 +73,7 @@ test('should work with custom reporter', async ({ runInlineTest }) => { expect(result.exitCode).toBe(0); expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ - '%%reporter-begin-begin-3%%', + '%%reporter-begin-begin%%', '%%reporter-testbegin-pass-foo%%', '%%reporter-stdout%%', '%%reporter-stderr%%',