From 4c4525c9e02707f08a4a195b5bb7acd3f766deb5 Mon Sep 17 00:00:00 2001
From: Max Schmitt <max@schmitt.mx>
Date: Wed, 16 Aug 2023 18:06:04 +0200
Subject: [PATCH] chore: make html report produce named attachments (#26421)

https://github.com/microsoft/playwright/issues/26326
---
 packages/html-reporter/src/links.tsx        | 11 +++++-
 tests/playwright-test/reporter-blob.spec.ts | 40 ++++++++++++---------
 tests/playwright-test/reporter-html.spec.ts | 34 ++++++++++++++++--
 3 files changed, 65 insertions(+), 20 deletions(-)

diff --git a/packages/html-reporter/src/links.tsx b/packages/html-reporter/src/links.tsx
index 03eae930a6..2aa835d7d6 100644
--- a/packages/html-reporter/src/links.tsx
+++ b/packages/html-reporter/src/links.tsx
@@ -68,13 +68,22 @@ export const AttachmentLink: React.FunctionComponent<{
 }> = ({ attachment, href, linkName }) => {
   return <TreeItem title={<span>
     {attachment.contentType === kMissingContentType ? icons.warning() : icons.attachment()}
-    {attachment.path && <a href={href || attachment.path} target='_blank'>{linkName || attachment.name}</a>}
+    {attachment.path && <a href={href || attachment.path} download={downloadFileNameForAttachment(attachment)}>{linkName || attachment.name}</a>}
     {attachment.body && <span>{attachment.name}</span>}
   </span>} loadChildren={attachment.body ? () => {
     return [<div className='attachment-body'>{attachment.body}</div>];
   } : undefined} depth={0} style={{ lineHeight: '32px' }}></TreeItem>;
 };
 
+function downloadFileNameForAttachment(attachment: TestAttachment): string {
+  if (attachment.name.includes('.') || !attachment.path)
+    return attachment.name;
+  const firstDotIndex = attachment.path.indexOf('.');
+  if (firstDotIndex === -1)
+    return attachment.name;
+  return attachment.name + attachment.path.slice(firstDotIndex, attachment.path.length);
+}
+
 export function generateTraceUrl(traces: TestAttachment[]) {
   return `trace/index.html?${traces.map((a, i) => `trace=${new URL(a.path!, window.location.href)}`).join('&')}`;
 }
diff --git a/tests/playwright-test/reporter-blob.spec.ts b/tests/playwright-test/reporter-blob.spec.ts
index 661eba7324..af138a6902 100644
--- a/tests/playwright-test/reporter-blob.spec.ts
+++ b/tests/playwright-test/reporter-blob.spec.ts
@@ -577,13 +577,13 @@ test('preserve attachments', async ({ runInlineTest, mergeReports, showReport, p
   await showReport();
 
   await page.getByText('first').click();
-  const popupPromise = page.waitForEvent('popup');
+  const downloadPromise = page.waitForEvent('download');
   // Check file attachment.
   await page.getByRole('link', { name: 'file-attachment' }).click();
-  const popup = await popupPromise;
+  const download = await downloadPromise;
   // Check file attachment content.
-  await expect(popup.locator('body')).toHaveText('hello!');
-  await popup.close();
+  expect(await readAllFromStreamAsString(await download.createReadStream())).toEqual('hello!');
+
   await page.goBack();
 
   await page.getByText('failing 1').click();
@@ -651,13 +651,13 @@ test('generate html with attachment urls', async ({ runInlineTest, mergeReports,
   await page.goto(`${server.PREFIX}/index.html`);
   await page.getByText('first').click();
 
-  const popupPromise = page.waitForEvent('popup');
+  const downloadPromise = page.waitForEvent('download');
   // Check file attachment.
   await page.getByRole('link', { name: 'file-attachment' }).click();
-  const popup = await popupPromise;
+  const download = await downloadPromise;
   // Check file attachment content.
-  await expect(popup.locator('body')).toHaveText('hello!');
-  await popup.close();
+  expect(await readAllFromStreamAsString(await download.createReadStream())).toEqual('hello!');
+
   await page.goBack();
 
   // Check inline attachment.
@@ -720,11 +720,10 @@ test('resource names should not clash between runs', async ({ runInlineTest, sho
     await page.getByText('first').click();
     await expect(fileAttachment).toBeVisible();
 
-    const popupPromise = page.waitForEvent('popup');
+    const downloadPromise = page.waitForEvent('download');
     await fileAttachment.click();
-    const popup = await popupPromise;
-    await expect(popup.locator('body')).toHaveText('hello!');
-    await popup.close();
+    const download = await downloadPromise;
+    expect(await readAllFromStreamAsString(await download.createReadStream())).toEqual('hello!');
     await page.goBack();
   }
 
@@ -733,11 +732,10 @@ test('resource names should not clash between runs', async ({ runInlineTest, sho
     await page.getByText('failing 2').click();
     await expect(fileAttachment).toBeVisible();
 
-    const popupPromise = page.waitForEvent('popup');
+    const downloadPromise = page.waitForEvent('download');
     await fileAttachment.click();
-    const popup = await popupPromise;
-    await expect(popup.locator('body')).toHaveText('bye!');
-    await popup.close();
+    const download = await downloadPromise;
+    expect(await readAllFromStreamAsString(await download.createReadStream())).toEqual('bye!');
     await page.goBack();
   }
 });
@@ -1371,4 +1369,12 @@ test('should merge blob reports with same name', async ({ runInlineTest, mergeRe
   await expect(page.locator('.subnav-item:has-text("Failed") .counter')).toHaveText('4');
   await expect(page.locator('.subnav-item:has-text("Flaky") .counter')).toHaveText('2');
   await expect(page.locator('.subnav-item:has-text("Skipped") .counter')).toHaveText('4');
-});
\ No newline at end of file
+});
+
+function readAllFromStreamAsString(stream: NodeJS.ReadableStream): Promise<string> {
+  return new Promise(resolve => {
+    const chunks: Buffer[] = [];
+    stream.on('data', chunk => chunks.push(chunk));
+    stream.on('end', () => resolve(Buffer.concat(chunks).toString('utf8')));
+  });
+}
diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts
index ac7cbf2fe0..23ba5bf395 100644
--- a/tests/playwright-test/reporter-html.spec.ts
+++ b/tests/playwright-test/reporter-html.spec.ts
@@ -684,7 +684,7 @@ for (const useIntermediateMergeReport of [false, true] as const) {
       await expect(page.locator('.attachment-body')).toHaveText(['foo', '{"foo":1}', 'utf16 encoded']);
     });
 
-    test('should use file-browser friendly extensions for buffer attachments based on contentType', async ({ runInlineTest }, testInfo) => {
+    test('should use file-browser friendly extensions for buffer attachments based on contentType', async ({ runInlineTest, showReport, page }, testInfo) => {
       const result = await runInlineTest({
         'a.test.js': `
           import { test, expect } from '@playwright/test';
@@ -700,6 +700,28 @@ for (const useIntermediateMergeReport of [false, true] as const) {
         `,
       }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' });
       expect(result.exitCode).toBe(0);
+      await showReport();
+      await page.getByRole('link', { name: 'passing' }).click();
+
+      const expectedAttachments = [
+        ['screenshot', 'screenshot.png', 'f6aa9785bc9c7b8fd40c3f6ede6f59112a939527.png'],
+        ['some-pdf', 'some-pdf.pdf', '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33.pdf'],
+        ['madeup-contentType', 'madeup-contentType.dat', '62cdb7020ff920e5aa642c3d4066950dd1f01f4d.dat'],
+        ['screenshot-that-already-has-an-extension-with-madeup.png', 'screenshot-that-already-has-an-extension-with-madeup.png', '86f7e437faa5a7fce15d1ddcb9eaeaea377667b8.png'],
+        ['screenshot-that-already-has-an-extension-with-correct-contentType.png', 'screenshot-that-already-has-an-extension-with-correct-contentType.png', '84a516841ba77a5b4648de2cd0dfcb30ea46dbb4.png'],
+        ['example.ext with spaces', 'example.ext with spaces', 'e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98.ext-with-spaces'],
+      ];
+
+      for (const [visibleAttachmentName, downloadFileName, sha1] of expectedAttachments) {
+        await test.step(`should download ${visibleAttachmentName}`, async () => {
+          const downloadPromise = page.waitForEvent('download');
+          await page.getByRole('link', { name: visibleAttachmentName, exact: true }).click();
+          const download = await downloadPromise;
+          expect(download.suggestedFilename()).toBe(downloadFileName);
+          expect(await readAllFromStream(await download.createReadStream())).toEqual(await fs.promises.readFile(path.join(testInfo.outputPath('playwright-report'), 'data', sha1)));
+        });
+      }
+
       const files = await fs.promises.readdir(path.join(testInfo.outputPath('playwright-report'), 'data'));
       expect(new Set(files)).toEqual(new Set([
         'f6aa9785bc9c7b8fd40c3f6ede6f59112a939527.png', // screenshot
@@ -2025,4 +2047,12 @@ for (const useIntermediateMergeReport of [false, true] as const) {
       await expect(page.getByText('passes title')).toBeVisible();
     });
   });
-}
\ No newline at end of file
+}
+
+function readAllFromStream(stream: NodeJS.ReadableStream): Promise<Buffer> {
+  return new Promise(resolve => {
+    const chunks: Buffer[] = [];
+    stream.on('data', chunk => chunks.push(chunk));
+    stream.on('end', () => resolve(Buffer.concat(chunks)));
+  });
+}