mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-23 14:16:00 +00:00
test: address review feedback on COLOR widget tests
- Drop shoehorn in favour of new LGraphNode() + vi.spyOn(), matching the pattern already used by useComboWidget.test.ts. No more 'as unknown as'. - Extract hex literals to named constants. - Add coverage for serialization, custom input name, and the no-default case (asserts widget still produces a usable color widget rather than pinning the specific fallback colour). - Split the e2e into two scenarios per review: one adds the node via the search box (real user flow), one loads a workflow asset that persists the colour value. Both assert against the user-visible hex string on the ColorPicker trigger button instead of the internal widget value. - Note: the FE-800 ticket reference in the describe block was a holdover from the synthetic-node draft; removed. - AGENTS.md addendum documenting the devtools-node pattern for Python-side test fixtures.
This commit is contained in:
@@ -122,6 +122,16 @@ Reserve `toPass()` for blocks with multiple assertions or complex async logic th
|
||||
| Context menu empty or wrong items | Node not selected | Select node first: `vueNodes.selectNode()` or `nodeRef.click('title')` |
|
||||
| `navigateIntoSubgraph` timeout | Node too small in test asset JSON | Use node size `[400, 200]` minimum |
|
||||
|
||||
## Backend Test Nodes (DevTools)
|
||||
|
||||
When a test needs a node, input shape, or backend behavior that no production node exposes, prefer **adding a node to `tools/devtools/nodes/`** over intercepting `/object_info` from the test.
|
||||
|
||||
- Add the new class to the appropriate file (e.g. `inputs.py` for input-shape variants), register it in that file's `NODE_CLASS_MAPPINGS` / `NODE_DISPLAY_NAME_MAPPINGS` with a `DevTools` prefix, and re-export it through `nodes/__init__.py` and `dev_nodes.py`.
|
||||
- In the spec, create the node via `comfyPage.nodeOps.addNode('DevToolsYourNode')` and assert against the rendered widget (see `vueNodes/widgets/legacy.spec.ts` and `vueNodes/widgets/color/colorWidget.spec.ts`).
|
||||
- Reserve `page.route(/\/object_info$/, ...)` for cases where the test needs to mutate an existing production node's shape mid-flight (e.g. `propertiesPanel/errorsTabMissingModels.spec.ts`).
|
||||
|
||||
Devtools nodes are loaded automatically into the test ComfyUI instance, so no fixture wiring is required.
|
||||
|
||||
## After Making Changes
|
||||
|
||||
- Run `pnpm typecheck:browser` after modifying TypeScript files in this directory
|
||||
|
||||
28
browser_tests/assets/vueNodes/color-widget-default.json
Normal file
28
browser_tests/assets/vueNodes/color-widget-default.json
Normal file
@@ -0,0 +1,28 @@
|
||||
{
|
||||
"id": "c0107e57-c01047-c01047-c01047-c0101010c010",
|
||||
"revision": 0,
|
||||
"last_node_id": 1,
|
||||
"last_link_id": 0,
|
||||
"nodes": [
|
||||
{
|
||||
"id": 1,
|
||||
"type": "DevToolsNodeWithColorInput",
|
||||
"pos": [400, 300],
|
||||
"size": [270, 58],
|
||||
"flags": {},
|
||||
"order": 0,
|
||||
"mode": 0,
|
||||
"inputs": [],
|
||||
"outputs": [],
|
||||
"properties": {
|
||||
"Node name for S&R": "DevToolsNodeWithColorInput"
|
||||
},
|
||||
"widgets_values": ["#00ff00"]
|
||||
}
|
||||
],
|
||||
"links": [],
|
||||
"groups": [],
|
||||
"config": {},
|
||||
"extra": {},
|
||||
"version": 0.4
|
||||
}
|
||||
@@ -3,20 +3,34 @@ import {
|
||||
comfyPageFixture as test
|
||||
} from '@e2e/fixtures/ComfyPage'
|
||||
|
||||
test.describe(
|
||||
'Vue Color Widget defaults (FE-800)',
|
||||
{ tag: '@vue-nodes' },
|
||||
() => {
|
||||
test('respects a non-black default declared on the node input', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
const nodeRef = await comfyPage.nodeOps.addNode(
|
||||
'DevToolsNodeWithColorInput'
|
||||
)
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
const COLOR_NODE_DISPLAY_NAME = 'Node With Color Input'
|
||||
const DECLARED_DEFAULT = '#00ff00'
|
||||
|
||||
const widget = await nodeRef.getWidgetByName('color_input')
|
||||
await expect.poll(() => widget.getValue()).toBe('#00ff00')
|
||||
test.describe('Vue Color Widget defaults', { tag: '@vue-nodes' }, () => {
|
||||
test('respects the declared default value in the input spec', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.page.mouse.dblclick(200, 200, { delay: 5 })
|
||||
await comfyPage.searchBox.fillAndSelectFirstNode(COLOR_NODE_DISPLAY_NAME)
|
||||
|
||||
const node = comfyPage.vueNodes.getNodeByTitle(COLOR_NODE_DISPLAY_NAME)
|
||||
const colorTrigger = node.getByRole('button', {
|
||||
name: new RegExp(DECLARED_DEFAULT, 'i')
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
await expect(colorTrigger).toBeVisible()
|
||||
})
|
||||
|
||||
test('restores a saved color value when loading a workflow', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.workflow.loadWorkflow('vueNodes/color-widget-default')
|
||||
|
||||
const node = comfyPage.vueNodes.getNodeByTitle(COLOR_NODE_DISPLAY_NAME)
|
||||
const colorTrigger = node.getByRole('button', {
|
||||
name: new RegExp(DECLARED_DEFAULT, 'i')
|
||||
})
|
||||
|
||||
await expect(colorTrigger).toBeVisible()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,92 +1,104 @@
|
||||
import { fromAny } from '@total-typescript/shoehorn'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import type {
|
||||
IBaseWidget,
|
||||
IColorWidget
|
||||
} from '@/lib/litegraph/src/types/widgets'
|
||||
import { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
|
||||
import type { ColorInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
|
||||
import { useColorWidget } from '@/renderer/extensions/vueNodes/widgets/composables/useColorWidget'
|
||||
|
||||
function createMockNode() {
|
||||
const addWidget = vi.fn(
|
||||
(
|
||||
type: string,
|
||||
name: string,
|
||||
value: unknown,
|
||||
_callback: unknown,
|
||||
options: Record<string, unknown>
|
||||
): IBaseWidget =>
|
||||
const TOP_LEVEL_DEFAULT = '#00ff00'
|
||||
const NESTED_DEFAULT = '#abcdef'
|
||||
const BLACK_FALLBACK = '#000000'
|
||||
|
||||
function createMockNode(): {
|
||||
node: LGraphNode
|
||||
addWidget: ReturnType<typeof vi.spyOn>
|
||||
} {
|
||||
const node = new LGraphNode('TestColorNode')
|
||||
const addWidget = vi.spyOn(node, 'addWidget').mockImplementation(
|
||||
(type, name, value, _callback, options) =>
|
||||
({
|
||||
type,
|
||||
name,
|
||||
value,
|
||||
options,
|
||||
options: typeof options === 'string' ? { property: options } : options,
|
||||
y: 0
|
||||
}) as unknown as IBaseWidget
|
||||
}) as IBaseWidget
|
||||
)
|
||||
const node = fromAny<LGraphNode, unknown>({ addWidget })
|
||||
return { node, addWidget }
|
||||
}
|
||||
|
||||
function createColorSpec(
|
||||
overrides: Partial<ColorInputSpec> = {}
|
||||
): ColorInputSpec {
|
||||
return { type: 'COLOR', name: 'color', ...overrides }
|
||||
}
|
||||
|
||||
describe('useColorWidget', () => {
|
||||
it('uses the top-level default produced by the V1 → V2 migration', () => {
|
||||
const { node, addWidget } = createMockNode()
|
||||
const inputSpec = fromAny<ColorInputSpec, unknown>({
|
||||
type: 'COLOR',
|
||||
name: 'color',
|
||||
default: '#00ff00'
|
||||
})
|
||||
const inputSpec = createColorSpec({ default: TOP_LEVEL_DEFAULT })
|
||||
|
||||
const widget = useColorWidget()(node, inputSpec) as IColorWidget
|
||||
const widget = useColorWidget()(node, inputSpec)
|
||||
|
||||
expect(addWidget).toHaveBeenCalledWith(
|
||||
'color',
|
||||
'color',
|
||||
'#00ff00',
|
||||
TOP_LEVEL_DEFAULT,
|
||||
expect.any(Function),
|
||||
{ serialize: true }
|
||||
)
|
||||
expect(widget.value).toBe('#00ff00')
|
||||
expect(widget.value).toBe(TOP_LEVEL_DEFAULT)
|
||||
})
|
||||
|
||||
it('uses options.default when the spec follows the V2 nested shape', () => {
|
||||
const { node, addWidget } = createMockNode()
|
||||
const inputSpec: ColorInputSpec = {
|
||||
type: 'COLOR',
|
||||
name: 'color',
|
||||
options: { default: '#abcdef' }
|
||||
}
|
||||
const inputSpec = createColorSpec({ options: { default: NESTED_DEFAULT } })
|
||||
|
||||
useColorWidget()(node, inputSpec)
|
||||
|
||||
expect(addWidget.mock.calls[0]![2]).toBe('#abcdef')
|
||||
expect(addWidget.mock.calls[0]![2]).toBe(NESTED_DEFAULT)
|
||||
})
|
||||
|
||||
it('prefers the top-level default when both locations are present', () => {
|
||||
const { node, addWidget } = createMockNode()
|
||||
const inputSpec = fromAny<ColorInputSpec, unknown>({
|
||||
type: 'COLOR',
|
||||
name: 'color',
|
||||
default: '#111111',
|
||||
options: { default: '#222222' }
|
||||
const inputSpec = createColorSpec({
|
||||
default: TOP_LEVEL_DEFAULT,
|
||||
options: { default: NESTED_DEFAULT }
|
||||
})
|
||||
|
||||
useColorWidget()(node, inputSpec)
|
||||
|
||||
expect(addWidget.mock.calls[0]![2]).toBe('#111111')
|
||||
expect(addWidget.mock.calls[0]![2]).toBe(TOP_LEVEL_DEFAULT)
|
||||
})
|
||||
|
||||
it('falls back to black when no default is provided', () => {
|
||||
it('still produces a usable widget when no default is supplied', () => {
|
||||
const { node, addWidget } = createMockNode()
|
||||
const inputSpec: ColorInputSpec = {
|
||||
type: 'COLOR',
|
||||
name: 'color'
|
||||
}
|
||||
const inputSpec = createColorSpec()
|
||||
|
||||
useColorWidget()(node, inputSpec)
|
||||
const widget = useColorWidget()(node, inputSpec)
|
||||
|
||||
expect(addWidget.mock.calls[0]![2]).toBe('#000000')
|
||||
expect(addWidget).toHaveBeenCalledOnce()
|
||||
expect(widget.type).toBe('color')
|
||||
expect(widget.name).toBe('color')
|
||||
expect(widget.value).toBe(BLACK_FALLBACK)
|
||||
})
|
||||
|
||||
it('serializes the widget so its value persists in saved workflows', () => {
|
||||
const { node, addWidget } = createMockNode()
|
||||
|
||||
useColorWidget()(node, createColorSpec({ default: TOP_LEVEL_DEFAULT }))
|
||||
|
||||
expect(addWidget.mock.calls[0]![4]).toEqual({ serialize: true })
|
||||
})
|
||||
|
||||
it('honours a custom input name from the spec', () => {
|
||||
const { node, addWidget } = createMockNode()
|
||||
|
||||
useColorWidget()(
|
||||
node,
|
||||
createColorSpec({ name: 'bg_color', default: TOP_LEVEL_DEFAULT })
|
||||
)
|
||||
|
||||
expect(addWidget.mock.calls[0]![1]).toBe('bg_color')
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user