-
Notifications
You must be signed in to change notification settings - Fork 449
Refactor Queue UI: Inline Progress & Cleanups #7230
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 34 commits
71c6da2
b790aaf
e65fc56
ca084c3
5c106bd
da4b241
5e1dc91
d965fb0
05d2cae
99019cf
0a5fc8b
c4b53c1
48aa3d9
87abea9
ad44241
73741df
8b4f174
f0b9d28
b428892
ab820bf
f6ffb1f
db80292
a5b5fe8
a48360d
96893f7
cbbf3eb
f7d3a8c
8fdf355
38b589c
3ce8cca
3fe2d39
ae1d6f9
b277cbf
b21cf86
8bec2df
268ab1d
36fda56
33626a5
d963b8e
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 |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import type { Page } from '@playwright/test' | ||
| import { expect } from '@playwright/test' | ||
|
|
||
| export class QueueList { | ||
| constructor(public readonly page: Page) {} | ||
|
|
||
| get toggleButton() { | ||
| return this.page.getByTestId('queue-toggle-button') | ||
| } | ||
|
|
||
| get inlineProgress() { | ||
| return this.page.getByTestId('queue-inline-progress') | ||
| } | ||
|
|
||
| get overlay() { | ||
| return this.page.getByTestId('queue-overlay') | ||
| } | ||
|
|
||
| get closeButton() { | ||
| return this.page.getByTestId('queue-overlay-close-button') | ||
| } | ||
|
|
||
| get jobItems() { | ||
| return this.page.getByTestId('queue-job-item') | ||
| } | ||
|
|
||
| get clearHistoryButton() { | ||
| return this.page.getByRole('button', { name: /Clear History/i }) | ||
| } | ||
|
|
||
| async open() { | ||
| if (!(await this.overlay.isVisible())) { | ||
| await this.toggleButton.click() | ||
| await expect(this.overlay).toBeVisible() | ||
| } | ||
| } | ||
|
|
||
| async close() { | ||
| if (await this.overlay.isVisible()) { | ||
| await this.closeButton.click() | ||
| await expect(this.overlay).not.toBeVisible() | ||
| } | ||
| } | ||
|
|
||
| async getJobCount(state?: string) { | ||
| if (state) { | ||
| return await this.page | ||
| .locator(`[data-testid="queue-job-item"][data-job-state="${state}"]`) | ||
| .count() | ||
| } | ||
| return await this.jobItems.count() | ||
| } | ||
|
|
||
| getJobAction(actionKey: string) { | ||
| return this.page.getByTestId(`job-action-${actionKey}`) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| import { expect, mergeTests } from '@playwright/test' | ||
|
|
||
| import type { ComfyPage } from '../../fixtures/ComfyPage' | ||
| import type { StatusWsMessage } from '../../../src/schemas/apiSchema' | ||
| import { comfyPageFixture } from '../../fixtures/ComfyPage' | ||
| import { webSocketFixture } from '../../fixtures/ws' | ||
|
|
||
| const test = mergeTests(comfyPageFixture, webSocketFixture) | ||
|
|
||
| type QueueState = { | ||
| running: QueueJob[] | ||
| pending: QueueJob[] | ||
| } | ||
|
|
||
| type QueueJob = [ | ||
| string, | ||
| string, | ||
| Record<string, unknown>, | ||
| Record<string, unknown>, | ||
| string[] | ||
| ] | ||
|
|
||
| type QueueController = { | ||
| state: QueueState | ||
| sync: ( | ||
| ws: { trigger(data: any, url?: string): Promise<void> }, | ||
| nextState: Partial<QueueState> | ||
| ) => Promise<void> | ||
| } | ||
|
|
||
| test.describe('Queue UI', () => { | ||
| let queue: QueueController | ||
|
|
||
| test.beforeEach(async ({ comfyPage }) => { | ||
| await comfyPage.page.route('**/api/prompt', async (route) => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ | ||
| prompt_id: 'mock-prompt-id', | ||
| number: 1, | ||
| node_errors: {} | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| // Mock history to avoid pulling real data | ||
| await comfyPage.page.route('**/api/history**', async (route) => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ History: [] }) | ||
| }) | ||
| }) | ||
|
|
||
| queue = await createQueueController(comfyPage) | ||
| }) | ||
|
|
||
| test('toggles overlay and updates count from status events', async ({ | ||
| comfyPage, | ||
| ws | ||
| }) => { | ||
| await queue.sync(ws, { running: [], pending: [] }) | ||
|
|
||
| await expect(comfyPage.queueList.toggleButton).toContainText('0') | ||
| await expect(comfyPage.queueList.toggleButton).toContainText(/queued/i) | ||
| await expect(comfyPage.queueList.overlay).toBeHidden() | ||
|
|
||
| await queue.sync(ws, { | ||
| pending: [queueJob('1', 'mock-pending', 'client-a')] | ||
| }) | ||
|
|
||
| await expect(comfyPage.queueList.toggleButton).toContainText('1') | ||
| await expect(comfyPage.queueList.toggleButton).toContainText(/queued/i) | ||
|
|
||
| await comfyPage.queueList.open() | ||
| await expect(comfyPage.queueList.overlay).toBeVisible() | ||
| await expect(comfyPage.queueList.jobItems).toHaveCount(1) | ||
|
|
||
| await comfyPage.queueList.close() | ||
| await expect(comfyPage.queueList.overlay).toBeHidden() | ||
| }) | ||
|
Comment on lines
+59
to
+82
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 Good coverage of overlay toggle and count; consider asserting inline progress as well This test validates the count text, overlay visibility, and item rendering end‑to‑end via mocked WS + 🤖 Prompt for AI Agents |
||
|
|
||
| test('displays running and pending jobs via status updates', async ({ | ||
| comfyPage, | ||
| ws | ||
| }) => { | ||
| await queue.sync(ws, { | ||
| running: [queueJob('2', 'mock-running', 'client-b')], | ||
| pending: [queueJob('3', 'mock-pending', 'client-c')] | ||
| }) | ||
|
|
||
| await comfyPage.queueList.open() | ||
| await expect(comfyPage.queueList.jobItems).toHaveCount(2) | ||
|
|
||
| const firstJob = comfyPage.queueList.jobItems.first() | ||
| await firstJob.hover() | ||
|
|
||
| const cancelAction = firstJob | ||
| .getByTestId('job-action-cancel-running') | ||
| .or(firstJob.getByTestId('job-action-cancel-hover')) | ||
|
|
||
| await expect(cancelAction).toBeVisible() | ||
| }) | ||
| }) | ||
|
|
||
| const queueJob = ( | ||
| queueIndex: string, | ||
| promptId: string, | ||
| clientId: string | ||
| ): QueueJob => [ | ||
| queueIndex, | ||
| promptId, | ||
| { client_id: clientId }, | ||
| { class_type: 'Note' }, | ||
| ['output'] | ||
| ] | ||
|
|
||
| const createQueueController = async ( | ||
| comfyPage: ComfyPage | ||
| ): Promise<QueueController> => { | ||
| const state: QueueState = { running: [], pending: [] } | ||
|
|
||
| // Single queue handler reads the latest in-memory state | ||
| await comfyPage.page.route('**/api/queue', async (route) => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ | ||
| queue_running: state.running, | ||
| queue_pending: state.pending | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| const sync = async ( | ||
| ws: { trigger(data: any, url?: string): Promise<void> }, | ||
| nextState: Partial<QueueState> | ||
| ) => { | ||
| if (nextState.running) state.running = nextState.running | ||
| if (nextState.pending) state.pending = nextState.pending | ||
|
|
||
| const total = state.running.length + state.pending.length | ||
| const queueResponse = comfyPage.page.waitForResponse('**/api/queue') | ||
|
|
||
| await ws.trigger({ | ||
| type: 'status', | ||
| data: { | ||
| status: { exec_info: { queue_remaining: total } } | ||
| } | ||
| } as StatusWsMessage) | ||
|
|
||
| await queueResponse | ||
| } | ||
|
|
||
| return { state, sync } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.