-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
fix(html-reporter): add filter for anonymous describe #30621
fix(html-reporter): add filter for anonymous describe #30621
Conversation
061bd98
to
c9bd2eb
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR! It looks good, but let's move the fix from the UI towards the report generation.
const testFilePathLinkTitle = await testFilePathLink.getAttribute('title'); | ||
expect(testFilePathLinkTitle).toEqual('Root describe › Test passed'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const testFilePathLinkTitle = await testFilePathLink.getAttribute('title'); | |
expect(testFilePathLinkTitle).toEqual('Root describe › Test passed'); | |
await expect(testFilePathLink).toHaveText('Root describe › Test passed'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is for validating the title attribute of the a tag.
It's not caught by toHaveText.
Is it possible to validate attributes with toHaveText?
I tried it just in case and got an error.
Here's an example
<a class="test-file-path-link" href="#?testId=0ae68002180e7361ff85-5ea21f3c6f68eb98910d" title="Root describe › Test passed" style="text-decoration: none; color: var(--color-fg-default);">
<span class="test-file-path">anonymous-describe.test.ts:6</span>
</a>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see, I missed the previous check for the text. We strongly prefer toHaveText()
over toHaveCount(1)
. To check the title, you can use toHaveAttribute()
. We also don't like CSS locators like .hbox a[title="..."]
, so I'd just drop that check. Overall, something like this would be a good test:
await showReport();
await expect(page.locator('.test-file-test')).toHaveCount(1);
const testFilePathLink = page.locator('.test-file-path-link');
await expect(testFilePathLink).toHaveText('Root describe › Test passed');
await expect(testFilePathLink).toHaveAttribute('title', 'Root describe › Test passed');
await testFilePathLink.click();
await expect(page.locator('.test-case-path')).toHaveText('Root describe');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the answer
I replaced that part with toHaveAttribute.
But .hbox a[title="..."]
would be hard to replace.
because the first a tag doesn't have a class.
The html code for one row looks like this ('Root describe › Test passed' appears three times.)
<div class="test-file-test test-file-test-outcome-expected">
<div class="hbox" style="align-items: flex-start;">
<div class="hbox">
<span class="test-file-test-status-icon">
<svg>...</svg>
</span>
<span>
<a class="" href="#?testId=0ae68002180e7361ff85-5ea21f3c6f68eb98910d" title="Root describe › Test passed" style="text-decoration: none; color: var(--color-fg-default);">
<span class="test-file-title">Root describe › Test passed</span>
</a>
</span>
</div>
<span data-testid="test-duration" style="min-width: 50px; text-align: right;">1.7s</span>
</div>
<div class="test-file-details-row">
<a class="test-file-path-link" href="#?testId=0ae68002180e7361ff85-5ea21f3c6f68eb98910d" title="Root describe › Test passed" style="text-decoration: none; color: var(--color-fg-default);">
<span class="test-file-path">anonymous-describe.test.ts:6</span>
</a>
<a class="test-file-badge" href="trace/index.html?trace=http://localhost:9323/data/41f3e012e4cd5dfb0adbef872a709158450a441e.zip" title="View trace" style="text-decoration: none; color: var(--color-fg-default);">
<svg>...</svg>
</a>
</div>
</div>
So I changed it to the following
Can you check if that's okay?
await showReport();
await expect(page.locator('.test-file-test')).toHaveCount(1);
await expect(page.locator('.test-file-test').locator('a').first()).toHaveAttribute('title', 'Root describe › Test passed');
await expect(page.locator('.test-file-title')).toHaveText('Root describe › Test passed');
const testFilePathLink = page.locator('.test-file-path-link');
await expect(testFilePathLink).toHaveAttribute('title', 'Root describe › Test passed');
await testFilePathLink.click();
await expect(page.locator('.test-case-path')).toHaveText('Root describe');
c9bd2eb
to
e1a939a
Compare
This comment has been minimized.
This comment has been minimized.
e1a939a
to
bf7716a
Compare
bf7716a
to
13e2a6d
Compare
This comment has been minimized.
This comment has been minimized.
Test results for "tests 1"27289 passed, 671 skipped Merge workflow run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, merging in. Thank you for the PR!
related issue: #30475
Motivation:
On #30475, we found that anonymous describe is rendered in html report
Modification:
Make filter for anonymous describe
Result:
anonymous describe will be filtered out.
Not render empty describe
Close #30475 issue