-
Notifications
You must be signed in to change notification settings - Fork 458
hotfix: stabilize flaky workflow sidebar browser tests #7280
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
Changes from 2 commits
de22caf
326de45
a58e121
b08b93a
8bdfde4
2f025e8
014e2d3
d714ec7
b287f46
6a55d53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -92,9 +92,26 @@ export class Topbar { | |
| ) | ||
| // Wait for the dialog to close. | ||
| await this.getSaveDialog().waitFor({ state: 'hidden', timeout: 500 }) | ||
|
|
||
| // Check if a confirmation dialog appeared (e.g., "Overwrite existing file?") | ||
| // If so, return early to let the test handle the confirmation | ||
| const confirmationDialog = this.page.locator( | ||
| '.p-dialog:has-text("Overwrite")' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ) | ||
| if (await confirmationDialog.isVisible()) { | ||
| return | ||
| } | ||
| } | ||
|
|
||
| async openTopbarMenu() { | ||
| // If menu is already open, close it first to reset state | ||
| const isAlreadyOpen = await this.menuLocator.isVisible() | ||
| if (isAlreadyOpen) { | ||
| // Click outside the menu to close it properly | ||
| await this.page.locator('body').click({ position: { x: 500, y: 300 } }) | ||
| await this.menuLocator.waitFor({ state: 'hidden', timeout: 1000 }) | ||
| } | ||
|
Comment on lines
106
to
+113
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider using a more reliable method to close the menu. Clicking on Consider one of these alternatives:
Example: // If menu is already open, close it first to reset state
const isAlreadyOpen = await this.menuLocator.isVisible()
if (isAlreadyOpen) {
// Press Escape to close the menu
await this.page.keyboard.press('Escape')
await this.menuLocator.waitFor({ state: 'hidden', timeout: 1000 })
}🤖 Prompt for AI Agents |
||
|
|
||
| await this.menuTrigger.click() | ||
| await this.menuLocator.waitFor({ state: 'visible' }) | ||
| return this.menuLocator | ||
|
|
@@ -146,6 +163,9 @@ export class Topbar { | |
| throw new Error('Path cannot be empty') | ||
| } | ||
|
|
||
| // Ensure no dialog masks are blocking before menu interaction | ||
| // await this.waitForDialogMaskHidden() | ||
Myestery marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
Myestery marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const menu = await this.openTopbarMenu() | ||
| const tabName = path[0] | ||
| const topLevelMenuItem = this.getMenuItem(tabName) | ||
|
|
@@ -162,15 +182,33 @@ export class Topbar { | |
|
|
||
| await topLevelMenu.hover() | ||
|
|
||
| // Hover over top-level menu with retry logic for flaky submenu appearance | ||
| const submenu = this.getVisibleSubmenu() | ||
|
Comment on lines
+182
to
+183
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the appearance nondeterministic? |
||
| try { | ||
| await submenu.waitFor({ state: 'visible', timeout: 1000 }) | ||
| } catch { | ||
| // Click outside to reset, then reopen menu | ||
| await this.page.locator('body').click({ position: { x: 500, y: 300 } }) | ||
| await this.menuLocator.waitFor({ state: 'hidden', timeout: 1000 }) | ||
| await this.menuTrigger.click() | ||
| await this.menuLocator.waitFor({ state: 'visible' }) | ||
| } | ||
|
|
||
Myestery marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| let currentMenu = topLevelMenu | ||
| for (let i = 1; i < path.length; i++) { | ||
| const commandName = path[i] | ||
| const menuItem = currentMenu | ||
| .locator( | ||
| `.p-tieredmenu-submenu .p-tieredmenu-item:has-text("${commandName}")` | ||
| ) | ||
| const menuItem = submenu | ||
| .locator(`.p-tieredmenu-item:has-text("${commandName}")`) | ||
| .first() | ||
| await menuItem.waitFor({ state: 'visible' }) | ||
|
|
||
| // For the last item, click it | ||
| if (i === path.length - 1) { | ||
| await menuItem.click() | ||
| return | ||
| } | ||
|
|
||
| // Otherwise, hover to open nested submenu | ||
| await menuItem.hover() | ||
| currentMenu = menuItem | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.