From 7727ebe758894232995967a76f32e7cfbcf21673 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 11 Jul 2022 19:47:15 -0700 Subject: [PATCH] feat(html report): improve test list view (#15543) - Two lines per test: title and location. - Align project labels. - Add trace badge that opens trace viewer. - Add video and image diff badges that show scrolled test result view. --- packages/html-reporter/src/chip.tsx | 9 +++-- packages/html-reporter/src/icons.tsx | 22 +++++++++++ packages/html-reporter/src/links.tsx | 20 +++++----- packages/html-reporter/src/reportView.tsx | 15 ++++--- .../html-reporter/src/testCaseView.spec.tsx | 2 +- packages/html-reporter/src/testCaseView.tsx | 8 ++-- packages/html-reporter/src/testFileView.css | 22 +++++++++-- packages/html-reporter/src/testFileView.tsx | 39 +++++++++++++++---- packages/html-reporter/src/testResultView.tsx | 25 +++++++++--- .../playwright-test/src/reporters/html.ts | 15 +++++-- 10 files changed, 134 insertions(+), 43 deletions(-) diff --git a/packages/html-reporter/src/chip.tsx b/packages/html-reporter/src/chip.tsx index 65f8863988..e98883d492 100644 --- a/packages/html-reporter/src/chip.tsx +++ b/packages/html-reporter/src/chip.tsx @@ -27,8 +27,9 @@ export const Chip: React.FC<{ setExpanded?: (expanded: boolean) => void, children?: any, dataTestId?: string, -}> = ({ header, expanded, setExpanded, children, noInsets, dataTestId }) => { - return
+ targetRef?: React.RefObject, +}> = ({ header, expanded, setExpanded, children, noInsets, dataTestId, targetRef }) => { + return
setExpanded?.(!expanded)} @@ -47,7 +48,8 @@ export const AutoChip: React.FC<{ noInsets?: boolean, children?: any, dataTestId?: string, -}> = ({ header, initialExpanded, noInsets, children, dataTestId }) => { + targetRef?: React.RefObject, +}> = ({ header, initialExpanded, noInsets, children, dataTestId, targetRef }) => { const [expanded, setExpanded] = React.useState(initialExpanded || initialExpanded === undefined); return {children} ; diff --git a/packages/html-reporter/src/icons.tsx b/packages/html-reporter/src/icons.tsx index 292954049a..234be15f1a 100644 --- a/packages/html-reporter/src/icons.tsx +++ b/packages/html-reporter/src/icons.tsx @@ -84,3 +84,25 @@ export const person = () => { export const commit = () => { return ; }; + +export const image = () => { + return ; +}; + +export const video = () => { + return ; +}; + +export const trace = () => { + return ; +}; + +export const empty = () => { + return ; +}; diff --git a/packages/html-reporter/src/links.tsx b/packages/html-reporter/src/links.tsx index b3cdc5cfed..8aaf8e56a3 100644 --- a/packages/html-reporter/src/links.tsx +++ b/packages/html-reporter/src/links.tsx @@ -27,20 +27,16 @@ export function navigate(href: string) { } export const Route: React.FunctionComponent<{ - params: string, + predicate: (params: URLSearchParams) => boolean, children: any -}> = ({ params, children }) => { - const initialParams = [...new URLSearchParams(window.location.hash.slice(1)).keys()].join('&'); - const [currentParams, setCurrentParam] = React.useState(initialParams); +}> = ({ predicate, children }) => { + const [matches, setMatches] = React.useState(predicate(new URLSearchParams(window.location.hash.slice(1)))); React.useEffect(() => { - const listener = () => { - const newParams = [...new URLSearchParams(window.location.hash.slice(1)).keys()].join('&'); - setCurrentParam(newParams); - }; + const listener = () => setMatches(predicate(new URLSearchParams(window.location.hash.slice(1)))); window.addEventListener('popstate', listener); return () => window.removeEventListener('popstate', listener); - }, []); - return currentParams === params ? children : null; + }, [predicate]); + return matches ? children : null; }; export const Link: React.FunctionComponent<{ @@ -79,4 +75,8 @@ export const AttachmentLink: React.FunctionComponent<{ } : undefined} depth={0} style={{ lineHeight: '32px' }}>; }; +export function generateTraceUrl(traces: TestAttachment[]) { + return `trace/index.html?${traces.map((a, i) => `trace=${new URL(a.path!, window.location.href)}`).join('&')}`; +} + const kMissingContentType = 'x-playwright/missing'; diff --git a/packages/html-reporter/src/reportView.tsx b/packages/html-reporter/src/reportView.tsx index 2594160e88..4b0a858217 100644 --- a/packages/html-reporter/src/reportView.tsx +++ b/packages/html-reporter/src/reportView.tsx @@ -35,6 +35,10 @@ declare global { } } +// These are extracted to preserve the function identity between renders to avoid re-triggering effects. +const testFilesRoutePredicate = (params: URLSearchParams) => !params.has('testId'); +const testCaseRoutePredicate = (params: URLSearchParams) => params.has('testId'); + export const ReportView: React.FC<{ report: LoadedReport | undefined, }> = ({ report }) => { @@ -48,13 +52,10 @@ export const ReportView: React.FC<{
{report?.json() && } {report?.json().metadata && } - + - - - - + {!!report && }
@@ -67,6 +68,8 @@ const TestCaseViewLoader: React.FC<{ const searchParams = new URLSearchParams(window.location.hash.slice(1)); const [test, setTest] = React.useState(); const testId = searchParams.get('testId'); + const anchor = (searchParams.get('anchor') || '') as 'video' | 'diff' | ''; + const run = +(searchParams.get('run') || '0'); React.useEffect(() => { (async () => { if (!testId || testId === test?.testId) @@ -83,5 +86,5 @@ const TestCaseViewLoader: React.FC<{ } })(); }, [test, report, testId]); - return ; + return ; }; diff --git a/packages/html-reporter/src/testCaseView.spec.tsx b/packages/html-reporter/src/testCaseView.spec.tsx index 4dedfecc0a..a40d0f13fc 100644 --- a/packages/html-reporter/src/testCaseView.spec.tsx +++ b/packages/html-reporter/src/testCaseView.spec.tsx @@ -62,7 +62,7 @@ const testCase: TestCase = { }; test('should render test case', async ({ mount }) => { - const component = await mount(); + const component = await mount(); await expect(component.locator('text=Annotation text').first()).toBeVisible(); await component.locator('text=Annotations').click(); await expect(component.locator('text=Annotation text')).not.toBeVisible(); diff --git a/packages/html-reporter/src/testCaseView.tsx b/packages/html-reporter/src/testCaseView.tsx index 397fc02dce..29a87bfe8c 100644 --- a/packages/html-reporter/src/testCaseView.tsx +++ b/packages/html-reporter/src/testCaseView.tsx @@ -27,8 +27,10 @@ import { TestResultView } from './testResultView'; export const TestCaseView: React.FC<{ projectNames: string[], test: TestCase | undefined, -}> = ({ projectNames, test }) => { - const [selectedResultIndex, setSelectedResultIndex] = React.useState(0); + anchor: 'video' | 'diff' | '', + run: number, +}> = ({ projectNames, test, run, anchor }) => { + const [selectedResultIndex, setSelectedResultIndex] = React.useState(run); return
{test &&
{test.path.join(' › ')}
} @@ -45,7 +47,7 @@ export const TestCaseView: React.FC<{ test.results.map((result, index) => ({ id: String(index), title:
{statusIcon(result.status)} {retryLabel(index)}
, - render: () => + render: () => })) || []} selectedTab={String(selectedResultIndex)} setSelectedTab={id => setSelectedResultIndex(+id)} />}
; }; diff --git a/packages/html-reporter/src/testFileView.css b/packages/html-reporter/src/testFileView.css index 78ddef899e..bc6b649de0 100644 --- a/packages/html-reporter/src/testFileView.css +++ b/packages/html-reporter/src/testFileView.css @@ -15,8 +15,7 @@ */ .test-file-test { - height: 38px; - line-height: 38px; + line-height: 32px; align-items: center; padding: 0 10px; white-space: nowrap; @@ -28,9 +27,24 @@ background-color: var(--color-canvas-subtle); } -.test-file-path { +.test-file-title { + font-weight: 500; +} + +.test-file-details-row { padding: 0 0 0 8px; - color: var(--color-fg-muted); + margin-left: 15px; + margin-top: -11px; + font-weight: normal; + color: var(--color-fg-subtle); +} + +.test-file-path { + margin-right: 7px; +} + +.test-file-badge { + margin-left: 7px; } .test-file-test-outcome-skipped { diff --git a/packages/html-reporter/src/testFileView.tsx b/packages/html-reporter/src/testFileView.tsx index a45c75b0d9..ba368a6f48 100644 --- a/packages/html-reporter/src/testFileView.tsx +++ b/packages/html-reporter/src/testFileView.tsx @@ -14,14 +14,15 @@ limitations under the License. */ -import type { HTMLReport, TestFileSummary } from '@playwright-test/reporters/html'; +import type { HTMLReport, TestCaseSummary, TestFileSummary } from '@playwright-test/reporters/html'; import * as React from 'react'; import { msToString } from './uiUtils'; import { Chip } from './chip'; import type { Filter } from './filter'; -import { Link, ProjectLink } from './links'; +import { generateTraceUrl, Link, ProjectLink } from './links'; import { statusIcon } from './statusIcon'; import './testFileView.css'; +import { video, image, trace } from './icons'; export const TestFileView: React.FC}> {file.tests.filter(t => filter.matches(t)).map(test =>
- {msToString(test.duration)} - {report.projectNames.length > 1 && !!test.projectName && - } - {statusIcon(test.outcome)} - {[...test.path, test.title].join(' › ')} - — {test.location.file}:{test.location.line} + {msToString(test.duration)} + {report.projectNames.length > 1 && !!test.projectName && + } + {statusIcon(test.outcome)} + {[...test.path, test.title].join(' › ')} +
+ {test.location.file}:{test.location.line} + {imageDiffBadge(test)} + {videoBadge(test)} + {traceBadge(test)} +
)} ; }; + +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 ? {image()} : undefined; +} + +function videoBadge(test: TestCaseSummary): JSX.Element | undefined { + const resultWithVideo = test.results.find(result => result.attachments.some(attachment => attachment.name === 'video')); + return resultWithVideo ? {video()} : undefined; +} + +function traceBadge(test: TestCaseSummary): JSX.Element | undefined { + const firstTraces = test.results.map(result => result.attachments.filter(attachment => attachment.name === 'trace')).filter(traces => traces.length > 0)[0]; + return firstTraces ? {trace()} : undefined; +} diff --git a/packages/html-reporter/src/testResultView.tsx b/packages/html-reporter/src/testResultView.tsx index f5b218a62f..7fcc4beb46 100644 --- a/packages/html-reporter/src/testResultView.tsx +++ b/packages/html-reporter/src/testResultView.tsx @@ -21,7 +21,7 @@ import { TreeItem } from './treeItem'; import { msToString } from './uiUtils'; import { AutoChip } from './chip'; import { traceImage } from './images'; -import { AttachmentLink } from './links'; +import { AttachmentLink, generateTraceUrl } from './links'; import { statusIcon } from './statusIcon'; import type { ImageDiff } from './imageDiffView'; import { ImageDiffView } from './imageDiffView'; @@ -64,7 +64,8 @@ function groupImageDiffs(screenshots: Set): ImageDiff[] { export const TestResultView: React.FC<{ test: TestCase, result: TestResult, -}> = ({ result }) => { + anchor: 'video' | 'diff' | '', +}> = ({ result, anchor }) => { const { screenshots, videos, traces, otherAttachments, diffs } = React.useMemo(() => { const attachments = result?.attachments || []; @@ -77,6 +78,20 @@ export const TestResultView: React.FC<{ return { screenshots: [...screenshots], videos, traces, otherAttachments, diffs }; }, [ result ]); + const videoRef = React.useRef(null); + const imageDiffRef = React.useRef(null); + + const [scrolled, setScrolled] = React.useState(false); + React.useEffect(() => { + if (scrolled) + return; + setScrolled(true); + if (anchor === 'video') + videoRef.current?.scrollIntoView({ block: 'start', inline: 'start' }); + if (anchor === 'diff') + imageDiffRef.current?.scrollIntoView({ block: 'start', inline: 'start' }); + }, [scrolled, anchor, setScrolled, videoRef]); + return
{!!result.errors.length && {result.errors.map((error, index) => )} @@ -86,7 +101,7 @@ export const TestResultView: React.FC<{ } {diffs.map((diff, index) => - + )} @@ -102,14 +117,14 @@ export const TestResultView: React.FC<{ {!!traces.length && {} } - {!!videos.length && + {!!videos.length && {videos.map((a, i) =>