Skip to content

feat(html-report): Keep query when clicking project filter + ability to right-click #35400

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

feat(html-report): Keep query when clicking project filter + ability to right-click #35400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1f9671e
feat(html-report): Keep query when clicking project filter + ability …
cpAdm Mar 28, 2025
779025d
test(html-reporter): Fix wrong URlL query
cpAdm Mar 28, 2025
41fbbae
test(html-reporter): Expect empty string if query param is not provided
cpAdm Mar 28, 2025
dbaa64e
test(html-reporter): Revert unnecessary new snapshot parts
cpAdm Mar 31, 2025
e25c86b
Merge branch 'main' into feat-toggle-project-filter-html-reporter
cpAdm Mar 31, 2025
da38ddb
fix: Undo accidently typo
cpAdm Mar 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 20 additions & 15 deletions packages/html-reporter/src/filter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,23 +207,28 @@ function cacheSearchValues(test: TestCaseSummary & { [searchValuesSymbol]?: Sear
return searchValues;
}

export function filterWithToken(tokens: string[], token: string, append: boolean): string {
export function filterWithToken(searchParams: URLSearchParams, token: string, append: boolean): string {
const tokens = searchParams.get('q')?.split(' ') ?? [];

let newTokens;
if (append) {
if (!tokens.includes(token))
return '#?q=' + [...tokens, token].join(' ').trim();
return '#?q=' + tokens.filter(t => t !== token).join(' ').trim();
newTokens = [...tokens, token];
else
newTokens = tokens.filter(t => t !== token);

} else {
// if metaKey or ctrlKey is not pressed, replace existing token with new token
let prefix: 's:' | 'p:' | '@';
if (token.startsWith('s:'))
prefix = 's:';
if (token.startsWith('p:'))
prefix = 'p:';
if (token.startsWith('@'))
prefix = '@';

newTokens = [...tokens.filter(t => !t.startsWith(prefix)), token];
}

// if metaKey or ctrlKey is not pressed, replace existing token with new token
let prefix: 's:' | 'p:' | '@';
if (token.startsWith('s:'))
prefix = 's:';
if (token.startsWith('p:'))
prefix = 'p:';
if (token.startsWith('@'))
prefix = '@';

const newTokens = tokens.filter(t => !t.startsWith(prefix));
newTokens.push(token);
return '#?q=' + newTokens.join(' ').trim();
return '#?q=' + encodeURIComponent(newTokens.join(' ').trim());
}
8 changes: 4 additions & 4 deletions packages/html-reporter/src/headerView.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,13 @@ test('should toggle filters', async ({ page, mount }) => {
</SearchParamsProvider>);
await component.locator('a', { hasText: 'All' }).click();
await component.locator('a', { hasText: 'Passed' }).click();
await expect(page).toHaveURL(/#\?q=s:passed/);
await expect(page).toHaveURL(/#\?q=s%3Apassed/);
await component.locator('a', { hasText: 'Failed' }).click();
await expect(page).toHaveURL(/#\?q=s:failed/);
await expect(page).toHaveURL(/#\?q=s%3Afailed/);
await component.locator('a', { hasText: 'Flaky' }).click();
await expect(page).toHaveURL(/#\?q=s:flaky/);
await expect(page).toHaveURL(/#\?q=s%3Aflaky/);
await component.locator('a', { hasText: 'Skipped' }).click();
await expect(page).toHaveURL(/#\?q=s:skipped/);
await expect(page).toHaveURL(/#\?q=s%3Askipped/);
await component.getByRole('textbox').fill('annot:annotation type=annotation description');
expect(filters).toEqual(['', '', 's:passed ', 's:failed ', 's:flaky ', 's:skipped ', 'annot:annotation type=annotation description']);
});
11 changes: 5 additions & 6 deletions packages/html-reporter/src/headerView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,22 +63,21 @@ const StatsNavView: React.FC<{
stats: Stats
}> = ({ stats }) => {
const searchParams = React.useContext(SearchParamsContext);
const q = searchParams.get('q')?.toString() || '';
const tokens = q.split(' ');

return <nav>
<Link className='subnav-item' href='#?'>
All <span className='d-inline counter'>{stats.total - stats.skipped}</span>
</Link>
<Link className='subnav-item' click={filterWithToken(tokens, 's:passed', false)} ctrlClick={filterWithToken(tokens, 's:passed', true)}>
<Link className='subnav-item' click={filterWithToken(searchParams, 's:passed', false)} ctrlClick={filterWithToken(searchParams, 's:passed', true)}>
Passed <span className='d-inline counter'>{stats.expected}</span>
</Link>
<Link className='subnav-item' click={filterWithToken(tokens, 's:failed', false)} ctrlClick={filterWithToken(tokens, 's:failed', true)}>
<Link className='subnav-item' click={filterWithToken(searchParams, 's:failed', false)} ctrlClick={filterWithToken(searchParams, 's:failed', true)}>
{!!stats.unexpected && statusIcon('unexpected')} Failed <span className='d-inline counter'>{stats.unexpected}</span>
</Link>
<Link className='subnav-item' click={filterWithToken(tokens, 's:flaky', false)} ctrlClick={filterWithToken(tokens, 's:flaky', true)}>
<Link className='subnav-item' click={filterWithToken(searchParams, 's:flaky', false)} ctrlClick={filterWithToken(searchParams, 's:flaky', true)}>
{!!stats.flaky && statusIcon('flaky')} Flaky <span className='d-inline counter'>{stats.flaky}</span>
</Link>
<Link className='subnav-item' click={filterWithToken(tokens, 's:skipped', false)} ctrlClick={filterWithToken(tokens, 's:skipped', true)}>
<Link className='subnav-item' click={filterWithToken(searchParams, 's:skipped', false)} ctrlClick={filterWithToken(searchParams, 's:skipped', true)}>
Skipped <span className='d-inline counter'>{stats.skipped}</span>
</Link>
</nav>;
Expand Down
8 changes: 4 additions & 4 deletions packages/html-reporter/src/links.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { CopyToClipboard } from './copyToClipboard';
import './links.css';
import { linkifyText } from '@web/renderUtils';
import { clsx, useFlash } from '@web/uiUtils';
import { filterWithToken } from './filter';

export function navigate(href: string | URL) {
window.history.pushState({}, '', href);
Expand Down Expand Up @@ -54,12 +55,11 @@ export const Link: React.FunctionComponent<{
};

export const ProjectLink: React.FunctionComponent<{
searchParams: URLSearchParams,
projectNames: string[],
projectName: string,
}> = ({ projectNames, projectName }) => {
const encoded = encodeURIComponent(projectName);
const value = projectName === encoded ? projectName : `"${encoded.replace(/%22/g, '%5C%22')}"`;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this logic was here to support project names with spaces/quotes inside. Otherwise, tokenizing p:my project will yield two tokens instead of a single one with "my project" as a project name. I guess we should add a test as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Should I make it generic such that it also works for annotations and such?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that would be great.

return <Link href={`#?q=p:${value}`}>
}> = ({ searchParams, projectNames, projectName }) => {
return <Link click={filterWithToken(searchParams, `p:${projectName}`, false)} ctrlClick={filterWithToken(searchParams, `p:${projectName}`, true)}>
<span className={clsx('label', `label-color-${projectNames.indexOf(projectName) % 6}`)} style={{ margin: '6px 0 0 6px' }}>
{projectName}
</span>
Expand Down
42 changes: 29 additions & 13 deletions packages/html-reporter/src/testCaseView.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,19 +113,19 @@ const annotationLinkRenderingTestCase: TestCase = {
test('should correctly render links in annotations', async ({ mount }) => {
const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={annotationLinkRenderingTestCase} prev={undefined} next={undefined} run={0}></TestCaseView>);

const firstLink = await component.getByText('https://playwright.dev/docs/intro').first();
const firstLink = component.getByText('https://playwright.dev/docs/intro').first();
await expect(firstLink).toBeVisible();
await expect(firstLink).toHaveAttribute('href', 'https://playwright.dev/docs/intro');

const secondLink = await component.getByText('https://playwright.dev/docs/api/class-playwright').first();
const secondLink = component.getByText('https://playwright.dev/docs/api/class-playwright').first();
await expect(secondLink).toBeVisible();
await expect(secondLink).toHaveAttribute('href', 'https://playwright.dev/docs/api/class-playwright');

const thirdLink = await component.getByText('https://github.com/microsoft/playwright/issues/23180').first();
const thirdLink = component.getByText('https://github.com/microsoft/playwright/issues/23180').first();
await expect(thirdLink).toBeVisible();
await expect(thirdLink).toHaveAttribute('href', 'https://github.com/microsoft/playwright/issues/23180');

const fourthLink = await component.getByText('https://github.com/microsoft/playwright/issues/23181').first();
const fourthLink = component.getByText('https://github.com/microsoft/playwright/issues/23181').first();
await expect(fourthLink).toBeVisible();
await expect(fourthLink).toHaveAttribute('href', 'https://github.com/microsoft/playwright/issues/23181');
});
Expand Down Expand Up @@ -170,7 +170,21 @@ const attachmentLinkRenderingTestCase: TestCase = {
results: [resultWithAttachment]
};

const testCaseSummary: TestCaseSummary = {
const previousTestCaseSummary: TestCaseSummary = {
testId: 'previousTestId',
title: 'previous test',
path: [],
projectName: 'chromium',
location: { file: 'test.spec.ts', line: 42, column: 0 },
tags: [],
outcome: 'expected',
duration: 10,
ok: true,
annotations: [],
results: [resultWithAttachment]
};

const nextTestCaseSummary: TestCaseSummary = {
testId: 'nextTestId',
title: 'next test',
path: [],
Expand All @@ -188,7 +202,7 @@ const testCaseSummary: TestCaseSummary = {
test('should correctly render links in attachments', async ({ mount }) => {
const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={attachmentLinkRenderingTestCase} prev={undefined} next={undefined} run={0}></TestCaseView>);
await component.getByText('first attachment').click();
const body = await component.getByText('The body with https://playwright.dev/docs/intro link');
const body = component.getByText('The body with https://playwright.dev/docs/intro link');
await expect(body).toBeVisible();
await expect(body.locator('a').filter({ hasText: 'playwright.dev' })).toHaveAttribute('href', 'https://playwright.dev/docs/intro');
await expect(body.locator('a').filter({ hasText: 'github.com' })).toHaveAttribute('href', 'https://github.com/microsoft/playwright/issues/31284');
Expand All @@ -209,12 +223,14 @@ test('should correctly render links in attachment name', async ({ mount }) => {
});

test('should correctly render prev and next', async ({ mount }) => {
const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={attachmentLinkRenderingTestCase} prev={testCaseSummary} next={testCaseSummary} run={0}></TestCaseView>);
const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={attachmentLinkRenderingTestCase} prev={previousTestCaseSummary} next={nextTestCaseSummary} run={0} />);
await expect(component).toMatchAriaSnapshot(`
- text: group
- link "« previous"
- link "next »"
- text: "My test test.spec.ts:42 10ms"
- link "« previous":
- /url: "#?testId=previousTestId"
- link "next »":
- /url: "#?testId=nextTestId"
- text: "My test test.spec.ts:42 10ms chromium"
`);
});

Expand All @@ -239,17 +255,17 @@ const testCaseWithTwoAttempts: TestCase = {
test('total duration is selected run duration', async ({ mount, page }) => {
const component = await mount(<TestCaseView projectNames={['chromium', 'webkit']} test={testCaseWithTwoAttempts} prev={undefined} next={undefined} run={0}></TestCaseView>);
await expect(component).toMatchAriaSnapshot(`
- text: "My test test.spec.ts:42 200ms"
- text: "My test test.spec.ts:42 200ms chromium"
- tablist:
- tab "Run 50ms"
- 'tab "Retry #1 150ms"'
`);
await page.getByRole('tab', { name: 'Run' }).click();
await expect(component).toMatchAriaSnapshot(`
- text: "My test test.spec.ts:42 200ms"
- text: "My test test.spec.ts:42 200ms chromium"
`);
await page.getByRole('tab', { name: 'Retry' }).click();
await expect(component).toMatchAriaSnapshot(`
- text: "My test test.spec.ts:42 200ms"
- text: "My test test.spec.ts:42 200ms chromium"
`);
});
2 changes: 1 addition & 1 deletion packages/html-reporter/src/testCaseView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export const TestCaseView: React.FC<{
<div className='test-case-duration'>{msToString(test.duration)}</div>
</div>}
{test && (!!test.projectName || labels) && <div className='test-case-project-labels-row'>
{test && !!test.projectName && <ProjectLink projectNames={projectNames} projectName={test.projectName}></ProjectLink>}
{test && !!test.projectName && <ProjectLink searchParams={searchParams} projectNames={projectNames} projectName={test.projectName} />}
{labels && <LabelsLinkView labels={labels} />}
</div>}
{!!visibleAnnotations.length && <AutoChip header='Annotations' dataTestId='test-case-annotations'>
Expand Down
12 changes: 4 additions & 8 deletions packages/html-reporter/src/testFileView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,8 @@ export const TestFileView: React.FC<React.PropsWithChildren<{
expanded={isFileExpanded(file.fileId)}
noInsets={true}
setExpanded={(expanded => setFileExpanded(file.fileId, expanded))}
header={<span>
{file.fileName}
</span>}>
header={<span>{file.fileName}</span>}
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like unrelated formatting change?

>
{file.tests.map(test =>
<div key={`test-${test.testId}`} className={clsx('test-file-test', 'test-file-test-outcome-' + test.outcome)}>
<div className='hbox' style={{ alignItems: 'flex-start' }}>
Expand All @@ -51,8 +50,7 @@ export const TestFileView: React.FC<React.PropsWithChildren<{
<Link href={testResultHref({ test }) + filterParam} title={[...test.path, test.title].join(' › ')}>
<span className='test-file-title'>{[...test.path, test.title].join(' › ')}</span>
</Link>
{projectNames.length > 1 && !!test.projectName &&
<ProjectLink projectNames={projectNames} projectName={test.projectName} />}
{projectNames.length > 1 && !!test.projectName && <ProjectLink searchParams={searchParams} projectNames={projectNames} projectName={test.projectName} />}
<LabelsClickView labels={test.tags} />
</span>
</div>
Expand Down Expand Up @@ -108,9 +106,7 @@ const LabelsClickView: React.FC<React.PropsWithChildren<{

const onClickHandle = (e: React.MouseEvent, label: string) => {
e.preventDefault();
const q = searchParams.get('q')?.toString() || '';
const tokens = q.split(' ');
navigate(filterWithToken(tokens, label, e.metaKey || e.ctrlKey));
navigate(filterWithToken(searchParams, label, e.metaKey || e.ctrlKey));
};

return labels.length > 0 ? (
Expand Down
Loading