Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(html): link from attachment step to attachment #33267

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
4 changes: 2 additions & 2 deletions packages/html-reporter/src/chip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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), []);
Expand Down
31 changes: 26 additions & 5 deletions packages/html-reporter/src/links.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const AttachmentLink: React.FunctionComponent<{
linkName?: string,
openInNewTab?: boolean,
}> = ({ attachment, href, linkName, openInNewTab }) => {
const isAnchored = useIsAnchored('attachment-' + attachment.name);
return <TreeItem title={<span>
{attachment.contentType === kMissingContentType ? icons.warning() : icons.attachment()}
{attachment.path && <a href={href || attachment.path} download={downloadFileNameForAttachment(attachment)}>{linkName || attachment.name}</a>}
Expand All @@ -82,7 +83,7 @@ export const AttachmentLink: React.FunctionComponent<{
)}
</span>} loadChildren={attachment.body ? () => {
return [<div key={1} className='attachment-body'><CopyToClipboard value={attachment.body!}/>{linkifyText(attachment.body!)}</div>];
} : undefined} depth={0} style={{ lineHeight: '32px' }}></TreeItem>;
} : undefined} depth={0} style={{ lineHeight: '32px' }} selected={isAnchored}></TreeItem>;
};

export const SearchParamsContext = React.createContext<URLSearchParams>(new URLSearchParams(window.location.hash.slice(1)));
Expand Down Expand Up @@ -114,7 +115,17 @@ 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;

function matchesAnchor(id: AnchorID, anchor: string): boolean {
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 useAnchor(id: AnchorID, onReveal: () => void) {
React.useEffect(() => {
Expand All @@ -123,16 +134,26 @@ export function useAnchor(id: AnchorID, onReveal: () => void) {

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)
if (params.has('anchor') && matchesAnchor(id, params.get('anchor')!))
onReveal();
};
window.addEventListener('popstate', listener);

// check if we're already on the anchor. required to make it work on page load.
listener();

return () => window.removeEventListener('popstate', listener);
}, [id, onReveal]);
}

export function useIsAnchored(id: AnchorID) {
const searchParams = React.useContext(SearchParamsContext);
if (!searchParams.has('anchor'))
return false;
const anchor = searchParams.get('anchor');
return matchesAnchor(id, anchor!);
}

export function Anchor({ id, children }: React.PropsWithChildren<{ id: AnchorID }>) {
const ref = React.useRef<HTMLDivElement>(null);
const onAnchorReveal = React.useCallback(() => {
Expand Down
12 changes: 7 additions & 5 deletions packages/html-reporter/src/testFileView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,17 @@ export const TestFileView: React.FC<React.PropsWithChildren<{
};

function imageDiffBadge(test: TestCaseSummary): JSX.Element | undefined {
const resultWithImageDiff = test.results.find(result => result.attachments.some(attachment => {
return attachment.contentType.startsWith('image/') && !!attachment.name.match(/-(expected|actual|diff)/);
}));
return resultWithImageDiff ? <Link href={`#?testId=${test.testId}&anchor=diff-0&run=${test.results.indexOf(resultWithImageDiff)}`} title='View images' className='test-file-badge'>{image()}</Link> : 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 <Link href={`#?testId=${test.testId}&anchor=attachment-${attachment.name}&run=${test.results.indexOf(result)}`} title='View images' className='test-file-badge'>{image()}</Link>;
}
}
}

function videoBadge(test: TestCaseSummary): JSX.Element | undefined {
const resultWithVideo = test.results.find(result => result.attachments.some(attachment => attachment.name === 'video'));
return resultWithVideo ? <Link href={`#?testId=${test.testId}&anchor=videos&run=${test.results.indexOf(resultWithVideo)}`} title='View video' className='test-file-badge'>{video()}</Link> : undefined;
return resultWithVideo ? <Link href={`#?testId=${test.testId}&anchor=attachment-video&run=${test.results.indexOf(resultWithVideo)}`} title='View video' className='test-file-badge'>{video()}</Link> : undefined;
}

function traceBadge(test: TestCaseSummary): JSX.Element | undefined {
Expand Down
61 changes: 37 additions & 24 deletions packages/html-reporter/src/testResultView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,15 @@ 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<TestAttachment>): ImageDiff[] {
const snapshotNameToImageDiff = new Map<string, ImageDiff>();
interface ImageDiffWithAnchors extends ImageDiff {
anchors: string[];
}

function groupImageDiffs(screenshots: Set<TestAttachment>): ImageDiffWithAnchors[] {
const snapshotNameToImageDiff = new Map<string, ImageDiffWithAnchors>();
for (const attachment of screenshots) {
const match = attachment.name.match(/^(.*)-(expected|actual|diff|previous)(\.[^.]+)?$/);
if (!match)
Expand All @@ -37,9 +42,10 @@ function groupImageDiffs(screenshots: Set<TestAttachment>): 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')
Expand All @@ -64,20 +70,22 @@ function groupImageDiffs(screenshots: Set<TestAttachment>): 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 } = React.useMemo(() => {
const attachments = result?.attachments || [];
const screenshots = new Set(attachments.filter(a => a.contentType.startsWith('image/')));
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<TestAttachment>(attachments);
[...screenshots, ...videos, ...traces, ...htmls].forEach(a => otherAttachments.delete(a));
[...screenshots, ...videos, ...traces].forEach(a => otherAttachments.delete(a));
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 };
}, [result]);

const screenshotAnchor = React.useMemo(() => screenshots.map(a => `attachment-${a.name}`), [screenshots]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • screenshotAnchors?
  • why not return this from the previous useMemo()?

Same for otherAttachmentsAnchor.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd also work, i'll make that change.

const otherAttachmentsAnchor = React.useMemo(() => [...otherAttachments].map(a => `attachment-${a.name}`), [otherAttachments]);

return <div className='test-result'>
{!!errors.length && <AutoChip header='Errors'>
{errors.map((error, index) => {
Expand All @@ -87,18 +95,18 @@ export const TestResultView: React.FC<{
})}
</AutoChip>}
{!!result.steps.length && <AutoChip header='Test Steps'>
{result.steps.map((step, i) => <StepTreeItem key={`step-${i}`} step={step} depth={0}></StepTreeItem>)}
{result.steps.map((step, i) => <StepTreeItem key={`step-${i}`} step={step} result={result} test={test} depth={0}/>)}
</AutoChip>}

{diffs.map((diff, index) =>
<Anchor key={`diff-${index}`} id={`diff-${index}`}>
<AutoChip dataTestId='test-results-image-diff' header={`Image mismatch: ${diff.name}`} revealOnAnchorId={`diff-${index}`}>
<Anchor key={`diff-${index}`} id={diff.anchors}>
<AutoChip dataTestId='test-results-image-diff' header={`Image mismatch: ${diff.name}`} revealOnAnchorId={diff.anchors}>
<ImageDiffView diff={diff}/>
</AutoChip>
</Anchor>
)}

{!!screenshots.length && <AutoChip header='Screenshots'>
{!!screenshots.length && <Anchor id={screenshotAnchor}><AutoChip header='Screenshots' revealOnAnchorId={screenshotAnchor}>
{screenshots.map((a, i) => {
return <div key={`screenshot-${i}`}>
<a href={a.path}>
Expand All @@ -107,9 +115,9 @@ export const TestResultView: React.FC<{
<AttachmentLink attachment={a}></AttachmentLink>
</div>;
})}
</AutoChip>}
</AutoChip></Anchor>}

{!!traces.length && <Anchor id='traces'><AutoChip header='Traces' revealOnAnchorId='traces'>
{!!traces.length && <Anchor id='attachment-trace'><AutoChip header='Traces' revealOnAnchorId='attachment-trace'>
{<div>
<a href={generateTraceUrl(traces)}>
<img className='screenshot' src={traceImage} style={{ width: 192, height: 117, marginLeft: 20 }} />
Expand All @@ -118,7 +126,7 @@ export const TestResultView: React.FC<{
</div>}
</AutoChip></Anchor>}

{!!videos.length && <Anchor id='videos'><AutoChip header='Videos' revealOnAnchorId='videos'>
{!!videos.length && <Anchor id='attachment-video'><AutoChip header='Videos' revealOnAnchorId='attachment-video'>
{videos.map((a, i) => <div key={`video-${i}`}>
<video controls>
<source src={a.path} type={a.contentType}/>
Expand All @@ -127,11 +135,12 @@ export const TestResultView: React.FC<{
</div>)}
</AutoChip></Anchor>}

{!!(otherAttachments.size + htmls.length) && <AutoChip header='Attachments'>
{[...htmls].map((a, i) => (
<AttachmentLink key={`html-link-${i}`} attachment={a} openInNewTab />)
{!!otherAttachments.size && <AutoChip header='Attachments' revealOnAnchorId={otherAttachmentsAnchor}>
{[...otherAttachments].map((a, i) =>
<Anchor key={`attachment-link-${i}`} id={`attachment-${a.name}`}>
<AttachmentLink attachment={a} openInNewTab={a.contentType.startsWith('text/html')} />
</Anchor>
)}
{[...otherAttachments].map((a, i) => <AttachmentLink key={`attachment-link-${i}`} attachment={a}></AttachmentLink>)}
</AutoChip>}
</div>;
};
Expand Down Expand Up @@ -161,19 +170,23 @@ function classifyErrors(testErrors: string[], diffs: ImageDiff[]) {
}

const StepTreeItem: React.FC<{
test: TestCase;
result: TestResult;
step: TestStep;
depth: number,
}> = ({ step, depth }) => {
return <TreeItem title={<span>
}> = ({ test, step, result, depth }) => {
const attachmentName = step.title.match(/^attach "(.*)"$/)?.[1];
return <TreeItem title={<span aria-label={step.title}>
<span style={{ float: 'right' }}>{msToString(step.duration)}</span>
{attachmentName && <a style={{ float: 'right' }} title='link to attachment' href={`#?testId=${test.testId}&anchor=attachment-${encodeURIComponent(attachmentName)}&run=${test.results.indexOf(result)}`} onClick={evt => { evt.stopPropagation(); }}>{icons.attachment()}</a>}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I wonder why do we use <Link> in some places, and plain <a> in others?
  • Can we simplify link generation to avoid passing test and result around somehow? Perhaps take existing ones from the current url? Or put them into a context? Something for a followup.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • no idea 🤷 I guess I can also rewrite this to <Link>, but I don't think there'll be a big difference since it's not a text link but a button
  • agree, that'd be nice. i'll look at it in a followup.

Copy link
Member Author

@Skn0tt Skn0tt Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've tried rewriting, but that doesn't work as well because <Link> doesn't allow passing in the float: 'right' style. Let's stick with <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}></StepTreeItem>);
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}></TestErrorView>);
children.unshift(<TestErrorView testId='test-snippet' key='line' error={step.snippet}/>);
return children;
} : undefined} depth={depth}></TreeItem>;
} : undefined} depth={depth}/>;
};
5 changes: 5 additions & 0 deletions packages/html-reporter/src/treeItem.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
4 changes: 2 additions & 2 deletions packages/html-reporter/src/treeItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 <div className={'tree-item'} style={style}>
<span className={className} style={{ whiteSpace: 'nowrap', paddingLeft: depth * 22 + 4 }} onClick={() => { onClick?.(); setExpanded(!expanded); }} >
<span className={clsx('tree-item-title', selected && 'selected')} style={{ whiteSpace: 'nowrap', paddingLeft: depth * 22 + 4 }} onClick={() => { onClick?.(); setExpanded(!expanded); }} >
{loadChildren && !!expanded && icons.downArrow()}
{loadChildren && !expanded && icons.rightArrow()}
{!loadChildren && <span style={{ visibility: 'hidden' }}>{icons.rightArrow()}</span>}
Expand Down
23 changes: 22 additions & 1 deletion tests/playwright-test/reporter-html.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: '<h1>Axe Report</h1>',
});
Expand Down Expand Up @@ -916,6 +916,27 @@ 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) => {
await testInfo.attach('foo', { body: 'bar' });
});
`,
}, { reporter: 'dot,html' }, { PLAYWRIGHT_HTML_OPEN: 'never' });
expect(result.exitCode).toBe(0);

await showReport();
await page.getByRole('link', { name: 'passing' }).click();
await page.getByLabel('attach "foo"').getByTitle('link to attachment').click();

await page.waitForURL(url => {
const navState = new URLSearchParams(url.hash.slice(1));
return navState.get('anchor') === 'attachment-foo';
});
});

test('should strikethrough textual diff', async ({ runInlineTest, showReport, page }) => {
const result = await runInlineTest({
'helper.ts': `
Expand Down
Loading