chore: move attachments link to step body (#34196)

microsoft · Jan 13, 2025 · a33659f · a33659f
1 parent 19c935c
commit a33659f
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 10 deletions.
diff --git a/packages/html-reporter/src/links.css b/packages/html-reporter/src/links.css
@@ -60,6 +60,11 @@
     color: var(--color-scale-orange-6);
     border: 1px solid var(--color-scale-orange-4);
   }
+  .label-color-gray {
+    background-color: var(--color-scale-gray-0);
+    color: var(--color-scale-gray-6);
+    border: 1px solid var(--color-scale-gray-4);
+  }
 }
 
 @media(prefers-color-scheme: dark) {
@@ -93,6 +98,11 @@
     color: var(--color-scale-orange-2);
     border: 1px solid var(--color-scale-orange-4);
   }
+  .label-color-gray {
+    background-color: var(--color-scale-gray-9);
+    color: var(--color-scale-gray-2);
+    border: 1px solid var(--color-scale-gray-4);
+  }
 }
 
 .attachment-body {

diff --git a/packages/html-reporter/src/testResultView.tsx b/packages/html-reporter/src/testResultView.tsx
@@ -134,7 +134,7 @@ export const TestResultView: React.FC<{
       </div>)}
     </AutoChip></Anchor>}
 
-    {!!otherAttachments.size && <AutoChip header='Attachments' revealOnAnchorId={otherAttachmentAnchors}>
+    {!!otherAttachments.size && <AutoChip header='Attachments' revealOnAnchorId={otherAttachmentAnchors} dataTestId='attachments'>
       {[...otherAttachments].map((a, i) =>
         <Anchor key={`attachment-link-${i}`} id={`attachment-${result.attachments.indexOf(a)}`}>
           <AttachmentLink attachment={a} result={result} openInNewTab={a.contentType.startsWith('text/html')} />
@@ -176,15 +176,27 @@ const StepTreeItem: React.FC<{
 }> = ({ test, step, result, depth }) => {
   return <TreeItem title={<span aria-label={step.title}>
     <span style={{ float: 'right' }}>{msToString(step.duration)}</span>
-    {step.attachments.length > 0 && <a style={{ float: 'right' }} title='link to attachment' href={testResultHref({ test, result, anchor: `attachment-${step.attachments[0]}` })} onClick={evt => { evt.stopPropagation(); }}>{icons.attachment()}</a>}
     {statusIcon(step.error || step.duration === -1 ? 'failed' : 'passed')}
     <span>{step.title}</span>
     {step.count > 1 && <> ✕ <span className='test-result-counter'>{step.count}</span></>}
     {step.location && <span className='test-result-path'>— {step.location.file}:{step.location.line}</span>}
   </span>} loadChildren={step.steps.length + (step.snippet ? 1 : 0) ? () => {
-    const children = step.steps.map((s, i) => <StepTreeItem key={i} step={s} depth={depth + 1} result={result} test={test} />);
-    if (step.snippet)
-      children.unshift(<TestErrorView testId='test-snippet' key='line' error={step.snippet}/>);
-    return children;
+    const snippet = step.snippet ? [<TestErrorView testId='test-snippet' key='line' error={step.snippet}/>] : [];
+    const steps = step.steps.map((s, i) => <StepTreeItem key={i} step={s} depth={depth + 1} result={result} test={test} />);
+    const attachments = step.attachments.map(attachmentIndex => (
+      <a key={'' + attachmentIndex}
+        href={testResultHref({ test, result, anchor: `attachment-${attachmentIndex}` })}
+        style={{ paddingLeft: depth * 22 + 4, textDecoration: 'none' }}
+      >
+        <span
+          style={{ margin: '8px 0 0 8px', padding: '2px 10px', cursor: 'pointer' }}
+          className='label label-color-gray'
+          title={`see "${result.attachments[attachmentIndex].name}"`}
+        >
+          {icons.attachment()}{result.attachments[attachmentIndex].name}
+        </span>
+      </a>
+    ));
+    return snippet.concat(steps, attachments);
   } : undefined} depth={depth}/>;
 };
diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts
@@ -932,9 +932,10 @@ for (const useIntermediateMergeReport of [true, false] as const) {
       await showReport();
       await page.getByRole('link', { name: 'passing' }).click();
 
-      const attachment = page.getByText('foo-2', { exact: true });
+      const attachment = page.getByTestId('attachments').getByText('foo-2', { exact: true });
       await expect(attachment).not.toBeInViewport();
-      await page.getByLabel('attach "foo-2"').getByTitle('link to attachment').click();
+      await page.getByLabel('attach "foo-2"').click();
+      await page.getByTitle('see "foo-2"').click();
       await expect(attachment).toBeInViewport();
 
       await page.reload();
@@ -961,9 +962,10 @@ for (const useIntermediateMergeReport of [true, false] as const) {
       await showReport();
       await page.getByRole('link', { name: 'passing' }).click();
 
-      const attachment = page.getByText('attachment', { exact: true });
+      const attachment = page.getByTestId('attachments').getByText('attachment', { exact: true });
       await expect(attachment).not.toBeInViewport();
-      await page.getByLabel('step').getByTitle('link to attachment').click();
+      await page.getByLabel('step').click();
+      await page.getByTitle('see "attachment"').click();
       await expect(attachment).toBeInViewport();
     });