mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-29 02:32:18 +00:00
test(browser): refactor browser tests for reliability and maintainability (#8510)
## Summary Major refactoring of browser tests to improve reliability, maintainability, and type safety. ## Changes ### Test Infrastructure Decomposition - Decomposed `ComfyPage.ts` (~1000 lines) into focused helpers: - `CanvasHelper`, `DebugHelper`, `SubgraphHelper`, `NodeOperationsHelper` - `SettingsHelper`, `WorkflowHelper`, `ClipboardHelper`, `KeyboardHelper` - Created `ContextMenu` page object, `BaseDialog` base class, and `BottomPanel` page object - Extracted `DefaultGraphPositions` constants ### Locator Stability - Added `data-testid` attributes to Vue components (sidebar, dialogs, node library) - Created centralized `selectors.ts` with test ID constants - Replaced fragile CSS selectors (`.nth()`, `:nth-child()`) with `getByTestId`/`getByRole` ### Performance & Reliability - Removed `setTimeout` anti-patterns (replaced with `waitForFunction`) - Replaced `waitForTimeout` with retrying assertions - Replaced hardcoded coordinates with computed `NodeReference` positions - Enforced LF line endings for all text files ### Type Safety - Enabled `no-explicit-any` lint rule for browser_tests via oxlint - Purged `as any` casts from browser_tests - Added Window type augmentation for standardized window access - Added proper type annotations throughout ### Bug Fixes - Restored `ExtensionManager` API contract - Removed test-only settings from production schema - Fixed flaky selectors and missing test setup ## Testing - All browser tests pass - Typecheck passes <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Overhauled browser E2E test infrastructure with many new helpers/fixtures, updated test APIs, and CI test container image bumped for consistency. * **Chores** * Standardized line endings and applied stricter lint rules for browser tests; workspace dependency version updated. * **Documentation** * Updated Playwright and TypeScript testing guidance and test-run commands. * **UI** * Added stable data-testids to multiple components to improve testability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This commit is contained in:
@@ -1,23 +1,32 @@
|
||||
import { expect } from '@playwright/test'
|
||||
|
||||
import type { Settings } from '../../src/schemas/apiSchema'
|
||||
import type { SettingParams } from '../../src/platform/settings/types'
|
||||
import { comfyPageFixture as test } from '../fixtures/ComfyPage'
|
||||
|
||||
/**
|
||||
* Type helper for test settings with arbitrary IDs.
|
||||
* Extensions can register settings with any ID, but SettingParams.id
|
||||
* is typed as keyof Settings for autocomplete. This helper allows
|
||||
* arbitrary IDs in tests while keeping type safety for other fields.
|
||||
*/
|
||||
type TestSettingId = keyof Settings
|
||||
|
||||
test.describe('Topbar commands', () => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
|
||||
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top')
|
||||
})
|
||||
|
||||
test('Should allow registering topbar commands', async ({ comfyPage }) => {
|
||||
await comfyPage.page.evaluate(() => {
|
||||
window['app'].registerExtension({
|
||||
window.app!.registerExtension({
|
||||
name: 'TestExtension1',
|
||||
commands: [
|
||||
{
|
||||
id: 'foo',
|
||||
label: 'foo-command',
|
||||
function: () => {
|
||||
window['foo'] = true
|
||||
window.foo = true
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -31,15 +40,15 @@ test.describe('Topbar commands', () => {
|
||||
})
|
||||
|
||||
await comfyPage.menu.topbar.triggerTopbarCommand(['ext', 'foo-command'])
|
||||
expect(await comfyPage.page.evaluate(() => window['foo'])).toBe(true)
|
||||
expect(await comfyPage.page.evaluate(() => window.foo)).toBe(true)
|
||||
})
|
||||
|
||||
test('Should not allow register command defined in other extension', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.registerCommand('foo', () => alert(1))
|
||||
await comfyPage.command.registerCommand('foo', () => alert(1))
|
||||
await comfyPage.page.evaluate(() => {
|
||||
window['app'].registerExtension({
|
||||
window.app!.registerExtension({
|
||||
name: 'TestExtension1',
|
||||
menuCommands: [
|
||||
{
|
||||
@@ -56,14 +65,14 @@ test.describe('Topbar commands', () => {
|
||||
|
||||
test('Should allow registering keybindings', async ({ comfyPage }) => {
|
||||
await comfyPage.page.evaluate(() => {
|
||||
const app = window['app']
|
||||
const app = window.app!
|
||||
app.registerExtension({
|
||||
name: 'TestExtension1',
|
||||
commands: [
|
||||
{
|
||||
id: 'TestCommand',
|
||||
function: () => {
|
||||
window['TestCommand'] = true
|
||||
window.TestCommand = true
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -77,68 +86,77 @@ test.describe('Topbar commands', () => {
|
||||
})
|
||||
|
||||
await comfyPage.page.keyboard.press('k')
|
||||
expect(await comfyPage.page.evaluate(() => window['TestCommand'])).toBe(
|
||||
true
|
||||
)
|
||||
expect(await comfyPage.page.evaluate(() => window.TestCommand)).toBe(true)
|
||||
})
|
||||
|
||||
test.describe('Settings', () => {
|
||||
test('Should allow adding settings', async ({ comfyPage }) => {
|
||||
await comfyPage.page.evaluate(() => {
|
||||
window['app'].registerExtension({
|
||||
window.app!.registerExtension({
|
||||
name: 'TestExtension1',
|
||||
settings: [
|
||||
{
|
||||
id: 'TestSetting',
|
||||
// Extensions can register arbitrary setting IDs
|
||||
id: 'TestSetting' as TestSettingId,
|
||||
name: 'Test Setting',
|
||||
type: 'text',
|
||||
defaultValue: 'Hello, world!',
|
||||
onChange: () => {
|
||||
window['changeCount'] = (window['changeCount'] ?? 0) + 1
|
||||
window.changeCount = (window.changeCount ?? 0) + 1
|
||||
}
|
||||
}
|
||||
]
|
||||
})
|
||||
})
|
||||
// onChange is called when the setting is first added
|
||||
expect(await comfyPage.page.evaluate(() => window['changeCount'])).toBe(1)
|
||||
expect(await comfyPage.getSetting('TestSetting')).toBe('Hello, world!')
|
||||
expect(await comfyPage.page.evaluate(() => window.changeCount)).toBe(1)
|
||||
expect(await comfyPage.settings.getSetting('TestSetting')).toBe(
|
||||
'Hello, world!'
|
||||
)
|
||||
|
||||
await comfyPage.setSetting('TestSetting', 'Hello, universe!')
|
||||
expect(await comfyPage.getSetting('TestSetting')).toBe('Hello, universe!')
|
||||
expect(await comfyPage.page.evaluate(() => window['changeCount'])).toBe(2)
|
||||
await comfyPage.settings.setSetting('TestSetting', 'Hello, universe!')
|
||||
expect(await comfyPage.settings.getSetting('TestSetting')).toBe(
|
||||
'Hello, universe!'
|
||||
)
|
||||
expect(await comfyPage.page.evaluate(() => window.changeCount)).toBe(2)
|
||||
})
|
||||
|
||||
test('Should allow setting boolean settings', async ({ comfyPage }) => {
|
||||
await comfyPage.page.evaluate(() => {
|
||||
window['app'].registerExtension({
|
||||
window.app!.registerExtension({
|
||||
name: 'TestExtension1',
|
||||
settings: [
|
||||
{
|
||||
id: 'Comfy.TestSetting',
|
||||
// Extensions can register arbitrary setting IDs
|
||||
id: 'Comfy.TestSetting' as TestSettingId,
|
||||
name: 'Test Setting',
|
||||
type: 'boolean',
|
||||
defaultValue: false,
|
||||
onChange: () => {
|
||||
window['changeCount'] = (window['changeCount'] ?? 0) + 1
|
||||
window.changeCount = (window.changeCount ?? 0) + 1
|
||||
}
|
||||
}
|
||||
]
|
||||
})
|
||||
})
|
||||
|
||||
expect(await comfyPage.getSetting('Comfy.TestSetting')).toBe(false)
|
||||
expect(await comfyPage.page.evaluate(() => window['changeCount'])).toBe(1)
|
||||
expect(await comfyPage.settings.getSetting('Comfy.TestSetting')).toBe(
|
||||
false
|
||||
)
|
||||
expect(await comfyPage.page.evaluate(() => window.changeCount)).toBe(1)
|
||||
|
||||
await comfyPage.settingDialog.open()
|
||||
await comfyPage.settingDialog.toggleBooleanSetting('Comfy.TestSetting')
|
||||
expect(await comfyPage.getSetting('Comfy.TestSetting')).toBe(true)
|
||||
expect(await comfyPage.page.evaluate(() => window['changeCount'])).toBe(2)
|
||||
expect(await comfyPage.settings.getSetting('Comfy.TestSetting')).toBe(
|
||||
true
|
||||
)
|
||||
expect(await comfyPage.page.evaluate(() => window.changeCount)).toBe(2)
|
||||
})
|
||||
|
||||
test.describe('Passing through attrs to setting components', () => {
|
||||
const testCases: Array<{
|
||||
config: Partial<SettingParams>
|
||||
config: Pick<SettingParams, 'type' | 'defaultValue'> &
|
||||
Partial<Omit<SettingParams, 'id' | 'type' | 'defaultValue'>>
|
||||
selector: string
|
||||
}> = [
|
||||
{
|
||||
@@ -191,13 +209,13 @@ test.describe('Topbar commands', () => {
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.page.evaluate((config) => {
|
||||
window['app'].registerExtension({
|
||||
window.app!.registerExtension({
|
||||
name: 'TestExtension1',
|
||||
settings: [
|
||||
{
|
||||
id: 'Comfy.TestSetting',
|
||||
// Extensions can register arbitrary setting IDs
|
||||
id: 'Comfy.TestSetting' as TestSettingId,
|
||||
name: 'Test',
|
||||
// The `disabled` attr is common to all settings components
|
||||
attrs: { disabled: true },
|
||||
...config
|
||||
}
|
||||
@@ -224,7 +242,7 @@ test.describe('Topbar commands', () => {
|
||||
test.describe('About panel', () => {
|
||||
test('Should allow adding badges', async ({ comfyPage }) => {
|
||||
await comfyPage.page.evaluate(() => {
|
||||
window['app'].registerExtension({
|
||||
window.app!.registerExtension({
|
||||
name: 'TestExtension1',
|
||||
aboutPageBadges: [
|
||||
{
|
||||
@@ -247,61 +265,71 @@ test.describe('Topbar commands', () => {
|
||||
test.describe('Dialog', () => {
|
||||
test('Should allow showing a prompt dialog', async ({ comfyPage }) => {
|
||||
await comfyPage.page.evaluate(() => {
|
||||
void window['app'].extensionManager.dialog
|
||||
.prompt({
|
||||
void window
|
||||
.app!.extensionManager.dialog.prompt({
|
||||
title: 'Test Prompt',
|
||||
message: 'Test Prompt Message'
|
||||
})
|
||||
.then((value: string) => {
|
||||
window['value'] = value
|
||||
.then((value: string | null) => {
|
||||
;(window as unknown as Record<string, unknown>)['value'] = value
|
||||
})
|
||||
})
|
||||
|
||||
await comfyPage.fillPromptDialog('Hello, world!')
|
||||
expect(await comfyPage.page.evaluate(() => window['value'])).toBe(
|
||||
'Hello, world!'
|
||||
)
|
||||
await comfyPage.nodeOps.fillPromptDialog('Hello, world!')
|
||||
expect(
|
||||
await comfyPage.page.evaluate(
|
||||
() => (window as unknown as Record<string, unknown>)['value']
|
||||
)
|
||||
).toBe('Hello, world!')
|
||||
})
|
||||
|
||||
test('Should allow showing a confirmation dialog', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.page.evaluate(() => {
|
||||
void window['app'].extensionManager.dialog
|
||||
.confirm({
|
||||
void window
|
||||
.app!.extensionManager.dialog.confirm({
|
||||
title: 'Test Confirm',
|
||||
message: 'Test Confirm Message'
|
||||
})
|
||||
.then((value: boolean) => {
|
||||
window['value'] = value
|
||||
.then((value: boolean | null) => {
|
||||
;(window as unknown as Record<string, unknown>)['value'] = value
|
||||
})
|
||||
})
|
||||
|
||||
await comfyPage.confirmDialog.click('confirm')
|
||||
expect(await comfyPage.page.evaluate(() => window['value'])).toBe(true)
|
||||
expect(
|
||||
await comfyPage.page.evaluate(
|
||||
() => (window as unknown as Record<string, unknown>)['value']
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
test('Should allow dismissing a dialog', async ({ comfyPage }) => {
|
||||
await comfyPage.page.evaluate(() => {
|
||||
window['value'] = 'foo'
|
||||
void window['app'].extensionManager.dialog
|
||||
.confirm({
|
||||
;(window as unknown as Record<string, unknown>)['value'] = 'foo'
|
||||
void window
|
||||
.app!.extensionManager.dialog.confirm({
|
||||
title: 'Test Confirm',
|
||||
message: 'Test Confirm Message'
|
||||
})
|
||||
.then((value: boolean) => {
|
||||
window['value'] = value
|
||||
.then((value: boolean | null) => {
|
||||
;(window as unknown as Record<string, unknown>)['value'] = value
|
||||
})
|
||||
})
|
||||
|
||||
await comfyPage.confirmDialog.click('reject')
|
||||
expect(await comfyPage.page.evaluate(() => window['value'])).toBeNull()
|
||||
expect(
|
||||
await comfyPage.page.evaluate(
|
||||
() => (window as unknown as Record<string, unknown>)['value']
|
||||
)
|
||||
).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
test.describe('Selection Toolbox', () => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.setSetting('Comfy.Canvas.SelectionToolbox', true)
|
||||
await comfyPage.settings.setSetting('Comfy.Canvas.SelectionToolbox', true)
|
||||
})
|
||||
|
||||
test('Should allow adding commands to selection toolbox', async ({
|
||||
@@ -309,7 +337,7 @@ test.describe('Topbar commands', () => {
|
||||
}) => {
|
||||
// Register an extension with a selection toolbox command
|
||||
await comfyPage.page.evaluate(() => {
|
||||
window['app'].registerExtension({
|
||||
window.app!.registerExtension({
|
||||
name: 'TestExtension1',
|
||||
commands: [
|
||||
{
|
||||
@@ -317,7 +345,9 @@ test.describe('Topbar commands', () => {
|
||||
label: 'Test Command',
|
||||
icon: 'pi pi-star',
|
||||
function: () => {
|
||||
window['selectionCommandExecuted'] = true
|
||||
;(window as unknown as Record<string, unknown>)[
|
||||
'selectionCommandExecuted'
|
||||
] = true
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -325,7 +355,7 @@ test.describe('Topbar commands', () => {
|
||||
})
|
||||
})
|
||||
|
||||
await comfyPage.selectNodes(['CLIP Text Encode (Prompt)'])
|
||||
await comfyPage.nodeOps.selectNodes(['CLIP Text Encode (Prompt)'])
|
||||
|
||||
// Click the command button in the selection toolbox
|
||||
const toolboxButton = comfyPage.page.locator(
|
||||
@@ -333,9 +363,13 @@ test.describe('Topbar commands', () => {
|
||||
)
|
||||
await toolboxButton.click()
|
||||
|
||||
// Verify the command was executed
|
||||
expect(
|
||||
await comfyPage.page.evaluate(() => window['selectionCommandExecuted'])
|
||||
await comfyPage.page.evaluate(
|
||||
() =>
|
||||
(window as unknown as Record<string, unknown>)[
|
||||
'selectionCommandExecuted'
|
||||
]
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user