Files
ComfyUI_frontend/src/components/rightSidePanel/errors/swapNodeGroups.test.ts
jaeone94 66daa6d645 refactor: error system cleanup — store separation, DDD fix, test improvements (#10302)
## Summary

Refactors the error system to improve separation of concerns, fix DDD
layer violations, and address code quality issues.

- Extract `missingNodesErrorStore` from `executionErrorStore`, removing
the delegation pattern that coupled missing-node logic into the
execution error store
- Extract `useNodeErrorFlagSync` composable for node error flag
reconciliation (previously inlined)
- Extract `useErrorClearingHooks` composable with explicit callback
cleanup on node removal
- Extract `useErrorActions` composable to deduplicate telemetry+command
patterns across error card components
- Move `getCnrIdFromNode`/`getCnrIdFromProperties` to
`platform/nodeReplacement` layer (DDD fix)
- Move `missingNodesErrorStore` to `platform/nodeReplacement` (DDD
alignment)
- Add unmount cancellation guard to `useErrorReport` async `onMounted`
- Return watch stop handle from `useNodeErrorFlagSync`
- Add `asyncResolvedIds` eviction on `missingNodesError` reset
- Add `console.warn` to silent catch blocks and empty array guard
- Hoist `useCommandStore` to setup scope, fix floating promises
- Add `data-testid` to error groups, image/video error spans, copy
button
- Update E2E tests to use scoped locators and testids
- Add unit tests for `onNodeRemoved` restoration and double-install
guard

Fixes #9875, Fixes #10027, Fixes #10033, Fixes #10085

## Test plan

- [x] Existing unit tests pass with updated imports and mocks
- [x] New unit tests for `useErrorClearingHooks` (callback restoration,
double-install guard)
- [x] E2E tests updated to use scoped locators and `data-testid`
- [ ] Manual: verify error tab shows runtime errors and missing nodes
correctly
- [ ] Manual: verify "Find on GitHub", "Copy", and "Get Help" buttons
work in error cards

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10302-refactor-error-system-cleanup-store-separation-DDD-fix-test-improvements-3286d73d365081838279d045b8dd957a)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-03-24 16:43:22 +09:00

187 lines
4.9 KiB
TypeScript

import { createPinia, setActivePinia } from 'pinia'
import { nextTick, ref } from 'vue'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import type { MissingNodeType } from '@/types/comfy'
vi.mock('@/scripts/app', () => ({
app: {
rootGraph: {
serialize: vi.fn(() => ({})),
getNodeById: vi.fn()
}
}
}))
vi.mock('@/utils/graphTraversalUtil', () => ({
getNodeByExecutionId: vi.fn(),
getExecutionIdByNode: vi.fn(),
getRootParentNode: vi.fn(() => null),
forEachNode: vi.fn(),
mapAllNodes: vi.fn(() => [])
}))
vi.mock('@/platform/distribution/types', () => ({
isCloud: false
}))
vi.mock('@/i18n', () => ({
st: vi.fn((_key: string, fallback: string) => fallback)
}))
vi.mock('@/stores/comfyRegistryStore', () => ({
useComfyRegistryStore: () => ({
inferPackFromNodeName: vi.fn()
})
}))
vi.mock('@/utils/nodeTitleUtil', () => ({
resolveNodeDisplayName: vi.fn(() => '')
}))
vi.mock('@/utils/litegraphUtil', () => ({
isLGraphNode: vi.fn(() => false)
}))
vi.mock('@/utils/executableGroupNodeDto', () => ({
isGroupNode: vi.fn(() => false)
}))
import { useMissingNodesErrorStore } from '@/platform/nodeReplacement/missingNodesErrorStore'
import { useErrorGroups } from './useErrorGroups'
function makeMissingNodeType(
type: string,
opts: {
nodeId?: string
isReplaceable?: boolean
replacement?: { new_node_id: string }
} = {}
): MissingNodeType {
return {
type,
nodeId: opts.nodeId ?? '1',
isReplaceable: opts.isReplaceable ?? false,
replacement: opts.replacement
? {
old_node_id: type,
new_node_id: opts.replacement.new_node_id,
old_widget_ids: null,
input_mapping: null,
output_mapping: null
}
: undefined
}
}
describe('swapNodeGroups computed', () => {
beforeEach(() => {
setActivePinia(createPinia())
})
function getSwapNodeGroups(nodeTypes: MissingNodeType[]) {
useMissingNodesErrorStore().surfaceMissingNodes(nodeTypes)
const searchQuery = ref('')
const t = (key: string) => key
const { swapNodeGroups } = useErrorGroups(searchQuery, t)
return swapNodeGroups
}
it('returns empty array when no missing nodes', () => {
const swap = getSwapNodeGroups([])
expect(swap.value).toEqual([])
})
it('returns empty array when no nodes are replaceable', () => {
const swap = getSwapNodeGroups([
makeMissingNodeType('NodeA', { isReplaceable: false }),
makeMissingNodeType('NodeB', { isReplaceable: false })
])
expect(swap.value).toEqual([])
})
it('groups replaceable nodes by type', async () => {
const swap = getSwapNodeGroups([
makeMissingNodeType('OldNode', {
nodeId: '1',
isReplaceable: true,
replacement: { new_node_id: 'NewNode' }
}),
makeMissingNodeType('OldNode', {
nodeId: '2',
isReplaceable: true,
replacement: { new_node_id: 'NewNode' }
})
])
await nextTick()
expect(swap.value).toHaveLength(1)
expect(swap.value[0].type).toBe('OldNode')
expect(swap.value[0].newNodeId).toBe('NewNode')
expect(swap.value[0].nodeTypes).toHaveLength(2)
})
it('creates separate groups for different types', async () => {
const swap = getSwapNodeGroups([
makeMissingNodeType('TypeA', {
nodeId: '1',
isReplaceable: true,
replacement: { new_node_id: 'NewA' }
}),
makeMissingNodeType('TypeB', {
nodeId: '2',
isReplaceable: true,
replacement: { new_node_id: 'NewB' }
})
])
await nextTick()
expect(swap.value).toHaveLength(2)
expect(swap.value.map((g) => g.type)).toEqual(['TypeA', 'TypeB'])
})
it('sorts groups alphabetically by type', async () => {
const swap = getSwapNodeGroups([
makeMissingNodeType('Zebra', {
nodeId: '1',
isReplaceable: true,
replacement: { new_node_id: 'NewZ' }
}),
makeMissingNodeType('Alpha', {
nodeId: '2',
isReplaceable: true,
replacement: { new_node_id: 'NewA' }
})
])
await nextTick()
expect(swap.value[0].type).toBe('Alpha')
expect(swap.value[1].type).toBe('Zebra')
})
it('excludes string nodeType entries', async () => {
const swap = getSwapNodeGroups([
'StringGroupNode' as unknown as MissingNodeType,
makeMissingNodeType('OldNode', {
nodeId: '1',
isReplaceable: true,
replacement: { new_node_id: 'NewNode' }
})
])
await nextTick()
expect(swap.value).toHaveLength(1)
expect(swap.value[0].type).toBe('OldNode')
})
it('sets newNodeId to undefined when replacement is missing', async () => {
const swap = getSwapNodeGroups([
makeMissingNodeType('OldNode', {
nodeId: '1',
isReplaceable: true
// no replacement
})
])
await nextTick()
expect(swap.value).toHaveLength(1)
expect(swap.value[0].newNodeId).toBeUndefined()
})
})