Compare commits

..

1 Commits

Author SHA1 Message Date
CodeRabbit Fixer
af29f850ce fix: refactor: Use real composable implementations in ComfyHubPublishDialog tests instead of heavy mocks (#9449)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-06 14:43:15 +01:00
7 changed files with 115 additions and 128 deletions

View File

@@ -178,7 +178,7 @@
"uploadAlreadyInProgress": "Upload already in progress",
"capture": "capture",
"nodes": "Nodes",
"nodesCount": "{count} node | {count} nodes",
"nodesCount": "{count} nodes | {count} node | {count} nodes",
"addNode": "Add a node...",
"filterBy": "Filter by:",
"filterByType": "Filter by {type}...",
@@ -222,7 +222,7 @@
"failed": "Failed",
"cancelled": "Cancelled",
"job": "Job",
"asset": "{count} asset | {count} assets",
"asset": "{count} assets | {count} asset | {count} assets",
"untitled": "Untitled",
"emDash": "—",
"enabling": "Enabling {id}",
@@ -3347,7 +3347,7 @@
}
},
"errorOverlay": {
"errorCount": "{count} ERROR | {count} ERRORS",
"errorCount": "{count} ERRORS | {count} ERROR | {count} ERRORS",
"seeErrors": "See Errors"
},
"help": {
@@ -3357,7 +3357,7 @@
"progressToast": {
"importingModels": "Importing Models",
"downloadingModel": "Downloading model...",
"downloadsFailed": "{count} download failed | {count} downloads failed",
"downloadsFailed": "{count} downloads failed | {count} download failed | {count} downloads failed",
"allDownloadsCompleted": "All downloads completed",
"noImportsInQueue": "No {filter} in queue",
"failed": "Failed",
@@ -3374,7 +3374,7 @@
"exportingAssets": "Exporting Assets",
"preparingExport": "Preparing export...",
"exportError": "Export failed",
"exportFailed": "{count} export failed | {count} exports failed",
"exportFailed": "{count} export failed | {count} export failed | {count} exports failed",
"allExportsCompleted": "All exports completed",
"noExportsInQueue": "No {filter} exports in queue",
"exportStarted": "Preparing ZIP download...",

View File

@@ -99,9 +99,7 @@ vi.mock('@/renderer/core/canvas/canvasStore', () => ({
vi.mock('@/renderer/core/thumbnail/useWorkflowThumbnail', () => ({
useWorkflowThumbnail: () => ({
storeThumbnail: vi.fn(),
getThumbnail: vi.fn(),
clearThumbnail: vi.fn(),
moveWorkflowThumbnail: vi.fn()
getThumbnail: vi.fn()
})
}))
@@ -855,13 +853,12 @@ describe('useWorkflowService', () => {
const existing = createSaveableWorkflow('workflows/test.app.json')
vi.spyOn(workflowStore, 'getWorkflowByPath').mockReturnValue(existing)
vi.spyOn(workflowStore, 'removeWorkflowEntry').mockResolvedValue()
vi.spyOn(workflowStore, 'deleteWorkflow').mockResolvedValue()
mockConfirm.mockResolvedValue(true)
await service.saveWorkflow(workflow)
expect(mockConfirm).toHaveBeenCalled()
expect(workflowStore.removeWorkflowEntry).toHaveBeenCalledWith(existing)
expect(workflowStore.renameWorkflow).toHaveBeenCalledWith(
workflow,
'workflows/test.app.json'

View File

@@ -130,20 +130,11 @@ export const useWorkflowService = () => {
if (existingWorkflow && !existingWorkflow.isTemporary) {
if ((await confirmOverwrite(newPath)) !== true) return false
}
const needsOverwrite =
!!existingWorkflow && !existingWorkflow.isTemporary && !isSelfOverwrite
// Close and remove the old workflow entry before saving the new content.
// The file on disk is intentionally kept so that a save failure does not
// cause data loss. The subsequent save with overwrite: true will
// atomically replace it.
if (needsOverwrite) {
if (workflowStore.isOpen(existingWorkflow)) {
await closeWorkflow(existingWorkflow, { warnIfUnsaved: false })
if (!isSelfOverwrite) {
const deleted = await deleteWorkflow(existingWorkflow, true)
if (!deleted) return false
}
await workflowStore.removeWorkflowEntry(existingWorkflow)
}
workflow.changeTracker?.checkState()
@@ -152,19 +143,11 @@ export const useWorkflowService = () => {
await saveWorkflow(workflow)
} else if (workflow.isTemporary) {
await renameWorkflow(workflow, newPath)
if (needsOverwrite) {
await workflowStore.saveWorkflow(workflow, { overwrite: true })
} else {
await workflowStore.saveWorkflow(workflow)
}
await workflowStore.saveWorkflow(workflow)
} else {
const tempWorkflow = workflowStore.saveAs(workflow, newPath)
await openWorkflow(tempWorkflow)
if (needsOverwrite) {
await workflowStore.saveWorkflow(tempWorkflow, { overwrite: true })
} else {
await workflowStore.saveWorkflow(tempWorkflow)
}
await workflowStore.saveWorkflow(tempWorkflow)
}
return true
}
@@ -191,12 +174,7 @@ export const useWorkflowService = () => {
await workflowStore.saveWorkflow(workflow)
return
}
// Remove the old entry without deleting the file; the rename
// will atomically replace it, preventing data loss on failure.
if (workflowStore.isOpen(existing)) {
await closeWorkflow(existing, { warnIfUnsaved: false })
}
await workflowStore.removeWorkflowEntry(existing)
await deleteWorkflow(existing, true)
}
await renameWorkflow(workflow, expectedPath)
toastStore.add({

View File

@@ -151,16 +151,14 @@ export class ComfyWorkflow extends UserFile {
super.unload()
}
override async save({
overwrite
}: { force?: boolean; overwrite?: boolean } = {}) {
override async save() {
const { useWorkflowDraftStore } =
await import('@/platform/workflow/persistence/stores/workflowDraftStore')
const draftStore = useWorkflowDraftStore()
this.content = JSON.stringify(this.activeState)
// Force save to ensure the content is updated in remote storage incase
// the isModified state is screwed by changeTracker.
const ret = await super.save({ force: true, overwrite })
const ret = await super.save({ force: true })
this.changeTracker?.reset()
this.isModified = false
draftStore.removeDraft(this.path)

View File

@@ -63,11 +63,7 @@ interface WorkflowStore {
) => ComfyWorkflow
renameWorkflow: (workflow: ComfyWorkflow, newPath: string) => Promise<void>
deleteWorkflow: (workflow: ComfyWorkflow) => Promise<void>
removeWorkflowEntry: (workflow: ComfyWorkflow) => Promise<void>
saveWorkflow: (
workflow: ComfyWorkflow,
options?: { overwrite?: boolean }
) => Promise<void>
saveWorkflow: (workflow: ComfyWorkflow) => Promise<void>
workflows: ComfyWorkflow[]
bookmarkedWorkflows: ComfyWorkflow[]
@@ -543,32 +539,14 @@ export const useWorkflowStore = defineStore('workflow', () => {
}
}
/**
* Remove a workflow entry from the store without deleting the file on disk.
* Used during atomic overwrite to clear the old entry before saving the new
* content, so that the save can use overwrite: true to replace the file.
*/
const removeWorkflowEntry = async (workflow: ComfyWorkflow) => {
useWorkflowDraftStore().removeDraft(workflow.path)
if (bookmarkStore.isBookmarked(workflow.path)) {
await bookmarkStore.setBookmarked(workflow.path, false)
}
clearThumbnail(workflow.key)
delete workflowLookup.value[workflow.path]
}
/**
* Save a workflow.
* @param workflow The workflow to save.
* @param options.overwrite Force overwrite of existing file at the path.
*/
const saveWorkflow = async (
workflow: ComfyWorkflow,
options?: { overwrite?: boolean }
) => {
const saveWorkflow = async (workflow: ComfyWorkflow) => {
isBusy.value = true
try {
await workflow.save({ overwrite: options?.overwrite })
await workflow.save()
// Synchronously detach and re-attach to force refresh the tree objects
// without an async gap that would cause the tab to disappear.
const openIndex = detachWorkflow(workflow)
@@ -796,7 +774,6 @@ export const useWorkflowStore = defineStore('workflow', () => {
createNewTemporary,
renameWorkflow,
deleteWorkflow,
removeWorkflowEntry,
saveAs,
saveWorkflow,
reorderWorkflows,

View File

@@ -1,59 +1,79 @@
import { flushPromises, mount } from '@vue/test-utils'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { ref } from 'vue'
import ComfyHubPublishDialog from '@/platform/workflow/sharing/components/publish/ComfyHubPublishDialog.vue'
import type { ComfyHubProfile } from '@/schemas/apiSchema'
const mockFetchProfile = vi.hoisted(() => vi.fn())
const mockGoToStep = vi.hoisted(() => vi.fn())
const mockGoNext = vi.hoisted(() => vi.fn())
const mockGoBack = vi.hoisted(() => vi.fn())
const mockOpenProfileCreationStep = vi.hoisted(() => vi.fn())
const mockCloseProfileCreationStep = vi.hoisted(() => vi.fn())
const mockFetchApi = vi.hoisted(() => vi.fn())
const mockToastErrorHandler = vi.hoisted(() => vi.fn())
const mockResolvedUserInfo = vi.hoisted(() => ({
value: { id: 'user-a' }
}))
vi.mock(
'@/platform/workflow/sharing/composables/useComfyHubProfileGate',
() => ({
useComfyHubProfileGate: () => ({
fetchProfile: mockFetchProfile
})
vi.mock('@/scripts/api', () => ({
api: {
fetchApi: mockFetchApi
}
}))
vi.mock('@/composables/auth/useCurrentUser', () => ({
useCurrentUser: () => ({
resolvedUserInfo: mockResolvedUserInfo
})
)
}))
vi.mock(
'@/platform/workflow/sharing/composables/useComfyHubPublishWizard',
() => ({
useComfyHubPublishWizard: () => ({
currentStep: ref('finish'),
formData: ref({
name: '',
description: '',
workflowType: '',
tags: [],
thumbnailType: 'image',
thumbnailFile: null,
comparisonBeforeFile: null,
comparisonAfterFile: null,
exampleImages: [],
selectedExampleIds: []
}),
isFirstStep: ref(false),
isLastStep: ref(true),
goToStep: mockGoToStep,
goNext: mockGoNext,
goBack: mockGoBack,
openProfileCreationStep: mockOpenProfileCreationStep,
closeProfileCreationStep: mockCloseProfileCreationStep
})
vi.mock('@/composables/useErrorHandling', () => ({
useErrorHandling: () => ({
toastErrorHandler: mockToastErrorHandler
})
)
}))
vi.mock('@/platform/workflow/management/stores/workflowStore', () => ({
useWorkflowStore: () => ({
activeWorkflow: { filename: 'test-workflow' }
})
}))
const mockProfile: ComfyHubProfile = {
username: 'testuser',
name: 'Test User',
description: 'A test profile'
}
function mockSuccessResponse(data?: unknown) {
return {
ok: true,
json: async () => data ?? mockProfile
} as Response
}
function mockErrorResponse(status = 404) {
return {
ok: false,
status,
json: async () => ({ message: 'Not found' })
} as Response
}
// Reset module-level singleton state in useComfyHubProfileGate between tests
async function resetProfileGateSingleton() {
const { useComfyHubProfileGate } =
await import('@/platform/workflow/sharing/composables/useComfyHubProfileGate')
const gate = useComfyHubProfileGate()
gate.hasProfile.value = null
gate.profile.value = null
gate.isCheckingProfile.value = false
gate.isFetchingProfile.value = false
}
describe('ComfyHubPublishDialog', () => {
const onClose = vi.fn()
beforeEach(() => {
beforeEach(async () => {
vi.clearAllMocks()
mockFetchProfile.mockResolvedValue(null)
mockResolvedUserInfo.value = { id: 'user-a' }
mockFetchApi.mockResolvedValue(mockErrorResponse())
await resetProfileGateSingleton()
})
function createWrapper() {
@@ -78,7 +98,7 @@ describe('ComfyHubPublishDialog', () => {
},
ComfyHubPublishWizardContent: {
template:
'<div><button data-testid="require-profile" @click="$props.onRequireProfile()" /><button data-testid="gate-complete" @click="$props.onGateComplete()" /><button data-testid="gate-close" @click="$props.onGateClose()" /></div>',
'<div><button data-testid="require-profile" @click="$props.onRequireProfile()" /><button data-testid="gate-complete" @click="$props.onGateComplete()" /><button data-testid="gate-close" @click="$props.onGateClose()" /><span data-testid="current-step">{{ $props.currentStep }}</span></div>',
props: [
'currentStep',
'formData',
@@ -88,7 +108,9 @@ describe('ComfyHubPublishDialog', () => {
'onGoBack',
'onRequireProfile',
'onGateComplete',
'onGateClose'
'onGateClose',
'onUpdateFormData',
'onPublish'
]
}
}
@@ -96,44 +118,62 @@ describe('ComfyHubPublishDialog', () => {
})
}
it('starts in publish wizard mode and prefetches profile asynchronously', async () => {
it('prefetches profile on mount via real composable', async () => {
createWrapper()
await flushPromises()
expect(mockFetchProfile).toHaveBeenCalledWith()
expect(mockFetchApi).toHaveBeenCalledWith('/hub/profile')
})
it('switches to profile creation step when final-step publish requires profile', async () => {
it('starts on the describe step with real wizard composable', async () => {
const wrapper = createWrapper()
await flushPromises()
expect(wrapper.find('[data-testid="current-step"]').text()).toBe('describe')
})
it('switches to profileCreation step when require-profile is triggered', async () => {
const wrapper = createWrapper()
await flushPromises()
await wrapper.find('[data-testid="require-profile"]').trigger('click')
expect(mockOpenProfileCreationStep).toHaveBeenCalledOnce()
expect(wrapper.find('[data-testid="current-step"]').text()).toBe(
'profileCreation'
)
})
it('returns to finish state after gate complete and does not auto-close', async () => {
it('returns to finish step and re-fetches profile after gate complete', async () => {
mockFetchApi.mockResolvedValue(mockSuccessResponse())
const wrapper = createWrapper()
await flushPromises()
await wrapper.find('[data-testid="require-profile"]').trigger('click')
expect(wrapper.find('[data-testid="current-step"]').text()).toBe(
'profileCreation'
)
await wrapper.find('[data-testid="gate-complete"]').trigger('click')
await flushPromises()
expect(mockOpenProfileCreationStep).toHaveBeenCalledOnce()
expect(mockCloseProfileCreationStep).toHaveBeenCalledOnce()
expect(mockFetchProfile).toHaveBeenCalledWith({ force: true })
expect(wrapper.find('[data-testid="current-step"]').text()).toBe('finish')
// Initial prefetch + force re-fetch after gate complete
expect(mockFetchApi).toHaveBeenCalledTimes(2)
expect(onClose).not.toHaveBeenCalled()
})
it('returns to finish state when profile gate is closed', async () => {
it('returns to finish step when profile gate is closed', async () => {
const wrapper = createWrapper()
await flushPromises()
await wrapper.find('[data-testid="require-profile"]').trigger('click')
expect(wrapper.find('[data-testid="current-step"]').text()).toBe(
'profileCreation'
)
await wrapper.find('[data-testid="gate-close"]').trigger('click')
expect(mockOpenProfileCreationStep).toHaveBeenCalledOnce()
expect(mockCloseProfileCreationStep).toHaveBeenCalledOnce()
expect(wrapper.find('[data-testid="current-step"]').text()).toBe('finish')
expect(onClose).not.toHaveBeenCalled()
})
})

View File

@@ -140,14 +140,11 @@ export class UserFile {
* Saves the file to the remote storage.
* @param force Whether to force the save even if the file is not modified.
*/
async save({
force = false,
overwrite
}: { force?: boolean; overwrite?: boolean } = {}): Promise<UserFile> {
async save({ force = false }: { force?: boolean } = {}): Promise<UserFile> {
if (this.isPersisted && !this.isModified && !force) return this
const resp = await api.storeUserData(this.path, this.content, {
overwrite: overwrite ?? this.isPersisted,
overwrite: this.isPersisted,
throwOnError: true,
full_info: true
})