diff --git a/packages/playwright/src/reporters/blob.ts b/packages/playwright/src/reporters/blob.ts index 057caab010..931345b01d 100644 --- a/packages/playwright/src/reporters/blob.ts +++ b/packages/playwright/src/reporters/blob.ts @@ -38,6 +38,7 @@ export type BlobReportMetadata = { userAgent: string; name?: string; shard?: { total: number, current: number }; + pathSeparator?: string; }; export class BlobReporter extends TeleReporterEmitter { @@ -59,6 +60,7 @@ export class BlobReporter extends TeleReporterEmitter { userAgent: getUserAgent(), name: process.env.PWTEST_BLOB_REPORT_NAME, shard: config.shard ?? undefined, + pathSeparator: path.sep, }; this._messages.push({ method: 'onBlobReportMetadata', diff --git a/packages/playwright/src/reporters/merge.ts b/packages/playwright/src/reporters/merge.ts index 8028d4d1e2..f2e4ec8eda 100644 --- a/packages/playwright/src/reporters/merge.ts +++ b/packages/playwright/src/reporters/merge.ts @@ -18,7 +18,7 @@ import fs from 'fs'; import path from 'path'; import type { ReporterDescription } from '../../types/test'; import type { FullConfigInternal } from '../common/config'; -import type { JsonConfig, JsonEvent, JsonFullResult, JsonProject, JsonSuite, JsonTestResultEnd } from '../isomorphic/teleReceiver'; +import type { JsonConfig, JsonEvent, JsonFullResult, JsonLocation, JsonProject, JsonSuite, JsonTestResultEnd, JsonTestStepStart } from '../isomorphic/teleReceiver'; import { TeleReporterReceiver } from '../isomorphic/teleReceiver'; import { JsonStringInternalizer, StringInternPool } from '../isomorphic/stringInternPool'; import { createReporters } from '../runner/reporters'; @@ -30,14 +30,13 @@ import { relativeFilePath } from '../util'; type StatusCallback = (message: string) => void; type ReportData = { - idsPatcher: IdsPatcher; + eventPatchers: JsonEventPatchers; reportFile: string; }; 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); const stringPool = new StringInternPool(); let printStatus: StatusCallback = () => {}; @@ -50,6 +49,9 @@ export async function createMergedReport(config: FullConfigInternal, dir: string if (shardFiles.length === 0) throw new Error(`No report files found in ${dir}`); const eventData = await mergeEvents(dir, shardFiles, stringPool, printStatus, rootDirOverride); + // If expicit config is provided, use platform path separator, otherwise use the one from the report (if any). + const pathSep = rootDirOverride ? path.sep : (eventData.pathSeparatorFromMetadata ?? path.sep); + const receiver = new TeleReporterReceiver(pathSep, multiplexer, false, config.config); printStatus(`processing test events`); const dispatchEvents = async (events: JsonEvent[]) => { @@ -63,30 +65,17 @@ export async function createMergedReport(config: FullConfigInternal, dir: string }; await dispatchEvents(eventData.prologue); - for (const { reportFile, idsPatcher } of eventData.reports) { + for (const { reportFile, eventPatchers } of eventData.reports) { const reportJsonl = await fs.promises.readFile(reportFile); const events = parseTestEvents(reportJsonl); new JsonStringInternalizer(stringPool).traverse(events); - idsPatcher.patchEvents(events); - patchAttachmentPaths(events, dir); + eventPatchers.patchers.push(new AttachmentPathPatcher(dir)); + eventPatchers.patchEvents(events); await dispatchEvents(events); } await dispatchEvents(eventData.epilogue); } -function patchAttachmentPaths(events: JsonEvent[], resourceDir: string) { - for (const event of events) { - if (event.method !== 'onTestEnd') - continue; - for (const attachment of (event.params.result as JsonTestResultEnd).attachments) { - if (!attachment.path) - continue; - - attachment.path = path.join(resourceDir, attachment.path); - } - } -} - const commonEventNames = ['onBlobReportMetadata', 'onConfigure', 'onProject', 'onBegin', 'onEnd']; const commonEvents = new Set(commonEventNames); const commonEventRegex = new RegExp(`${commonEventNames.join('|')}`); @@ -186,8 +175,12 @@ async function mergeEvents(dir: string, shardReportFiles: string[], stringPool: salt = sha1 + '-' + i; saltSet.add(salt); - const idsPatcher = new IdsPatcher(stringPool, metadata.name, salt); - idsPatcher.patchEvents(parsedEvents); + const eventPatchers = new JsonEventPatchers(); + eventPatchers.patchers.push(new IdsPatcher(stringPool, metadata.name, salt)); + // Only patch path separators if we are merging reports with explicit config. + if (rootDirOverride) + eventPatchers.patchers.push(new PathSeparatorPatcher(metadata.pathSeparator)); + eventPatchers.patchEvents(parsedEvents); for (const event of parsedEvents) { if (event.method === 'onConfigure') @@ -200,7 +193,7 @@ async function mergeEvents(dir: string, shardReportFiles: string[], stringPool: // Save information about the reports to stream their test events later. reports.push({ - idsPatcher, + eventPatchers, reportFile: localPath, }); } @@ -215,7 +208,8 @@ async function mergeEvents(dir: string, shardReportFiles: string[], stringPool: epilogue: [ mergeEndEvents(endEvents), { method: 'onExit', params: undefined }, - ] + ], + pathSeparatorFromMetadata: blobs[0]?.metadata.pathSeparator, }; } @@ -338,26 +332,27 @@ class UniqueFileNameGenerator { } class IdsPatcher { - constructor(private _stringPool: StringInternPool, private _reportName: string | undefined, private _salt: string) { + constructor( + private _stringPool: StringInternPool, + private _reportName: string | undefined, + private _salt: string) { } - patchEvents(events: JsonEvent[]) { - for (const event of events) { - const { method, params } = event; - switch (method) { - case 'onProject': - this._onProject(params.project); - continue; - case 'onTestBegin': - case 'onStepBegin': - case 'onStepEnd': - case 'onStdIO': - params.testId = this._mapTestId(params.testId); - continue; - case 'onTestEnd': - params.test.testId = this._mapTestId(params.test.testId); - continue; - } + patchEvent(event: JsonEvent) { + const { method, params } = event; + switch (method) { + case 'onProject': + this._onProject(params.project); + return; + case 'onTestBegin': + case 'onStepBegin': + case 'onStepEnd': + case 'onStdIO': + params.testId = this._mapTestId(params.testId); + return; + case 'onTestEnd': + params.test.testId = this._mapTestId(params.test.testId); + return; } } @@ -377,3 +372,92 @@ class IdsPatcher { return this._stringPool.internString(testId + this._salt); } } + +class AttachmentPathPatcher { + constructor(private _resourceDir: string) { + } + + patchEvent(event: JsonEvent) { + if (event.method !== 'onTestEnd') + return; + for (const attachment of (event.params.result as JsonTestResultEnd).attachments) { + if (!attachment.path) + continue; + + attachment.path = path.join(this._resourceDir, attachment.path); + } + } +} + +class PathSeparatorPatcher { + private _from: string; + private _to: string; + constructor(from?: string) { + this._from = from ?? (path.sep === '/' ? '\\' : '/'); + this._to = path.sep; + } + + patchEvent(jsonEvent: JsonEvent) { + if (this._from === this._to) + return; + if (jsonEvent.method === 'onProject') { + this._updateProject(jsonEvent.params.project as JsonProject); + return; + } + if (jsonEvent.method === 'onTestEnd') { + const testResult = jsonEvent.params.result as JsonTestResultEnd; + testResult.errors.forEach(error => this._updateLocation(error.location)); + testResult.attachments.forEach(attachment => { + if (attachment.path) + attachment.path = this._updatePath(attachment.path); + }); + return; + } + if (jsonEvent.method === 'onStepBegin') { + const step = jsonEvent.params.step as JsonTestStepStart; + this._updateLocation(step.location); + return; + } + } + + private _updateProject(project: JsonProject) { + project.outputDir = this._updatePath(project.outputDir); + project.testDir = this._updatePath(project.testDir); + project.snapshotDir = this._updatePath(project.snapshotDir); + project.suites.forEach(suite => this._updateSuite(suite, true)); + } + + private _updateSuite(suite: JsonSuite, isFileSuite: boolean = false) { + this._updateLocation(suite.location); + if (isFileSuite) + suite.title = this._updatePath(suite.title); + for (const child of suite.suites) + this._updateSuite(child); + for (const test of suite.tests) + this._updateLocation(test.location); + } + + private _updateLocation(location?: JsonLocation) { + if (location) + location.file = this._updatePath(location.file); + } + + private _updatePath(text: string): string { + return text.split(this._from).join(this._to); + } +} + +interface JsonEventPatcher { + patchEvent(event: JsonEvent): void; +} + +class JsonEventPatchers { + readonly patchers: JsonEventPatcher[] = []; + + patchEvents(events: JsonEvent[]) { + for (const event of events) { + for (const patcher of this.patchers) + patcher.patchEvent(event); + } + } +} diff --git a/tests/playwright-test/reporter-blob.spec.ts b/tests/playwright-test/reporter-blob.spec.ts index 379306ef55..a2f1e04418 100644 --- a/tests/playwright-test/reporter-blob.spec.ts +++ b/tests/playwright-test/reporter-blob.spec.ts @@ -1259,15 +1259,36 @@ test('blob report should include version', async ({ runInlineTest }) => { const reportFiles = await fs.promises.readdir(reportDir); expect(reportFiles).toEqual(['report.zip']); - await extractZip(test.info().outputPath('blob-report', reportFiles[0]), { dir: test.info().outputPath('blob-report') }); - const reportFile = test.info().outputPath('blob-report', reportFiles[0].replace(/\.zip$/, '.jsonl')); - const data = await fs.promises.readFile(reportFile, 'utf8'); - const events = data.split('\n').filter(Boolean).map(line => JSON.parse(line)); + const events = await extractReport(test.info().outputPath('blob-report', 'report.zip'), test.info().outputPath('tmp')); const metadataEvent = events.find(e => e.method === 'onBlobReportMetadata'); expect(metadataEvent.params.version).toBe(1); expect(metadataEvent.params.userAgent).toBe(getUserAgent()); }); +async function extractReport(reportZipFile: string, unzippedReportDir: string): Promise { + await extractZip(reportZipFile, { dir: unzippedReportDir }); + const reportFile = path.join(unzippedReportDir, path.basename(reportZipFile).replace(/\.zip$/, '.jsonl')); + const data = await fs.promises.readFile(reportFile, 'utf8'); + const events = data.split('\n').filter(Boolean).map(line => JSON.parse(line)); + return events; +} + +async function zipReport(events: any[], zipFilePath: string) { + const lines = events.map(e => JSON.stringify(e) + '\n'); + const zipFile = new yazl.ZipFile(); + const zipFinishPromise = new Promise((resolve, reject) => { + (zipFile as any).on('error', error => reject(error)); + zipFile.outputStream.pipe(fs.createWriteStream(zipFilePath)).on('close', () => { + resolve(undefined); + }).on('error', error => reject(error)); + }); + const content = Readable.from(lines); + zipFile.addReadStream(content, 'report.jsonl'); + zipFile.end(); + await zipFinishPromise; +} + + test('merge-reports should throw if report version is from the future', async ({ runInlineTest, mergeReports }) => { const reportDir = test.info().outputPath('blob-report'); const files = { @@ -1293,28 +1314,15 @@ test('merge-reports should throw if report version is from the future', async ({ // Extract report and modify version. const reportZipFile = test.info().outputPath('blob-report', reportFiles[1]); - await extractZip(reportZipFile, { dir: test.info().outputPath('tmp') }); - const reportFile = test.info().outputPath('tmp', reportFiles[1].replace(/\.zip$/, '.jsonl')); - const data = await fs.promises.readFile(reportFile, 'utf8'); - const events = data.split('\n').filter(Boolean).map(line => JSON.parse(line)); + const events = await extractReport(reportZipFile, test.info().outputPath('tmp')); + const metadataEvent = events.find(e => e.method === 'onBlobReportMetadata'); expect(metadataEvent.params.version).toBeTruthy(); ++metadataEvent.params.version; - const modifiedLines = events.map(e => JSON.stringify(e) + '\n'); // Zip it back. await fs.promises.rm(reportZipFile, { force: true }); - const zipFile = new yazl.ZipFile(); - const zipFinishPromise = new Promise((resolve, reject) => { - (zipFile as any).on('error', error => reject(error)); - zipFile.outputStream.pipe(fs.createWriteStream(reportZipFile)).on('close', () => { - resolve(undefined); - }).on('error', error => reject(error)); - }); - const content = Readable.from(modifiedLines); - zipFile.addReadStream(content, path.basename(reportFile)); - zipFile.end(); - await zipFinishPromise; + await zipReport(events, reportZipFile); const { exitCode, output } = await mergeReports(reportDir, { 'PW_TEST_HTML_REPORT_OPEN': 'never' }, { additionalArgs: ['--reporter', 'html'] }); expect(exitCode).toBe(1); @@ -1512,3 +1520,135 @@ test('merge reports same rootDirs', async ({ runInlineTest, mergeReports }) => { expect(output).toContain(`test: ${test.info().outputPath('tests', 'dir1', 'a.test.js')}`); expect(output).toContain(`test: ${test.info().outputPath('tests', 'dir2', 'b.test.js')}`); }); + + +function patchPathSeparators(json: any) { + const from = (path.sep === '/') ? /\//g : /\\/g; + const to = (path.sep === '/') ? '\\' : '/'; + + function patchPathSeparatorsRecursive(obj: any) { + if (typeof obj !== 'object') + return; + for (const key in obj) { + if (/file|dir|path|^title$/i.test(key) && typeof obj[key] === 'string') + obj[key] = obj[key].replace(from, to); + patchPathSeparatorsRecursive(obj[key]); + } + } + patchPathSeparatorsRecursive(json); +} + +test('merge reports with different rootDirs and path separators', 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); + console.log('test title:', test.titlePath()[2]); + } + }; + `, + 'merge.config.ts': `module.exports = { + testDir: 'mergeRoot', + reporter: './echo-reporter.js' + };`, + 'dir1/playwright.config.ts': `module.exports = { + 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.mkdir(allReportsDir, { recursive: true }); + + // Extract report and change path separators. + const reportZipFile = test.info().outputPath('dir1', 'blob-report', 'report.zip'); + const events = await extractReport(reportZipFile, test.info().outputPath('tmp')); + events.forEach(patchPathSeparators); + + // Zip it back. + const report1 = path.join(allReportsDir, 'report-1.zip'); + await zipReport(events, report1); + + // Copy second report as is. + await fs.promises.cp(test.info().outputPath('dir2', 'blob-report', 'report.zip'), path.join(allReportsDir, 'report-2.zip')); + + { + 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', 'tests1', 'a.test.js')}`); + expect(output).toContain(`test title: ${'tests1' + path.sep + 'a.test.js'}`); + expect(output).toContain(`test: ${test.info().outputPath('mergeRoot', 'tests2', 'b.test.js')}`); + expect(output).toContain(`test title: ${'tests2' + path.sep + 'b.test.js'}`); + } +}); + +test('merge reports without --config preserves path separators', 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); + console.log('test title:', test.titlePath()[2]); + } + }; + `, + 'dir1/playwright.config.ts': `module.exports = { + reporter: [['blob', { outputDir: 'blob-report' }]] + };`, + 'dir1/tests1/a.test.js': ` + import { test, expect } from '@playwright/test'; + test('math 1', async ({}) => { }); + `, + 'dir1/tests2/b.test.js': ` + import { test, expect } from '@playwright/test'; + test('math 2', async ({}) => { }); + `, + }; + await runInlineTest(files1, { workers: 1 }, undefined, { additionalArgs: ['--config', test.info().outputPath('dir1/playwright.config.ts')] }); + + const allReportsDir = test.info().outputPath('all-blob-reports'); + await fs.promises.mkdir(allReportsDir, { recursive: true }); + + // Extract report and change path separators. + const reportZipFile = test.info().outputPath('dir1', 'blob-report', 'report.zip'); + const events = await extractReport(reportZipFile, test.info().outputPath('tmp')); + events.forEach(patchPathSeparators); + + // Zip it back. + const report1 = path.join(allReportsDir, 'report-1.zip'); + await zipReport(events, report1); + + const { exitCode, output } = await mergeReports(allReportsDir, undefined, { additionalArgs: ['--reporter', './echo-reporter.js'] }); + expect(exitCode).toBe(0); + const otherSeparator = path.sep === '/' ? '\\' : '/'; + expect(output).toContain(`rootDir: ${test.info().outputPath('dir1').replaceAll(path.sep, otherSeparator)}`); + expect(output).toContain(`test: ${test.info().outputPath('dir1', 'tests1', 'a.test.js').replaceAll(path.sep, otherSeparator)}`); + expect(output).toContain(`test title: ${'tests1' + otherSeparator + 'a.test.js'}`); + expect(output).toContain(`test: ${test.info().outputPath('dir1', 'tests2', 'b.test.js').replaceAll(path.sep, otherSeparator)}`); + expect(output).toContain(`test title: ${'tests2' + otherSeparator + 'b.test.js'}`); +}); diff --git a/tests/playwright-test/reporter-markdown.spec.ts b/tests/playwright-test/reporter-markdown.spec.ts index 0241cb495c..d24f2561c1 100644 --- a/tests/playwright-test/reporter-markdown.spec.ts +++ b/tests/playwright-test/reporter-markdown.spec.ts @@ -14,7 +14,8 @@ * limitations under the License. */ -import * as fs from 'fs'; +import fs from 'fs'; +import path from 'path'; import { expect, test } from './playwright-test-fixtures'; test('simple report', async ({ runInlineTest }) => { @@ -25,7 +26,7 @@ test('simple report', async ({ runInlineTest }) => { reporter: 'markdown', }; `, - 'a.test.js': ` + 'dir1/a.test.js': ` import { test, expect } from '@playwright/test'; test('math 1', async ({}) => { expect(1 + 1).toBe(2); @@ -38,7 +39,7 @@ test('simple report', async ({ runInlineTest }) => { }); test.skip('skipped 1', async ({}) => {}); `, - 'b.test.js': ` + 'dir2/b.test.js': ` import { test, expect } from '@playwright/test'; test('math 2', async ({}) => { expect(1 + 1).toBe(2); @@ -63,13 +64,13 @@ test('simple report', async ({ runInlineTest }) => { expect(exitCode).toBe(1); const reportFile = await fs.promises.readFile(test.info().outputPath('report.md')); expect(reportFile.toString()).toContain(`**2 failed** -:x: a.test.js:6:11 › failing 1 -:x: b.test.js:6:11 › failing 2 +:x: dir1${path.sep}a.test.js:6:11 › failing 1 +:x: dir2${path.sep}b.test.js:6:11 › failing 2
2 flaky -:warning: a.test.js:9:11 › flaky 1
:warning: c.test.js:6:11 › flaky 2
+:warning: dir1${path.sep}a.test.js:9:11 › flaky 1