Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
104 changes: 97 additions & 7 deletions src/components/TopMenuSection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { createTestingPinia } from '@pinia/testing'
import { mount } from '@vue/test-utils'
import type { MenuItem } from 'primevue/menuitem'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { computed, nextTick } from 'vue'
import { computed, defineComponent, h, nextTick } from 'vue'
import type { Component } from 'vue'
import { createI18n } from 'vue-i18n'

import TopMenuSection from '@/components/TopMenuSection.vue'
Expand Down Expand Up @@ -36,7 +37,17 @@ vi.mock('@/stores/firebaseAuthStore', () => ({
}))
}))

function createWrapper(pinia = createTestingPinia({ createSpy: vi.fn })) {
type WrapperOptions = {
pinia?: ReturnType<typeof createTestingPinia>
stubs?: Record<string, boolean | Component>
attachTo?: HTMLElement
}

function createWrapper({
pinia = createTestingPinia({ createSpy: vi.fn }),
stubs = {},
attachTo
}: WrapperOptions = {}) {
const i18n = createI18n({
legacy: false,
locale: 'en',
Expand All @@ -55,18 +66,21 @@ function createWrapper(pinia = createTestingPinia({ createSpy: vi.fn })) {
})

return mount(TopMenuSection, {
attachTo,
global: {
plugins: [pinia, i18n],
stubs: {
SubgraphBreadcrumb: true,
QueueProgressOverlay: true,
QueueInlineProgressSummary: true,
CurrentUserButton: true,
LoginButton: true,
ContextMenu: {
name: 'ContextMenu',
props: ['model'],
template: '<div />'
}
},
...stubs
},
directives: {
tooltip: () => {}
Expand All @@ -91,6 +105,7 @@ function createTask(id: string, status: JobStatus): TaskItemImpl {
describe('TopMenuSection', () => {
beforeEach(() => {
vi.resetAllMocks()
localStorage.clear()
})

describe('authentication state', () => {
Expand Down Expand Up @@ -151,7 +166,7 @@ describe('TopMenuSection', () => {
vi.mocked(settingStore.get).mockImplementation((key) =>
key === 'Comfy.Queue.QPOV2' ? true : undefined
)
const wrapper = createWrapper(pinia)
const wrapper = createWrapper({ pinia })

await nextTick()

Expand All @@ -169,7 +184,7 @@ describe('TopMenuSection', () => {
vi.mocked(settingStore.get).mockImplementation((key) =>
key === 'Comfy.Queue.QPOV2' ? false : undefined
)
const wrapper = createWrapper(pinia)
const wrapper = createWrapper({ pinia })
const commandStore = useCommandStore(pinia)

await wrapper.find('[data-testid="queue-overlay-toggle"]').trigger('click')
Expand All @@ -185,7 +200,7 @@ describe('TopMenuSection', () => {
vi.mocked(settingStore.get).mockImplementation((key) =>
key === 'Comfy.Queue.QPOV2' ? true : undefined
)
const wrapper = createWrapper(pinia)
const wrapper = createWrapper({ pinia })
const sidebarTabStore = useSidebarTabStore(pinia)

await wrapper.find('[data-testid="queue-overlay-toggle"]').trigger('click')
Expand All @@ -199,7 +214,7 @@ describe('TopMenuSection', () => {
vi.mocked(settingStore.get).mockImplementation((key) =>
key === 'Comfy.Queue.QPOV2' ? true : undefined
)
const wrapper = createWrapper(pinia)
const wrapper = createWrapper({ pinia })
const sidebarTabStore = useSidebarTabStore(pinia)
const toggleButton = wrapper.find('[data-testid="queue-overlay-toggle"]')

Expand All @@ -210,6 +225,81 @@ describe('TopMenuSection', () => {
expect(sidebarTabStore.activeSidebarTabId).toBe(null)
})

describe('inline progress summary', () => {
const configureSettings = (
pinia: ReturnType<typeof createTestingPinia>,
qpoV2Enabled: boolean
) => {
const settingStore = useSettingStore(pinia)
vi.mocked(settingStore.get).mockImplementation((key) => {
if (key === 'Comfy.Queue.QPOV2') return qpoV2Enabled
if (key === 'Comfy.UseNewMenu') return 'Top'
return undefined
})
}
Comment on lines +230 to +240

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using explicit return type annotation for type safety.

The configureSettings helper function lacks a return type annotation. While it implicitly returns void, adding an explicit annotation improves readability and catches accidental return value bugs.

♻️ Suggested improvement
-    const configureSettings = (
+    const configureSettings = (
       pinia: ReturnType<typeof createTestingPinia>,
       qpoV2Enabled: boolean
-    ) => {
+    ): void => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const configureSettings = (
pinia: ReturnType<typeof createTestingPinia>,
qpoV2Enabled: boolean
) => {
const settingStore = useSettingStore(pinia)
vi.mocked(settingStore.get).mockImplementation((key) => {
if (key === 'Comfy.Queue.QPOV2') return qpoV2Enabled
if (key === 'Comfy.UseNewMenu') return 'Top'
return undefined
})
}
const configureSettings = (
pinia: ReturnType<typeof createTestingPinia>,
qpoV2Enabled: boolean
): void => {
const settingStore = useSettingStore(pinia)
vi.mocked(settingStore.get).mockImplementation((key) => {
if (key === 'Comfy.Queue.QPOV2') return qpoV2Enabled
if (key === 'Comfy.UseNewMenu') return 'Top'
return undefined
})
}
🤖 Prompt for AI Agents
In `@src/components/TopMenuSection.test.ts` around lines 230 - 240, The helper
function configureSettings currently has no explicit return type; update its
signature to declare an explicit void return type (e.g., configureSettings(...):
void) to improve type safety and readability, and ensure any implementation
(including the vi.mocked(settingStore.get).mockImplementation callback) does not
accidentally return a value; reference configureSettings, useSettingStore, and
vi.mocked in your change so the type annotation and behavior are clear.


it('renders inline progress summary when QPO V2 is enabled', async () => {
const pinia = createTestingPinia({ createSpy: vi.fn })
configureSettings(pinia, true)

const wrapper = createWrapper({ pinia })

await nextTick()

expect(
wrapper.findComponent({ name: 'QueueInlineProgressSummary' }).exists()
).toBe(true)
})

it('does not render inline progress summary when QPO V2 is disabled', async () => {
const pinia = createTestingPinia({ createSpy: vi.fn })
configureSettings(pinia, false)

const wrapper = createWrapper({ pinia })

await nextTick()

expect(
wrapper.findComponent({ name: 'QueueInlineProgressSummary' }).exists()
).toBe(false)
})

it('teleports inline progress summary when actionbar is floating', async () => {
localStorage.setItem('Comfy.MenuPosition.Docked', 'false')
const actionbarTarget = document.createElement('div')
document.body.appendChild(actionbarTarget)
const pinia = createTestingPinia({ createSpy: vi.fn })
configureSettings(pinia, true)

const ComfyActionbarStub = defineComponent({
name: 'ComfyActionbar',
setup(_, { expose }) {
expose({ panelElement: actionbarTarget })
return () => h('div')
}
})

const wrapper = createWrapper({
pinia,
attachTo: document.body,
stubs: {
ComfyActionbar: ComfyActionbarStub
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
})

try {
await nextTick()

expect(
actionbarTarget.querySelector('queue-inline-progress-summary-stub')
).not.toBeNull()
} finally {
wrapper.unmount()
actionbarTarget.remove()
}
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
})

it('disables the clear queue context menu item when no queued jobs exist', () => {
const wrapper = createWrapper()
const menu = wrapper.findComponent({ name: 'ContextMenu' })
Expand Down
Loading