test: simplify test file mocking patterns (#8320)

Simplifies test mocking patterns across multiple test files.

- Removes redundant `vi.hoisted()` calls
- Cleans up mock implementations
- Removes unused imports and variables

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8320-test-simplify-test-file-mocking-patterns-2f46d73d36508150981bd8ecb99a6a11)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
Alexander Brown
2026-01-28 12:17:16 -08:00
committed by GitHub
parent 3e2352423b
commit e44b411ff6
24 changed files with 627 additions and 511 deletions

View File

@@ -1,9 +1,17 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { extractWorkflow } from '@/platform/remote/comfyui/jobs/fetchJobs'
import { api } from '@/scripts/api'
import type {
JobDetail,
JobListItem
} from '@/platform/remote/comfyui/jobs/jobTypes'
import {
findActiveIndex,
getJobDetail,
getJobWorkflow,
getOutputsForTask
} from '@/services/jobOutputCache'
import { ResultItemImpl, TaskItemImpl } from '@/stores/queueStore'
vi.mock('@/platform/remote/comfyui/jobs/fetchJobs', () => ({
@@ -11,6 +19,15 @@ vi.mock('@/platform/remote/comfyui/jobs/fetchJobs', () => ({
extractWorkflow: vi.fn()
}))
vi.mock('@/scripts/api', () => ({
api: {
getJobDetail: vi.fn(),
apiURL: vi.fn((path: string) => `/api${path}`),
addEventListener: vi.fn(),
removeEventListener: vi.fn()
}
}))
function createResultItem(url: string, supportsPreview = true): ResultItemImpl {
const item = new ResultItemImpl({
filename: url,
@@ -48,15 +65,19 @@ function createTask(
return new TaskItemImpl(job, {}, flatOutputs)
}
// Generate unique IDs per test to avoid cache collisions
let testCounter = 0
function uniqueId(prefix: string): string {
return `${prefix}-${++testCounter}-${Date.now()}`
}
describe('jobOutputCache', () => {
beforeEach(() => {
vi.resetModules()
vi.clearAllMocks()
})
describe('findActiveIndex', () => {
it('returns index of matching URL', async () => {
const { findActiveIndex } = await import('@/services/jobOutputCache')
it('returns index of matching URL', () => {
const items = [
createResultItem('a'),
createResultItem('b'),
@@ -66,15 +87,13 @@ describe('jobOutputCache', () => {
expect(findActiveIndex(items, 'b')).toBe(1)
})
it('returns 0 when URL not found', async () => {
const { findActiveIndex } = await import('@/services/jobOutputCache')
it('returns 0 when URL not found', () => {
const items = [createResultItem('a'), createResultItem('b')]
expect(findActiveIndex(items, 'missing')).toBe(0)
})
it('returns 0 when URL is undefined', async () => {
const { findActiveIndex } = await import('@/services/jobOutputCache')
it('returns 0 when URL is undefined', () => {
const items = [createResultItem('a'), createResultItem('b')]
expect(findActiveIndex(items, undefined)).toBe(0)
@@ -83,7 +102,6 @@ describe('jobOutputCache', () => {
describe('getOutputsForTask', () => {
it('returns previewable outputs directly when no lazy load needed', async () => {
const { getOutputsForTask } = await import('@/services/jobOutputCache')
const outputs = [createResultItem('p-1'), createResultItem('p-2')]
const task = createTask(undefined, outputs, 1)
@@ -93,14 +111,13 @@ describe('jobOutputCache', () => {
})
it('lazy loads when outputsCount > 1', async () => {
const { getOutputsForTask } = await import('@/services/jobOutputCache')
const previewOutput = createResultItem('preview')
const fullOutputs = [
createResultItem('full-1'),
createResultItem('full-2')
]
const job = createMockJob('task-1', 3)
const job = createMockJob(uniqueId('task'), 3)
const task = new TaskItemImpl(job, {}, [previewOutput])
const loadedTask = new TaskItemImpl(job, {}, fullOutputs)
task.loadFullOutputs = vi.fn().mockResolvedValue(loadedTask)
@@ -112,10 +129,9 @@ describe('jobOutputCache', () => {
})
it('caches loaded tasks', async () => {
const { getOutputsForTask } = await import('@/services/jobOutputCache')
const fullOutputs = [createResultItem('full-1')]
const job = createMockJob('task-1', 3)
const job = createMockJob(uniqueId('task'), 3)
const task = new TaskItemImpl(job, {}, [createResultItem('preview')])
const loadedTask = new TaskItemImpl(job, {}, fullOutputs)
task.loadFullOutputs = vi.fn().mockResolvedValue(loadedTask)
@@ -130,10 +146,9 @@ describe('jobOutputCache', () => {
})
it('falls back to preview outputs on load error', async () => {
const { getOutputsForTask } = await import('@/services/jobOutputCache')
const previewOutput = createResultItem('preview')
const job = createMockJob('task-1', 3)
const job = createMockJob(uniqueId('task'), 3)
const task = new TaskItemImpl(job, {}, [previewOutput])
task.loadFullOutputs = vi
.fn()
@@ -145,9 +160,8 @@ describe('jobOutputCache', () => {
})
it('returns null when request is superseded', async () => {
const { getOutputsForTask } = await import('@/services/jobOutputCache')
const job1 = createMockJob('task-1', 3)
const job2 = createMockJob('task-2', 3)
const job1 = createMockJob(uniqueId('task'), 3)
const job2 = createMockJob(uniqueId('task'), 3)
const task1 = new TaskItemImpl(job1, {}, [createResultItem('preview-1')])
const task2 = new TaskItemImpl(job2, {}, [createResultItem('preview-2')])
@@ -182,57 +196,51 @@ describe('jobOutputCache', () => {
describe('getJobDetail', () => {
it('fetches and caches job detail', async () => {
const { getJobDetail } = await import('@/services/jobOutputCache')
const { fetchJobDetail } =
await import('@/platform/remote/comfyui/jobs/fetchJobs')
const jobId = uniqueId('job')
const mockDetail: JobDetail = {
id: 'job-1',
id: jobId,
status: 'completed',
create_time: Date.now(),
priority: 0,
outputs: {}
}
vi.mocked(fetchJobDetail).mockResolvedValue(mockDetail)
vi.mocked(api.getJobDetail).mockResolvedValue(mockDetail)
const result = await getJobDetail('job-1')
const result = await getJobDetail(jobId)
expect(result).toEqual(mockDetail)
expect(fetchJobDetail).toHaveBeenCalledWith(expect.any(Function), 'job-1')
expect(api.getJobDetail).toHaveBeenCalledWith(jobId)
})
it('returns cached job detail on subsequent calls', async () => {
const { getJobDetail } = await import('@/services/jobOutputCache')
const { fetchJobDetail } =
await import('@/platform/remote/comfyui/jobs/fetchJobs')
const jobId = uniqueId('job')
const mockDetail: JobDetail = {
id: 'job-2',
id: jobId,
status: 'completed',
create_time: Date.now(),
priority: 0,
outputs: {}
}
vi.mocked(fetchJobDetail).mockResolvedValue(mockDetail)
vi.mocked(api.getJobDetail).mockResolvedValue(mockDetail)
// First call
await getJobDetail('job-2')
expect(fetchJobDetail).toHaveBeenCalledTimes(1)
await getJobDetail(jobId)
expect(api.getJobDetail).toHaveBeenCalledTimes(1)
// Second call should use cache
const result = await getJobDetail('job-2')
const result = await getJobDetail(jobId)
expect(result).toEqual(mockDetail)
expect(fetchJobDetail).toHaveBeenCalledTimes(1)
expect(api.getJobDetail).toHaveBeenCalledTimes(1)
})
it('returns undefined on fetch error', async () => {
const { getJobDetail } = await import('@/services/jobOutputCache')
const { fetchJobDetail } =
await import('@/platform/remote/comfyui/jobs/fetchJobs')
const jobId = uniqueId('job-error')
vi.mocked(fetchJobDetail).mockRejectedValue(new Error('Network error'))
vi.mocked(api.getJobDetail).mockRejectedValue(new Error('Network error'))
const result = await getJobDetail('job-error')
const result = await getJobDetail(jobId)
expect(result).toBeUndefined()
})
@@ -240,12 +248,10 @@ describe('jobOutputCache', () => {
describe('getJobWorkflow', () => {
it('fetches job detail and extracts workflow', async () => {
const { getJobWorkflow } = await import('@/services/jobOutputCache')
const { fetchJobDetail, extractWorkflow } =
await import('@/platform/remote/comfyui/jobs/fetchJobs')
const jobId = uniqueId('job-wf')
const mockDetail: JobDetail = {
id: 'job-wf',
id: jobId,
status: 'completed',
create_time: Date.now(),
priority: 0,
@@ -253,24 +259,22 @@ describe('jobOutputCache', () => {
}
const mockWorkflow = { version: 1 }
vi.mocked(fetchJobDetail).mockResolvedValue(mockDetail)
vi.mocked(api.getJobDetail).mockResolvedValue(mockDetail)
vi.mocked(extractWorkflow).mockResolvedValue(mockWorkflow as any)
const result = await getJobWorkflow('job-wf')
const result = await getJobWorkflow(jobId)
expect(result).toEqual(mockWorkflow)
expect(extractWorkflow).toHaveBeenCalledWith(mockDetail)
})
it('returns undefined when job detail not found', async () => {
const { getJobWorkflow } = await import('@/services/jobOutputCache')
const { fetchJobDetail, extractWorkflow } =
await import('@/platform/remote/comfyui/jobs/fetchJobs')
const jobId = uniqueId('missing')
vi.mocked(fetchJobDetail).mockResolvedValue(undefined)
vi.mocked(api.getJobDetail).mockResolvedValue(undefined)
vi.mocked(extractWorkflow).mockResolvedValue(undefined)
const result = await getJobWorkflow('missing')
const result = await getJobWorkflow(jobId)
expect(result).toBeUndefined()
})

View File

@@ -1,74 +0,0 @@
import type { useSettingStore } from '@/platform/settings/settingStore'
let pendingCallbacks: Array<() => Promise<void>> = []
let isNewUserDetermined = false
let isNewUserCached: boolean | null = null
export const newUserService = () => {
function checkIsNewUser(
settingStore: ReturnType<typeof useSettingStore>
): boolean {
const isNewUserSettings =
Object.keys(settingStore.settingValues).length === 0 ||
!settingStore.get('Comfy.TutorialCompleted')
const hasNoWorkflow = !localStorage.getItem('workflow')
const hasNoPreviousWorkflow = !localStorage.getItem(
'Comfy.PreviousWorkflow'
)
return isNewUserSettings && hasNoWorkflow && hasNoPreviousWorkflow
}
async function registerInitCallback(callback: () => Promise<void>) {
if (isNewUserDetermined) {
if (isNewUserCached) {
try {
await callback()
} catch (error) {
console.error('New user initialization callback failed:', error)
}
}
} else {
pendingCallbacks.push(callback)
}
}
async function initializeIfNewUser(
settingStore: ReturnType<typeof useSettingStore>
) {
if (isNewUserDetermined) return
isNewUserCached = checkIsNewUser(settingStore)
isNewUserDetermined = true
if (!isNewUserCached) {
pendingCallbacks = []
return
}
await settingStore.set(
'Comfy.InstalledVersion',
__COMFYUI_FRONTEND_VERSION__
)
for (const callback of pendingCallbacks) {
try {
await callback()
} catch (error) {
console.error('New user initialization callback failed:', error)
}
}
pendingCallbacks = []
}
function isNewUser(): boolean | null {
return isNewUserDetermined ? isNewUserCached : null
}
return {
registerInitCallback,
initializeIfNewUser,
isNewUser
}
}

View File

@@ -7,6 +7,12 @@ const mockLocalStorage = vi.hoisted(() => ({
clear: vi.fn()
}))
const mockSettingStore = vi.hoisted(() => ({
settingValues: {} as Record<string, unknown>,
get: vi.fn(),
set: vi.fn()
}))
Object.defineProperty(window, 'localStorage', {
value: mockLocalStorage,
writable: true
@@ -16,31 +22,26 @@ vi.mock('@/config/version', () => ({
__COMFYUI_FRONTEND_VERSION__: '1.24.0'
}))
vi.mock('@/platform/settings/settingStore', () => ({
useSettingStore: () => mockSettingStore
}))
//@ts-expect-error Define global for the test
global.__COMFYUI_FRONTEND_VERSION__ = '1.24.0'
import type { newUserService as NewUserServiceType } from '@/services/newUserService'
import { useNewUserService } from '@/services/useNewUserService'
describe('newUserService', () => {
let service: ReturnType<typeof NewUserServiceType>
let mockSettingStore: any
let newUserService: typeof NewUserServiceType
describe('useNewUserService', () => {
let service: ReturnType<typeof useNewUserService>
beforeEach(async () => {
beforeEach(() => {
vi.clearAllMocks()
mockSettingStore.settingValues = {}
mockSettingStore.get.mockReset()
mockSettingStore.set.mockReset()
vi.resetModules()
const module = await import('@/services/newUserService')
newUserService = module.newUserService
service = newUserService()
mockSettingStore = {
settingValues: {},
get: vi.fn(),
set: vi.fn()
}
service = useNewUserService()
service.reset()
mockLocalStorage.getItem.mockReturnValue(null)
})
@@ -54,7 +55,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(service.isNewUser()).toBe(true)
})
@@ -69,7 +70,7 @@ describe('newUserService', () => {
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(service.isNewUser()).toBe(true)
})
@@ -82,7 +83,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(service.isNewUser()).toBe(false)
})
@@ -98,7 +99,7 @@ describe('newUserService', () => {
return null
})
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(service.isNewUser()).toBe(false)
})
@@ -114,7 +115,7 @@ describe('newUserService', () => {
return null
})
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(service.isNewUser()).toBe(false)
})
@@ -127,7 +128,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(service.isNewUser()).toBe(true)
})
@@ -143,7 +144,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(service.isNewUser()).toBe(false)
})
@@ -160,7 +161,7 @@ describe('newUserService', () => {
return null
})
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(service.isNewUser()).toBe(false)
})
@@ -177,7 +178,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(service.isNewUser()).toBe(true)
await service.registerInitCallback(mockCallback)
@@ -207,7 +208,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
await service.registerInitCallback(mockCallback)
@@ -228,7 +229,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(mockSettingStore.set).toHaveBeenCalledWith(
'Comfy.InstalledVersion',
@@ -244,7 +245,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(mockSettingStore.set).not.toHaveBeenCalled()
})
@@ -263,7 +264,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(mockCallback1).toHaveBeenCalledTimes(1)
expect(mockCallback2).toHaveBeenCalledTimes(1)
@@ -281,7 +282,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(mockCallback).not.toHaveBeenCalled()
})
@@ -299,7 +300,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(consoleSpy).toHaveBeenCalledWith(
'New user initialization callback failed:',
@@ -316,10 +317,10 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(mockSettingStore.set).toHaveBeenCalledTimes(1)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(mockSettingStore.set).toHaveBeenCalledTimes(1)
})
@@ -331,15 +332,12 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
// Before initialization, isNewUser should return null
expect(service.isNewUser()).toBeNull()
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
// After initialization, isNewUser should return true for a new user
expect(service.isNewUser()).toBe(true)
// Should set the installed version for new users
expect(mockSettingStore.set).toHaveBeenCalledWith(
'Comfy.InstalledVersion',
expect.any(String)
@@ -357,7 +355,7 @@ describe('newUserService', () => {
mockSettingStore.get.mockReturnValue(undefined)
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(service.isNewUser()).toBe(true)
})
@@ -372,7 +370,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
expect(service.isNewUser()).toBe(true)
})
@@ -388,7 +386,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service.initializeIfNewUser(mockSettingStore)
await service.initializeIfNewUser()
await service.registerInitCallback(mockCallback1)
await service.registerInitCallback(mockCallback2)
@@ -399,9 +397,9 @@ describe('newUserService', () => {
})
describe('state sharing between instances', () => {
it('should share state between multiple service instances', async () => {
const service1 = newUserService()
const service2 = newUserService()
it('should share state between multiple service calls', async () => {
const service1 = useNewUserService()
const service2 = useNewUserService()
mockSettingStore.settingValues = {}
mockSettingStore.get.mockImplementation((key: string) => {
@@ -410,15 +408,15 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service1.initializeIfNewUser(mockSettingStore)
await service1.initializeIfNewUser()
expect(service2.isNewUser()).toBe(true)
expect(service1.isNewUser()).toBe(service2.isNewUser())
})
it('should execute callbacks registered on different instances', async () => {
const service1 = newUserService()
const service2 = newUserService()
it('should execute callbacks registered on different service calls', async () => {
const service1 = useNewUserService()
const service2 = useNewUserService()
const mockCallback1 = vi.fn().mockResolvedValue(undefined)
const mockCallback2 = vi.fn().mockResolvedValue(undefined)
@@ -433,7 +431,7 @@ describe('newUserService', () => {
})
mockLocalStorage.getItem.mockReturnValue(null)
await service1.initializeIfNewUser(mockSettingStore)
await service1.initializeIfNewUser()
expect(mockCallback1).toHaveBeenCalledTimes(1)
expect(mockCallback2).toHaveBeenCalledTimes(1)

View File

@@ -0,0 +1,82 @@
import { ref, shallowRef } from 'vue'
import { createSharedComposable } from '@vueuse/core'
import { useSettingStore } from '@/platform/settings/settingStore'
function _useNewUserService() {
const settingStore = useSettingStore()
const pendingCallbacks = shallowRef<Array<() => Promise<void>>>([])
const isNewUserDetermined = ref(false)
const isNewUserCached = ref<boolean | null>(null)
function reset() {
pendingCallbacks.value = []
isNewUserDetermined.value = false
isNewUserCached.value = null
}
function checkIsNewUser(): boolean {
const isNewUserSettings =
Object.keys(settingStore.settingValues).length === 0 ||
!settingStore.get('Comfy.TutorialCompleted')
const hasNoWorkflow = !localStorage.getItem('workflow')
const hasNoPreviousWorkflow = !localStorage.getItem(
'Comfy.PreviousWorkflow'
)
return isNewUserSettings && hasNoWorkflow && hasNoPreviousWorkflow
}
async function registerInitCallback(callback: () => Promise<void>) {
if (isNewUserDetermined.value) {
if (isNewUserCached.value) {
try {
await callback()
} catch (error) {
console.error('New user initialization callback failed:', error)
}
}
} else {
pendingCallbacks.value = [...pendingCallbacks.value, callback]
}
}
async function initializeIfNewUser() {
if (isNewUserDetermined.value) return
isNewUserCached.value = checkIsNewUser()
isNewUserDetermined.value = true
if (!isNewUserCached.value) {
pendingCallbacks.value = []
return
}
await settingStore.set(
'Comfy.InstalledVersion',
__COMFYUI_FRONTEND_VERSION__
)
for (const callback of pendingCallbacks.value) {
try {
await callback()
} catch (error) {
console.error('New user initialization callback failed:', error)
}
}
pendingCallbacks.value = []
}
function isNewUser(): boolean | null {
return isNewUserDetermined.value ? isNewUserCached.value : null
}
return {
registerInitCallback,
initializeIfNewUser,
isNewUser,
reset
}
}
export const useNewUserService = createSharedComposable(_useNewUserService)