diff --git a/docs/testing/unit-testing.md b/docs/testing/unit-testing.md index a17592643..9be403659 100644 --- a/docs/testing/unit-testing.md +++ b/docs/testing/unit-testing.md @@ -11,6 +11,7 @@ This guide covers patterns and examples for unit testing utilities, composables, 5. [Mocking Utility Functions](#mocking-utility-functions) 6. [Testing with Debounce and Throttle](#testing-with-debounce-and-throttle) 7. [Mocking Node Definitions](#mocking-node-definitions) +8. [Mocking Composables with Reactive State](#mocking-composables-with-reactive-state) ## Testing Vue Composables with Reactivity @@ -253,3 +254,79 @@ it('should validate node definition', () => { expect(validateComfyNodeDef(EXAMPLE_NODE_DEF)).not.toBeNull() }) ``` + +## Mocking Composables with Reactive State + +When mocking composables that return reactive refs, define the mock implementation inline in `vi.mock()`'s factory function. This ensures stable singleton instances across all test invocations. + +### Rules + +1. **Define mocks in the factory function** — Create `vi.fn()` and `ref()` instances directly inside `vi.mock()`, not in `beforeEach` +2. **Use singleton pattern** — The factory runs once; all calls to the composable return the same mock object +3. **Access mocks per-test** — Call the composable directly in each test to get the singleton instance rather than storing in a shared variable +4. **Wrap in `vi.mocked()` for type safety** — Use `vi.mocked(service.method).mockResolvedValue(...)` when configuring +5. **Rely on `vi.resetAllMocks()`** — Resets call counts without recreating instances; ref values may need manual reset if mutated + +### Pattern + +```typescript +// Example from: src/platform/updates/common/releaseStore.test.ts +import { ref } from 'vue' + +vi.mock('@/path/to/composable', () => { + const doSomething = vi.fn() + const isLoading = ref(false) + const error = ref(null) + return { + useMyComposable: () => ({ + doSomething, + isLoading, + error + }) + } +}) + +describe('MyStore', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('should call the composable method', async () => { + const service = useMyComposable() + vi.mocked(service.doSomething).mockResolvedValue({ data: 'test' }) + + await store.initialize() + + expect(service.doSomething).toHaveBeenCalledWith(expectedArgs) + }) + + it('should handle errors from the composable', async () => { + const service = useMyComposable() + vi.mocked(service.doSomething).mockResolvedValue(null) + service.error.value = 'Something went wrong' + + await store.initialize() + + expect(store.error).toBe('Something went wrong') + }) +}) +``` + +### Anti-patterns + +```typescript +// ❌ Don't configure mock return values in beforeEach with shared variable +let mockService: { doSomething: Mock } +beforeEach(() => { + mockService = { doSomething: vi.fn() } + vi.mocked(useMyComposable).mockReturnValue(mockService) +}) + +// ❌ Don't auto-mock then override — reactive refs won't work correctly +vi.mock('@/path/to/composable') +vi.mocked(useMyComposable).mockReturnValue({ isLoading: ref(false) }) +``` + +``` + +``` diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue index 08e7573a9..27c038076 100644 --- a/src/components/graph/GraphCanvas.vue +++ b/src/components/graph/GraphCanvas.vue @@ -149,7 +149,7 @@ import { app as comfyApp } from '@/scripts/app' import { ChangeTracker } from '@/scripts/changeTracker' import { IS_CONTROL_WIDGET, updateControlWidgetLabel } from '@/scripts/widgets' import { useColorPaletteService } from '@/services/colorPaletteService' -import { newUserService } from '@/services/newUserService' +import { useNewUserService } from '@/services/useNewUserService' import { storeToRefs } from 'pinia' import { useBootstrapStore } from '@/stores/bootstrapStore' @@ -457,11 +457,9 @@ onMounted(async () => { // Register core settings immediately after settings are ready CORE_SETTINGS.forEach(settingStore.addSetting) - // Wait for both i18n and newUserService in parallel - // (newUserService only needs settings, not i18n) await Promise.all([ until(() => isI18nReady.value || !!i18nError.value).toBe(true), - newUserService().initializeIfNewUser(settingStore) + useNewUserService().initializeIfNewUser() ]) if (i18nError.value) { console.warn( diff --git a/src/components/graph/SelectionToolbox.test.ts b/src/components/graph/SelectionToolbox.test.ts index 441ca027f..bbbe9d786 100644 --- a/src/components/graph/SelectionToolbox.test.ts +++ b/src/components/graph/SelectionToolbox.test.ts @@ -13,6 +13,8 @@ import { createMockCanvas, createMockPositionable } from '@/utils/__tests__/litegraphTestUtils' +import * as litegraphUtil from '@/utils/litegraphUtil' +import * as nodeFilterUtil from '@/utils/nodeFilterUtil' function createMockExtensionService(): ReturnType { return { @@ -289,9 +291,8 @@ describe('SelectionToolbox', () => { ) }) - it('should show mask editor only for single image nodes', async () => { - const mockUtils = await import('@/utils/litegraphUtil') - const isImageNodeSpy = vi.spyOn(mockUtils, 'isImageNode') + it('should show mask editor only for single image nodes', () => { + const isImageNodeSpy = vi.spyOn(litegraphUtil, 'isImageNode') // Single image node isImageNodeSpy.mockReturnValue(true) @@ -307,9 +308,8 @@ describe('SelectionToolbox', () => { expect(wrapper2.find('.mask-editor-button').exists()).toBe(false) }) - it('should show Color picker button only for single Load3D nodes', async () => { - const mockUtils = await import('@/utils/litegraphUtil') - const isLoad3dNodeSpy = vi.spyOn(mockUtils, 'isLoad3dNode') + it('should show Color picker button only for single Load3D nodes', () => { + const isLoad3dNodeSpy = vi.spyOn(litegraphUtil, 'isLoad3dNode') // Single Load3D node isLoad3dNodeSpy.mockReturnValue(true) @@ -325,13 +325,9 @@ describe('SelectionToolbox', () => { expect(wrapper2.find('.load-3d-viewer-button').exists()).toBe(false) }) - it('should show ExecuteButton only when output nodes are selected', async () => { - const mockNodeFilterUtil = await import('@/utils/nodeFilterUtil') - const isOutputNodeSpy = vi.spyOn(mockNodeFilterUtil, 'isOutputNode') - const filterOutputNodesSpy = vi.spyOn( - mockNodeFilterUtil, - 'filterOutputNodes' - ) + it('should show ExecuteButton only when output nodes are selected', () => { + const isOutputNodeSpy = vi.spyOn(nodeFilterUtil, 'isOutputNode') + const filterOutputNodesSpy = vi.spyOn(nodeFilterUtil, 'filterOutputNodes') // With output node selected isOutputNodeSpy.mockReturnValue(true) diff --git a/src/components/graph/selectionToolbox/ExecuteButton.test.ts b/src/components/graph/selectionToolbox/ExecuteButton.test.ts index 6fe236d5b..370b96b1d 100644 --- a/src/components/graph/selectionToolbox/ExecuteButton.test.ts +++ b/src/components/graph/selectionToolbox/ExecuteButton.test.ts @@ -7,6 +7,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { createI18n } from 'vue-i18n' import ExecuteButton from '@/components/graph/selectionToolbox/ExecuteButton.vue' +import { useSelectionState } from '@/composables/graph/useSelectionState' import type { LGraphCanvas, LGraphNode } from '@/lib/litegraph/src/litegraph' import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' import { useCommandStore } from '@/stores/commandStore' @@ -47,7 +48,7 @@ describe('ExecuteButton', () => { } }) - beforeEach(async () => { + beforeEach(() => { // Set up Pinia with testing utilities setActivePinia( createTestingPinia({ @@ -71,10 +72,7 @@ describe('ExecuteButton', () => { vi.spyOn(commandStore, 'execute').mockResolvedValue() // Update the useSelectionState mock - const { useSelectionState } = vi.mocked( - await import('@/composables/graph/useSelectionState') - ) - useSelectionState.mockReturnValue({ + vi.mocked(useSelectionState).mockReturnValue({ selectedNodes: { value: mockSelectedNodes } diff --git a/src/composables/useTemplateFiltering.test.ts b/src/composables/useTemplateFiltering.test.ts index a44db0e99..48e7f6fd8 100644 --- a/src/composables/useTemplateFiltering.test.ts +++ b/src/composables/useTemplateFiltering.test.ts @@ -4,6 +4,7 @@ import { nextTick, ref } from 'vue' import type { IFuseOptions } from 'fuse.js' import type { TemplateInfo } from '@/platform/workflow/templates/types/template' +import { useTemplateFiltering } from '@/composables/useTemplateFiltering' const defaultSettingStore = { get: vi.fn((key: string) => { @@ -50,9 +51,6 @@ vi.mock('@/scripts/api', () => ({ } })) -const { useTemplateFiltering } = - await import('@/composables/useTemplateFiltering') - describe('useTemplateFiltering', () => { beforeEach(() => { setActivePinia(createPinia()) diff --git a/src/lib/litegraph/src/LGraph.test.ts b/src/lib/litegraph/src/LGraph.test.ts index 1c2f38da2..ed119aec5 100644 --- a/src/lib/litegraph/src/LGraph.test.ts +++ b/src/lib/litegraph/src/LGraph.test.ts @@ -46,12 +46,9 @@ describe('LGraph', () => { expect(graph.extra).toBe('TestGraph') }) - test('is exactly the same type', async ({ expect }) => { - const directImport = await import('@/lib/litegraph/src/LGraph') - const entryPointImport = await import('@/lib/litegraph/src/litegraph') - - expect(LiteGraph.LGraph).toBe(directImport.LGraph) - expect(LiteGraph.LGraph).toBe(entryPointImport.LGraph) + test('is exactly the same type', ({ expect }) => { + // LGraph from barrel export and LiteGraph.LGraph should be the same + expect(LiteGraph.LGraph).toBe(LGraph) }) test('populates optional values', ({ expect, minimalSerialisableGraph }) => { diff --git a/src/lib/litegraph/src/litegraph.test.ts b/src/lib/litegraph/src/litegraph.test.ts index 936b82f6e..19645a682 100644 --- a/src/lib/litegraph/src/litegraph.test.ts +++ b/src/lib/litegraph/src/litegraph.test.ts @@ -1,12 +1,15 @@ import { clamp } from 'es-toolkit/compat' -import { beforeEach, describe, expect, vi } from 'vitest' +import { describe, expect } from 'vitest' import { LiteGraphGlobal, LGraphCanvas, - LiteGraph + LiteGraph, + LGraph } from '@/lib/litegraph/src/litegraph' +import { LGraph as DirectLGraph } from '@/lib/litegraph/src/LGraph' + import { test } from './__fixtures__/testExtensions' describe('Litegraph module', () => { @@ -27,22 +30,9 @@ describe('Litegraph module', () => { }) describe('Import order dependency', () => { - beforeEach(() => { - vi.resetModules() - }) - - test('Imports without error when entry point is imported first', async ({ - expect - }) => { - async function importNormally() { - const entryPointImport = await import('@/lib/litegraph/src/litegraph') - const directImport = await import('@/lib/litegraph/src/LGraph') - - // Sanity check that imports were cleared. - expect(Object.is(LiteGraph, entryPointImport.LiteGraph)).toBe(false) - expect(Object.is(LiteGraph.LGraph, directImport.LGraph)).toBe(false) - } - - await expect(importNormally()).resolves.toBeUndefined() + test('Imports reference the same types', ({ expect }) => { + // Both imports should reference the same LGraph class + expect(LiteGraph.LGraph).toBe(DirectLGraph) + expect(LiteGraph.LGraph).toBe(LGraph) }) }) diff --git a/src/platform/assets/composables/useMediaAssetActions.test.ts b/src/platform/assets/composables/useMediaAssetActions.test.ts index 6c8f680a0..2d0a7a291 100644 --- a/src/platform/assets/composables/useMediaAssetActions.test.ts +++ b/src/platform/assets/composables/useMediaAssetActions.test.ts @@ -4,6 +4,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { LGraphNode } from '@/lib/litegraph/src/litegraph' import type { AssetItem } from '@/platform/assets/schemas/assetSchema' +import { useMediaAssetActions } from './useMediaAssetActions' + // Use vi.hoisted to create a mutable reference for isCloud const mockIsCloud = vi.hoisted(() => ({ value: false })) @@ -126,7 +128,6 @@ describe('useMediaAssetActions', () => { }) it('should use asset.name as filename', async () => { - const { useMediaAssetActions } = await import('./useMediaAssetActions') const actions = useMediaAssetActions() const asset = createMockAsset({ @@ -146,7 +147,6 @@ describe('useMediaAssetActions', () => { }) it('should use asset_hash as filename when available', async () => { - const { useMediaAssetActions } = await import('./useMediaAssetActions') const actions = useMediaAssetActions() const asset = createMockAsset({ @@ -160,7 +160,6 @@ describe('useMediaAssetActions', () => { }) it('should fall back to asset.name when asset_hash is not available', async () => { - const { useMediaAssetActions } = await import('./useMediaAssetActions') const actions = useMediaAssetActions() const asset = createMockAsset({ @@ -174,7 +173,6 @@ describe('useMediaAssetActions', () => { }) it('should fall back to asset.name when asset_hash is null', async () => { - const { useMediaAssetActions } = await import('./useMediaAssetActions') const actions = useMediaAssetActions() const asset = createMockAsset({ @@ -196,7 +194,6 @@ describe('useMediaAssetActions', () => { }) it('should use asset_hash for each asset', async () => { - const { useMediaAssetActions } = await import('./useMediaAssetActions') const actions = useMediaAssetActions() const assets = [ diff --git a/src/platform/surveys/useFeatureUsageTracker.test.ts b/src/platform/surveys/useFeatureUsageTracker.test.ts index 5313e9eaf..f37b41ee1 100644 --- a/src/platform/surveys/useFeatureUsageTracker.test.ts +++ b/src/platform/surveys/useFeatureUsageTracker.test.ts @@ -1,27 +1,27 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useFeatureUsageTracker } from './useFeatureUsageTracker' + const STORAGE_KEY = 'Comfy.FeatureUsage' describe('useFeatureUsageTracker', () => { beforeEach(() => { localStorage.clear() - vi.resetModules() + vi.clearAllMocks() }) afterEach(() => { localStorage.clear() }) - it('initializes with zero count for new feature', async () => { - const { useFeatureUsageTracker } = await import('./useFeatureUsageTracker') - const { useCount } = useFeatureUsageTracker('test-feature') + it('initializes with zero count for new feature', () => { + const { useCount } = useFeatureUsageTracker('test-feature-1') expect(useCount.value).toBe(0) }) - it('increments count on trackUsage', async () => { - const { useFeatureUsageTracker } = await import('./useFeatureUsageTracker') - const { useCount, trackUsage } = useFeatureUsageTracker('test-feature') + it('increments count on trackUsage', () => { + const { useCount, trackUsage } = useFeatureUsageTracker('test-feature-2') expect(useCount.value).toBe(0) @@ -32,14 +32,12 @@ describe('useFeatureUsageTracker', () => { expect(useCount.value).toBe(2) }) - it('sets firstUsed only on first use', async () => { + it('sets firstUsed only on first use', () => { vi.useFakeTimers() const firstTs = 1000000 vi.setSystemTime(firstTs) try { - const { useFeatureUsageTracker } = - await import('./useFeatureUsageTracker') - const { usage, trackUsage } = useFeatureUsageTracker('test-feature') + const { usage, trackUsage } = useFeatureUsageTracker('test-feature-3') trackUsage() expect(usage.value?.firstUsed).toBe(firstTs) @@ -52,12 +50,10 @@ describe('useFeatureUsageTracker', () => { } }) - it('updates lastUsed on each use', async () => { + it('updates lastUsed on each use', () => { vi.useFakeTimers() try { - const { useFeatureUsageTracker } = - await import('./useFeatureUsageTracker') - const { usage, trackUsage } = useFeatureUsageTracker('test-feature') + const { usage, trackUsage } = useFeatureUsageTracker('test-feature-4') trackUsage() const firstLastUsed = usage.value?.lastUsed ?? 0 @@ -71,10 +67,9 @@ describe('useFeatureUsageTracker', () => { } }) - it('reset clears feature data', async () => { - const { useFeatureUsageTracker } = await import('./useFeatureUsageTracker') + it('reset clears feature data', () => { const { useCount, trackUsage, reset } = - useFeatureUsageTracker('test-feature') + useFeatureUsageTracker('test-feature-5') trackUsage() trackUsage() @@ -84,8 +79,7 @@ describe('useFeatureUsageTracker', () => { expect(useCount.value).toBe(0) }) - it('tracks multiple features independently', async () => { - const { useFeatureUsageTracker } = await import('./useFeatureUsageTracker') + it('tracks multiple features independently', () => { const featureA = useFeatureUsageTracker('feature-a') const featureB = useFeatureUsageTracker('feature-b') @@ -100,8 +94,6 @@ describe('useFeatureUsageTracker', () => { it('persists to localStorage', async () => { vi.useFakeTimers() try { - const { useFeatureUsageTracker } = - await import('./useFeatureUsageTracker') const { trackUsage } = useFeatureUsageTracker('persisted-feature') trackUsage() @@ -114,7 +106,7 @@ describe('useFeatureUsageTracker', () => { } }) - it('loads existing data from localStorage', async () => { + it('loads existing data from localStorage', () => { localStorage.setItem( STORAGE_KEY, JSON.stringify({ @@ -122,8 +114,6 @@ describe('useFeatureUsageTracker', () => { }) ) - vi.resetModules() - const { useFeatureUsageTracker } = await import('./useFeatureUsageTracker') const { useCount } = useFeatureUsageTracker('existing-feature') expect(useCount.value).toBe(5) diff --git a/src/platform/telemetry/topupTracker.test.ts b/src/platform/telemetry/topupTracker.test.ts index 1ea07ff90..9f8e38ccf 100644 --- a/src/platform/telemetry/topupTracker.test.ts +++ b/src/platform/telemetry/topupTracker.test.ts @@ -1,6 +1,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import type * as TopupTrackerModule from '@/platform/telemetry/topupTracker' +import { + startTopupTracking, + checkForCompletedTopup, + clearTopupTracking +} from '@/platform/telemetry/topupTracker' import type { AuditLog } from '@/services/customerEventsService' // Mock localStorage @@ -25,19 +29,15 @@ vi.mock('@/platform/telemetry', () => ({ })) describe('topupTracker', () => { - let topupTracker: typeof TopupTrackerModule - - beforeEach(async () => { + beforeEach(() => { vi.clearAllMocks() - // Dynamically import to ensure fresh module state - topupTracker = await import('@/platform/telemetry/topupTracker') }) describe('startTopupTracking', () => { it('should save current timestamp to localStorage', () => { const beforeTimestamp = Date.now() - topupTracker.startTopupTracking() + startTopupTracking() expect(mockLocalStorage.setItem).toHaveBeenCalledWith( 'pending_topup_timestamp', @@ -57,7 +57,7 @@ describe('topupTracker', () => { it('should return false if no pending topup exists', () => { mockLocalStorage.getItem.mockReturnValue(null) - const result = topupTracker.checkForCompletedTopup([]) + const result = checkForCompletedTopup([]) expect(result).toBe(false) expect(mockTelemetry.trackApiCreditTopupSucceeded).not.toHaveBeenCalled() @@ -66,7 +66,7 @@ describe('topupTracker', () => { it('should return false if events array is empty', () => { mockLocalStorage.getItem.mockReturnValue(Date.now().toString()) - const result = topupTracker.checkForCompletedTopup([]) + const result = checkForCompletedTopup([]) expect(result).toBe(false) expect(mockTelemetry.trackApiCreditTopupSucceeded).not.toHaveBeenCalled() @@ -75,7 +75,7 @@ describe('topupTracker', () => { it('should return false if events array is null', () => { mockLocalStorage.getItem.mockReturnValue(Date.now().toString()) - const result = topupTracker.checkForCompletedTopup(null) + const result = checkForCompletedTopup(null) expect(result).toBe(false) expect(mockTelemetry.trackApiCreditTopupSucceeded).not.toHaveBeenCalled() @@ -94,7 +94,7 @@ describe('topupTracker', () => { } ] - const result = topupTracker.checkForCompletedTopup(events) + const result = checkForCompletedTopup(events) expect(result).toBe(false) expect(mockLocalStorage.removeItem).toHaveBeenCalledWith( @@ -122,7 +122,7 @@ describe('topupTracker', () => { } ] - const result = topupTracker.checkForCompletedTopup(events) + const result = checkForCompletedTopup(events) expect(result).toBe(true) expect(mockTelemetry.trackApiCreditTopupSucceeded).toHaveBeenCalledOnce() @@ -144,7 +144,7 @@ describe('topupTracker', () => { } ] - const result = topupTracker.checkForCompletedTopup(events) + const result = checkForCompletedTopup(events) expect(result).toBe(false) expect(mockTelemetry.trackApiCreditTopupSucceeded).not.toHaveBeenCalled() @@ -164,7 +164,7 @@ describe('topupTracker', () => { } ] - const result = topupTracker.checkForCompletedTopup(events) + const result = checkForCompletedTopup(events) expect(result).toBe(false) expect(mockTelemetry.trackApiCreditTopupSucceeded).not.toHaveBeenCalled() @@ -189,7 +189,7 @@ describe('topupTracker', () => { } ] - const result = topupTracker.checkForCompletedTopup(events) + const result = checkForCompletedTopup(events) expect(result).toBe(false) expect(mockTelemetry.trackApiCreditTopupSucceeded).not.toHaveBeenCalled() @@ -198,7 +198,7 @@ describe('topupTracker', () => { describe('clearTopupTracking', () => { it('should remove pending topup from localStorage', () => { - topupTracker.clearTopupTracking() + clearTopupTracking() expect(mockLocalStorage.removeItem).toHaveBeenCalledWith( 'pending_topup_timestamp' diff --git a/src/platform/telemetry/useTelemetry.test.ts b/src/platform/telemetry/useTelemetry.test.ts index b5c887c74..d3d565c2e 100644 --- a/src/platform/telemetry/useTelemetry.test.ts +++ b/src/platform/telemetry/useTelemetry.test.ts @@ -1,5 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' +import { useTelemetry } from '@/platform/telemetry' + vi.mock('@/platform/distribution/types', () => ({ isCloud: false })) @@ -9,17 +11,14 @@ describe('useTelemetry', () => { vi.clearAllMocks() }) - it('should return null when not in cloud distribution', async () => { - const { useTelemetry } = await import('@/platform/telemetry') + it('should return null when not in cloud distribution', () => { const provider = useTelemetry() // Should return null for OSS builds expect(provider).toBeNull() - }, 10000) - - it('should return null consistently for OSS builds', async () => { - const { useTelemetry } = await import('@/platform/telemetry') + }) + it('should return null consistently for OSS builds', () => { const provider1 = useTelemetry() const provider2 = useTelemetry() diff --git a/src/platform/updates/common/releaseStore.test.ts b/src/platform/updates/common/releaseStore.test.ts index 2f22d18e0..b54c8e87d 100644 --- a/src/platform/updates/common/releaseStore.test.ts +++ b/src/platform/updates/common/releaseStore.test.ts @@ -1,19 +1,96 @@ -import { createPinia, setActivePinia } from 'pinia' -import { compare, valid } from 'semver' -import type { Mock } from 'vitest' +import { until } from '@vueuse/core' +import { setActivePinia } from 'pinia' +import { compare } from 'semver' import { beforeEach, describe, expect, it, vi } from 'vitest' import { ref } from 'vue' import type { ReleaseNote } from '@/platform/updates/common/releaseService' +import { useSettingStore } from '@/platform/settings/settingStore' import { useReleaseStore } from '@/platform/updates/common/releaseStore' +import { useReleaseService } from '@/platform/updates/common/releaseService' +import { useSystemStatsStore } from '@/stores/systemStatsStore' +import { isElectron } from '@/utils/envUtil' +import { createTestingPinia } from '@pinia/testing' +import type { SystemStats } from '@/types' // Mock the dependencies -vi.mock('semver') -vi.mock('@/utils/envUtil') +vi.mock('semver', () => ({ + compare: vi.fn(), + valid: vi.fn(() => '1.0.0') +})) + +vi.mock('@/utils/envUtil', () => ({ + isElectron: vi.fn(() => true) +})) + vi.mock('@/platform/distribution/types', () => ({ isCloud: false })) -vi.mock('@/platform/updates/common/releaseService') -vi.mock('@/platform/settings/settingStore') -vi.mock('@/stores/systemStatsStore') + +vi.mock('@/platform/updates/common/releaseService', () => { + const getReleases = vi.fn() + const isLoading = ref(false) + const error = ref(null) + return { + useReleaseService: () => ({ + getReleases, + isLoading, + error + }) + } +}) + +vi.mock('@/platform/settings/settingStore', () => { + const get = vi.fn((key: string) => { + if (key === 'Comfy.Notification.ShowVersionUpdates') return true + return null + }) + const set = vi.fn() + return { + useSettingStore: () => ({ get, set }) + } +}) + +const mockSystemStatsState = vi.hoisted(() => ({ + systemStats: { + system: { + comfyui_version: '1.0.0', + argv: [] + } + } satisfies { + system: Partial + }, + isInitialized: true, + reset() { + this.systemStats = { + system: { + comfyui_version: '1.0.0', + argv: [] + } satisfies Partial + } + this.isInitialized = true + } +})) +vi.mock('@/stores/systemStatsStore', () => { + const refetchSystemStats = vi.fn() + const getFormFactor = vi.fn(() => 'git-windows') + return { + useSystemStatsStore: () => ({ + get systemStats() { + return mockSystemStatsState.systemStats + }, + set systemStats(val) { + mockSystemStatsState.systemStats = val + }, + get isInitialized() { + return mockSystemStatsState.isInitialized + }, + set isInitialized(val) { + mockSystemStatsState.isInitialized = val + }, + refetchSystemStats, + getFormFactor + }) + } +}) vi.mock('@vueuse/core', () => ({ until: vi.fn(() => Promise.resolve()), useStorage: vi.fn(() => ({ value: {} })), @@ -21,27 +98,6 @@ vi.mock('@vueuse/core', () => ({ })) describe('useReleaseStore', () => { - let store: ReturnType - let mockReleaseService: { - getReleases: Mock - isLoading: ReturnType> - error: ReturnType> - } - let mockSettingStore: { get: Mock; set: Mock } - let mockSystemStatsStore: { - systemStats: { - system: { - comfyui_version: string - argv?: string[] - [key: string]: unknown - } - devices?: unknown[] - } | null - isInitialized: boolean - refetchSystemStats: Mock - getFormFactor: Mock - } - const mockRelease = { id: 1, project: 'comfyui' as const, @@ -51,71 +107,16 @@ describe('useReleaseStore', () => { attention: 'high' as const } - beforeEach(async () => { - setActivePinia(createPinia()) + beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) - // Reset all mocks - vi.clearAllMocks() - - // Setup mock services with proper refs - mockReleaseService = { - getReleases: vi.fn(), - isLoading: ref(false), - error: ref(null) - } - - mockSettingStore = { - get: vi.fn(), - set: vi.fn() - } - - mockSystemStatsStore = { - systemStats: { - system: { - comfyui_version: '1.0.0' - } - }, - isInitialized: true, - refetchSystemStats: vi.fn(), - getFormFactor: vi.fn(() => 'git-windows') - } - - // Setup mock implementations - const { useReleaseService } = - await import('@/platform/updates/common/releaseService') - const { useSettingStore } = await import('@/platform/settings/settingStore') - const { useSystemStatsStore } = await import('@/stores/systemStatsStore') - const { isElectron } = await import('@/utils/envUtil') - - vi.mocked(useReleaseService).mockReturnValue( - mockReleaseService as Partial< - ReturnType - > as ReturnType - ) - vi.mocked(useSettingStore).mockReturnValue( - mockSettingStore as Partial< - ReturnType - > as ReturnType - ) - vi.mocked(useSystemStatsStore).mockReturnValue( - mockSystemStatsStore as Partial< - ReturnType - > as ReturnType - ) - vi.mocked(isElectron).mockReturnValue(true) - vi.mocked(valid).mockReturnValue('1.0.0') - - // Default showVersionUpdates to true - mockSettingStore.get.mockImplementation((key: string) => { - if (key === 'Comfy.Notification.ShowVersionUpdates') return true - return null - }) - - store = useReleaseStore() + vi.resetAllMocks() + mockSystemStatsState.reset() }) describe('initial state', () => { it('should initialize with default state', () => { + const store = useReleaseStore() expect(store.releases).toEqual([]) expect(store.isLoading).toBe(false) expect(store.error).toBeNull() @@ -124,6 +125,7 @@ describe('useReleaseStore', () => { describe('computed properties', () => { it('should return most recent release', () => { + const store = useReleaseStore() const olderRelease = { ...mockRelease, id: 2, @@ -136,6 +138,7 @@ describe('useReleaseStore', () => { }) it('should return 3 most recent releases', () => { + const store = useReleaseStore() const releases = [ mockRelease, { ...mockRelease, id: 2, version: '1.1.0' }, @@ -148,6 +151,7 @@ describe('useReleaseStore', () => { }) it('should show update button (shouldShowUpdateButton)', () => { + const store = useReleaseStore() vi.mocked(compare).mockReturnValue(1) // newer version available store.releases = [mockRelease] @@ -155,6 +159,7 @@ describe('useReleaseStore', () => { }) it('should not show update button when no new version', () => { + const store = useReleaseStore() vi.mocked(compare).mockReturnValue(-1) // current version is newer store.releases = [mockRelease] @@ -163,19 +168,17 @@ describe('useReleaseStore', () => { }) describe('showVersionUpdates setting', () => { - beforeEach(async () => { - store.releases = [mockRelease] - }) - describe('when notifications are enabled', () => { - beforeEach(async () => { - mockSettingStore.get.mockImplementation((key: string) => { + beforeEach(() => { + const settingStore = useSettingStore() + vi.mocked(settingStore.get).mockImplementation((key: string) => { if (key === 'Comfy.Notification.ShowVersionUpdates') return true return null }) }) it('should show toast for medium/high attention releases', () => { + const store = useReleaseStore() vi.mocked(compare).mockReturnValue(1) store.releases = [mockRelease] @@ -183,6 +186,7 @@ describe('useReleaseStore', () => { }) it('should not show toast for low attention releases', () => { + const store = useReleaseStore() vi.mocked(compare).mockReturnValue(1) const lowAttentionRelease = { @@ -196,13 +200,18 @@ describe('useReleaseStore', () => { }) it('should show red dot for new versions', () => { + const store = useReleaseStore() + store.releases = [mockRelease] vi.mocked(compare).mockReturnValue(1) expect(store.shouldShowRedDot).toBe(true) }) it('should show popup for latest version', () => { - mockSystemStatsStore.systemStats!.system.comfyui_version = '1.2.0' + const store = useReleaseStore() + store.releases = [mockRelease] + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats!.system.comfyui_version = '1.2.0' vi.mocked(compare).mockReturnValue(0) @@ -210,11 +219,13 @@ describe('useReleaseStore', () => { }) it('should fetch releases during initialization', async () => { - mockReleaseService.getReleases.mockResolvedValue([mockRelease]) + const store = useReleaseStore() + const releaseService = useReleaseService() + vi.mocked(releaseService.getReleases).mockResolvedValue([mockRelease]) await store.initialize() - expect(mockReleaseService.getReleases).toHaveBeenCalledWith({ + expect(releaseService.getReleases).toHaveBeenCalledWith({ project: 'comfyui', current_version: '1.0.0', form_factor: 'git-windows', @@ -224,27 +235,35 @@ describe('useReleaseStore', () => { }) describe('when notifications are disabled', () => { - beforeEach(async () => { - mockSettingStore.get.mockImplementation((key: string) => { + beforeEach(() => { + const settingStore = useSettingStore() + vi.mocked(settingStore.get).mockImplementation((key: string) => { if (key === 'Comfy.Notification.ShowVersionUpdates') return false return null }) }) it('should not show toast even with new version available', () => { + const store = useReleaseStore() + store.releases = [mockRelease] vi.mocked(compare).mockReturnValue(1) expect(store.shouldShowToast).toBe(false) }) it('should not show red dot even with new version available', () => { + const store = useReleaseStore() + store.releases = [mockRelease] vi.mocked(compare).mockReturnValue(1) expect(store.shouldShowRedDot).toBe(false) }) it('should not show popup even for latest version', () => { - mockSystemStatsStore.systemStats!.system.comfyui_version = '1.2.0' + const store = useReleaseStore() + store.releases = [mockRelease] + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats!.system.comfyui_version = '1.2.0' vi.mocked(compare).mockReturnValue(0) @@ -252,15 +271,19 @@ describe('useReleaseStore', () => { }) it('should skip fetching releases during initialization', async () => { + const store = useReleaseStore() + const releaseService = useReleaseService() await store.initialize() - expect(mockReleaseService.getReleases).not.toHaveBeenCalled() + expect(releaseService.getReleases).not.toHaveBeenCalled() }) it('should not fetch releases when calling fetchReleases directly', async () => { + const store = useReleaseStore() + const releaseService = useReleaseService() await store.fetchReleases() - expect(mockReleaseService.getReleases).not.toHaveBeenCalled() + expect(releaseService.getReleases).not.toHaveBeenCalled() expect(store.isLoading).toBe(false) }) }) @@ -268,11 +291,13 @@ describe('useReleaseStore', () => { describe('release initialization', () => { it('should fetch releases successfully', async () => { - mockReleaseService.getReleases.mockResolvedValue([mockRelease]) + const store = useReleaseStore() + const releaseService = useReleaseService() + vi.mocked(releaseService.getReleases).mockResolvedValue([mockRelease]) await store.initialize() - expect(mockReleaseService.getReleases).toHaveBeenCalledWith({ + expect(releaseService.getReleases).toHaveBeenCalledWith({ project: 'comfyui', current_version: '1.0.0', form_factor: 'git-windows', @@ -282,12 +307,15 @@ describe('useReleaseStore', () => { }) it('should include form_factor in API call', async () => { - mockSystemStatsStore.getFormFactor.mockReturnValue('desktop-mac') - mockReleaseService.getReleases.mockResolvedValue([mockRelease]) + const store = useReleaseStore() + const releaseService = useReleaseService() + const systemStatsStore = useSystemStatsStore() + vi.mocked(systemStatsStore.getFormFactor).mockReturnValue('desktop-mac') + vi.mocked(releaseService.getReleases).mockResolvedValue([mockRelease]) await store.initialize() - expect(mockReleaseService.getReleases).toHaveBeenCalledWith({ + expect(releaseService.getReleases).toHaveBeenCalledWith({ project: 'comfyui', current_version: '1.0.0', form_factor: 'desktop-mac', @@ -296,16 +324,22 @@ describe('useReleaseStore', () => { }) it('should skip fetching when --disable-api-nodes is present', async () => { - mockSystemStatsStore.systemStats!.system.argv = ['--disable-api-nodes'] + const store = useReleaseStore() + const releaseService = useReleaseService() + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats!.system.argv = ['--disable-api-nodes'] await store.initialize() - expect(mockReleaseService.getReleases).not.toHaveBeenCalled() + expect(releaseService.getReleases).not.toHaveBeenCalled() expect(store.isLoading).toBe(false) }) it('should skip fetching when --disable-api-nodes is one of multiple args', async () => { - mockSystemStatsStore.systemStats!.system.argv = [ + const store = useReleaseStore() + const releaseService = useReleaseService() + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats!.system.argv = [ '--port', '8080', '--disable-api-nodes', @@ -314,37 +348,46 @@ describe('useReleaseStore', () => { await store.initialize() - expect(mockReleaseService.getReleases).not.toHaveBeenCalled() + expect(releaseService.getReleases).not.toHaveBeenCalled() expect(store.isLoading).toBe(false) }) it('should fetch normally when --disable-api-nodes is not present', async () => { - mockSystemStatsStore.systemStats!.system.argv = [ + const store = useReleaseStore() + const releaseService = useReleaseService() + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats!.system.argv = [ '--port', '8080', '--verbose' ] - mockReleaseService.getReleases.mockResolvedValue([mockRelease]) + vi.mocked(releaseService.getReleases).mockResolvedValue([mockRelease]) await store.initialize() - expect(mockReleaseService.getReleases).toHaveBeenCalled() + expect(releaseService.getReleases).toHaveBeenCalled() expect(store.releases).toEqual([mockRelease]) }) it('should fetch normally when argv is undefined', async () => { - mockSystemStatsStore.systemStats!.system.argv = undefined - mockReleaseService.getReleases.mockResolvedValue([mockRelease]) + const store = useReleaseStore() + const releaseService = useReleaseService() + const systemStatsStore = useSystemStatsStore() + // TODO: Consider deleting this test since the types have to be violated for it to be relevant + delete (systemStatsStore.systemStats!.system as { argv?: string[] }).argv + vi.mocked(releaseService.getReleases).mockResolvedValue([mockRelease]) await store.initialize() - expect(mockReleaseService.getReleases).toHaveBeenCalled() + expect(releaseService.getReleases).toHaveBeenCalled() expect(store.releases).toEqual([mockRelease]) }) it('should handle API errors gracefully', async () => { - mockReleaseService.getReleases.mockResolvedValue(null) - mockReleaseService.error.value = 'API Error' + const store = useReleaseStore() + const releaseService = useReleaseService() + vi.mocked(releaseService.getReleases).mockResolvedValue(null) + releaseService.error.value = 'API Error' await store.initialize() @@ -353,7 +396,9 @@ describe('useReleaseStore', () => { }) it('should handle non-Error objects', async () => { - mockReleaseService.getReleases.mockRejectedValue('String error') + const store = useReleaseStore() + const releaseService = useReleaseService() + vi.mocked(releaseService.getReleases).mockRejectedValue('String error') await store.initialize() @@ -361,12 +406,14 @@ describe('useReleaseStore', () => { }) it('should set loading state correctly', async () => { + const store = useReleaseStore() + const releaseService = useReleaseService() let resolvePromise: (value: ReleaseNote[] | null) => void const promise = new Promise((resolve) => { resolvePromise = resolve }) - mockReleaseService.getReleases.mockReturnValue(promise) + vi.mocked(releaseService.getReleases).mockReturnValue(promise) const initPromise = store.initialize() expect(store.isLoading).toBe(true) @@ -378,19 +425,23 @@ describe('useReleaseStore', () => { }) it('should fetch system stats if not available', async () => { - const { until } = await import('@vueuse/core') - mockSystemStatsStore.systemStats = null - mockSystemStatsStore.isInitialized = false - mockReleaseService.getReleases.mockResolvedValue([mockRelease]) + const store = useReleaseStore() + const releaseService = useReleaseService() + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats = null + systemStatsStore.isInitialized = false + vi.mocked(releaseService.getReleases).mockResolvedValue([mockRelease]) await store.initialize() - expect(until).toHaveBeenCalled() - expect(mockReleaseService.getReleases).toHaveBeenCalled() + expect(vi.mocked(until)).toHaveBeenCalled() + expect(releaseService.getReleases).toHaveBeenCalled() }) it('should not set loading state when notifications disabled', async () => { - mockSettingStore.get.mockImplementation((key: string) => { + const store = useReleaseStore() + const settingStore = useSettingStore() + vi.mocked(settingStore.get).mockImplementation((key: string) => { if (key === 'Comfy.Notification.ShowVersionUpdates') return false return null }) @@ -403,16 +454,22 @@ describe('useReleaseStore', () => { describe('--disable-api-nodes argument handling', () => { it('should skip fetchReleases when --disable-api-nodes is present', async () => { - mockSystemStatsStore.systemStats!.system.argv = ['--disable-api-nodes'] + const store = useReleaseStore() + const releaseService = useReleaseService() + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats!.system.argv = ['--disable-api-nodes'] await store.fetchReleases() - expect(mockReleaseService.getReleases).not.toHaveBeenCalled() + expect(releaseService.getReleases).not.toHaveBeenCalled() expect(store.isLoading).toBe(false) }) it('should skip fetchReleases when --disable-api-nodes is among other args', async () => { - mockSystemStatsStore.systemStats!.system.argv = [ + const store = useReleaseStore() + const releaseService = useReleaseService() + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats!.system.argv = [ '--port', '8080', '--disable-api-nodes', @@ -421,96 +478,109 @@ describe('useReleaseStore', () => { await store.fetchReleases() - expect(mockReleaseService.getReleases).not.toHaveBeenCalled() + expect(releaseService.getReleases).not.toHaveBeenCalled() expect(store.isLoading).toBe(false) }) it('should proceed with fetchReleases when --disable-api-nodes is not present', async () => { - mockSystemStatsStore.systemStats!.system.argv = [ + const store = useReleaseStore() + const releaseService = useReleaseService() + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats!.system.argv = [ '--port', '8080', '--verbose' ] - mockReleaseService.getReleases.mockResolvedValue([mockRelease]) + vi.mocked(releaseService.getReleases).mockResolvedValue([mockRelease]) await store.fetchReleases() - expect(mockReleaseService.getReleases).toHaveBeenCalled() + expect(releaseService.getReleases).toHaveBeenCalled() }) it('should proceed with fetchReleases when argv is undefined', async () => { - mockSystemStatsStore.systemStats!.system.argv = undefined - mockReleaseService.getReleases.mockResolvedValue([mockRelease]) + const store = useReleaseStore() + const releaseService = useReleaseService() + const systemStatsStore = useSystemStatsStore() + delete (systemStatsStore.systemStats!.system as { argv?: string[] }).argv + vi.mocked(releaseService.getReleases).mockResolvedValue([mockRelease]) await store.fetchReleases() - expect(mockReleaseService.getReleases).toHaveBeenCalled() + expect(releaseService.getReleases).toHaveBeenCalled() }) it('should proceed with fetchReleases when system stats are not available', async () => { - const { until } = await import('@vueuse/core') - mockSystemStatsStore.systemStats = null - mockSystemStatsStore.isInitialized = false - mockReleaseService.getReleases.mockResolvedValue([mockRelease]) + const store = useReleaseStore() + const releaseService = useReleaseService() + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats = null + systemStatsStore.isInitialized = false + vi.mocked(releaseService.getReleases).mockResolvedValue([mockRelease]) await store.fetchReleases() expect(until).toHaveBeenCalled() - expect(mockReleaseService.getReleases).toHaveBeenCalled() + expect(releaseService.getReleases).toHaveBeenCalled() }) }) describe('action handlers', () => { - beforeEach(async () => { - store.releases = [mockRelease] - }) - it('should handle skip release', async () => { + const store = useReleaseStore() + store.releases = [mockRelease] + const settingStore = useSettingStore() await store.handleSkipRelease('1.2.0') - expect(mockSettingStore.set).toHaveBeenCalledWith( + expect(settingStore.set).toHaveBeenCalledWith( 'Comfy.Release.Version', '1.2.0' ) - expect(mockSettingStore.set).toHaveBeenCalledWith( + expect(settingStore.set).toHaveBeenCalledWith( 'Comfy.Release.Status', 'skipped' ) - expect(mockSettingStore.set).toHaveBeenCalledWith( + expect(settingStore.set).toHaveBeenCalledWith( 'Comfy.Release.Timestamp', expect.any(Number) ) }) it('should handle show changelog', async () => { + const store = useReleaseStore() + store.releases = [mockRelease] + const settingStore = useSettingStore() await store.handleShowChangelog('1.2.0') - expect(mockSettingStore.set).toHaveBeenCalledWith( + expect(settingStore.set).toHaveBeenCalledWith( 'Comfy.Release.Version', '1.2.0' ) - expect(mockSettingStore.set).toHaveBeenCalledWith( + expect(settingStore.set).toHaveBeenCalledWith( 'Comfy.Release.Status', 'changelog seen' ) - expect(mockSettingStore.set).toHaveBeenCalledWith( + expect(settingStore.set).toHaveBeenCalledWith( 'Comfy.Release.Timestamp', expect.any(Number) ) }) it('should handle whats new seen', async () => { + const store = useReleaseStore() + store.releases = [mockRelease] + const settingStore = useSettingStore() await store.handleWhatsNewSeen('1.2.0') - expect(mockSettingStore.set).toHaveBeenCalledWith( + expect(settingStore.set).toHaveBeenCalledWith( 'Comfy.Release.Version', '1.2.0' ) - expect(mockSettingStore.set).toHaveBeenCalledWith( + expect(settingStore.set).toHaveBeenCalledWith( 'Comfy.Release.Status', "what's new seen" ) - expect(mockSettingStore.set).toHaveBeenCalledWith( + expect(settingStore.set).toHaveBeenCalledWith( 'Comfy.Release.Timestamp', expect.any(Number) ) @@ -519,7 +589,9 @@ describe('useReleaseStore', () => { describe('popup visibility', () => { it('should show toast for medium/high attention releases', () => { - mockSettingStore.get.mockImplementation((key: string) => { + const store = useReleaseStore() + const settingStore = useSettingStore() + vi.mocked(settingStore.get).mockImplementation((key: string) => { if (key === 'Comfy.Release.Version') return null if (key === 'Comfy.Release.Status') return null if (key === 'Comfy.Notification.ShowVersionUpdates') return true @@ -534,8 +606,10 @@ describe('useReleaseStore', () => { }) it('should show red dot for new versions', () => { + const store = useReleaseStore() + const settingStore = useSettingStore() vi.mocked(compare).mockReturnValue(1) - mockSettingStore.get.mockImplementation((key: string) => { + vi.mocked(settingStore.get).mockImplementation((key: string) => { if (key === 'Comfy.Notification.ShowVersionUpdates') return true return null }) @@ -546,8 +620,11 @@ describe('useReleaseStore', () => { }) it('should show popup for latest version', () => { - mockSystemStatsStore.systemStats!.system.comfyui_version = '1.2.0' // Same as release - mockSettingStore.get.mockImplementation((key: string) => { + const store = useReleaseStore() + const systemStatsStore = useSystemStatsStore() + const settingStore = useSettingStore() + systemStatsStore.systemStats!.system.comfyui_version = '1.2.0' // Same as release + vi.mocked(settingStore.get).mockImplementation((key: string) => { if (key === 'Comfy.Notification.ShowVersionUpdates') return true return null }) @@ -562,8 +639,11 @@ describe('useReleaseStore', () => { describe('edge cases', () => { it('should handle missing system stats gracefully', async () => { - mockSystemStatsStore.systemStats = null - mockSettingStore.get.mockImplementation((key: string) => { + const store = useReleaseStore() + const systemStatsStore = useSystemStatsStore() + const settingStore = useSettingStore() + systemStatsStore.systemStats = null + vi.mocked(settingStore.get).mockImplementation((key: string) => { if (key === 'Comfy.Notification.ShowVersionUpdates') return false return null }) @@ -571,11 +651,13 @@ describe('useReleaseStore', () => { await store.initialize() // Should not fetch system stats when notifications disabled - expect(mockSystemStatsStore.refetchSystemStats).not.toHaveBeenCalled() + expect(systemStatsStore.refetchSystemStats).not.toHaveBeenCalled() }) it('should handle concurrent fetchReleases calls', async () => { - mockReleaseService.getReleases.mockImplementation( + const store = useReleaseStore() + const releaseService = useReleaseService() + vi.mocked(releaseService.getReleases).mockImplementation( () => new Promise((resolve) => setTimeout(() => resolve([mockRelease]), 100) @@ -589,41 +671,37 @@ describe('useReleaseStore', () => { await Promise.all([promise1, promise2]) // Should only call API once due to loading check - expect(mockReleaseService.getReleases).toHaveBeenCalledTimes(1) + expect(releaseService.getReleases).toHaveBeenCalledTimes(1) }) }) describe('isElectron environment checks', () => { - beforeEach(async () => { - // Set up a new version available - store.releases = [mockRelease] - mockSettingStore.get.mockImplementation((key: string) => { - if (key === 'Comfy.Notification.ShowVersionUpdates') return true - return null - }) - }) - describe('when running in Electron (desktop)', () => { - beforeEach(async () => { - const { isElectron } = await import('@/utils/envUtil') + beforeEach(() => { vi.mocked(isElectron).mockReturnValue(true) }) it('should show toast when conditions are met', () => { - vi.mocked(compare).mockReturnValue(1) + const store = useReleaseStore() store.releases = [mockRelease] + vi.mocked(compare).mockReturnValue(1) expect(store.shouldShowToast).toBe(true) }) it('should show red dot when new version available', () => { + const store = useReleaseStore() + store.releases = [mockRelease] vi.mocked(compare).mockReturnValue(1) expect(store.shouldShowRedDot).toBe(true) }) it('should show popup for latest version', () => { - mockSystemStatsStore.systemStats!.system.comfyui_version = '1.2.0' + const store = useReleaseStore() + store.releases = [mockRelease] + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats!.system.comfyui_version = '1.2.0' vi.mocked(compare).mockReturnValue(0) @@ -632,12 +710,12 @@ describe('useReleaseStore', () => { }) describe('when NOT running in Electron (web)', () => { - beforeEach(async () => { - const { isElectron } = await import('@/utils/envUtil') + beforeEach(() => { vi.mocked(isElectron).mockReturnValue(false) }) it('should NOT show toast even when all other conditions are met', () => { + const store = useReleaseStore() vi.mocked(compare).mockReturnValue(1) // Set up all conditions that would normally show toast @@ -647,12 +725,15 @@ describe('useReleaseStore', () => { }) it('should NOT show red dot even when new version available', () => { + const store = useReleaseStore() + store.releases = [mockRelease] vi.mocked(compare).mockReturnValue(1) expect(store.shouldShowRedDot).toBe(false) }) it('should NOT show toast regardless of attention level', () => { + const store = useReleaseStore() vi.mocked(compare).mockReturnValue(1) // Test with high attention releases @@ -672,6 +753,7 @@ describe('useReleaseStore', () => { }) it('should NOT show red dot even with high attention release', () => { + const store = useReleaseStore() vi.mocked(compare).mockReturnValue(1) store.releases = [{ ...mockRelease, attention: 'high' as const }] @@ -680,7 +762,10 @@ describe('useReleaseStore', () => { }) it('should NOT show popup even for latest version', () => { - mockSystemStatsStore.systemStats!.system.comfyui_version = '1.2.0' + const store = useReleaseStore() + store.releases = [mockRelease] + const systemStatsStore = useSystemStatsStore() + systemStatsStore.systemStats!.system.comfyui_version = '1.2.0' vi.mocked(compare).mockReturnValue(0) diff --git a/src/platform/updates/common/versionCompatibilityStore.test.ts b/src/platform/updates/common/versionCompatibilityStore.test.ts index 78aa87c74..3fdb80eaa 100644 --- a/src/platform/updates/common/versionCompatibilityStore.test.ts +++ b/src/platform/updates/common/versionCompatibilityStore.test.ts @@ -1,3 +1,4 @@ +import { until } from '@vueuse/core' import { createPinia, setActivePinia } from 'pinia' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { ref } from 'vue' @@ -357,17 +358,15 @@ describe('useVersionCompatibilityStore', () => { describe('initialization', () => { it('should fetch system stats if not available', async () => { - const { until } = await import('@vueuse/core') mockSystemStatsStore.systemStats = null mockSystemStatsStore.isInitialized = false await store.initialize() - expect(until).toHaveBeenCalled() + expect(vi.mocked(until)).toHaveBeenCalled() }) it('should not fetch system stats if already available', async () => { - const { until } = await import('@vueuse/core') mockSystemStatsStore.systemStats = { system: { comfyui_version: '1.24.0', @@ -378,7 +377,7 @@ describe('useVersionCompatibilityStore', () => { await store.initialize() - expect(until).not.toHaveBeenCalled() + expect(vi.mocked(until)).not.toHaveBeenCalled() }) }) }) diff --git a/src/renderer/core/thumbnail/useWorkflowThumbnail.test.ts b/src/renderer/core/thumbnail/useWorkflowThumbnail.test.ts index 39f8de56a..4f7ea6ef6 100644 --- a/src/renderer/core/thumbnail/useWorkflowThumbnail.test.ts +++ b/src/renderer/core/thumbnail/useWorkflowThumbnail.test.ts @@ -3,6 +3,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { ComfyWorkflow } from '@/platform/workflow/management/stores/workflowStore' import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore' +import { createGraphThumbnail } from '@/renderer/core/thumbnail/graphThumbnailRenderer' +import { useWorkflowThumbnail } from '@/renderer/core/thumbnail/useWorkflowThumbnail' +import { api } from '@/scripts/api' vi.mock('@/renderer/core/thumbnail/graphThumbnailRenderer', () => ({ createGraphThumbnail: vi.fn() @@ -19,12 +22,6 @@ vi.mock('@/scripts/api', () => ({ } })) -const { useWorkflowThumbnail } = - await import('@/renderer/core/thumbnail/useWorkflowThumbnail') -const { createGraphThumbnail } = - await import('@/renderer/core/thumbnail/graphThumbnailRenderer') -const { api } = await import('@/scripts/api') - describe('useWorkflowThumbnail', () => { let workflowStore: ReturnType diff --git a/src/renderer/extensions/minimap/composables/useMinimap.test.ts b/src/renderer/extensions/minimap/composables/useMinimap.test.ts index 1ab52999c..db050ad1f 100644 --- a/src/renderer/extensions/minimap/composables/useMinimap.test.ts +++ b/src/renderer/extensions/minimap/composables/useMinimap.test.ts @@ -200,9 +200,8 @@ vi.mock('@/platform/workflow/management/stores/workflowStore', () => ({ })) })) -const { useMinimap } = - await import('@/renderer/extensions/minimap/composables/useMinimap') -const { api } = await import('@/scripts/api') +import { useMinimap } from '@/renderer/extensions/minimap/composables/useMinimap' +import { api } from '@/scripts/api' describe('useMinimap', () => { let moduleMockCanvasElement: HTMLCanvasElement diff --git a/src/renderer/extensions/minimap/composables/useMinimapRenderer.test.ts b/src/renderer/extensions/minimap/composables/useMinimapRenderer.test.ts index a2de2a759..fbdff5142 100644 --- a/src/renderer/extensions/minimap/composables/useMinimapRenderer.test.ts +++ b/src/renderer/extensions/minimap/composables/useMinimapRenderer.test.ts @@ -103,9 +103,7 @@ describe('useMinimapRenderer', () => { expect(vi.mocked(renderMinimapToCanvas)).not.toHaveBeenCalled() }) - it('should only render when redraw is needed', async () => { - const { renderMinimapToCanvas } = - await import('@/renderer/extensions/minimap/minimapCanvasRenderer') + it('should only render when redraw is needed', () => { const canvasRef = ref(mockCanvas) const graphRef = ref(mockGraph) as Ref const boundsRef = ref({ minX: 0, minY: 0, width: 100, height: 100 }) diff --git a/src/renderer/extensions/minimap/composables/useMinimapViewport.test.ts b/src/renderer/extensions/minimap/composables/useMinimapViewport.test.ts index 76f0ca710..c2416edc3 100644 --- a/src/renderer/extensions/minimap/composables/useMinimapViewport.test.ts +++ b/src/renderer/extensions/minimap/composables/useMinimapViewport.test.ts @@ -4,6 +4,11 @@ import { ref } from 'vue' import type { Ref } from 'vue' import type { LGraph } from '@/lib/litegraph/src/litegraph' +import { + calculateMinimapScale, + calculateNodeBounds, + enforceMinimumBounds +} from '@/renderer/core/spatial/boundsCalculator' import { useMinimapViewport } from '@/renderer/extensions/minimap/composables/useMinimapViewport' import type { MinimapCanvas } from '@/renderer/extensions/minimap/types' @@ -66,10 +71,7 @@ describe('useMinimapViewport', () => { expect(viewport.scale.value).toBe(1) }) - it('should calculate graph bounds from nodes', async () => { - const { calculateNodeBounds, enforceMinimumBounds } = - await import('@/renderer/core/spatial/boundsCalculator') - + it('should calculate graph bounds from nodes', () => { vi.mocked(calculateNodeBounds).mockReturnValue({ minX: 100, minY: 100, @@ -92,10 +94,7 @@ describe('useMinimapViewport', () => { expect(enforceMinimumBounds).toHaveBeenCalled() }) - it('should handle empty graph', async () => { - const { calculateNodeBounds } = - await import('@/renderer/core/spatial/boundsCalculator') - + it('should handle empty graph', () => { vi.mocked(calculateNodeBounds).mockReturnValue(null) const canvasRef = ref(mockCanvas) as Ref @@ -131,11 +130,7 @@ describe('useMinimapViewport', () => { }) }) - it('should calculate viewport transform', async () => { - const { calculateNodeBounds, enforceMinimumBounds, calculateMinimapScale } = - await import('@/renderer/core/spatial/boundsCalculator') - - // Mock the bounds calculation + it('should calculate viewport transform', () => { vi.mocked(calculateNodeBounds).mockReturnValue({ minX: 0, minY: 0, @@ -236,10 +231,7 @@ describe('useMinimapViewport', () => { expect(() => viewport.centerViewOn(100, 100)).not.toThrow() }) - it('should calculate scale correctly', async () => { - const { calculateMinimapScale, calculateNodeBounds, enforceMinimumBounds } = - await import('@/renderer/core/spatial/boundsCalculator') - + it('should calculate scale correctly', () => { const testBounds = { minX: 0, minY: 0, diff --git a/src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.test.ts b/src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.test.ts index ef6d15359..599a3bfab 100644 --- a/src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.test.ts +++ b/src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.test.ts @@ -2,6 +2,7 @@ import axios from 'axios' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { IWidget } from '@/lib/litegraph/src/litegraph' +import { api } from '@/scripts/api' import { LGraphNode } from '@/lib/litegraph/src/litegraph' import { useRemoteWidget } from '@/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget' import type { RemoteWidgetConfig } from '@/schemas/nodeDefSchema' @@ -610,7 +611,6 @@ describe('useRemoteWidget', () => { }) it('should register event listener when enabled', async () => { - const { api } = await import('@/scripts/api') const addEventListenerSpy = vi.spyOn(api, 'addEventListener') const mockNode = { @@ -636,7 +636,6 @@ describe('useRemoteWidget', () => { }) it('should refresh widget when workflow completes successfully', async () => { - const { api } = await import('@/scripts/api') let executionSuccessHandler: (() => void) | undefined vi.spyOn(api, 'addEventListener').mockImplementation((event, handler) => { @@ -674,7 +673,6 @@ describe('useRemoteWidget', () => { }) it('should not refresh when toggle is disabled', async () => { - const { api } = await import('@/scripts/api') let executionSuccessHandler: (() => void) | undefined vi.spyOn(api, 'addEventListener').mockImplementation((event, handler) => { @@ -707,7 +705,6 @@ describe('useRemoteWidget', () => { }) it('should cleanup event listener on node removal', async () => { - const { api } = await import('@/scripts/api') let executionSuccessHandler: (() => void) | undefined vi.spyOn(api, 'addEventListener').mockImplementation((event, handler) => { diff --git a/src/services/jobOutputCache.test.ts b/src/services/jobOutputCache.test.ts index 38e47e973..99fbd22f1 100644 --- a/src/services/jobOutputCache.test.ts +++ b/src/services/jobOutputCache.test.ts @@ -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() }) diff --git a/src/services/newUserService.ts b/src/services/newUserService.ts deleted file mode 100644 index fff9eb72c..000000000 --- a/src/services/newUserService.ts +++ /dev/null @@ -1,74 +0,0 @@ -import type { useSettingStore } from '@/platform/settings/settingStore' - -let pendingCallbacks: Array<() => Promise> = [] -let isNewUserDetermined = false -let isNewUserCached: boolean | null = null - -export const newUserService = () => { - function checkIsNewUser( - settingStore: ReturnType - ): 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) { - 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 - ) { - 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 - } -} diff --git a/src/services/newUserService.test.ts b/src/services/useNewUserService.test.ts similarity index 84% rename from src/services/newUserService.test.ts rename to src/services/useNewUserService.test.ts index 43fcc92cd..1fd590b21 100644 --- a/src/services/newUserService.test.ts +++ b/src/services/useNewUserService.test.ts @@ -7,6 +7,12 @@ const mockLocalStorage = vi.hoisted(() => ({ clear: vi.fn() })) +const mockSettingStore = vi.hoisted(() => ({ + settingValues: {} as Record, + 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 - let mockSettingStore: any - let newUserService: typeof NewUserServiceType +describe('useNewUserService', () => { + let service: ReturnType - 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) diff --git a/src/services/useNewUserService.ts b/src/services/useNewUserService.ts new file mode 100644 index 000000000..093c273bd --- /dev/null +++ b/src/services/useNewUserService.ts @@ -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 Promise>>([]) + const isNewUserDetermined = ref(false) + const isNewUserCached = ref(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) { + 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) diff --git a/src/stores/subgraphNavigationStore.test.ts b/src/stores/subgraphNavigationStore.test.ts index d7753bb0c..e16700602 100644 --- a/src/stores/subgraphNavigationStore.test.ts +++ b/src/stores/subgraphNavigationStore.test.ts @@ -6,6 +6,7 @@ import { useWorkflowStore } from '@/platform/workflow/management/stores/workflow import type { ComfyWorkflow } from '@/platform/workflow/management/stores/workflowStore' import { app } from '@/scripts/app' import { useSubgraphNavigationStore } from '@/stores/subgraphNavigationStore' +import { findSubgraphPathById } from '@/utils/graphTraversalUtil' vi.mock('@/scripts/app', () => { const mockCanvas = { @@ -136,7 +137,6 @@ describe('useSubgraphNavigationStore', () => { it('should clear navigation when activeSubgraph becomes undefined', async () => { const navigationStore = useSubgraphNavigationStore() const workflowStore = useWorkflowStore() - const { findSubgraphPathById } = await import('@/utils/graphTraversalUtil') // Create mock subgraph and graph structure const mockSubgraph = { diff --git a/src/workbench/extensions/manager/components/manager/packCard/PackCard.test.ts b/src/workbench/extensions/manager/components/manager/packCard/PackCard.test.ts index 9cf0da2ca..29af937f5 100644 --- a/src/workbench/extensions/manager/components/manager/packCard/PackCard.test.ts +++ b/src/workbench/extensions/manager/components/manager/packCard/PackCard.test.ts @@ -1,5 +1,5 @@ +import { createTestingPinia } from '@pinia/testing' import { mount } from '@vue/test-utils' -import { createPinia, setActivePinia } from 'pinia' import ProgressSpinner from 'primevue/progressspinner' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -15,6 +15,7 @@ const translateMock = vi.hoisted(() => ) ) const dateMock = vi.hoisted(() => vi.fn(() => '2024. 1. 1.')) +const storageMap = vi.hoisted(() => new Map()) // Mock dependencies vi.mock('vue-i18n', () => ({ @@ -45,16 +46,17 @@ vi.mock('@/stores/workspace/colorPaletteStore', () => ({ })) })) -vi.mock('@vueuse/core', async () => { - const { ref } = await import('vue') - return { - whenever: vi.fn(), - useStorage: vi.fn((_key, defaultValue) => { - return ref(defaultValue) - }), - createSharedComposable: vi.fn((fn) => fn) - } -}) +vi.mock('@vueuse/core', () => ({ + whenever: vi.fn(), + useStorage: vi.fn((key: string, defaultValue: unknown) => { + if (!storageMap.has(key)) storageMap.set(key, defaultValue) + return storageMap.get(key) + }), + createSharedComposable: vi.fn((fn) => { + let cached: ReturnType + return (...args: Parameters) => (cached ??= fn(...args)) + }) +})) vi.mock('@/config', () => ({ default: { @@ -72,12 +74,9 @@ vi.mock('@/stores/systemStatsStore', () => ({ })) describe('PackCard', () => { - let pinia: ReturnType - beforeEach(() => { vi.clearAllMocks() - pinia = createPinia() - setActivePinia(pinia) + storageMap.clear() }) const createWrapper = (props: { @@ -87,7 +86,7 @@ describe('PackCard', () => { const wrapper = mount(PackCard, { props, global: { - plugins: [pinia], + plugins: [createTestingPinia({ stubActions: false })], components: { ProgressSpinner },