mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-25 07:05:26 +00:00
## Summary Improved type safety in test files by eliminating unsafe type assertions and adopting official testing patterns. Reduced unsafe `as unknown as` type assertions and eliminated all `null!` assertions. ## Changes - **Adopted @pinia/testing patterns** - Replaced manual Pinia store mocking with `createTestingPinia()` in `useSelectionState.test.ts` - Eliminated ~120 lines of mock boilerplate - Created `createMockSettingStore()` helper to replace duplicated store mocks in `useCoreCommands.test.ts` - **Eliminated unsafe null assertions** - Created explicit `MockMaskEditorStore` interface with proper nullable types in `useCanvasTools.test.ts` - Replaced `null!` initializations with `null` and used `!` at point of use or `?.` for optional chaining - **Made partial mock intent explicit** - Updated test utilities in `litegraphTestUtils.ts` to use explicit `Partial<T>` typing - Changed cast pattern from `as T` to `as Partial<T> as T` to show incomplete mock intent - Applied to `createMockLGraphNode()`, `createMockPositionable()`, and `createMockLGraphGroup()` - **Created centralized mock utilities** in `src/utils/__tests__/litegraphTestUtils.ts` - `createMockLGraphNode()`, `createMockPositionable()`, `createMockLGraphGroup()`, `createMockSubgraphNode()` - Updated 8+ test files to use centralized utilities - Used union types `Partial<T> | Record<string, unknown>` for flexible mock creation ## Results - ✅ 0 typecheck errors - ✅ 0 lint errors - ✅ All tests passing in modified files - ✅ Eliminated all `null!` assertions - ✅ Reduced unsafe double-cast patterns significantly ## Files Modified (18) - `src/components/graph/SelectionToolbox.test.ts` - `src/components/graph/selectionToolbox/{BypassButton,ColorPickerButton,ExecuteButton}.test.ts` - `src/components/sidebar/tabs/queue/ResultGallery.test.ts` - `src/composables/canvas/useSelectedLiteGraphItems.test.ts` - `src/composables/graph/{useGraphHierarchy,useSelectionState}.test.ts` - `src/composables/maskeditor/{useCanvasHistory,useCanvasManager,useCanvasTools,useCanvasTransform}.test.ts` - `src/composables/node/{useNodePricing,useWatchWidget}.test.ts` - `src/composables/{useBrowserTabTitle,useCoreCommands}.test.ts` - `src/utils/__tests__/litegraphTestUtils.ts` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8258-refactor-eliminate-unsafe-type-assertions-from-Group-2-test-files-2f16d73d365081549c65fd546cc7c765) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: AustinMroz <austin@comfy.org> Co-authored-by: Christian Byrne <cbyrne@comfy.org> Co-authored-by: Benjamin Lu <benjaminlu1107@gmail.com>
184 lines
5.5 KiB
TypeScript
184 lines
5.5 KiB
TypeScript
import { describe, expect, it, vi } from 'vitest'
|
|
import { nextTick } from 'vue'
|
|
|
|
import { useComputedWithWidgetWatch } from '@/composables/node/useWatchWidget'
|
|
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
|
import { createMockLGraphNode } from '@/utils/__tests__/litegraphTestUtils'
|
|
|
|
// Mock useChainCallback
|
|
vi.mock('@/composables/functional/useChainCallback', () => ({
|
|
useChainCallback: vi.fn((original, newCallback) => {
|
|
return function (this: unknown, ...args: unknown[]) {
|
|
original?.call(this, ...args)
|
|
newCallback.call(this, ...args)
|
|
}
|
|
})
|
|
}))
|
|
|
|
describe('useComputedWithWidgetWatch', () => {
|
|
const createMockNode = (
|
|
widgets: Array<{
|
|
name: string
|
|
value: unknown
|
|
callback?: (...args: unknown[]) => void
|
|
}> = []
|
|
): LGraphNode => {
|
|
const baseNode = createMockLGraphNode()
|
|
return Object.assign(baseNode, {
|
|
widgets: widgets.map((widget) => ({
|
|
name: widget.name,
|
|
value: widget.value,
|
|
callback: widget.callback
|
|
})),
|
|
graph: {
|
|
setDirtyCanvas: vi.fn()
|
|
}
|
|
})
|
|
}
|
|
|
|
it('should create a reactive computed that responds to widget changes', async () => {
|
|
const mockNode = createMockNode([
|
|
{ name: 'width', value: 100 },
|
|
{ name: 'height', value: 200 }
|
|
])
|
|
|
|
const computedWithWidgetWatch = useComputedWithWidgetWatch(mockNode)
|
|
|
|
const computedValue = computedWithWidgetWatch(() => {
|
|
const width =
|
|
(mockNode.widgets?.find((w) => w.name === 'width')?.value as number) ||
|
|
0
|
|
const height =
|
|
(mockNode.widgets?.find((w) => w.name === 'height')?.value as number) ||
|
|
0
|
|
return width * height
|
|
})
|
|
|
|
// Initial value should be computed correctly
|
|
expect(computedValue.value).toBe(20000)
|
|
|
|
// Change widget value and trigger callback
|
|
const widthWidget = mockNode.widgets?.find((w) => w.name === 'width')
|
|
if (widthWidget && widthWidget.callback) {
|
|
widthWidget.value = 150
|
|
widthWidget.callback(widthWidget.value)
|
|
}
|
|
|
|
await nextTick()
|
|
|
|
// Computed should update
|
|
expect(computedValue.value).toBe(30000)
|
|
})
|
|
|
|
it('should only observe specific widgets when widgetNames is provided', async () => {
|
|
const mockNode = createMockNode([
|
|
{ name: 'width', value: 100 },
|
|
{ name: 'height', value: 200 },
|
|
{ name: 'depth', value: 50 }
|
|
])
|
|
|
|
const computedWithWidgetWatch = useComputedWithWidgetWatch(mockNode, {
|
|
widgetNames: ['width', 'height']
|
|
})
|
|
|
|
const computedValue = computedWithWidgetWatch(() => {
|
|
return mockNode.widgets?.map((w) => w.value).join('-') || ''
|
|
})
|
|
|
|
expect(computedValue.value).toBe('100-200-50')
|
|
|
|
// Change observed widget
|
|
const widthWidget = mockNode.widgets?.find((w) => w.name === 'width')
|
|
if (widthWidget && widthWidget.callback) {
|
|
widthWidget.value = 150
|
|
widthWidget.callback(widthWidget.value)
|
|
}
|
|
|
|
await nextTick()
|
|
expect(computedValue.value).toBe('150-200-50')
|
|
|
|
// Change non-observed widget - should not trigger update
|
|
const depthWidget = mockNode.widgets?.find((w) => w.name === 'depth')
|
|
if (depthWidget) {
|
|
depthWidget.value = 75
|
|
// Depth widget callback should not have been modified
|
|
expect(depthWidget.callback).toBeUndefined()
|
|
}
|
|
})
|
|
|
|
it('should trigger canvas redraw when triggerCanvasRedraw is true', async () => {
|
|
const mockNode = createMockNode([{ name: 'value', value: 10 }])
|
|
|
|
const computedWithWidgetWatch = useComputedWithWidgetWatch(mockNode, {
|
|
triggerCanvasRedraw: true
|
|
})
|
|
|
|
computedWithWidgetWatch(() => mockNode.widgets?.[0]?.value || 0)
|
|
|
|
// Change widget value
|
|
const widget = mockNode.widgets?.[0]
|
|
if (widget && widget.callback) {
|
|
widget.value = 20
|
|
widget.callback(widget.value)
|
|
}
|
|
|
|
await nextTick()
|
|
|
|
// Canvas redraw should have been triggered
|
|
expect(mockNode.graph?.setDirtyCanvas).toHaveBeenCalledWith(true, true)
|
|
})
|
|
|
|
it('should not trigger canvas redraw when triggerCanvasRedraw is false', async () => {
|
|
const mockNode = createMockNode([{ name: 'value', value: 10 }])
|
|
|
|
const computedWithWidgetWatch = useComputedWithWidgetWatch(mockNode, {
|
|
triggerCanvasRedraw: false
|
|
})
|
|
|
|
computedWithWidgetWatch(() => mockNode.widgets?.[0]?.value || 0)
|
|
|
|
// Change widget value
|
|
const widget = mockNode.widgets?.[0]
|
|
if (widget && widget.callback) {
|
|
widget.value = 20
|
|
widget.callback(widget.value)
|
|
}
|
|
|
|
await nextTick()
|
|
|
|
// Canvas redraw should not have been triggered
|
|
expect(mockNode.graph?.setDirtyCanvas).not.toHaveBeenCalled()
|
|
})
|
|
|
|
it('should handle nodes without widgets gracefully', () => {
|
|
const mockNode = createMockNode([])
|
|
|
|
const computedWithWidgetWatch = useComputedWithWidgetWatch(mockNode)
|
|
|
|
const computedValue = computedWithWidgetWatch(() => 'no widgets')
|
|
|
|
expect(computedValue.value).toBe('no widgets')
|
|
})
|
|
|
|
it('should chain with existing widget callbacks', async () => {
|
|
const existingCallback = vi.fn()
|
|
const mockNode = createMockNode([
|
|
{ name: 'value', value: 10, callback: existingCallback }
|
|
])
|
|
|
|
const computedWithWidgetWatch = useComputedWithWidgetWatch(mockNode)
|
|
computedWithWidgetWatch(() => mockNode.widgets?.[0]?.value || 0)
|
|
|
|
// Trigger widget callback
|
|
const widget = mockNode.widgets?.[0]
|
|
if (widget && widget.callback) {
|
|
widget.callback(widget.value)
|
|
}
|
|
|
|
await nextTick()
|
|
|
|
// Both existing callback and our callback should have been called
|
|
expect(existingCallback).toHaveBeenCalled()
|
|
})
|
|
})
|