From 04286c033adab9472e3dc55969f3b7cb8efe5c55 Mon Sep 17 00:00:00 2001 From: Johnpaul Chiwetelu <49923152+Myestery@users.noreply.github.com> Date: Wed, 10 Dec 2025 07:30:40 +0100 Subject: [PATCH] hotfix: stabilize flaky workflow sidebar browser tests (#7280) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fix flaky workflow sidebar browser tests that were failing in headless mode - Add retry logic for menu hover operations in Topbar - Add proper timing/wait helpers for dialog masks and workflow service completion - Fix test isolation issues in setupWorkflowsDirectory and drop workflow test ## Test plan - [x] Run `pnpm test:browser -- browser_tests/tests/sidebar/workflows.spec.ts` multiple times - [x] Verify the 3 previously failing tests now pass consistently: - "Can overwrite other workflows with save as" - "Can rename nested workflow from opened workflow item" - "Can drop workflow from workflows sidebar" ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7280-hotfix-stabilize-flaky-workflow-sidebar-browser-tests-2c46d73d365081c5b3badfafe35a63dc) by [Unito](https://www.unito.io) --------- Co-authored-by: Terry Jia Co-authored-by: Alexander Brown Co-authored-by: Luke Mino-Altherr Co-authored-by: Claude Co-authored-by: GitHub Action Co-authored-by: Benjamin Lu --- browser_tests/fixtures/ComfyPage.ts | 17 +++++++ .../fixtures/components/SidebarTab.ts | 7 +++ browser_tests/fixtures/components/Topbar.ts | 46 +++++++++++++++++-- browser_tests/tests/sidebar/workflows.spec.ts | 11 ++++- 4 files changed, 76 insertions(+), 5 deletions(-) diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index 73a8b8461..e29f6ea5b 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -126,6 +126,20 @@ class ConfirmDialog { const loc = this[locator] await expect(loc).toBeVisible() await loc.click() + + // Wait for the dialog mask to disappear after confirming + const mask = this.page.locator('.p-dialog-mask') + const count = await mask.count() + if (count > 0) { + await mask.first().waitFor({ state: 'hidden', timeout: 3000 }) + } + + // Wait for workflow service to finish if it's busy + await this.page.waitForFunction( + () => window['app']?.extensionManager?.workflow?.isBusy === false, + undefined, + { timeout: 3000 } + ) } } @@ -242,6 +256,9 @@ export class ComfyPage { await this.page.evaluate(async () => { await window['app'].extensionManager.workflow.syncWorkflows() }) + + // Wait for Vue to re-render the workflow list + await this.nextFrame() } async setupUser(username: string) { diff --git a/browser_tests/fixtures/components/SidebarTab.ts b/browser_tests/fixtures/components/SidebarTab.ts index 00ec8a5b0..3254e27c8 100644 --- a/browser_tests/fixtures/components/SidebarTab.ts +++ b/browser_tests/fixtures/components/SidebarTab.ts @@ -137,6 +137,13 @@ export class WorkflowsSidebarTab extends SidebarTab { .click() await this.page.keyboard.type(newName) await this.page.keyboard.press('Enter') + + // Wait for workflow service to finish renaming + await this.page.waitForFunction( + () => !window['app']?.extensionManager?.workflow?.isBusy, + undefined, + { timeout: 3000 } + ) } async insertWorkflow(locator: Locator) { diff --git a/browser_tests/fixtures/components/Topbar.ts b/browser_tests/fixtures/components/Topbar.ts index 289ca641b..c5e7c8155 100644 --- a/browser_tests/fixtures/components/Topbar.ts +++ b/browser_tests/fixtures/components/Topbar.ts @@ -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")' + ) + 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 }) + } + await this.menuTrigger.click() await this.menuLocator.waitFor({ state: 'visible' }) return this.menuLocator @@ -162,15 +179,36 @@ export class Topbar { await topLevelMenu.hover() + // Hover over top-level menu with retry logic for flaky submenu appearance + const submenu = this.getVisibleSubmenu() + 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' }) + // Re-hover on top-level menu to trigger submenu + await topLevelMenu.hover() + await submenu.waitFor({ state: 'visible', timeout: 1000 }) + } + 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 } diff --git a/browser_tests/tests/sidebar/workflows.spec.ts b/browser_tests/tests/sidebar/workflows.spec.ts index b14ba8eb0..86f37b23d 100644 --- a/browser_tests/tests/sidebar/workflows.spec.ts +++ b/browser_tests/tests/sidebar/workflows.spec.ts @@ -340,6 +340,11 @@ test.describe('Workflows sidebar', () => { await comfyPage.menu.workflowsTab.open() + // Wait for workflow to appear in Browse section after sync + const workflowItem = + comfyPage.menu.workflowsTab.getPersistedItem('workflow1.json') + await expect(workflowItem).toBeVisible({ timeout: 3000 }) + const nodeCount = await comfyPage.getGraphNodesCount() // Get the bounding box of the canvas element @@ -358,6 +363,10 @@ test.describe('Workflows sidebar', () => { '#graph-canvas', { targetPosition } ) - expect(await comfyPage.getGraphNodesCount()).toBe(nodeCount * 2) + + // Wait for nodes to be inserted after drag-drop with retryable assertion + await expect + .poll(() => comfyPage.getGraphNodesCount(), { timeout: 3000 }) + .toBe(nodeCount * 2) }) })