From 22ff808d5983c637f192392f11b6178f2d533efd Mon Sep 17 00:00:00 2001 From: Alexander Brown <448862+DrJKL@users.noreply.github.com> Date: Sat, 31 Jan 2026 10:46:11 -0800 Subject: [PATCH] fix(browser_tests): use getByRole for interactive elements Replace fragile CSS selectors with Playwright getByRole() for better accessibility-based testing: - bottomPanelShortcuts.spec.ts: button[aria-label] -> getByRole - execution.spec.ts: .p-dialog-close-button -> getByRole - backgroundImageUpload.spec.ts: button:has(.pi-*) -> getByRole - nodeHelp.spec.ts: button:has(.pi-question) -> getByRole - BaseDialog.ts: closeButton uses getByRole - ComfyNodeSearchBox.ts: Add button uses getByRole - Topbar.ts: close-button uses getByRole Amp-Thread-ID: https://ampcode.com/threads/T-019c155c-92e1-76f3-b6ce-7fc3ffa11582 Co-authored-by: Amp --- browser_tests/fixtures/ComfyPage.ts | 5 +- .../fixtures/components/BaseDialog.ts | 2 +- .../fixtures/components/ComfyNodeSearchBox.ts | 2 +- browser_tests/fixtures/components/Topbar.ts | 2 +- .../tests/backgroundImageUpload.spec.ts | 54 +++++++++++-------- .../tests/bottomPanelShortcuts.spec.ts | 28 +++++----- browser_tests/tests/execution.spec.ts | 5 +- browser_tests/tests/nodeHelp.spec.ts | 8 ++- 8 files changed, 64 insertions(+), 42 deletions(-) diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index cb37200f7..a974096b1 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -839,7 +839,10 @@ export class ComfyPage { * @deprecated Use dialog-specific close methods instead (e.g., settingDialog.close()) */ async closeDialog() { - await this.page.locator('.p-dialog-close-button').click({ force: true }) + await this.page + .locator('.p-dialog') + .getByRole('button', { name: 'Close' }) + .click({ force: true }) await this.page.locator('.p-dialog').waitFor({ state: 'hidden' }) } diff --git a/browser_tests/fixtures/components/BaseDialog.ts b/browser_tests/fixtures/components/BaseDialog.ts index 1fbcf899b..62b4ff64c 100644 --- a/browser_tests/fixtures/components/BaseDialog.ts +++ b/browser_tests/fixtures/components/BaseDialog.ts @@ -9,7 +9,7 @@ export class BaseDialog { testId?: string ) { this.root = testId ? page.getByTestId(testId) : page.locator('.p-dialog') - this.closeButton = this.root.locator('.p-dialog-close-button') + this.closeButton = this.root.getByRole('button', { name: 'Close' }) } async isVisible(): Promise { diff --git a/browser_tests/fixtures/components/ComfyNodeSearchBox.ts b/browser_tests/fixtures/components/ComfyNodeSearchBox.ts index 59a33e254..82f0945a6 100644 --- a/browser_tests/fixtures/components/ComfyNodeSearchBox.ts +++ b/browser_tests/fixtures/components/ComfyNodeSearchBox.ts @@ -30,7 +30,7 @@ export class ComfyNodeSearchFilterSelectionPanel { async addFilter(filterValue: string, filterType: string) { await this.selectFilterType(filterType) await this.selectFilterValue(filterValue) - await this.page.locator('button:has-text("Add")').click() + await this.page.getByRole('button', { name: 'Add' }).click() } } diff --git a/browser_tests/fixtures/components/Topbar.ts b/browser_tests/fixtures/components/Topbar.ts index 46cebe5e3..9c96efe60 100644 --- a/browser_tests/fixtures/components/Topbar.ts +++ b/browser_tests/fixtures/components/Topbar.ts @@ -56,7 +56,7 @@ export class Topbar { async closeWorkflowTab(tabName: string) { const tab = this.getWorkflowTab(tabName) - await tab.locator('.close-button').click({ force: true }) + await tab.getByRole('button', { name: 'Close' }).click({ force: true }) } getSaveDialog(): Locator { diff --git a/browser_tests/tests/backgroundImageUpload.spec.ts b/browser_tests/tests/backgroundImageUpload.spec.ts index b620f5441..da93b01fe 100644 --- a/browser_tests/tests/backgroundImageUpload.spec.ts +++ b/browser_tests/tests/backgroundImageUpload.spec.ts @@ -34,16 +34,18 @@ test.describe('Background Image Upload', () => { await expect(backgroundImageSetting).toBeVisible() // Verify the component has the expected elements using semantic selectors - const urlInput = backgroundImageSetting.locator('input[type="text"]') + const urlInput = backgroundImageSetting.getByRole('textbox') await expect(urlInput).toBeVisible() await expect(urlInput).toHaveAttribute('placeholder') - const uploadButton = backgroundImageSetting.locator( - 'button:has(.pi-upload)' - ) + const uploadButton = backgroundImageSetting.getByRole('button', { + name: /upload/i + }) await expect(uploadButton).toBeVisible() - const clearButton = backgroundImageSetting.locator('button:has(.pi-trash)') + const clearButton = backgroundImageSetting.getByRole('button', { + name: /clear/i + }) await expect(clearButton).toBeVisible() await expect(clearButton).toBeDisabled() // Should be disabled when no image }) @@ -63,9 +65,9 @@ test.describe('Background Image Upload', () => { '#Comfy\\.Canvas\\.BackgroundImage' ) // Click the upload button to trigger file input - const uploadButton = backgroundImageSetting.locator( - 'button:has(.pi-upload)' - ) + const uploadButton = backgroundImageSetting.getByRole('button', { + name: /upload/i + }) // Set up file upload handler const fileChooserPromise = comfyPage.page.waitForEvent('filechooser') @@ -76,11 +78,13 @@ test.describe('Background Image Upload', () => { await fileChooser.setFiles(comfyPage.assetPath('image32x32.webp')) // Verify the URL input now has an API URL - const urlInput = backgroundImageSetting.locator('input[type="text"]') + const urlInput = backgroundImageSetting.getByRole('textbox') await expect(urlInput).toHaveValue(/^\/api\/view\?.*subfolder=backgrounds/) // Verify clear button is now enabled - const clearButton = backgroundImageSetting.locator('button:has(.pi-trash)') + const clearButton = backgroundImageSetting.getByRole('button', { + name: /clear/i + }) await expect(clearButton).toBeEnabled() // Verify the setting value was actually set @@ -107,14 +111,16 @@ test.describe('Background Image Upload', () => { '#Comfy\\.Canvas\\.BackgroundImage' ) // Enter URL in the input field - const urlInput = backgroundImageSetting.locator('input[type="text"]') + const urlInput = backgroundImageSetting.getByRole('textbox') await urlInput.fill(testImageUrl) // Trigger blur event to ensure the value is set await urlInput.blur() // Verify clear button is now enabled - const clearButton = backgroundImageSetting.locator('button:has(.pi-trash)') + const clearButton = backgroundImageSetting.getByRole('button', { + name: /clear/i + }) await expect(clearButton).toBeEnabled() // Verify the setting value was updated @@ -144,11 +150,13 @@ test.describe('Background Image Upload', () => { '#Comfy\\.Canvas\\.BackgroundImage' ) // Verify the input has the test URL - const urlInput = backgroundImageSetting.locator('input[type="text"]') + const urlInput = backgroundImageSetting.getByRole('textbox') await expect(urlInput).toHaveValue(testImageUrl) // Verify clear button is enabled - const clearButton = backgroundImageSetting.locator('button:has(.pi-trash)') + const clearButton = backgroundImageSetting.getByRole('button', { + name: /clear/i + }) await expect(clearButton).toBeEnabled() // Click the clear button @@ -182,9 +190,9 @@ test.describe('Background Image Upload', () => { '#Comfy\\.Canvas\\.BackgroundImage' ) // Hover over upload button and verify tooltip appears - const uploadButton = backgroundImageSetting.locator( - 'button:has(.pi-upload)' - ) + const uploadButton = backgroundImageSetting.getByRole('button', { + name: /upload/i + }) await uploadButton.hover() const uploadTooltip = comfyPage.page.locator('.p-tooltip:visible') @@ -194,12 +202,14 @@ test.describe('Background Image Upload', () => { await comfyPage.page.locator('body').hover() // Set a background to enable clear button - const urlInput = backgroundImageSetting.locator('input[type="text"]') + const urlInput = backgroundImageSetting.getByRole('textbox') await urlInput.fill('https://example.com/test.png') await urlInput.blur() // Hover over clear button and verify tooltip appears - const clearButton = backgroundImageSetting.locator('button:has(.pi-trash)') + const clearButton = backgroundImageSetting.getByRole('button', { + name: /clear/i + }) await clearButton.hover() const clearTooltip = comfyPage.page.locator('.p-tooltip:visible') @@ -220,8 +230,10 @@ test.describe('Background Image Upload', () => { const backgroundImageSetting = comfyPage.page.locator( '#Comfy\\.Canvas\\.BackgroundImage' ) - const urlInput = backgroundImageSetting.locator('input[type="text"]') - const clearButton = backgroundImageSetting.locator('button:has(.pi-trash)') + const urlInput = backgroundImageSetting.getByRole('textbox') + const clearButton = backgroundImageSetting.getByRole('button', { + name: /clear/i + }) // Initially clear button should be disabled await expect(clearButton).toBeDisabled() diff --git a/browser_tests/tests/bottomPanelShortcuts.spec.ts b/browser_tests/tests/bottomPanelShortcuts.spec.ts index 354f0fb51..516c53b7b 100644 --- a/browser_tests/tests/bottomPanelShortcuts.spec.ts +++ b/browser_tests/tests/bottomPanelShortcuts.spec.ts @@ -13,7 +13,7 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { // Click shortcuts toggle button in sidebar await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() // Shortcuts panel should now be visible @@ -21,7 +21,7 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { // Click toggle button again to hide await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() // Panel should be hidden again @@ -31,7 +31,7 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { test('should display essentials shortcuts tab', async ({ comfyPage }) => { // Open shortcuts panel await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() // Essentials tab should be visible and active by default @@ -65,7 +65,7 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { test('should display view controls shortcuts tab', async ({ comfyPage }) => { // Open shortcuts panel await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() // Click view controls tab @@ -91,7 +91,7 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { test('should switch between shortcuts tabs', async ({ comfyPage }) => { // Open shortcuts panel await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() // Essentials should be active initially @@ -125,7 +125,7 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { test('should display formatted keyboard shortcuts', async ({ comfyPage }) => { // Open shortcuts panel await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() // Wait for shortcuts to load @@ -149,13 +149,13 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { }) => { // Open shortcuts panel first await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() await expect(comfyPage.page.locator('.bottom-panel')).toBeVisible() // Open terminal panel (should switch panels) await comfyPage.page - .locator('button[aria-label*="Toggle Bottom Panel"]') + .getByRole('button', { name: /Toggle Bottom Panel/i }) .click() // Panel should still be visible but showing terminal content @@ -163,7 +163,7 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { // Switch back to shortcuts await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() // Should show shortcuts content again @@ -175,7 +175,7 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { test('should handle keyboard navigation', async ({ comfyPage }) => { // Open shortcuts panel await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() // Focus the first tab @@ -203,13 +203,13 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { }) => { // Open shortcuts panel await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() await expect(comfyPage.page.locator('.bottom-panel')).toBeVisible() // Click shortcuts button again to close await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() // Panel should be hidden @@ -221,7 +221,7 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { }) => { // Open shortcuts panel await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() // Should have 3-column grid layout @@ -256,7 +256,7 @@ test.describe('Bottom Panel Shortcuts', { tag: '@ui' }, () => { }) => { // Open shortcuts panel await comfyPage.page - .locator('button[aria-label*="Keyboard Shortcuts"]') + .getByRole('button', { name: /Keyboard Shortcuts/i }) .click() // Manage shortcuts button should be visible diff --git a/browser_tests/tests/execution.spec.ts b/browser_tests/tests/execution.spec.ts index 65884f819..49667f076 100644 --- a/browser_tests/tests/execution.spec.ts +++ b/browser_tests/tests/execution.spec.ts @@ -16,7 +16,10 @@ test.describe('Execution', { tag: ['@smoke', '@workflow'] }, () => { await comfyPage.executeCommand('Comfy.QueuePrompt') await expect(comfyPage.page.locator('.comfy-error-report')).toBeVisible() - await comfyPage.page.locator('.p-dialog-close-button').click() + await comfyPage.page + .locator('.p-dialog') + .getByRole('button', { name: 'Close' }) + .click() await comfyPage.page.locator('.comfy-error-report').waitFor({ state: 'hidden' }) diff --git a/browser_tests/tests/nodeHelp.spec.ts b/browser_tests/tests/nodeHelp.spec.ts index ec9b4057a..e1134de52 100644 --- a/browser_tests/tests/nodeHelp.spec.ts +++ b/browser_tests/tests/nodeHelp.spec.ts @@ -88,7 +88,9 @@ test.describe('Node Help', { tag: ['@slow', '@ui'] }, () => { await ksamplerNode.hover() // Click the help button - const helpButton = ksamplerNode.locator('button:has(.pi-question)') + const helpButton = ksamplerNode.getByRole('button', { + name: /learn more/i + }) await expect(helpButton).toBeVisible() await helpButton.click() @@ -118,7 +120,9 @@ test.describe('Node Help', { tag: ['@slow', '@ui'] }, () => { .filter({ hasText: 'KSampler' }) .first() await ksamplerNode.hover() - const helpButton = ksamplerNode.locator('button:has(.pi-question)') + const helpButton = ksamplerNode.getByRole('button', { + name: /learn more/i + }) await helpButton.click() // Verify help page is shown