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

@@ -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

.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: |

.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:

.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:

.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:

.gitignore

@@ -18,6 +18,7 @@ nohup.out
 .trace
 .tmp
 allure*
+blob-report
 playwright-report
 test-results
 /demo/

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<Promise<void>>();
 
-  private _outputDir!: string;
+  private _outputDir!: Promise<string>;
   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<void> {
     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<undefined>();
     (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<void> = fs.promises.copyFile(from, to)
+    const copyPromise: Promise<void> = 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);

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) {

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(`</details>`);
     }
 
-    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'));
   }

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`);

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' }]
   ];

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;
   }

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']
   ];

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);
+});