From 53a78a315e15820de82524810e2f77b156aca9da Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Fri, 3 Nov 2023 13:49:47 -0700 Subject: [PATCH] fix(merge): preserve original "rootDir" by default (#27963) Fixes https://github.com/microsoft/playwright/issues/27877 --- docs/src/test-sharding-js.md | 3 + packages/playwright/src/cli.ts | 3 +- .../playwright/src/isomorphic/teleReceiver.ts | 3 +- packages/playwright/src/reporters/merge.ts | 32 +++++- tests/playwright-test/reporter-blob.spec.ts | 98 ++++++++++++++++++- 5 files changed, 130 insertions(+), 9 deletions(-) diff --git a/docs/src/test-sharding-js.md b/docs/src/test-sharding-js.md index 2f6438966c..a18132304d 100644 --- a/docs/src/test-sharding-js.md +++ b/docs/src/test-sharding-js.md @@ -141,6 +141,8 @@ You can now see the reports have been merged and a combined HTML report is avail `npx playwright merge-reports path/to/blob-reports-dir` reads all blob reports from the passed directory and merges them into a single report. +When merging reports from different OS'es you'll have to provide an explicit merge config to disambiguate which directory should be used as tests root. + Supported options: - `--reporter reporter-to-use` @@ -166,6 +168,7 @@ Supported options: ```ts title="merge.config.ts" export default { + testDir: 'e2e', reporter: [['html', { open: 'never' }]], }; ``` diff --git a/packages/playwright/src/cli.ts b/packages/playwright/src/cli.ts index 5f9182cf6f..27328a91bd 100644 --- a/packages/playwright/src/cli.ts +++ b/packages/playwright/src/cli.ts @@ -199,7 +199,8 @@ async function mergeReports(reportDir: string | undefined, opts: { [key: string] reporterDescriptions = config.config.reporter; if (!reporterDescriptions) reporterDescriptions = [[defaultReporter]]; - await createMergedReport(config, dir, reporterDescriptions!); + const rootDirOverride = configFile ? config.config.rootDir : undefined; + await createMergedReport(config, dir, reporterDescriptions!, rootDirOverride); } function overridesFromOptions(options: { [key: string]: any }): ConfigCLIOverrides { diff --git a/packages/playwright/src/isomorphic/teleReceiver.ts b/packages/playwright/src/isomorphic/teleReceiver.ts index c2ff531fbf..2850fcdc74 100644 --- a/packages/playwright/src/isomorphic/teleReceiver.ts +++ b/packages/playwright/src/isomorphic/teleReceiver.ts @@ -196,7 +196,7 @@ export class TeleReporterReceiver { } private _onConfigure(config: JsonConfig) { - this._rootDir = this._reportConfig?.rootDir || config.rootDir; + this._rootDir = config.rootDir; this._listOnly = config.listOnly; this._config = this._parseConfig(config); this._reporter.onConfigure?.(this._config); @@ -324,7 +324,6 @@ export class TeleReporterReceiver { const result = { ...baseFullConfig, ...config }; if (this._reportConfig) { result.configFile = this._reportConfig.configFile; - result.rootDir = this._reportConfig.rootDir; result.reportSlowTests = this._reportConfig.reportSlowTests; result.quiet = this._reportConfig.quiet; result.reporter = [...this._reportConfig.reporter]; diff --git a/packages/playwright/src/reporters/merge.ts b/packages/playwright/src/reporters/merge.ts index e114455eab..8028d4d1e2 100644 --- a/packages/playwright/src/reporters/merge.ts +++ b/packages/playwright/src/reporters/merge.ts @@ -34,7 +34,7 @@ type ReportData = { reportFile: string; }; -export async function createMergedReport(config: FullConfigInternal, dir: string, reporterDescriptions: ReporterDescription[]) { +export async function createMergedReport(config: FullConfigInternal, dir: string, reporterDescriptions: ReporterDescription[], rootDirOverride: string | undefined) { const reporters = await createReporters(config, 'merge', reporterDescriptions); const multiplexer = new Multiplexer(reporters); const receiver = new TeleReporterReceiver(path.sep, multiplexer, false, config.config); @@ -49,7 +49,7 @@ export async function createMergedReport(config: FullConfigInternal, dir: string const shardFiles = await sortedShardFiles(dir); if (shardFiles.length === 0) throw new Error(`No report files found in ${dir}`); - const eventData = await mergeEvents(dir, shardFiles, stringPool, printStatus); + const eventData = await mergeEvents(dir, shardFiles, stringPool, printStatus, rootDirOverride); printStatus(`processing test events`); const dispatchEvents = async (events: JsonEvent[]) => { @@ -148,7 +148,7 @@ function findMetadata(events: JsonEvent[], file: string): BlobReportMetadata { return metadata; } -async function mergeEvents(dir: string, shardReportFiles: string[], stringPool: StringInternPool, printStatus: StatusCallback) { +async function mergeEvents(dir: string, shardReportFiles: string[], stringPool: StringInternPool, printStatus: StatusCallback, rootDirOverride: string | undefined) { const internalizer = new JsonStringInternalizer(stringPool); const configureEvents: JsonEvent[] = []; @@ -207,7 +207,7 @@ async function mergeEvents(dir: string, shardReportFiles: string[], stringPool: return { prologue: [ - mergeConfigureEvents(configureEvents), + mergeConfigureEvents(configureEvents, rootDirOverride), ...projectEvents, { method: 'onBegin', params: undefined }, ], @@ -219,7 +219,7 @@ async function mergeEvents(dir: string, shardReportFiles: string[], stringPool: }; } -function mergeConfigureEvents(configureEvents: JsonEvent[]): JsonEvent { +function mergeConfigureEvents(configureEvents: JsonEvent[], rootDirOverride: string | undefined): JsonEvent { if (!configureEvents.length) throw new Error('No configure events found'); let config: JsonConfig = { @@ -235,6 +235,28 @@ function mergeConfigureEvents(configureEvents: JsonEvent[]): JsonEvent { }; for (const event of configureEvents) config = mergeConfigs(config, event.params.config); + + if (rootDirOverride) { + config.rootDir = rootDirOverride; + } else { + const rootDirs = new Set(configureEvents.map(e => e.params.config.rootDir)); + if (rootDirs.size > 1) { + throw new Error([ + `Blob reports being merged were recorded with different test directories, and`, + `merging cannot proceed. This may happen if you are merging reports from`, + `machines with different environments, like different operating systems or`, + `if the tests ran with different playwright configs.`, + ``, + `You can force merge by specifying a merge config file with "-c" option. If`, + `you'd like all test paths to be correct, make sure 'testDir' in the merge config`, + `file points to the actual tests location.`, + ``, + `Found directories:`, + ...rootDirs + ].join('\n')); + } + } + return { method: 'onConfigure', params: { diff --git a/tests/playwright-test/reporter-blob.spec.ts b/tests/playwright-test/reporter-blob.spec.ts index e44216f063..379306ef55 100644 --- a/tests/playwright-test/reporter-blob.spec.ts +++ b/tests/playwright-test/reporter-blob.spec.ts @@ -639,7 +639,7 @@ test('generate html with attachment urls', async ({ runInlineTest, mergeReports, const htmlReportDir = test.info().outputPath('playwright-report'); for (const entry of await fs.promises.readdir(htmlReportDir)) - await (fs.promises as any).cp(path.join(htmlReportDir, entry), path.join(reportDir, entry), { recursive: true }); + await fs.promises.cp(path.join(htmlReportDir, entry), path.join(reportDir, entry), { recursive: true }); const oldSeveFile = server.serveFile; server.serveFile = async (req, res) => { @@ -1416,3 +1416,99 @@ test('reporter list in the custom config', async ({ runInlineTest, mergeReports expect(text).toContain('testReporter.js'); expect(text).toContain('{ myOpt: 1 }'); }); + +test('merge reports with different rootDirs', async ({ runInlineTest, mergeReports }) => { + test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/27877' }); + const files1 = { + 'echo-reporter.js': ` + export default class EchoReporter { + onBegin(config, suite) { + console.log('rootDir:', config.rootDir); + } + onTestBegin(test) { + console.log('test:', test.location.file); + } + }; + `, + 'merge.config.ts': `module.exports = { + testDir: 'mergeRoot', + reporter: './echo-reporter.js' + };`, + 'dir1/playwright.config.ts': `module.exports = { + testDir: 'tests1', + reporter: [['blob', { outputDir: 'blob-report' }]] + };`, + 'dir1/tests1/a.test.js': ` + import { test, expect } from '@playwright/test'; + test('math 1', async ({}) => { }); + `, + }; + await runInlineTest(files1, { workers: 1 }, undefined, { additionalArgs: ['--config', test.info().outputPath('dir1/playwright.config.ts')] }); + + const files2 = { + 'dir2/playwright.config.ts': `module.exports = { + reporter: [['blob', { outputDir: 'blob-report' }]] + };`, + 'dir2/tests2/b.test.js': ` + import { test, expect } from '@playwright/test'; + test('math 2', async ({}) => { }); + `, + }; + await runInlineTest(files2, { workers: 1 }, undefined, { additionalArgs: ['--config', test.info().outputPath('dir2/playwright.config.ts')] }); + + const allReportsDir = test.info().outputPath('all-blob-reports'); + await fs.promises.cp(test.info().outputPath('dir1', 'blob-report', 'report.zip'), path.join(allReportsDir, 'report-1.zip')); + await fs.promises.cp(test.info().outputPath('dir2', 'blob-report', 'report.zip'), path.join(allReportsDir, 'report-2.zip')); + + { + const { exitCode, output } = await mergeReports(allReportsDir); + expect(exitCode).toBe(1); + expect(output).toContain(`Blob reports being merged were recorded with different test directories`); + } + + { + const { exitCode, output } = await mergeReports(allReportsDir, undefined, { additionalArgs: ['--config', 'merge.config.ts'] }); + expect(exitCode).toBe(0); + expect(output).toContain(`rootDir: ${test.info().outputPath('mergeRoot')}`); + expect(output).toContain(`test: ${test.info().outputPath('mergeRoot', 'a.test.js')}`); + expect(output).toContain(`test: ${test.info().outputPath('mergeRoot', 'tests2', 'b.test.js')}`); + } +}); + +test('merge reports same rootDirs', async ({ runInlineTest, mergeReports }) => { + test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/27877' }); + const files = { + 'echo-reporter.js': ` + export default class EchoReporter { + onBegin(config, suite) { + console.log('rootDir:', config.rootDir); + } + onTestBegin(test) { + console.log('test:', test.location.file); + } + }; + `, + 'playwright.config.ts': `module.exports = { + testDir: 'tests', + reporter: [['blob', { outputDir: 'blob-report' }]] + };`, + 'tests/dir1/a.test.js': ` + import { test, expect } from '@playwright/test'; + test('math 1', async ({}) => { }); + `, + 'tests/dir2/b.test.js': ` + import { test, expect } from '@playwright/test'; + test('math 2', async ({}) => { }); + `, + }; + await runInlineTest(files, { shard: `1/2` }); + await runInlineTest(files, { shard: `2/2` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' }); + + const allReportsDir = test.info().outputPath('blob-report'); + + const { exitCode, output } = await mergeReports(allReportsDir, undefined, { additionalArgs: ['--reporter', './echo-reporter.js'] }); + expect(exitCode).toBe(0); + expect(output).toContain(`rootDir: ${test.info().outputPath('tests')}`); + expect(output).toContain(`test: ${test.info().outputPath('tests', 'dir1', 'a.test.js')}`); + expect(output).toContain(`test: ${test.info().outputPath('tests', 'dir2', 'b.test.js')}`); +});