From 512cb36c9bb1b454ac861876a673cdaa17a0b8ca Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 16 Dec 2024 15:25:32 +0100 Subject: [PATCH] feat(html): link from attachment step to attachment (#33267) --- packages/html-reporter/src/chip.tsx | 4 +- packages/html-reporter/src/links.tsx | 50 ++++++++++----- packages/html-reporter/src/testCaseView.tsx | 6 +- packages/html-reporter/src/testFileView.tsx | 18 +++--- packages/html-reporter/src/testResultView.tsx | 64 +++++++++++-------- packages/html-reporter/src/treeItem.css | 5 ++ packages/html-reporter/src/treeItem.tsx | 4 +- tests/playwright-test/reporter-html.spec.ts | 24 ++++++- 8 files changed, 117 insertions(+), 58 deletions(-) diff --git a/packages/html-reporter/src/chip.tsx b/packages/html-reporter/src/chip.tsx index f94dcbc6d680f..cdd07777a67f7 100644 --- a/packages/html-reporter/src/chip.tsx +++ b/packages/html-reporter/src/chip.tsx @@ -20,7 +20,7 @@ import './colors.css'; import './common.css'; import * as icons from './icons'; import { clsx } from '@web/uiUtils'; -import { useAnchor } from './links'; +import { type AnchorID, useAnchor } from './links'; export const Chip: React.FC<{ header: JSX.Element | string, @@ -53,7 +53,7 @@ export const AutoChip: React.FC<{ noInsets?: boolean, children?: any, dataTestId?: string, - revealOnAnchorId?: string, + revealOnAnchorId?: AnchorID, }> = ({ header, initialExpanded, noInsets, children, dataTestId, revealOnAnchorId }) => { const [expanded, setExpanded] = React.useState(initialExpanded ?? true); const onReveal = React.useCallback(() => setExpanded(true), []); diff --git a/packages/html-reporter/src/links.tsx b/packages/html-reporter/src/links.tsx index 1e5cad48c1de6..a6ea1e669537d 100644 --- a/packages/html-reporter/src/links.tsx +++ b/packages/html-reporter/src/links.tsx @@ -14,7 +14,7 @@ limitations under the License. */ -import type { TestAttachment } from './types'; +import type { TestAttachment, TestCase, TestCaseSummary, TestResult, TestResultSummary } from './types'; import * as React from 'react'; import * as icons from './icons'; import { TreeItem } from './treeItem'; @@ -72,6 +72,7 @@ export const AttachmentLink: React.FunctionComponent<{ linkName?: string, openInNewTab?: boolean, }> = ({ attachment, href, linkName, openInNewTab }) => { + const isAnchored = useIsAnchored('attachment-' + attachment.name); return {attachment.contentType === kMissingContentType ? icons.warning() : icons.attachment()} {attachment.path && {linkName || attachment.name}} @@ -82,7 +83,7 @@ export const AttachmentLink: React.FunctionComponent<{ )} } loadChildren={attachment.body ? () => { return [
{linkifyText(attachment.body!)}
]; - } : undefined} depth={0} style={{ lineHeight: '32px' }}>; + } : undefined} depth={0} style={{ lineHeight: '32px' }} selected={isAnchored}>; }; export const SearchParamsContext = React.createContext(new URLSearchParams(window.location.hash.slice(1))); @@ -114,23 +115,29 @@ export function generateTraceUrl(traces: TestAttachment[]) { const kMissingContentType = 'x-playwright/missing'; -type AnchorID = string | ((id: string | null) => boolean) | undefined; +export type AnchorID = string | string[] | ((id: string) => boolean) | undefined; export function useAnchor(id: AnchorID, onReveal: () => void) { + const searchParams = React.useContext(SearchParamsContext); + const isAnchored = useIsAnchored(id); React.useEffect(() => { - if (typeof id === 'undefined') - return; - - const listener = () => { - const params = new URLSearchParams(window.location.hash.slice(1)); - const anchor = params.get('anchor'); - const isRevealed = typeof id === 'function' ? id(anchor) : anchor === id; - if (isRevealed) - onReveal(); - }; - window.addEventListener('popstate', listener); - return () => window.removeEventListener('popstate', listener); - }, [id, onReveal]); + if (isAnchored) + onReveal(); + }, [isAnchored, onReveal, searchParams]); +} + +export function useIsAnchored(id: AnchorID) { + const searchParams = React.useContext(SearchParamsContext); + const anchor = searchParams.get('anchor'); + if (anchor === null) + return false; + if (typeof id === 'undefined') + return false; + if (typeof id === 'string') + return id === anchor; + if (Array.isArray(id)) + return id.includes(anchor); + return id(anchor); } export function Anchor({ id, children }: React.PropsWithChildren<{ id: AnchorID }>) { @@ -142,3 +149,14 @@ export function Anchor({ id, children }: React.PropsWithChildren<{ id: AnchorID return
{children}
; } + +export function testResultHref({ test, result, anchor }: { test?: TestCase | TestCaseSummary, result?: TestResult | TestResultSummary, anchor?: string }) { + const params = new URLSearchParams(); + if (test) + params.set('testId', test.testId); + if (test && result) + params.set('run', '' + test.results.indexOf(result as any)); + if (anchor) + params.set('anchor', anchor); + return `#?` + params; +} diff --git a/packages/html-reporter/src/testCaseView.tsx b/packages/html-reporter/src/testCaseView.tsx index 4e9785ad8ae35..e4ffa9c15b9a4 100644 --- a/packages/html-reporter/src/testCaseView.tsx +++ b/packages/html-reporter/src/testCaseView.tsx @@ -19,7 +19,7 @@ import * as React from 'react'; import { TabbedPane } from './tabbedPane'; import { AutoChip } from './chip'; import './common.css'; -import { Link, ProjectLink, SearchParamsContext } from './links'; +import { Link, ProjectLink, SearchParamsContext, testResultHref } from './links'; import { statusIcon } from './statusIcon'; import './testCaseView.css'; import { TestResultView } from './testResultView'; @@ -53,9 +53,9 @@ export const TestCaseView: React.FC<{ {test &&
{test.path.join(' › ')}
-
« previous
+
« previous
-
next »
+
next »
} {test &&
{test?.title}
} {test &&
diff --git a/packages/html-reporter/src/testFileView.tsx b/packages/html-reporter/src/testFileView.tsx index 6b31d2ebe2d53..f8fad1d646de7 100644 --- a/packages/html-reporter/src/testFileView.tsx +++ b/packages/html-reporter/src/testFileView.tsx @@ -19,7 +19,7 @@ import * as React from 'react'; import { hashStringToInt, msToString } from './utils'; import { Chip } from './chip'; import { filterWithToken } from './filter'; -import { generateTraceUrl, Link, navigate, ProjectLink, SearchParamsContext } from './links'; +import { generateTraceUrl, Link, navigate, ProjectLink, SearchParamsContext, testResultHref } from './links'; import { statusIcon } from './statusIcon'; import './testFileView.css'; import { video, image, trace } from './icons'; @@ -48,7 +48,7 @@ export const TestFileView: React.FC - + {[...test.path, test.title].join(' › ')} {projectNames.length > 1 && !!test.projectName && @@ -59,7 +59,7 @@ export const TestFileView: React.FC{msToString(test.duration)}
- + {test.location.file}:{test.location.line} {imageDiffBadge(test)} @@ -72,15 +72,17 @@ export const TestFileView: React.FC result.attachments.some(attachment => { - return attachment.contentType.startsWith('image/') && !!attachment.name.match(/-(expected|actual|diff)/); - })); - return resultWithImageDiff ? {image()} : undefined; + for (const result of test.results) { + for (const attachment of result.attachments) { + if (attachment.contentType.startsWith('image/') && !!attachment.name.match(/-(expected|actual|diff)/)) + return {image()}; + } + } } function videoBadge(test: TestCaseSummary): JSX.Element | undefined { const resultWithVideo = test.results.find(result => result.attachments.some(attachment => attachment.name === 'video')); - return resultWithVideo ? {video()} : undefined; + return resultWithVideo ? {video()} : undefined; } function traceBadge(test: TestCaseSummary): JSX.Element | undefined { diff --git a/packages/html-reporter/src/testResultView.tsx b/packages/html-reporter/src/testResultView.tsx index 9170f2023dceb..410677cb02fa8 100644 --- a/packages/html-reporter/src/testResultView.tsx +++ b/packages/html-reporter/src/testResultView.tsx @@ -20,15 +20,20 @@ import { TreeItem } from './treeItem'; import { msToString } from './utils'; import { AutoChip } from './chip'; import { traceImage } from './images'; -import { Anchor, AttachmentLink, generateTraceUrl } from './links'; +import { Anchor, AttachmentLink, generateTraceUrl, testResultHref } from './links'; import { statusIcon } from './statusIcon'; import type { ImageDiff } from '@web/shared/imageDiffView'; import { ImageDiffView } from '@web/shared/imageDiffView'; import { TestErrorView, TestScreenshotErrorView } from './testErrorView'; +import * as icons from './icons'; import './testResultView.css'; -function groupImageDiffs(screenshots: Set): ImageDiff[] { - const snapshotNameToImageDiff = new Map(); +interface ImageDiffWithAnchors extends ImageDiff { + anchors: string[]; +} + +function groupImageDiffs(screenshots: Set): ImageDiffWithAnchors[] { + const snapshotNameToImageDiff = new Map(); for (const attachment of screenshots) { const match = attachment.name.match(/^(.*)-(expected|actual|diff|previous)(\.[^.]+)?$/); if (!match) @@ -37,9 +42,10 @@ function groupImageDiffs(screenshots: Set): ImageDiff[] { const snapshotName = name + extension; let imageDiff = snapshotNameToImageDiff.get(snapshotName); if (!imageDiff) { - imageDiff = { name: snapshotName }; + imageDiff = { name: snapshotName, anchors: [`attachment-${name}`] }; snapshotNameToImageDiff.set(snapshotName, imageDiff); } + imageDiff.anchors.push(`attachment-${attachment.name}`); if (category === 'actual') imageDiff.actual = { attachment }; if (category === 'expected') @@ -64,18 +70,19 @@ function groupImageDiffs(screenshots: Set): ImageDiff[] { export const TestResultView: React.FC<{ test: TestCase, result: TestResult, -}> = ({ result }) => { - const { screenshots, videos, traces, otherAttachments, diffs, errors, htmls } = React.useMemo(() => { +}> = ({ test, result }) => { + const { screenshots, videos, traces, otherAttachments, diffs, errors, otherAttachmentAnchors, screenshotAnchors } = React.useMemo(() => { const attachments = result?.attachments || []; const screenshots = new Set(attachments.filter(a => a.contentType.startsWith('image/'))); + const screenshotAnchors = [...screenshots].map(a => `attachment-${a.name}`); const videos = attachments.filter(a => a.contentType.startsWith('video/')); const traces = attachments.filter(a => a.name === 'trace'); - const htmls = attachments.filter(a => a.contentType.startsWith('text/html')); const otherAttachments = new Set(attachments); - [...screenshots, ...videos, ...traces, ...htmls].forEach(a => otherAttachments.delete(a)); + [...screenshots, ...videos, ...traces].forEach(a => otherAttachments.delete(a)); + const otherAttachmentAnchors = [...otherAttachments].map(a => `attachment-${a.name}`); const diffs = groupImageDiffs(screenshots); const errors = classifyErrors(result.errors, diffs); - return { screenshots: [...screenshots], videos, traces, otherAttachments, diffs, errors, htmls }; + return { screenshots: [...screenshots], videos, traces, otherAttachments, diffs, errors, otherAttachmentAnchors, screenshotAnchors }; }, [result]); return
@@ -87,29 +94,29 @@ export const TestResultView: React.FC<{ })} } {!!result.steps.length && - {result.steps.map((step, i) => )} + {result.steps.map((step, i) => )} } {diffs.map((diff, index) => - - + + )} - {!!screenshots.length && + {!!screenshots.length && {screenshots.map((a, i) => { - return
+ return -
; + ; })} } - {!!traces.length && + {!!traces.length && {} } - {!!videos.length && + {!!videos.length && {videos.map((a, i) =>
)} } - {!!(otherAttachments.size + htmls.length) && - {[...htmls].map((a, i) => ( - ) + {!!otherAttachments.size && + {[...otherAttachments].map((a, i) => + + + )} - {[...otherAttachments].map((a, i) => )} }
; }; @@ -161,19 +169,23 @@ function classifyErrors(testErrors: string[], diffs: ImageDiff[]) { } const StepTreeItem: React.FC<{ + test: TestCase; + result: TestResult; step: TestStep; depth: number, -}> = ({ step, depth }) => { - return +}> = ({ test, step, result, depth }) => { + const attachmentName = step.title.match(/^attach "(.*)"$/)?.[1]; + return {msToString(step.duration)} + {attachmentName && { evt.stopPropagation(); }}>{icons.attachment()}} {statusIcon(step.error || step.duration === -1 ? 'failed' : 'passed')} {step.title} {step.count > 1 && <> ✕ {step.count}} {step.location && — {step.location.file}:{step.location.line}} } loadChildren={step.steps.length + (step.snippet ? 1 : 0) ? () => { - const children = step.steps.map((s, i) => ); + const children = step.steps.map((s, i) => ); if (step.snippet) - children.unshift(); + children.unshift(); return children; - } : undefined} depth={depth}>; + } : undefined} depth={depth}/>; }; diff --git a/packages/html-reporter/src/treeItem.css b/packages/html-reporter/src/treeItem.css index a8cedc4f6af38..f37a759c2d4fa 100644 --- a/packages/html-reporter/src/treeItem.css +++ b/packages/html-reporter/src/treeItem.css @@ -25,6 +25,11 @@ cursor: pointer; } +.tree-item-title.selected { + text-decoration: underline var(--color-underlinenav-icon); + text-decoration-thickness: 1.5px; +} + .tree-item-body { min-height: 18px; } diff --git a/packages/html-reporter/src/treeItem.tsx b/packages/html-reporter/src/treeItem.tsx index 507a9c0e716f5..926a398a053f7 100644 --- a/packages/html-reporter/src/treeItem.tsx +++ b/packages/html-reporter/src/treeItem.tsx @@ -17,6 +17,7 @@ import * as React from 'react'; import './treeItem.css'; import * as icons from './icons'; +import { clsx } from '@web/uiUtils'; export const TreeItem: React.FunctionComponent<{ title: JSX.Element, @@ -28,9 +29,8 @@ export const TreeItem: React.FunctionComponent<{ style?: React.CSSProperties, }> = ({ title, loadChildren, onClick, expandByDefault, depth, selected, style }) => { const [expanded, setExpanded] = React.useState(expandByDefault || false); - const className = selected ? 'tree-item-title selected' : 'tree-item-title'; return
- { onClick?.(); setExpanded(!expanded); }} > + { onClick?.(); setExpanded(!expanded); }} > {loadChildren && !!expanded && icons.downArrow()} {loadChildren && !expanded && icons.rightArrow()} {!loadChildren && {icons.rightArrow()}} diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts index 048976ea2326e..5568455d6320d 100644 --- a/tests/playwright-test/reporter-html.spec.ts +++ b/tests/playwright-test/reporter-html.spec.ts @@ -847,7 +847,7 @@ for (const useIntermediateMergeReport of [true, false] as const) { 'a.test.js': ` import { test, expect } from '@playwright/test'; test('passing', async ({ page }, testInfo) => { - testInfo.attach('axe-report.html', { + await testInfo.attach('axe-report.html', { contentType: 'text/html', body: '

Axe Report

', }); @@ -916,6 +916,28 @@ for (const useIntermediateMergeReport of [true, false] as const) { ])); }); + test('should link from attach step to attachment view', async ({ runInlineTest, page, showReport }) => { + const result = await runInlineTest({ + 'a.test.js': ` + import { test, expect } from '@playwright/test'; + test('passing', async ({ page }, testInfo) => { + for (let i = 0; i < 100; i++) + await testInfo.attach('foo-1', { body: 'bar' }); + await testInfo.attach('foo-2', { body: 'bar' }); + }); + `, + }, { reporter: 'dot,html' }, { PLAYWRIGHT_HTML_OPEN: 'never' }); + expect(result.exitCode).toBe(0); + + await showReport(); + await page.getByRole('link', { name: 'passing' }).click(); + + const attachment = page.getByText('foo-2', { exact: true }); + await expect(attachment).not.toBeInViewport(); + await page.getByLabel('attach "foo-2"').getByTitle('link to attachment').click(); + await expect(attachment).toBeInViewport(); + }); + test('should highlight textual diff', async ({ runInlineTest, showReport, page }) => { const result = await runInlineTest({ 'helper.ts': `