From d92fe16b76272afb19e7af5a2496f7efce45441d Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 28 Jul 2023 15:49:31 -0700 Subject: [PATCH] fix(blob report): default location relative to package.json (#24481) Also: - remove `blob-report` directory at the start; - markdown's `report.md` next to package.json; - use default location in playwright's workflows. References #24451. --- .github/actions/upload-blob-report/action.yml | 2 +- .github/workflows/create_test_report.yml | 2 +- .github/workflows/tests_primary.yml | 6 +-- .github/workflows/tests_secondary.yml | 8 ++-- .github/workflows/tests_service.yml | 4 +- .gitignore | 1 + .../playwright-test/src/reporters/blob.ts | 24 +++++------ .../playwright-test/src/reporters/html.ts | 19 ++------- .../playwright-test/src/reporters/markdown.ts | 3 +- packages/playwright-test/src/util.ts | 8 ++++ tests/library/playwright.config.ts | 2 +- .../playwright-test-fixtures.ts | 10 ++++- tests/playwright-test/playwright.config.ts | 2 +- tests/playwright-test/reporter-blob.spec.ts | 40 ++++++++++++++----- 14 files changed, 79 insertions(+), 52 deletions(-) diff --git a/.github/actions/upload-blob-report/action.yml b/.github/actions/upload-blob-report/action.yml index 1747c5f69c..6fd17587db 100644 --- a/.github/actions/upload-blob-report/action.yml +++ b/.github/actions/upload-blob-report/action.yml @@ -5,7 +5,7 @@ inputs: description: 'Directory containing blob report' required: true type: string - default: 'test-results/blob-report' + default: 'blob-report' connection_string: description: 'Azure connection string' required: true diff --git a/.github/workflows/create_test_report.yml b/.github/workflows/create_test_report.yml index e519ac0f7b..84ef24fc91 100644 --- a/.github/workflows/create_test_report.yml +++ b/.github/workflows/create_test_report.yml @@ -31,7 +31,7 @@ jobs: - name: Merge reports run: | - npx playwright merge-reports --reporter markdown,html blob-report + npx playwright merge-reports --reporter markdown,html ./blob-report - name: Upload HTML report to Azure run: | diff --git a/.github/workflows/tests_primary.yml b/.github/workflows/tests_primary.yml index 08d2e9043c..eaecf877a4 100644 --- a/.github/workflows/tests_primary.yml +++ b/.github/workflows/tests_primary.yml @@ -64,7 +64,7 @@ jobs: if: always() && github.event_name == 'pull_request' uses: ./.github/actions/upload-blob-report with: - report_dir: test-results/blob-report + report_dir: blob-report connection_string: '${{ secrets.AZURE_CONNECTION_STRING_FOR_BLOB_REPORT }}' test_linux_chromium_tot: @@ -96,7 +96,7 @@ jobs: if: always() && github.event_name == 'pull_request' uses: ./.github/actions/upload-blob-report with: - report_dir: test-results/blob-report + report_dir: blob-report connection_string: '${{ secrets.AZURE_CONNECTION_STRING_FOR_BLOB_REPORT }}' test_test_runner: @@ -146,7 +146,7 @@ jobs: if: always() && github.event_name == 'pull_request' uses: ./.github/actions/upload-blob-report with: - report_dir: test-results/blob-report + report_dir: blob-report connection_string: '${{ secrets.AZURE_CONNECTION_STRING_FOR_BLOB_REPORT }}' test_web_components: diff --git a/.github/workflows/tests_secondary.yml b/.github/workflows/tests_secondary.yml index 84bb4be9e5..4e5fb66987 100644 --- a/.github/workflows/tests_secondary.yml +++ b/.github/workflows/tests_secondary.yml @@ -51,7 +51,7 @@ jobs: if: always() && github.event_name == 'pull_request' uses: ./.github/actions/upload-blob-report with: - report_dir: test-results/blob-report + report_dir: blob-report connection_string: '${{ secrets.AZURE_CONNECTION_STRING_FOR_BLOB_REPORT }}' test_mac: @@ -83,7 +83,7 @@ jobs: if: always() && github.event_name == 'pull_request' uses: ./.github/actions/upload-blob-report with: - report_dir: test-results/blob-report + report_dir: blob-report connection_string: '${{ secrets.AZURE_CONNECTION_STRING_FOR_BLOB_REPORT }}' test_win: @@ -115,7 +115,7 @@ jobs: if: always() && github.event_name == 'pull_request' uses: ./.github/actions/upload-blob-report with: - report_dir: test-results/blob-report + report_dir: blob-report connection_string: '${{ secrets.AZURE_CONNECTION_STRING_FOR_BLOB_REPORT }}' test-package-installations-other-node-versions: @@ -240,7 +240,7 @@ jobs: if: always() && github.event_name == 'pull_request' uses: ./.github/actions/upload-blob-report with: - report_dir: test-results/blob-report + report_dir: blob-report connection_string: '${{ secrets.AZURE_CONNECTION_STRING_FOR_BLOB_REPORT }}' chrome_stable_linux: diff --git a/.github/workflows/tests_service.yml b/.github/workflows/tests_service.yml index df76791ceb..e7c10f49db 100644 --- a/.github/workflows/tests_service.yml +++ b/.github/workflows/tests_service.yml @@ -34,13 +34,13 @@ jobs: - name: Zip blob report if: always() run: zip -r ../blob-report-${{ matrix.browser }}-${{ matrix.service-os }}.zip . - working-directory: test-results/blob-report + working-directory: blob-report - name: Upload blob report to GitHub if: always() uses: actions/upload-artifact@v3 with: name: blob-report-${{ github.run_attempt }} - path: test-results/blob-report-${{ matrix.browser }}-${{ matrix.service-os }}.zip + path: blob-report-${{ matrix.browser }}-${{ matrix.service-os }}.zip retention-days: 2 merge_reports: diff --git a/.gitignore b/.gitignore index 3d26132809..1866dfff9c 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,7 @@ nohup.out .trace .tmp allure* +blob-report playwright-report test-results /demo/ diff --git a/packages/playwright-test/src/reporters/blob.ts b/packages/playwright-test/src/reporters/blob.ts index 5b91e1beca..2105b806e7 100644 --- a/packages/playwright-test/src/reporters/blob.ts +++ b/packages/playwright-test/src/reporters/blob.ts @@ -16,15 +16,15 @@ import fs from 'fs'; import path from 'path'; -import { ManualPromise, calculateSha1, createGuid } from 'playwright-core/lib/utils'; +import { ManualPromise, calculateSha1, createGuid, removeFolders } from 'playwright-core/lib/utils'; import { mime } from 'playwright-core/lib/utilsBundle'; import { Readable } from 'stream'; import type { EventEmitter } from 'events'; import type { FullConfig, FullResult, TestResult } from '../../types/testReporter'; -import type { Suite } from '../common/test'; import type { JsonAttachment, JsonEvent } from '../isomorphic/teleReceiver'; import { TeleReporterEmitter } from './teleEmitter'; import { yazl } from 'playwright-core/lib/zipBundle'; +import { resolveReporterOutputPath } from '../util'; type BlobReporterOptions = { configDir: string; @@ -42,7 +42,7 @@ export class BlobReporter extends TeleReporterEmitter { private _salt: string; private _copyFilePromises = new Set>(); - private _outputDir!: string; + private _outputDir!: Promise; private _reportName!: string; constructor(options: BlobReporterOptions) { @@ -52,7 +52,9 @@ export class BlobReporter extends TeleReporterEmitter { } override onConfigure(config: FullConfig) { - this._outputDir = path.resolve(this._options.configDir, this._options.outputDir || 'blob-report'); + const outputDir = resolveReporterOutputPath('blob-report', this._options.configDir, this._options.outputDir); + const removePromise = process.env.PWTEST_BLOB_DO_NOT_REMOVE ? Promise.resolve() : removeFolders([outputDir]); + this._outputDir = removePromise.then(() => fs.promises.mkdir(path.join(outputDir, 'resources'), { recursive: true })).then(() => outputDir); this._reportName = `report-${createGuid()}`; const metadata: BlobReportMetadata = { projectSuffix: process.env.PWTEST_BLOB_SUFFIX, @@ -65,21 +67,16 @@ export class BlobReporter extends TeleReporterEmitter { super.onConfigure(config); } - override onBegin(suite: Suite): void { - // Note: config.outputDir is cleared betwee onConfigure and onBegin, so we call mkdir here. - fs.mkdirSync(path.join(this._outputDir, 'resources'), { recursive: true }); - super.onBegin(suite); - } - override async onEnd(result: FullResult): Promise { await super.onEnd(result); + const outputDir = await this._outputDir; const lines = this._messages.map(m => JSON.stringify(m) + '\n'); const content = Readable.from(lines); const zipFile = new yazl.ZipFile(); const zipFinishPromise = new ManualPromise(); (zipFile as any as EventEmitter).on('error', error => zipFinishPromise.reject(error)); - const zipFileName = path.join(this._outputDir, this._reportName + '.zip'); + const zipFileName = path.join(outputDir, this._reportName + '.zip'); zipFile.outputStream.pipe(fs.createWriteStream(zipFileName)).on('close', () => { zipFinishPromise.resolve(undefined); }).on('error', error => zipFinishPromise.reject(error)); @@ -103,7 +100,7 @@ export class BlobReporter extends TeleReporterEmitter { const sha1 = calculateSha1(attachment.path + this._salt); const extension = mime.getExtension(attachment.contentType) || 'dat'; const newPath = `resources/${sha1}.${extension}`; - this._startCopyingFile(attachment.path, path.join(this._outputDir, newPath)); + this._startCopyingFile(attachment.path, newPath); return { ...attachment, path: newPath, @@ -112,7 +109,8 @@ export class BlobReporter extends TeleReporterEmitter { } private _startCopyingFile(from: string, to: string) { - const copyPromise: Promise = fs.promises.copyFile(from, to) + const copyPromise: Promise = this._outputDir + .then(dir => fs.promises.copyFile(from, path.join(dir, to))) .catch(e => { console.error(`Failed to copy file from "${from}" to "${to}": ${e}`); }) .then(() => { this._copyFilePromises.delete(copyPromise); }); this._copyFilePromises.add(copyPromise); diff --git a/packages/playwright-test/src/reporters/html.ts b/packages/playwright-test/src/reporters/html.ts index 07705543fc..f40cabc8fa 100644 --- a/packages/playwright-test/src/reporters/html.ts +++ b/packages/playwright-test/src/reporters/html.ts @@ -24,7 +24,7 @@ import { HttpServer, assert, calculateSha1, copyFileAndMakeWritable, gracefullyP import type { JsonAttachment, JsonReport, JsonSuite, JsonTestCase, JsonTestResult, JsonTestStep } from './raw'; import RawReporter from './raw'; import { stripAnsiEscapes } from './base'; -import { getPackageJsonPath, sanitizeForFilePath } from '../util'; +import { resolveReporterOutputPath, sanitizeForFilePath } from '../util'; import type { Metadata } from '../../types/test'; import type { ZipFile } from 'playwright-core/lib/zipBundle'; import { yazl } from 'playwright-core/lib/zipBundle'; @@ -95,11 +95,9 @@ class HtmlReporter extends EmptyReporter { } _resolveOptions(): { outputFolder: string, open: HtmlReportOpenOption, attachmentsBaseURL: string } { - let { outputFolder } = this._options; - if (outputFolder) - outputFolder = path.resolve(this._options.configDir, outputFolder); + const outputFolder = reportFolderFromEnv() ?? resolveReporterOutputPath('playwright-report', this._options.configDir, this._options.outputFolder); return { - outputFolder: reportFolderFromEnv() ?? outputFolder ?? defaultReportFolder(this._options.configDir), + outputFolder, open: process.env.PW_TEST_HTML_REPORT_OPEN as any || this._options.open || 'on-failure', attachmentsBaseURL: this._options.attachmentsBaseURL || 'data/' }; @@ -142,17 +140,8 @@ function reportFolderFromEnv(): string | undefined { return undefined; } -function defaultReportFolder(searchForPackageJson: string): string { - let basePath = getPackageJsonPath(searchForPackageJson); - if (basePath) - basePath = path.dirname(basePath); - else - basePath = process.cwd(); - return path.resolve(basePath, 'playwright-report'); -} - function standaloneDefaultFolder(): string { - return reportFolderFromEnv() ?? defaultReportFolder(process.cwd()); + return reportFolderFromEnv() ?? resolveReporterOutputPath('playwright-report', process.cwd(), undefined); } export async function showHTMLReport(reportFolder: string | undefined, host: string = 'localhost', port?: number, testId?: string) { diff --git a/packages/playwright-test/src/reporters/markdown.ts b/packages/playwright-test/src/reporters/markdown.ts index 114026f864..a3db49fc19 100644 --- a/packages/playwright-test/src/reporters/markdown.ts +++ b/packages/playwright-test/src/reporters/markdown.ts @@ -18,6 +18,7 @@ import fs from 'fs'; import path from 'path'; import type { FullResult, TestCase, TestError } from '../../types/testReporter'; import { BaseReporter, formatError, formatTestTitle, stripAnsiEscapes } from './base'; +import { resolveReporterOutputPath } from '../util'; type MarkdownReporterOptions = { configDir: string, @@ -70,7 +71,7 @@ class MarkdownReporter extends BaseReporter { lines.push(``); } - const reportFile = path.resolve(this._options.configDir, this._options.outputFile || 'report.md'); + const reportFile = resolveReporterOutputPath('report.md', this._options.configDir, this._options.outputFile); await fs.promises.mkdir(path.dirname(reportFile), { recursive: true }); await fs.promises.writeFile(reportFile, lines.join('\n')); } diff --git a/packages/playwright-test/src/util.ts b/packages/playwright-test/src/util.ts index e67ca4719d..77db4a7514 100644 --- a/packages/playwright-test/src/util.ts +++ b/packages/playwright-test/src/util.ts @@ -261,6 +261,14 @@ export function getPackageJsonPath(folderPath: string): string { return result; } +export function resolveReporterOutputPath(defaultValue: string, configDir: string, configValue: string | undefined) { + if (configValue) + return path.resolve(configDir, configValue); + let basePath = getPackageJsonPath(configDir); + basePath = basePath ? path.dirname(basePath) : process.cwd(); + return path.resolve(basePath, defaultValue); +} + export async function normalizeAndSaveAttachment(outputPath: string, name: string, options: { path?: string, body?: string | Buffer, contentType?: string } = {}): Promise<{ name: string; path?: string | undefined; body?: Buffer | undefined; contentType: string; }> { if ((options.path !== undefined ? 1 : 0) + (options.body !== undefined ? 1 : 0) !== 1) throw new Error(`Exactly one of "path" and "body" must be specified`); diff --git a/tests/library/playwright.config.ts b/tests/library/playwright.config.ts index 64fb01ccc0..ef2cd5e295 100644 --- a/tests/library/playwright.config.ts +++ b/tests/library/playwright.config.ts @@ -46,7 +46,7 @@ const reporters = () => { const result: ReporterDescription[] = process.env.CI ? [ ['dot'], ['json', { outputFile: path.join(outputDir, 'report.json') }], - ['blob', { outputDir: path.join(outputDir, 'blob-report') }], + ['blob'], ] : [ ['html', { open: 'on-failure' }] ]; diff --git a/tests/playwright-test/playwright-test-fixtures.ts b/tests/playwright-test/playwright-test-fixtures.ts index aaaaec9a0b..ace24a1b5b 100644 --- a/tests/playwright-test/playwright-test-fixtures.ts +++ b/tests/playwright-test/playwright-test-fixtures.ts @@ -93,6 +93,12 @@ const configFile = (baseDir: string, files: Files): string | undefined => { return undefined; }; +function findPackageJSONDir(files: Files, dir: string) { + while (dir && !files[dir + '/package.json']) + dir = path.dirname(dir); + return dir; +} + function startPlaywrightTest(childProcess: CommonFixtures['childProcess'], baseDir: string, params: any, env: NodeJS.ProcessEnv, options: RunOptions): TestChildProcess { const paramList: string[] = []; for (const key of Object.keys(params)) { @@ -138,7 +144,9 @@ async function runPlaywrightTest(childProcess: CommonFixtures['childProcess'], b if (config) additionalArgs.push('--config', config); const cwd = options.cwd ? path.resolve(baseDir, options.cwd) : baseDir; - const mergeResult = await mergeReports('blob-report', env, { cwd, additionalArgs }); + const packageRoot = path.resolve(baseDir, findPackageJSONDir(files, options.cwd ?? '')); + const relativeBlobReportPath = path.relative(cwd, path.join(packageRoot, 'blob-report')); + const mergeResult = await mergeReports(relativeBlobReportPath, env, { cwd, additionalArgs }); expect(mergeResult.exitCode).toBe(0); output = mergeResult.output; } diff --git a/tests/playwright-test/playwright.config.ts b/tests/playwright-test/playwright.config.ts index 3a8431228f..02a87efa10 100644 --- a/tests/playwright-test/playwright.config.ts +++ b/tests/playwright-test/playwright.config.ts @@ -25,7 +25,7 @@ const reporters = () => { const result: ReporterDescription[] = process.env.CI ? [ ['dot'], ['json', { outputFile: path.join(outputDir, 'report.json') }], - ['blob', { outputDir: path.join(outputDir, 'blob-report') }], + ['blob', { outputDir: path.join(__dirname, '..', '..', 'blob-report') }], ] : [ ['list'] ]; diff --git a/tests/playwright-test/reporter-blob.spec.ts b/tests/playwright-test/reporter-blob.spec.ts index 8f0078bbd0..f3452c61ce 100644 --- a/tests/playwright-test/reporter-blob.spec.ts +++ b/tests/playwright-test/reporter-blob.spec.ts @@ -127,7 +127,7 @@ test('should call methods in right order', async ({ runInlineTest, mergeReports ` }; await runInlineTest(files, { shard: `1/3` }); - await runInlineTest(files, { shard: `3/3` }); + await runInlineTest(files, { shard: `3/3` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' }); const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), 'resources']); @@ -201,7 +201,7 @@ test('should merge into html with dependencies', async ({ runInlineTest, mergeRe }; const totalShards = 3; for (let i = 0; i < totalShards; i++) - await runInlineTest(files, { shard: `${i + 1}/${totalShards}` }); + await runInlineTest(files, { shard: `${i + 1}/${totalShards}` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' }); const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), 'resources']); @@ -270,7 +270,7 @@ test('be able to merge incomplete shards', async ({ runInlineTest, mergeReports, ` }; await runInlineTest(files, { shard: `1/3` }); - await runInlineTest(files, { shard: `3/3` }); + await runInlineTest(files, { shard: `3/3` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' }); const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); @@ -312,7 +312,7 @@ test('total time is from test run not from merge', async ({ runInlineTest, merge `, }; await runInlineTest(files, { shard: `1/2` }); - await runInlineTest(files, { shard: `2/2` }); + await runInlineTest(files, { shard: `2/2` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' }); const { exitCode, output } = await mergeReports(reportDir, { 'PW_TEST_HTML_REPORT_OPEN': 'never' }, { additionalArgs: ['--reporter', 'html'] }); expect(exitCode).toBe(0); @@ -377,7 +377,7 @@ test('merge into list report by default', async ({ runInlineTest, mergeReports } const totalShards = 3; for (let i = 0; i < totalShards; i++) - await runInlineTest(files, { shard: `${i + 1}/${totalShards}` }); + await runInlineTest(files, { shard: `${i + 1}/${totalShards}` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' }); const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); expect(reportFiles).toEqual([expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), expect.stringMatching(/report-.*.zip/), 'resources']); @@ -584,7 +584,7 @@ test('resource names should not clash between runs', async ({ runInlineTest, sho ` }; await runInlineTest(files, { shard: `1/2` }); - await runInlineTest(files, { shard: `2/2` }); + await runInlineTest(files, { shard: `2/2` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' }); const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); @@ -720,7 +720,7 @@ test('multiple output reports based on config', async ({ runInlineTest, mergeRep ` }; await runInlineTest(files, { shard: `1/2` }); - await runInlineTest(files, { shard: `2/2` }); + await runInlineTest(files, { shard: `2/2` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' }); const reportFiles = await fs.promises.readdir(reportDir); reportFiles.sort(); @@ -857,7 +857,7 @@ test('preserve config fields', async ({ runInlineTest, mergeReports }) => { }; await runInlineTest(files, { shard: `1/3`, workers: 1 }); - await runInlineTest(files, { shard: `3/3`, workers: 1 }); + await runInlineTest(files, { shard: `3/3`, workers: 1 }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' }); const mergeConfig = { reportSlowTests: { @@ -1122,7 +1122,7 @@ test('same project different suffixes', async ({ runInlineTest, mergeReports }) }; await runInlineTest(files, undefined, { PWTEST_BLOB_SUFFIX: '-first' }); - await runInlineTest(files, undefined, { PWTEST_BLOB_SUFFIX: '-second' }); + await runInlineTest(files, undefined, { PWTEST_BLOB_SUFFIX: '-second', PWTEST_BLOB_DO_NOT_REMOVE: '1' }); const reportDir = test.info().outputPath('blob-report'); const { exitCode, output } = await mergeReports(reportDir, {}, { additionalArgs: ['--reporter', test.info().outputPath('echo-reporter.js')] }); @@ -1137,3 +1137,25 @@ test('no reports error', async ({ runInlineTest, mergeReports }) => { expect(exitCode).toBe(1); expect(output).toContain(`No report files found in`); }); + +test('blob-report should be next to package.json', async ({ runInlineTest }, testInfo) => { + const result = await runInlineTest({ + 'foo/package.json': `{ "name": "foo" }`, + // unused config along "search path" + 'foo/bar/playwright.config.js': ` + module.exports = { projects: [ {} ] }; + `, + 'foo/bar/baz/tests/a.spec.js': ` + import { test, expect } from '@playwright/test'; + const fs = require('fs'); + test('pass', ({}, testInfo) => { + }); + `, + }, { reporter: 'blob' }, {}, { cwd: 'foo/bar/baz/tests' }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(1); + expect(fs.existsSync(testInfo.outputPath('blob-report'))).toBe(false); + expect(fs.existsSync(testInfo.outputPath('foo', 'blob-report'))).toBe(true); + expect(fs.existsSync(testInfo.outputPath('foo', 'bar', 'blob-report'))).toBe(false); + expect(fs.existsSync(testInfo.outputPath('foo', 'bar', 'baz', 'tests', 'blob-report'))).toBe(false); +});