-
Notifications
You must be signed in to change notification settings - Fork 160
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
chore(e2e): add e2e tests for global header #2412
base: main
Are you sure you want to change the base?
chore(e2e): add e2e tests for global header #2412
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test e2e-tests |
741341b
to
b49056d
Compare
845f5a7
to
99f4cbf
Compare
Signed-off-by: Yi Cai <[email protected]>
Signed-off-by: Yi Cai <[email protected]>
Signed-off-by: Yi Cai <[email protected]>
Signed-off-by: Yi Cai <[email protected]>
Signed-off-by: Yi Cai <[email protected]>
Signed-off-by: Yi Cai <[email protected]>
Signed-off-by: Yi Cai <[email protected]>
Signed-off-by: Yi Cai <[email protected]>
Signed-off-by: Yi Cai <[email protected]>
6f6fd76
to
8dfbd1b
Compare
/test e2e-tests |
Signed-off-by: Yi Cai <[email protected]>
@@ -445,7 +445,8 @@ for (const version of ["RHBK"]) { | |||
expect(statusBefore).toBe(403); | |||
|
|||
// logout | |||
await uiHelper.openSidebar("Settings"); | |||
await uiHelper.clickByDataTestId("KeyboardArrowDownOutlinedIcon"); |
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.
what if there are multiple Icon on sidebar? instead can we click based on the nav item like setting?
"[data-testid='KeyboardArrowDownOutlinedIcon']", | ||
); | ||
await profileDropdown.click(); | ||
expect(await uiHelper.isLinkVisible("Settings")).toBeTruthy(); |
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.
expect(await uiHelper.isLinkVisible("Settings")).toBeTruthy(); | |
await uiHelper.verifyLink("Settings"); |
similarly can we replace all expect(await isLinkVisible.()) as above?
@@ -95,6 +95,26 @@ export class UIhelper { | |||
await this.page.locator(`a`).filter({ hasText: linkText }).first().click(); | |||
} | |||
|
|||
async clickLinkByAriaLabel(ariaLabel: string) { | |||
await this.page.locator(`div[aria-label='${ariaLabel}'] a`).first().click(); |
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.
you can update the existing clickLink() method to handle this locator.
|
||
async goToSettingsPage() { | ||
await expect(this.page.locator("nav[id='global-header']")).toBeVisible(); | ||
await this.clickByDataTestId("KeyboardArrowDownOutlinedIcon"); |
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.
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.
We should find the item by the label, not the icon
await link.dispatchEvent("click"); | ||
} | ||
|
||
async clickPbyText(text: string) { |
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.
lets not create methods for tags, you can directly use this await this.page.locator(
p).getByText(text).first().click();
in test instead of putting here.
@@ -150,11 +170,21 @@ export class UIhelper { | |||
return await this.isElementVisible(locator, timeout); | |||
} | |||
|
|||
async isSearchBarVisible(): Promise<boolean> { |
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.
you may directly use expect(await this.isElementVisible(locator)).toBeTruthy();
in test instead of new method in uihelper.
Idea is to have generic methods in uihelper while keeping it small. if you still want you can move it to respective pages under e2e-tests/playwright/support/pages
and the locators to e2e-tests/playwright/support/pageObjects
async isLinkVisible(text: string): Promise<boolean> { | ||
const locator = `a:has-text("${text}")`; | ||
return await this.isElementVisible(locator); | ||
} | ||
|
||
async isLinkVisibleByLabel(label: string): Promise<boolean> { |
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.
instead of new method you may update verifyLink() to handle this.
await this.page.locator(`div[aria-label='${ariaLabel}'] a`).first().click(); | ||
} | ||
|
||
async clickLinkByHref(href: string) { |
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.
you can update the existing clickLink() method to handle this
async clickLinkByHref(href: string) { | ||
const link = this.page.locator(`a[href="${href}"]`); | ||
await link.waitFor({ state: "visible" }); | ||
await link.dispatchEvent("click"); |
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.
await link.dispatchEvent("click"); | |
await link.click({force:true}); |
Description
Update e2e tests for global header.
Changes including:
support
field to the default configMapapp-config-rhdh.yaml
appConfig
field toupstream.backstage
config in order to test notification unread amount badgeopenSidebar("Settings") with a new uiHelper function
goToSettingsPage()`default-global-header.spec.ts
openSidebar("Create...")
with newclickLinkByAriaLabel("Create...")
getByPlaceholder("Search")
togetByPlaceholder("Search", { exact: true })
so that it only specify the filter search element from main contentWhich issue(s) does this PR fix
PR acceptance criteria
Please make sure that the following steps are complete:
How to test changes / Special notes to the reviewer