Skip to content

Commit c9440fc

Browse files
author
Connor Byrne
committed
test: add failing repro for run acknowledgement queued behind outcome banners
A queue notification banner holds the screen for 4s and new notifications are appended to a FIFO. A run the user just triggered is therefore not acknowledged until every already-queued outcome banner has drained, so clicking Run after a batch of failures looks like it did nothing.
1 parent 2ab67a6 commit c9440fc

2 files changed

Lines changed: 118 additions & 3 deletions

File tree

browser_tests/tests/queueNotificationBanners.spec.ts

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,16 @@
11
import type { Page } from '@playwright/test'
2-
import { expect } from '@playwright/test'
3-
4-
import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage'
2+
import { expect, mergeTests } from '@playwright/test'
3+
4+
import { comfyPageFixture } from '@e2e/fixtures/ComfyPage'
5+
import { ExecutionHelper } from '@e2e/fixtures/helpers/ExecutionHelper'
6+
import {
7+
createRouteMockJob,
8+
jobsRouteFixture
9+
} from '@e2e/fixtures/jobsRouteFixture'
510
import { TestIds } from '@e2e/fixtures/selectors'
11+
import { webSocketFixture } from '@e2e/fixtures/ws'
12+
13+
const test = mergeTests(comfyPageFixture, webSocketFixture, jobsRouteFixture)
614

715
// Mirrors BANNER_DISMISS_DELAY_MS in src/composables/queue/useQueueNotificationBanners.ts.
816
// Duplicated here to avoid pulling production source (and its litegraph
@@ -147,6 +155,37 @@ test.describe('Queue notification banners', { tag: ['@ui'] }, () => {
147155
})
148156
})
149157

158+
test.describe('Run acknowledgement priority', () => {
159+
test('a new run is acknowledged over an outcome banner still on screen', async ({
160+
comfyPage,
161+
getWebSocket,
162+
jobsRoutes
163+
}) => {
164+
const failedJobId = 'failed-job-1'
165+
await jobsRoutes.mockJobsHistory([
166+
createRouteMockJob({
167+
id: failedJobId,
168+
status: 'failed',
169+
execution_end_time: Date.now()
170+
})
171+
])
172+
173+
const exec = new ExecutionHelper(comfyPage, await getWebSocket())
174+
exec.executionStart(failedJobId)
175+
exec.executionError(failedJobId, '1', 'boom')
176+
exec.status(0)
177+
178+
const banner = bannerLocator(comfyPage.page)
179+
await expect(banner).toContainText('failed', {
180+
timeout: BANNER_ASSERT_TIMEOUT_MS
181+
})
182+
183+
await dispatchPromptQueueing(comfyPage.page)
184+
185+
await expect(banner).toContainText('queuing')
186+
})
187+
})
188+
150189
test.describe('Direct queued event (no pending predecessor)', () => {
151190
test('promptQueued without prior queueing shows queued banner directly', async ({
152191
comfyPage

src/composables/queue/useQueueNotificationBanners.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,4 +343,80 @@ describe(useQueueNotificationBanners, () => {
343343
unmount()
344344
}
345345
})
346+
347+
it('acknowledges a new run over an outcome notification still on screen', async () => {
348+
const { unmount, composable } = mountComposable()
349+
350+
try {
351+
await runBatch({
352+
start: 5_000,
353+
finish: 5_100,
354+
tasks: [createTask({ state: 'Failed', ts: 5_050 })]
355+
})
356+
357+
expect(composable.currentNotification.value).toEqual({
358+
type: 'failed',
359+
count: 1
360+
})
361+
362+
mockApi.dispatchEvent(
363+
new CustomEvent('promptQueueing', {
364+
detail: { requestId: 7, batchCount: 1 }
365+
})
366+
)
367+
await nextTick()
368+
369+
expect(composable.currentNotification.value).toEqual({
370+
type: 'queuedPending',
371+
count: 1,
372+
requestId: 7
373+
})
374+
} finally {
375+
unmount()
376+
}
377+
})
378+
379+
it('acknowledges a new run ahead of outcome notifications still waiting', async () => {
380+
const { unmount, composable } = mountComposable()
381+
382+
try {
383+
await runBatch({
384+
start: 6_000,
385+
finish: 6_100,
386+
tasks: [
387+
createTask({ ts: 6_050 }),
388+
createTask({ state: 'Failed', ts: 6_060 })
389+
]
390+
})
391+
392+
expect(composable.currentNotification.value).toEqual({
393+
type: 'completed',
394+
count: 1,
395+
thumbnailUrls: []
396+
})
397+
398+
mockApi.dispatchEvent(
399+
new CustomEvent('promptQueueing', {
400+
detail: { requestId: 8, batchCount: 1 }
401+
})
402+
)
403+
await nextTick()
404+
405+
expect(composable.currentNotification.value).toEqual({
406+
type: 'queuedPending',
407+
count: 1,
408+
requestId: 8
409+
})
410+
411+
await vi.advanceTimersByTimeAsync(4000)
412+
await nextTick()
413+
414+
expect(composable.currentNotification.value).toEqual({
415+
type: 'failed',
416+
count: 1
417+
})
418+
} finally {
419+
unmount()
420+
}
421+
})
346422
})

0 commit comments

Comments
 (0)