mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 14:30:41 +00:00
refactor(test): extract dialog page objects from inline getByRole usage (#10822)
## Summary
Extract inline `getByRole('dialog')` calls across E2E tests into
reusable page objects.
## Changes
- **What**: Extract `ConfirmDialog` class from `ComfyPage.ts` into
`browser_tests/fixtures/components/ConfirmDialog.ts` with new `save`
button locator. Add `MediaLightbox` and `TemplatesDialog` page objects.
Refactor 4 test files to use these page objects instead of raw dialog
locators.
- **Skipped**: `appModeDropdownClipping.spec.ts` uses
`getByRole('dialog')` for a PrimeVue Popover (not a true dialog), left
as-is.
## Review Focus
The `ConfirmDialog.click()` method now supports a `save` action used by
`workflowPersistence.spec.ts`, which also waits for the dialog mask to
disappear and workflow service to settle.
Fixes #10723
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10822-refactor-test-extract-dialog-page-objects-from-inline-getByRole-usage-3366d73d365081b3bc0ee7ef0ddce658)
by [Unito](https://www.unito.io)
This commit is contained in:
@@ -14,9 +14,12 @@ import { VueNodeHelpers } from '@e2e/fixtures/VueNodeHelpers'
|
||||
import { BottomPanel } from '@e2e/fixtures/components/BottomPanel'
|
||||
import { ComfyNodeSearchBox } from '@e2e/fixtures/components/ComfyNodeSearchBox'
|
||||
import { ComfyNodeSearchBoxV2 } from '@e2e/fixtures/components/ComfyNodeSearchBoxV2'
|
||||
import { ConfirmDialog } from '@e2e/fixtures/components/ConfirmDialog'
|
||||
import { ContextMenu } from '@e2e/fixtures/components/ContextMenu'
|
||||
import { MediaLightbox } from '@e2e/fixtures/components/MediaLightbox'
|
||||
import { QueuePanel } from '@e2e/fixtures/components/QueuePanel'
|
||||
import { SettingDialog } from '@e2e/fixtures/components/SettingDialog'
|
||||
import { TemplatesDialog } from '@e2e/fixtures/components/TemplatesDialog'
|
||||
import {
|
||||
AssetsSidebarTab,
|
||||
ModelLibrarySidebarTab,
|
||||
@@ -131,48 +134,6 @@ class ComfyMenu {
|
||||
}
|
||||
}
|
||||
|
||||
type KeysOfType<T, Match> = {
|
||||
[K in keyof T]: T[K] extends Match ? K : never
|
||||
}[keyof T]
|
||||
|
||||
class ConfirmDialog {
|
||||
public readonly root: Locator
|
||||
public readonly delete: Locator
|
||||
public readonly overwrite: Locator
|
||||
public readonly reject: Locator
|
||||
public readonly confirm: Locator
|
||||
|
||||
constructor(public readonly page: Page) {
|
||||
this.root = page.getByRole('dialog')
|
||||
this.delete = this.root.getByRole('button', { name: 'Delete' })
|
||||
this.overwrite = this.root.getByRole('button', { name: 'Overwrite' })
|
||||
this.reject = this.root.getByRole('button', { name: 'Cancel' })
|
||||
this.confirm = this.root.getByRole('button', { name: 'Confirm' })
|
||||
}
|
||||
|
||||
async click(locator: KeysOfType<ConfirmDialog, Locator>) {
|
||||
const loc = this[locator]
|
||||
await loc.waitFor({ state: 'visible' })
|
||||
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 as WorkspaceStore | undefined)?.workflow
|
||||
?.isBusy === false,
|
||||
undefined,
|
||||
{ timeout: 3000 }
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
export class ComfyPage {
|
||||
public readonly url: string
|
||||
// All canvas position operations are based on default view of canvas.
|
||||
@@ -196,6 +157,8 @@ export class ComfyPage {
|
||||
public readonly templates: ComfyTemplates
|
||||
public readonly settingDialog: SettingDialog
|
||||
public readonly confirmDialog: ConfirmDialog
|
||||
public readonly templatesDialog: TemplatesDialog
|
||||
public readonly mediaLightbox: MediaLightbox
|
||||
public readonly vueNodes: VueNodeHelpers
|
||||
public readonly appMode: AppModeHelper
|
||||
public readonly subgraph: SubgraphHelper
|
||||
@@ -244,6 +207,8 @@ export class ComfyPage {
|
||||
this.templates = new ComfyTemplates(page)
|
||||
this.settingDialog = new SettingDialog(page, this)
|
||||
this.confirmDialog = new ConfirmDialog(page)
|
||||
this.templatesDialog = new TemplatesDialog(page)
|
||||
this.mediaLightbox = new MediaLightbox(page)
|
||||
this.vueNodes = new VueNodeHelpers(page)
|
||||
this.appMode = new AppModeHelper(this)
|
||||
this.subgraph = new SubgraphHelper(this)
|
||||
|
||||
44
browser_tests/fixtures/components/ConfirmDialog.ts
Normal file
44
browser_tests/fixtures/components/ConfirmDialog.ts
Normal file
@@ -0,0 +1,44 @@
|
||||
import type { Locator, Page } from '@playwright/test'
|
||||
|
||||
import type { WorkspaceStore } from '../../types/globals'
|
||||
|
||||
type KeysOfType<T, Match> = {
|
||||
[K in keyof T]: T[K] extends Match ? K : never
|
||||
}[keyof T]
|
||||
|
||||
export class ConfirmDialog {
|
||||
public readonly root: Locator
|
||||
public readonly delete: Locator
|
||||
public readonly overwrite: Locator
|
||||
public readonly reject: Locator
|
||||
public readonly confirm: Locator
|
||||
public readonly save: Locator
|
||||
|
||||
constructor(public readonly page: Page) {
|
||||
this.root = page.getByRole('dialog')
|
||||
this.delete = this.root.getByRole('button', { name: 'Delete' })
|
||||
this.overwrite = this.root.getByRole('button', { name: 'Overwrite' })
|
||||
this.reject = this.root.getByRole('button', { name: 'Cancel' })
|
||||
this.confirm = this.root.getByRole('button', { name: 'Confirm' })
|
||||
this.save = this.root.getByRole('button', { name: 'Save' })
|
||||
}
|
||||
|
||||
async click(locator: KeysOfType<ConfirmDialog, Locator>) {
|
||||
const loc = this[locator]
|
||||
await loc.waitFor({ state: 'visible' })
|
||||
await loc.click()
|
||||
|
||||
// Wait for this confirm dialog to close (not all dialogs — another
|
||||
// dialog like save-as may open immediately after).
|
||||
await this.root.waitFor({ state: 'hidden', timeout: 5000 }).catch(() => {})
|
||||
|
||||
// Wait for workflow service to finish if it's busy
|
||||
await this.page.waitForFunction(
|
||||
() =>
|
||||
(window.app?.extensionManager as WorkspaceStore | undefined)?.workflow
|
||||
?.isBusy === false,
|
||||
undefined,
|
||||
{ timeout: 3000 }
|
||||
)
|
||||
}
|
||||
}
|
||||
11
browser_tests/fixtures/components/MediaLightbox.ts
Normal file
11
browser_tests/fixtures/components/MediaLightbox.ts
Normal file
@@ -0,0 +1,11 @@
|
||||
import type { Locator, Page } from '@playwright/test'
|
||||
|
||||
export class MediaLightbox {
|
||||
public readonly root: Locator
|
||||
public readonly closeButton: Locator
|
||||
|
||||
constructor(public readonly page: Page) {
|
||||
this.root = page.getByRole('dialog')
|
||||
this.closeButton = this.root.getByLabel('Close')
|
||||
}
|
||||
}
|
||||
19
browser_tests/fixtures/components/TemplatesDialog.ts
Normal file
19
browser_tests/fixtures/components/TemplatesDialog.ts
Normal file
@@ -0,0 +1,19 @@
|
||||
import type { Locator, Page } from '@playwright/test'
|
||||
|
||||
export class TemplatesDialog {
|
||||
public readonly root: Locator
|
||||
|
||||
constructor(public readonly page: Page) {
|
||||
this.root = page.getByRole('dialog')
|
||||
}
|
||||
|
||||
filterByHeading(name: string): Locator {
|
||||
return this.root.filter({
|
||||
has: this.page.getByRole('heading', { name, exact: true })
|
||||
})
|
||||
}
|
||||
|
||||
getCombobox(name: RegExp | string): Locator {
|
||||
return this.root.getByRole('combobox', { name })
|
||||
}
|
||||
}
|
||||
@@ -18,15 +18,13 @@ test.describe('Confirm dialog text wrapping', { tag: ['@mobile'] }, () => {
|
||||
.catch(() => {})
|
||||
}, longFilename)
|
||||
|
||||
const dialog = comfyPage.page.getByRole('dialog')
|
||||
await expect(dialog).toBeVisible()
|
||||
const { root, confirm, reject } = comfyPage.confirmDialog
|
||||
await expect(root).toBeVisible()
|
||||
|
||||
const confirmButton = dialog.getByRole('button', { name: 'Confirm' })
|
||||
await expect(confirmButton).toBeVisible()
|
||||
await expect(confirmButton).toBeInViewport()
|
||||
await expect(confirm).toBeVisible()
|
||||
await expect(confirm).toBeInViewport()
|
||||
|
||||
const cancelButton = dialog.getByRole('button', { name: 'Cancel' })
|
||||
await expect(cancelButton).toBeVisible()
|
||||
await expect(cancelButton).toBeInViewport()
|
||||
await expect(reject).toBeVisible()
|
||||
await expect(reject).toBeInViewport()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -41,30 +41,28 @@ test.describe('MediaLightbox', { tag: ['@slow'] }, () => {
|
||||
await assetCard.hover()
|
||||
await assetCard.getByLabel('Zoom in').click()
|
||||
|
||||
const gallery = comfyPage.page.getByRole('dialog')
|
||||
await expect(gallery).toBeVisible()
|
||||
|
||||
return { gallery }
|
||||
const { root } = comfyPage.mediaLightbox
|
||||
await expect(root).toBeVisible()
|
||||
}
|
||||
|
||||
test('opens gallery and shows dialog with close button', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
const { gallery } = await runAndOpenGallery(comfyPage)
|
||||
await expect(gallery.getByLabel('Close')).toBeVisible()
|
||||
await runAndOpenGallery(comfyPage)
|
||||
await expect(comfyPage.mediaLightbox.closeButton).toBeVisible()
|
||||
})
|
||||
|
||||
test('closes gallery on Escape key', async ({ comfyPage }) => {
|
||||
await runAndOpenGallery(comfyPage)
|
||||
|
||||
await comfyPage.page.keyboard.press('Escape')
|
||||
await expect(comfyPage.page.getByRole('dialog')).not.toBeVisible()
|
||||
await expect(comfyPage.mediaLightbox.root).not.toBeVisible()
|
||||
})
|
||||
|
||||
test('closes gallery when clicking close button', async ({ comfyPage }) => {
|
||||
const { gallery } = await runAndOpenGallery(comfyPage)
|
||||
await runAndOpenGallery(comfyPage)
|
||||
|
||||
await gallery.getByLabel('Close').click()
|
||||
await expect(comfyPage.page.getByRole('dialog')).not.toBeVisible()
|
||||
await comfyPage.mediaLightbox.closeButton.click()
|
||||
await expect(comfyPage.mediaLightbox.root).not.toBeVisible()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -116,9 +116,7 @@ test.describe('Templates', { tag: ['@slow', '@workflow'] }, () => {
|
||||
|
||||
await comfyPage.command.executeCommand('Comfy.BrowseTemplates')
|
||||
|
||||
const dialog = comfyPage.page.getByRole('dialog').filter({
|
||||
has: comfyPage.page.getByRole('heading', { name: 'Modèles', exact: true })
|
||||
})
|
||||
const dialog = comfyPage.templatesDialog.filterByHeading('Modèles')
|
||||
await expect(dialog).toBeVisible()
|
||||
|
||||
// Validate that French-localized strings from the templates index are rendered
|
||||
@@ -220,8 +218,7 @@ test.describe('Templates', { tag: ['@slow', '@workflow'] }, () => {
|
||||
await expect(comfyPage.templates.content).toBeVisible()
|
||||
|
||||
// Wait for filter bar select components to render
|
||||
const dialog = comfyPage.page.getByRole('dialog')
|
||||
const sortBySelect = dialog.getByRole('combobox', { name: /Sort/ })
|
||||
const sortBySelect = comfyPage.templatesDialog.getCombobox(/Sort/)
|
||||
await expect(sortBySelect).toBeVisible()
|
||||
|
||||
// Screenshot the filter bar containing MultiSelect and SingleSelect
|
||||
|
||||
@@ -460,11 +460,8 @@ test.describe('Workflow Persistence', () => {
|
||||
.getWorkflowTab('Unsaved Workflow')
|
||||
.click({ button: 'middle' })
|
||||
|
||||
// Click "Save" in the dirty close dialog (scoped to dialog)
|
||||
const dialog = comfyPage.page.getByRole('dialog')
|
||||
const saveButton = dialog.getByRole('button', { name: 'Save' })
|
||||
await saveButton.waitFor({ state: 'visible' })
|
||||
await saveButton.click()
|
||||
// Click "Save" in the dirty close dialog
|
||||
await comfyPage.confirmDialog.click('save')
|
||||
|
||||
// Fill in the filename dialog
|
||||
const saveDialog = comfyPage.menu.topbar.getSaveDialog()
|
||||
|
||||
Reference in New Issue
Block a user