Skip to content
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

[BrowserTest] Fix flaky gallery test #2150

Merged
merged 7 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added browser_tests/assets/image32x32.webp
Binary file not shown.
Binary file added browser_tests/assets/image64x64.webp
Binary file not shown.
26 changes: 20 additions & 6 deletions browser_tests/fixtures/components/SidebarTab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,10 @@ export class QueueSidebarTab extends SidebarTab {
return this.root.locator('.no-results-placeholder')
}

get galleryImage() {
return this.page.locator('.galleria-image')
}

private getToggleExpandButton(isExpanded: boolean) {
const iconSelector = isExpanded ? '.pi-image' : '.pi-images'
return this.root.locator(`.toggle-expanded-button ${iconSelector}`)
Expand Down Expand Up @@ -249,14 +253,24 @@ export class QueueSidebarTab extends SidebarTab {

async openTaskPreview(taskIndex: number) {
const previewButton = this.getTaskPreviewButton(taskIndex)
await previewButton.hover()
await previewButton.click()
return this.getGalleryImage(taskIndex).waitFor({ state: 'visible' })
return this.galleryImage.waitFor({ state: 'visible' })
}

getGalleryImage(imageFilename: string) {
return this.galleryImage.and(this.page.getByAltText(imageFilename))
}

getGalleryImage(galleryItemIndex: number) {
// Aria labels of Galleria items are 1-based indices
const galleryLabel = `${galleryItemIndex + 1}`
return this.page.getByLabel(galleryLabel).locator('.galleria-image')
getTaskImage(imageFilename: string) {
return this.tasks.getByAltText(imageFilename)
}

/** Trigger the queue store and tasks to update */
async triggerTasksUpdate() {
await this.page.evaluate(() => {
window['app']['api'].dispatchCustomEvent('status', {
exec_info: { queue_remaining: 0 }
})
})
}
}
7 changes: 4 additions & 3 deletions browser_tests/fixtures/utils/taskHistory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,12 @@ export default class TaskHistory {
private addTask(task: HistoryTaskItem) {
setPromptId(task)
setQueueIndex(task)
this.tasks.push(task)
this.tasks.unshift(task) // Tasks are added to the front of the queue
}

clearTasks() {
clearTasks(): this {
this.tasks = []
return this
}

withTask(
Expand All @@ -155,7 +156,7 @@ export default class TaskHistory {
/** Repeats the last task in the task history a specified number of times. */
repeat(n: number): this {
for (let i = 0; i < n; i++)
this.addTask(structuredClone(this.tasks.at(-1)) as HistoryTaskItem)
this.addTask(structuredClone(this.tasks.at(0)) as HistoryTaskItem)
return this
}
}
70 changes: 31 additions & 39 deletions browser_tests/menu.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { expect, mergeTests } from '@playwright/test'
import { expect } from '@playwright/test'

import { comfyPageFixture } from './fixtures/ComfyPage'
import { webSocketFixture } from './fixtures/ws'

const test = mergeTests(comfyPageFixture, webSocketFixture)
import { comfyPageFixture as test } from './fixtures/ComfyPage'

test.describe('Menu', () => {
test.beforeEach(async ({ comfyPage }) => {
Expand Down Expand Up @@ -878,66 +875,61 @@ test.describe('Queue sidebar', () => {
})

test.describe('Gallery', () => {
const firstImage = 'example.webp'
const secondImage = 'image32x32.webp'

test.beforeEach(async ({ comfyPage }) => {
await comfyPage
.setupHistory()
.withTask(['example.webp'])
.repeat(1)
.withTask([secondImage])
.withTask([firstImage])
.setupRoutes()
await comfyPage.menu.queueTab.open()
await comfyPage.menu.queueTab.waitForTasks()
await comfyPage.menu.queueTab.openTaskPreview(0)
})

test('displays gallery image after opening task preview', async ({
comfyPage
}) => {
await comfyPage.menu.queueTab.openTaskPreview(0)
expect(comfyPage.menu.queueTab.getGalleryImage(0)).toBeVisible()
await comfyPage.nextFrame()
expect(comfyPage.menu.queueTab.getGalleryImage(firstImage)).toBeVisible()
})

test('should maintain active gallery item when new tasks are added', async ({
comfyPage,
ws
test('maintains active gallery item when new tasks are added', async ({
comfyPage
}) => {
const initialIndex = 0
await comfyPage.menu.queueTab.openTaskPreview(initialIndex)

// Add a new task while the gallery is still open
comfyPage.setupHistory().withTask(['example.webp'])
await ws.trigger({
type: 'status',
data: {
status: { exec_info: { queue_remaining: 0 } }
}
})
await comfyPage.menu.queueTab.waitForTasks()

// The index of all tasks increments when a new task is prepended
const expectIndex = initialIndex + 1
expect(comfyPage.menu.queueTab.getGalleryImage(expectIndex)).toBeVisible()
const newImage = 'image64x64.webp'
comfyPage.setupHistory().withTask([newImage])
await comfyPage.menu.queueTab.triggerTasksUpdate()
await comfyPage.page.waitForTimeout(500)
const newTask = comfyPage.menu.queueTab.tasks.getByAltText(newImage)
await newTask.waitFor({ state: 'visible' })
// The active gallery item should still be the initial image
expect(comfyPage.menu.queueTab.getGalleryImage(firstImage)).toBeVisible()
})

test.describe('Gallery navigation', () => {
const paths: {
description: string
path: ('Right' | 'Left')[]
expectIndex: number
end: string
}[] = [
{ description: 'Right', path: ['Right'], expectIndex: 1 },
{ description: 'Left', path: ['Right', 'Left'], expectIndex: 0 },
{ description: 'Right wrap', path: ['Right', 'Right'], expectIndex: 0 },
{ description: 'Left wrap', path: ['Left'], expectIndex: 1 }
{ description: 'Right', path: ['Right'], end: secondImage },
{ description: 'Left', path: ['Right', 'Left'], end: firstImage },
{ description: 'Left wrap', path: ['Left'], end: secondImage },
{ description: 'Right wrap', path: ['Right', 'Right'], end: firstImage }
]

paths.forEach(({ description, path, expectIndex }) => {
paths.forEach(({ description, path, end }) => {
test(`can navigate gallery ${description}`, async ({ comfyPage }) => {
await comfyPage.menu.queueTab.openTaskPreview(0)
for (const direction of path)
await comfyPage.page.keyboard.press(`Arrow${direction}`)

expect(
comfyPage.menu.queueTab.getGalleryImage(expectIndex)
).toBeVisible()
await comfyPage.page.keyboard.press(`Arrow${direction}`, {
delay: 256
})
await comfyPage.nextFrame()
expect(comfyPage.menu.queueTab.getGalleryImage(end)).toBeVisible()
})
})
})
Expand Down
Loading