fix(blob): properly communicate attachments through blob merges (#35517)

This commit is contained in:
Adam Gastineau 2025-04-16 13:03:45 -07:00 committed by GitHub
parent 37a9535a27
commit 039e87f5cb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 160 additions and 10 deletions

View File

@ -87,14 +87,15 @@ export type JsonTestResultStart = {
startTime: number;
};
export type JsonAttachment = Omit<reporterTypes.TestResult['attachments'][0], 'body'> & { base64?: string };
export type JsonAttachment = Omit<reporterTypes.TestResult['attachments'][0], 'body'> & { base64?: string; };
export type JsonTestResultEnd = {
id: string;
duration: number;
status: reporterTypes.TestStatus;
errors: reporterTypes.TestError[];
attachments: JsonAttachment[];
/** No longer emitted, but kept for backwards compatibility */
attachments?: JsonAttachment[];
annotations?: Annotation[];
};
@ -115,6 +116,12 @@ export type JsonTestStepEnd = {
annotations?: Annotation[];
};
export type JsonTestResultOnAttach = {
testId: string;
resultId: string;
attachments: JsonAttachment[];
};
export type JsonFullResult = {
status: reporterTypes.FullResult['status'];
startTime: number;
@ -180,6 +187,10 @@ export class TeleReporterReceiver {
this._onStepBegin(params.testId, params.resultId, params.step);
return;
}
if (method === 'onAttach') {
this._onAttach(params.testId, params.resultId, params.attachments);
return;
}
if (method === 'onStepEnd') {
this._onStepEnd(params.testId, params.resultId, params.step);
return;
@ -241,7 +252,9 @@ export class TeleReporterReceiver {
result.status = payload.status;
result.errors = payload.errors;
result.error = result.errors?.[0];
result.attachments = this._parseAttachments(payload.attachments);
// Attachments are only present here from legacy blobs. These override all _onAttach events
if (!!payload.attachments)
result.attachments = this._parseAttachments(payload.attachments);
if (payload.annotations) {
result.annotations = payload.annotations;
test.annotations = result.annotations;
@ -276,6 +289,17 @@ export class TeleReporterReceiver {
this._reporter.onStepEnd?.(test, result, step);
}
private _onAttach(testId: string, resultId: string, attachments: JsonAttachment[]) {
const test = this._tests.get(testId)!;
const result = test.results.find(r => r._id === resultId)!;
result.attachments.push(...attachments.map(a => ({
name: a.name,
contentType: a.contentType,
path: a.path,
body: a.base64 && (globalThis as any).Buffer ? Buffer.from(a.base64, 'base64') : undefined,
})));
}
private _onError(error: reporterTypes.TestError) {
this._reporter.onError?.(error);
}

View File

@ -30,7 +30,7 @@ import type { BlobReportMetadata } from './blob';
import type { ReporterDescription } from '../../types/test';
import type { TestError } from '../../types/testReporter';
import type { FullConfigInternal } from '../common/config';
import type { JsonConfig, JsonEvent, JsonFullResult, JsonLocation, JsonProject, JsonSuite, JsonTestCase, JsonTestResultEnd, JsonTestStepEnd, JsonTestStepStart } from '../isomorphic/teleReceiver';
import type { JsonAttachment, JsonConfig, JsonEvent, JsonFullResult, JsonLocation, JsonProject, JsonSuite, JsonTestCase, JsonTestResultEnd, JsonTestResultOnAttach, JsonTestStepEnd, JsonTestStepStart } from '../isomorphic/teleReceiver';
import type * as blobV1 from './versions/blobV1';
type StatusCallback = (message: string) => void;
@ -390,6 +390,7 @@ class IdsPatcher {
case 'onProject':
this._onProject(params.project);
return;
case 'onAttach':
case 'onTestBegin':
case 'onStepBegin':
case 'onStepEnd':
@ -447,9 +448,14 @@ class AttachmentPathPatcher {
}
patchEvent(event: JsonEvent) {
if (event.method !== 'onTestEnd')
return;
for (const attachment of (event.params.result as JsonTestResultEnd).attachments) {
if (event.method === 'onAttach')
this._patchAttachments((event.params as JsonTestResultOnAttach).attachments);
else if (event.method === 'onTestEnd')
this._patchAttachments((event.params as JsonTestResultEnd).attachments ?? []);
}
private _patchAttachments(attachments: JsonAttachment[]) {
for (const attachment of attachments) {
if (!attachment.path)
continue;
@ -476,7 +482,7 @@ class PathSeparatorPatcher {
if (jsonEvent.method === 'onTestEnd') {
const testResult = jsonEvent.params.result as JsonTestResultEnd;
testResult.errors.forEach(error => this._updateErrorLocations(error));
testResult.attachments.forEach(attachment => {
(testResult.attachments ?? []).forEach(attachment => {
if (attachment.path)
attachment.path = this._updatePath(attachment.path);
});
@ -492,6 +498,14 @@ class PathSeparatorPatcher {
this._updateErrorLocations(step.error);
return;
}
if (jsonEvent.method === 'onAttach') {
const attach = jsonEvent.params as JsonTestResultOnAttach;
attach.attachments.forEach(attachment => {
if (attachment.path)
attachment.path = this._updatePath(attachment.path);
});
return;
}
}
private _updateProject(project: JsonProject) {

View File

@ -33,6 +33,7 @@ export class TeleReporterEmitter implements ReporterV2 {
private _messageSink: (message: teleReceiver.JsonEvent) => void;
private _rootDir!: string;
private _emitterOptions: TeleReporterEmitterOptions;
private _resultKnownAttachmentCounts = new Map<string, number>();
// In case there is blob reporter and UI mode, make sure one does override
// the id assigned by the other.
private readonly _idSymbol = Symbol('id');
@ -76,6 +77,7 @@ export class TeleReporterEmitter implements ReporterV2 {
timeout: test.timeout,
annotations: []
};
this._sendNewAttachments(result, test.id);
this._messageSink({
method: 'onTestEnd',
params: {
@ -83,6 +85,8 @@ export class TeleReporterEmitter implements ReporterV2 {
result: this._serializeResultEnd(result),
}
});
this._resultKnownAttachmentCounts.delete((result as any)[this._idSymbol]);
}
onStepBegin(test: reporterTypes.TestCase, result: reporterTypes.TestResult, step: reporterTypes.TestStep): void {
@ -98,11 +102,15 @@ export class TeleReporterEmitter implements ReporterV2 {
}
onStepEnd(test: reporterTypes.TestCase, result: reporterTypes.TestResult, step: reporterTypes.TestStep): void {
// Create synthetic onAttach event so we serialize the entire attachment along with the step
const resultId = (result as any)[this._idSymbol] as string;
this._sendNewAttachments(result, test.id);
this._messageSink({
method: 'onStepEnd',
params: {
testId: test.id,
resultId: (result as any)[this._idSymbol],
resultId,
step: this._serializeStepEnd(step, result)
}
});
@ -236,11 +244,28 @@ export class TeleReporterEmitter implements ReporterV2 {
duration: result.duration,
status: result.status,
errors: result.errors,
attachments: this._serializeAttachments(result.attachments),
annotations: result.annotations?.length ? result.annotations : undefined,
};
}
private _sendNewAttachments(result: reporterTypes.TestResult, testId: string) {
const resultId = (result as any)[this._idSymbol] as string;
// Track whether this step (or something else since the last step) has added attachments and send them
const knownAttachmentCount = this._resultKnownAttachmentCounts.get(resultId) ?? 0;
if (result.attachments.length > knownAttachmentCount) {
this._messageSink({
method: 'onAttach',
params: {
testId,
resultId,
attachments: this._serializeAttachments((result.attachments.slice(knownAttachmentCount))),
}
});
}
this._resultKnownAttachmentCounts.set(resultId, result.attachments.length);
}
_serializeAttachments(attachments: reporterTypes.TestResult['attachments']): teleReceiver.JsonAttachment[] {
return attachments.map(a => {
const { body, ...rest } = a;

View File

@ -25,6 +25,7 @@ import extractZip from '../../packages/playwright-core/bundles/zip/node_modules/
import * as yazl from '../../packages/playwright-core/bundles/zip/node_modules/yazl';
import { getUserAgent } from '../../packages/playwright-core/lib/server/utils/userAgent';
import { Readable } from 'stream';
import type { JSONReportTestResult } from '../../packages/playwright-test/reporter';
const DOES_NOT_SUPPORT_UTF8_IN_TERMINAL = process.platform === 'win32' && process.env.TERM_PROGRAM !== 'vscode' && !process.env.WT_SESSION;
const POSITIVE_STATUS_MARK = DOES_NOT_SUPPORT_UTF8_IN_TERMINAL ? 'ok' : '✓ ';
@ -1824,6 +1825,92 @@ test('merge reports without --config preserves path separators', async ({ runInl
expect(output).toContain(`test title: ${'tests2' + otherSeparator + 'b.test.js'}`);
});
test('merge reports should preserve attachments', async ({ runInlineTest, mergeReports, showReport, page }) => {
const reportDir = test.info().outputPath('blob-report-orig');
const files = {
'playwright.config.ts': `
module.exports = {
retries: 1,
reporter: [['blob', { outputDir: '${reportDir.replace(/\\/g, '/')}' }]]
};
`,
'a.test.js': `
import { test, expect } from '@playwright/test';
import * as fs from 'fs';
test('attachment A', async ({}) => {
const attachmentPath = test.info().outputPath('foo.txt');
fs.writeFileSync(attachmentPath, 'hello!');
await test.info().attach('file-attachment1', { path: attachmentPath });
await test.info().attachments.push({ name: 'file-attachment2', path: attachmentPath, contentType: 'text/html' });
await test.info().attach('file-attachment3', { path: attachmentPath });
await test.info().attach('file-attachment4', { path: attachmentPath });
await test.info().attachments.push({ name: 'file-attachment5', path: attachmentPath, contentType: 'text/html' });
await test.info().attachments.push({ name: 'file-attachment6', path: attachmentPath, contentType: 'text/html' });
await test.info().attach('file-attachment7', { path: attachmentPath });
});
`,
'b.test.js': `
import { test, expect } from '@playwright/test';
import * as fs from 'fs';
test('attachment B', async ({}) => {
const attachmentPath = test.info().outputPath('bar.txt');
fs.writeFileSync(attachmentPath, 'goodbye!');
await test.info().attach('file-attachment8', { path: attachmentPath });
await test.info().attachments.push({ name: 'file-attachment9', path: attachmentPath, contentType: 'application/json' });
});
`
};
await runInlineTest(files, { shard: `1/2` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' });
await runInlineTest(files, { shard: `2/2` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' });
const reportFiles = await fs.promises.readdir(reportDir);
reportFiles.sort();
expect(reportFiles).toEqual(['report-1.zip', 'report-2.zip']);
const { exitCode } = await mergeReports(reportDir, { 'PLAYWRIGHT_HTML_OPEN': 'never' }, { additionalArgs: ['--reporter', 'blob,html'] });
expect(exitCode).toBe(0);
const reportZipFile = test.info().outputPath('blob-report', 'report.zip');
const events = await extractReport(reportZipFile, test.info().outputPath('tmp'));
type Attachment = Omit<JSONReportTestResult['attachments'][number], 'path'> & {
path: any
};
const attachment1: Attachment = { name: 'file-attachment1', path: expect.stringContaining(''), contentType: 'text/plain' };
const attachment2: Attachment = { name: 'file-attachment2', path: expect.stringContaining(''), contentType: 'text/html' };
const attachment3: Attachment = { name: 'file-attachment3', path: expect.stringContaining(''), contentType: 'text/plain' };
const attachment4: Attachment = { name: 'file-attachment4', path: expect.stringContaining(''), contentType: 'text/plain' };
const attachment5: Attachment = { name: 'file-attachment5', path: expect.stringContaining(''), contentType: 'text/html' };
const attachment6: Attachment = { name: 'file-attachment6', path: expect.stringContaining(''), contentType: 'text/html' };
const attachment7: Attachment = { name: 'file-attachment7', path: expect.stringContaining(''), contentType: 'text/plain' };
const attachment8: Attachment = { name: 'file-attachment8', path: expect.stringContaining(''), contentType: 'text/plain' };
const attachment9: Attachment = { name: 'file-attachment9', path: expect.stringContaining(''), contentType: 'application/json' };
const aAttachments = [attachment1, attachment2, attachment3, attachment4, attachment5, attachment6, attachment7];
const bAttachments = [attachment8, attachment9];
const allStepAttachments = events.flatMap(e => e.method === 'onStepEnd' ? e?.params?.step?.attachments ?? [] : []);
expect(allStepAttachments).toEqual([0, 2, 3, 6, 0]);
const allTestAttachments = events.flatMap(e => e.method === 'onAttach' ? e?.params?.attachments ?? [] : []);
expect(allTestAttachments).toEqual([...aAttachments, ...bAttachments]);
await showReport();
{
await page.getByRole('link', { name: 'Attachment A' }).click();
for (const attachment of aAttachments)
await expect(page.getByRole('link', { name: attachment.name })).toBeVisible();
await page.goBack();
}
{
await page.getByRole('link', { name: 'Attachment B' }).click();
for (const attachment of bAttachments)
await expect(page.getByRole('link', { name: attachment.name })).toBeVisible();
await page.goBack();
}
});
test('merge reports must not change test ids when there is no need to', async ({ runInlineTest, mergeReports }) => {
const files = {
'echo-test-id-reporter.js': `