From d0280ec8c7b81ebf9aadb764628bd48458224c81 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 18 Jul 2023 09:29:25 -0700 Subject: [PATCH] chore(blob): drop shard number from report name (#24270) We store shard number in the report metadata event and then sort shard files by shard number. This guarantees that within each project sharded events will always go in stable order. --- .../playwright-test/src/reporters/blob.ts | 27 +++++++------------ .../playwright-test/src/reporters/merge.ts | 19 +++++++++++++ tests/playwright-test/reporter-blob.spec.ts | 20 +++++++------- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/packages/playwright-test/src/reporters/blob.ts b/packages/playwright-test/src/reporters/blob.ts index 6eb33b0536..5b91e1beca 100644 --- a/packages/playwright-test/src/reporters/blob.ts +++ b/packages/playwright-test/src/reporters/blob.ts @@ -33,6 +33,7 @@ type BlobReporterOptions = { export type BlobReportMetadata = { projectSuffix?: string; + shard?: { total: number, current: number }; }; export class BlobReporter extends TeleReporterEmitter { @@ -48,18 +49,19 @@ export class BlobReporter extends TeleReporterEmitter { super(message => this._messages.push(message), false); this._options = options; this._salt = createGuid(); - - this._messages.push({ - method: 'onBlobReportMetadata', - params: { - projectSuffix: process.env.PWTEST_BLOB_SUFFIX, - } - }); } override onConfigure(config: FullConfig) { this._outputDir = path.resolve(this._options.configDir, this._options.outputDir || 'blob-report'); - this._reportName = this._computeReportName(config); + this._reportName = `report-${createGuid()}`; + const metadata: BlobReportMetadata = { + projectSuffix: process.env.PWTEST_BLOB_SUFFIX, + shard: config.shard ? config.shard : undefined, + }; + this._messages.push({ + method: 'onBlobReportMetadata', + params: metadata + }); super.onConfigure(config); } @@ -109,15 +111,6 @@ export class BlobReporter extends TeleReporterEmitter { }); } - private _computeReportName(config: FullConfig) { - let shardSuffix = ''; - if (config.shard) { - const paddedNumber = `${config.shard.current}`.padStart(`${config.shard.total}`.length, '0'); - shardSuffix = `${paddedNumber}-of-${config.shard.total}-`; - } - return `report-${shardSuffix}${createGuid()}`; - } - private _startCopyingFile(from: string, to: string) { const copyPromise: Promise = fs.promises.copyFile(from, to) .catch(e => { console.error(`Failed to copy file from "${from}" to "${to}": ${e}`); }) diff --git a/packages/playwright-test/src/reporters/merge.ts b/packages/playwright-test/src/reporters/merge.ts index 8f1fac6d94..4b28fcc301 100644 --- a/packages/playwright-test/src/reporters/merge.ts +++ b/packages/playwright-test/src/reporters/merge.ts @@ -24,6 +24,7 @@ import { TeleReporterReceiver } from '../isomorphic/teleReceiver'; import { createReporters } from '../runner/reporters'; import { Multiplexer } from './multiplexer'; import { ZipFile } from 'playwright-core/lib/utils'; +import type { BlobReportMetadata } from './blob'; export async function createMergedReport(config: FullConfigInternal, dir: string, reporterDescriptions: ReporterDescription[], resolvePaths: boolean) { const shardFiles = await sortedShardFiles(dir); @@ -71,14 +72,32 @@ async function extractReportFromZip(file: string): Promise { throw new Error(`Cannot find *.jsonl file in ${file}`); } +function findMetadata(events: JsonEvent[], file: string): BlobReportMetadata { + if (events[0]?.method !== 'onBlobReportMetadata') + throw new Error(`No metadata event found in ${file}`); + return events[0].params; +} + async function mergeEvents(dir: string, shardReportFiles: string[]) { const events: JsonEvent[] = []; const configureEvents: JsonEvent[] = []; const beginEvents: JsonEvent[] = []; const endEvents: JsonEvent[] = []; + const shardEvents: { metadata: BlobReportMetadata, parsedEvents: JsonEvent[] }[] = []; for (const reportFile of shardReportFiles) { const reportJsonl = await extractReportFromZip(path.join(dir, reportFile)); const parsedEvents = parseEvents(reportJsonl); + shardEvents.push({ + metadata: findMetadata(parsedEvents, reportFile), + parsedEvents + }); + } + shardEvents.sort((a, b) => { + const shardA = a.metadata.shard?.current ?? 0; + const shardB = b.metadata.shard?.current ?? 0; + return shardA - shardB; + }); + for (const { parsedEvents } of shardEvents) { for (const event of parsedEvents) { if (event.method === 'onConfigure') configureEvents.push(event); diff --git a/tests/playwright-test/reporter-blob.spec.ts b/tests/playwright-test/reporter-blob.spec.ts index c33b6d47cd..9aefabd6f4 100644 --- a/tests/playwright-test/reporter-blob.spec.ts +++ b/tests/playwright-test/reporter-blob.spec.ts @@ -130,7 +130,7 @@ test('should call methods in right order', async ({ runInlineTest, mergeReports await runInlineTest(files, { shard: `3/3` }); const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); - expect(reportFiles).toEqual([expect.stringMatching(/report-1-of-3.*.zip/), expect.stringMatching(/report-3-of-3.*.zip/), 'resources']); + expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), 'resources']); const { exitCode, output } = await mergeReports(reportDir, {}, { additionalArgs: ['--reporter', test.info().outputPath('echo-reporter.js')] }); expect(exitCode).toBe(0); const lines = output.split('\n').filter(l => l.trim().length); @@ -194,7 +194,7 @@ test('should merge into html', async ({ runInlineTest, mergeReports, showReport, await runInlineTest(files, { shard: `${i + 1}/${totalShards}` }); const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); - expect(reportFiles).toEqual([expect.stringMatching(/report-1-of-3.*.zip/), expect.stringMatching(/report-2-of-3.*.zip/), expect.stringMatching(/report-3-of-3.*.zip/), 'resources']); + expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), 'resources']); const { exitCode, output } = await mergeReports(reportDir, { 'PW_TEST_HTML_REPORT_OPEN': 'never' }, { additionalArgs: ['--reporter', 'html'] }); expect(exitCode).toBe(0); @@ -254,7 +254,7 @@ test('be able to merge incomplete shards', async ({ runInlineTest, mergeReports, const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); - expect(reportFiles).toEqual([expect.stringMatching(/report-1-of-3.*.zip/), expect.stringMatching(/report-3-of-3.*.zip/), 'resources']); + expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), 'resources']); const { exitCode } = await mergeReports(reportDir, { 'PW_TEST_HTML_REPORT_OPEN': 'never' }, { additionalArgs: ['--reporter', 'html'] }); expect(exitCode).toBe(0); @@ -360,7 +360,7 @@ test('merge into list report by default', async ({ runInlineTest, mergeReports } await runInlineTest(files, { shard: `${i + 1}/${totalShards}` }); const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); - expect(reportFiles).toEqual([expect.stringMatching(/report-1-of-3.*.zip/), expect.stringMatching(/report-2-of-3.*.zip/), expect.stringMatching(/report-3-of-3.*.zip/), 'resources']); + expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), 'resources']); const { exitCode, output } = await mergeReports(reportDir, { PW_TEST_DEBUG_REPORTERS: '1', PW_TEST_DEBUG_REPORTERS_PRINT_STEPS: '1', PWTEST_TTY_WIDTH: '80' }, { additionalArgs: ['--reporter', 'list'] }); expect(exitCode).toBe(0); @@ -431,7 +431,7 @@ test('preserve attachments', async ({ runInlineTest, mergeReports, showReport, p const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); - expect(reportFiles).toEqual([expect.stringMatching(/report-1-of-2.*.zip/), 'resources']); + expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), 'resources']); const { exitCode } = await mergeReports(reportDir, { 'PW_TEST_HTML_REPORT_OPEN': 'never' }, { additionalArgs: ['--reporter', 'html'] }); expect(exitCode).toBe(0); @@ -494,7 +494,7 @@ test('generate html with attachment urls', async ({ runInlineTest, mergeReports, const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); - expect(reportFiles).toEqual([expect.stringMatching(/report-1-of-2.*.zip/), 'resources']); + expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), 'resources']); const { exitCode } = await mergeReports(reportDir, { 'PW_TEST_HTML_REPORT_OPEN': 'never' }, { additionalArgs: ['--reporter', 'html', '--attachments', 'missing'] }); expect(exitCode).toBe(0); @@ -568,7 +568,7 @@ test('resource names should not clash between runs', async ({ runInlineTest, sho const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); - expect(reportFiles).toEqual([expect.stringMatching(/report-1-of-2.*.zip/), expect.stringMatching(/report-2-of-2.*.zip/), 'resources']); + expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), 'resources']); const { exitCode } = await mergeReports(reportDir, {}, { additionalArgs: ['--reporter', 'html'] }); expect(exitCode).toBe(0); @@ -643,7 +643,7 @@ test('multiple output reports', async ({ runInlineTest, mergeReports, showReport const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); - expect(reportFiles).toEqual([expect.stringMatching(/report-1-of-2.*.zip/), 'resources']); + expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), 'resources']); const { exitCode, output } = await mergeReports(reportDir, { 'PW_TEST_HTML_REPORT_OPEN': 'never' }, { additionalArgs: ['--reporter', 'html,line'] }); expect(exitCode).toBe(0); @@ -704,7 +704,7 @@ test('multiple output reports based on config', async ({ runInlineTest, mergeRep const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); - expect(reportFiles).toEqual([expect.stringMatching(/report-1-of-2.*.zip/), expect.stringMatching(/report-2-of-2.*.zip/), 'resources']); + expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), 'resources']); const { exitCode, output } = await mergeReports(reportDir, undefined, { additionalArgs: ['--config', test.info().outputPath('merged/playwright.config.ts')] }); expect(exitCode).toBe(0); @@ -850,7 +850,7 @@ test('preserve config fields', async ({ runInlineTest, mergeReports }) => { const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); - expect(reportFiles).toEqual([expect.stringMatching(/report-1-of-3.*.zip/), expect.stringMatching(/report-3-of-3.*.zip/), 'resources']); + expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), 'resources']); const { exitCode } = await mergeReports(reportDir, {}, { additionalArgs: ['--reporter', test.info().outputPath('echo-reporter.js'), '-c', test.info().outputPath('merge.config.ts')] }); expect(exitCode).toBe(0); const json = JSON.parse(fs.readFileSync(test.info().outputPath('config.json')).toString());