Conversation
5a684b9 to
f3f84b9
Compare
f3f84b9 to
7cf9fbb
Compare
d5def38 to
a510274
Compare
a510274 to
048b804
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive UI testing capabilities for advisory exploration functionality, including search, download, and display features for security advisories.
- Adds step definitions for advisory-specific UI tests including search, download, and verification of UI elements
- Creates new helper methods in DetailsPage for button and panel visibility verification
- Introduces BDD-style feature tests with multiple scenarios covering advisory search, display, and interaction
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/ui/steps/details-page.ts | Adds new step definitions for button/panel verification and file download testing |
| tests/ui/helpers/DetailsPage.ts | Extends DetailsPage class with button clicking and element visibility verification methods |
| tests/ui/features/@advisory-explorer/advisory-explorer.step.ts | Creates advisory-specific step definitions for search and table verification |
| tests/ui/features/@advisory-explorer/advisory-explorer.feature | Defines BDD feature scenarios for advisory exploration functionality |
| "File with the name {string} is downloaded", | ||
| async ({ page }, expectedFilename) => { | ||
| const downloadPromise = page.waitForEvent("download"); |
There was a problem hiding this comment.
The download promise is created before the download action is triggered. This should be set up before the user action that triggers the download, but here it appears to be waiting for a download that may have already been initiated by a previous step.
| "File with the name {string} is downloaded", | |
| async ({ page }, expectedFilename) => { | |
| const downloadPromise = page.waitForEvent("download"); | |
| "User downloads the file {string} by clicking the {string} button", | |
| async ({ page }, expectedFilename, buttonName) => { | |
| const downloadPromise = page.waitForEvent("download"); | |
| const detailsPage = new DetailsPage(page); | |
| await detailsPage.clickOnPageButton(buttonName); |
| this.page | ||
| .locator(".pf-v6-c-card__title-text") | ||
| .filter({ hasText: panel }) | ||
| .first() |
There was a problem hiding this comment.
Using a specific CSS class selector .pf-v6-c-card__title-text makes the test brittle to UI framework changes. Consider using a more semantic selector like getByRole('heading') or adding a test-specific data attribute.
| this.page | |
| .locator(".pf-v6-c-card__title-text") | |
| .filter({ hasText: panel }) | |
| .first() | |
| this.page.getByRole("heading", { name: panel }) |
|
So, I took a look and there is still a feature file, which seems to be an older version of the feature file, which is newly added in this PR. We can keep the old file if it has additional tests, which are not implemented in this PR, but I think duplicate scenarios should be cleared from the older file to prevent confusion. |
|
I ran the tests against an environment, which has about 500 000 advisories in it and this was the result:
I think a more strict selector is needed in this case. |
|
Also, when searching an advisory, another strict mode violation is present. This is most likely due to new version of TPA.
` |
No description provided.