From 8f136850f4601c02e79c26b464b9c6e51c1b89f5 Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Sun, 15 Oct 2023 13:34:28 +0100 Subject: [PATCH 01/13] Add test for converted widgets on missing nodes + fix crash --- tests-ui/tests/widgetInputs.test.js | 47 +++++++++++++++++++++++++++++ tests-ui/utils/litegraph.js | 3 ++ web/extensions/core/widgetInputs.js | 9 ++++-- 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/tests-ui/tests/widgetInputs.test.js b/tests-ui/tests/widgetInputs.test.js index 40f0528df..826c2bbb1 100644 --- a/tests-ui/tests/widgetInputs.test.js +++ b/tests-ui/tests/widgetInputs.test.js @@ -165,4 +165,51 @@ describe("widget inputs", () => { expect(clone.widgets.ckpt_name.isConvertedToInput).toBeFalsy(); expect(clone.inputs.ckpt_name).toBeFalsy(); }); + + test("shows missing node error on custom node with converted input", async () => { + const { graph } = await start(); + + const dialogShow = jest.spyOn(graph.app.ui.dialog, "show") + + await graph.app.loadGraphData({ + last_node_id: 3, + last_link_id: 4, + nodes: [ + { + id: 1, + type: "TestNode", + pos: [41.87329101561909, 389.7381480823742], + size: { 0: 220, 1: 374 }, + flags: {}, + order: 1, + mode: 0, + inputs: [{ name: "test", type: "FLOAT", link: 4, widget: { name: "test" }, slot_index: 0 }], + outputs: [], + properties: { "Node name for S&R": "TestNode" }, + widgets_values: [1], + }, + { + id: 3, + type: "PrimitiveNode", + pos: [-312, 433], + size: { 0: 210, 1: 82 }, + flags: {}, + order: 0, + mode: 0, + outputs: [{ links: [4], widget: { name: "test" } }], + title: "test", + properties: {}, + }, + ], + links: [[4, 3, 0, 1, 6, "FLOAT"]], + groups: [], + config: {}, + extra: {}, + version: 0.4, + }); + + expect(dialogShow).toBeCalledTimes(1); + expect(dialogShow.mock.calls[0][0]).toContain("the following node types were not found"); + expect(dialogShow.mock.calls[0][0]).toContain("TestNode"); + }); }); \ No newline at end of file diff --git a/tests-ui/utils/litegraph.js b/tests-ui/utils/litegraph.js index 821bbe38b..777f8c3ba 100644 --- a/tests-ui/utils/litegraph.js +++ b/tests-ui/utils/litegraph.js @@ -30,4 +30,7 @@ export function setup(ctx) { export function teardown(ctx) { forEachKey((k) => delete ctx[k]); + + // Clear document after each run + document.getElementsByTagName("html")[0].innerHTML = ""; } diff --git a/web/extensions/core/widgetInputs.js b/web/extensions/core/widgetInputs.js index ce05a29e9..bc0fc2bb0 100644 --- a/web/extensions/core/widgetInputs.js +++ b/web/extensions/core/widgetInputs.js @@ -315,7 +315,7 @@ app.registerExtension({ onAfterGraphConfigured() { if (this.outputs[0].links?.length && !this.widgets?.length) { - this.#onFirstConnection(); + if(!this.#onFirstConnection()) return; // Populate widget values from config data if (this.widgets) { @@ -386,13 +386,16 @@ app.registerExtension({ widget = input.widget; } - const { type } = getWidgetType(widget[GET_CONFIG]()); + const config = widget[GET_CONFIG]?.(); + if(!config) return; + + const { type } = getWidgetType(config); // Update our output to restrict to the widget type this.outputs[0].type = type; this.outputs[0].name = type; this.outputs[0].widget = widget; - this.#createWidget(widget[CONFIG] ?? widget[GET_CONFIG](), theirNode, widget.name, recreating); + this.#createWidget(widget[CONFIG] ?? config, theirNode, widget.name, recreating); } #createWidget(inputData, node, widgetName, recreating) { From 9d17f784769b6e49a81100d59a7a702f9ec6c76d Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Sun, 15 Oct 2023 13:35:38 +0100 Subject: [PATCH 02/13] tidy --- tests-ui/jest.config.js | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests-ui/jest.config.js b/tests-ui/jest.config.js index a5fa0e035..cb5a4da18 100644 --- a/tests-ui/jest.config.js +++ b/tests-ui/jest.config.js @@ -1,16 +1,7 @@ -const path = require("path"); /** @type {import('jest').Config} */ const config = { testEnvironment: "jsdom", - // transform: { - // "^.+\\.[t|j]sx?$": "babel-jest", - // }, setupFiles: ["./globalSetup.js"], - // moduleDirectories: ["node_modules", path.resolve("../web/scripts")], - // moduleNameMapper: { - // "./api.js": path.resolve("../web/scripts/api.js"), - // "./api": path.resolve("../web/scripts/api.js"), - // }, }; module.exports = config; From efea06dda53edba7c60c05ae609052aa8d2fed0e Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Sun, 15 Oct 2023 14:54:01 +0100 Subject: [PATCH 03/13] mores tests + refactor --- tests-ui/jest.config.js | 2 + tests-ui/tests/widgetInputs.test.js | 188 ++++++++++++++++++---------- tests-ui/utils/ezgraph.js | 112 ++++++++++------- tests-ui/utils/index.js | 42 +++++++ 4 files changed, 235 insertions(+), 109 deletions(-) diff --git a/tests-ui/jest.config.js b/tests-ui/jest.config.js index cb5a4da18..b5a5d646d 100644 --- a/tests-ui/jest.config.js +++ b/tests-ui/jest.config.js @@ -2,6 +2,8 @@ const config = { testEnvironment: "jsdom", setupFiles: ["./globalSetup.js"], + clearMocks: true, + resetModules: true, }; module.exports = config; diff --git a/tests-ui/tests/widgetInputs.test.js b/tests-ui/tests/widgetInputs.test.js index 826c2bbb1..7fed1f28f 100644 --- a/tests-ui/tests/widgetInputs.test.js +++ b/tests-ui/tests/widgetInputs.test.js @@ -1,9 +1,49 @@ -/// // @ts-check +/// -const { start } = require("../utils"); +const { start, makeNodeDef, checkBeforeAndAfterReload, assertNotNullOrUndefined } = require("../utils"); const lg = require("../utils/litegraph"); +/** + * @typedef { import("../utils/ezgraph") } Ez + * @typedef { ReturnType["ez"] } EzNodeFactory + */ + +/** + * @param { EzNodeFactory } ez + * @param { InstanceType } graph + * @param { InstanceType } input + * @param { string } widgetType + * @param { boolean } hasControlWidget + * @returns + */ +async function connectPrimitiveAndReload(ez, graph, input, widgetType, hasControlWidget) { + // Connect to primitive and ensure its still connected after + let primitive = ez.PrimitiveNode(); + primitive.outputs[0].connectTo(input); + + await checkBeforeAndAfterReload(graph, async () => { + primitive = graph.find(primitive); + let { connections } = primitive.outputs[0]; + expect(connections).toHaveLength(1); + expect(connections[0].targetNode.id).toBe(input.node.node.id); + + // Ensure widget is correct type + const valueWidget = primitive.widgets.value; + expect(valueWidget.widget.type).toBe(widgetType); + + // Check if control_after_generate should be added + if (hasControlWidget) { + const controlWidget = primitive.widgets.control_after_generate; + expect(controlWidget.widget.type).toBe("combo"); + } + + // Ensure we dont have other widgets + expect(primitive.node.widgets).toHaveLength(1 + +!!hasControlWidget); + }); + + return primitive; +} describe("widget inputs", () => { beforeEach(() => { @@ -12,7 +52,6 @@ describe("widget inputs", () => { afterEach(() => { lg.teardown(global); - jest.resetModules(); }); [ @@ -28,58 +67,27 @@ describe("widget inputs", () => { { name: "combo", type: ["a", "b", "c"], control: true }, ].forEach((c) => { test(`widget conversion + primitive works on ${c.name}`, async () => { - /** - * Test node with widgets of each type - * @type { import("../../web/types/comfy").ComfyObjectInfo } ComfyObjectInfo - */ - const WidgetTestNode = { - category: "test", - name: "WidgetTestNode", - output_name: [], - input: { - required: { - [c.name]: [c.type, c.opt ?? {}], - }, - }, - }; - - const { ez } = await start({ + const { ez, graph } = await start({ mockNodeDefs: { - WidgetTestNode, + ["TestNode"]: makeNodeDef("TestNode", { [c.name]: [c.type, c.opt ?? {}] }), }, }); // Create test node and convert to input - const n = ez.WidgetTestNode(); + const n = ez.TestNode(); const w = n.widgets[c.name]; w.convertToInput(); expect(w.isConvertedToInput).toBeTruthy(); const input = w.getConvertedInput(); expect(input).toBeTruthy(); - // Connect to primitive - const p1 = ez.PrimitiveNode(); - // @ts-ignore : input is valid - p1.outputs[0].connectTo(input); - expect(p1.outputs[0].connectTo).toHaveLength(1); - - // Ensure widget is correct type - const valueWidget = p1.widgets.value; - expect(valueWidget.widget.type).toBe(c.widget ?? c.name); - - // Check if control_after_generate should be added - if (c.control) { - const controlWidget = p1.widgets.control_after_generate; - expect(controlWidget.widget.type).toBe("combo"); - } - - // Ensure we dont have other widgets - expect(p1.node.widgets).toHaveLength(1 + +!!c.control); + // @ts-ignore : input is valid here + await connectPrimitiveAndReload(ez, graph, input, c.widget ?? c.name, c.control); }); }); test("converted widget works after reload", async () => { - const { graph, ez } = await start(); + const { ez, graph } = await start(); let n = ez.CheckpointLoaderSimple(); const inputCount = n.inputs.length; @@ -100,31 +108,14 @@ describe("widget inputs", () => { n.widgets.ckpt_name.convertToInput(); expect(n.inputs.length).toEqual(inputCount + 1); - let primitive = ez.PrimitiveNode(); - primitive.outputs[0].connectTo(n.inputs.ckpt_name); - - await graph.reload(); - - // Find the reloaded nodes in the graph - n = graph.find(n); - primitive = graph.find(primitive); - - // Ensure widget is converted - expect(n.widgets.ckpt_name.isConvertedToInput).toBeTruthy(); - expect(n.inputs.ckpt_name).toBeTruthy(); - expect(n.inputs.length).toEqual(inputCount + 1); - - // Ensure primitive is connected - let { connections } = primitive.outputs[0]; - expect(connections).toHaveLength(1); - expect(connections[0].targetNode.id).toBe(n.node.id); + const primitive = await connectPrimitiveAndReload(ez, graph, n.inputs.ckpt_name, "combo", true); // Disconnect & reconnect - connections[0].disconnect(); - ({ connections } = primitive.outputs[0]); + primitive.outputs[0].connections[0].disconnect(); + let { connections } = primitive.outputs[0]; expect(connections).toHaveLength(0); - primitive.outputs[0].connectTo(n.inputs.ckpt_name); + primitive.outputs[0].connectTo(n.inputs.ckpt_name); ({ connections } = primitive.outputs[0]); expect(connections).toHaveLength(1); expect(connections[0].targetNode.id).toBe(n.node.id); @@ -169,7 +160,7 @@ describe("widget inputs", () => { test("shows missing node error on custom node with converted input", async () => { const { graph } = await start(); - const dialogShow = jest.spyOn(graph.app.ui.dialog, "show") + const dialogShow = jest.spyOn(graph.app.ui.dialog, "show"); await graph.app.loadGraphData({ last_node_id: 3, @@ -212,4 +203,75 @@ describe("widget inputs", () => { expect(dialogShow.mock.calls[0][0]).toContain("the following node types were not found"); expect(dialogShow.mock.calls[0][0]).toContain("TestNode"); }); -}); \ No newline at end of file + + test("defaultInput widgets can be converted back to inputs", async () => { + const { graph, ez } = await start({ + mockNodeDefs: { + ["TestNode"]: makeNodeDef("TestNode", { example: ["INT", { defaultInput: true }] }), + }, + }); + + // Create test node and ensure it starts as an input + let n = ez.TestNode(); + let w = n.widgets.example; + expect(w.isConvertedToInput).toBeTruthy(); + let input = w.getConvertedInput(); + expect(input).toBeTruthy(); + + // Ensure it can be converted to + w.convertToWidget(); + expect(w.isConvertedToInput).toBeFalsy(); + expect(n.inputs.length).toEqual(0); + // and from + w.convertToInput(); + expect(w.isConvertedToInput).toBeTruthy(); + input = w.getConvertedInput(); + + // Reload and ensure it still only has 1 converted widget + if (!assertNotNullOrUndefined(input)) return; + + await connectPrimitiveAndReload(ez, graph, input, "number", true); + n = graph.find(n); + expect(n.widgets).toHaveLength(1); + w = n.widgets.example; + expect(w.isConvertedToInput).toBeTruthy(); + + // Convert back to widget and ensure it is still a widget after reload + w.convertToWidget(); + await graph.reload(); + n = graph.find(n); + expect(n.widgets).toHaveLength(1); + expect(n.widgets[0].isConvertedToInput).toBeFalsy(); + expect(n.inputs.length).toEqual(0); + }); + + test("forceInput widgets can not be converted back to inputs", async () => { + const { graph, ez } = await start({ + mockNodeDefs: { + ["TestNode"]: makeNodeDef("TestNode", { example: ["INT", { forceInput: true }] }), + }, + }); + + // Create test node and ensure it starts as an input + let n = ez.TestNode(); + let w = n.widgets.example; + expect(w.isConvertedToInput).toBeTruthy(); + const input = w.getConvertedInput(); + expect(input).toBeTruthy(); + + // Convert to widget should error + expect(() => w.convertToWidget()).toThrow(); + + // Reload and ensure it still only has 1 converted widget + if (assertNotNullOrUndefined(input)) { + await connectPrimitiveAndReload(ez, graph, input, "number", true); + n = graph.find(n); + expect(n.widgets).toHaveLength(1); + expect(n.widgets.example.isConvertedToInput).toBeTruthy(); + } + }); + + test("primitive combo cannot connect to non matching list", () => { + throw new Error("not implemented"); + }); +}); diff --git a/tests-ui/utils/ezgraph.js b/tests-ui/utils/ezgraph.js index fd1d2dc7f..1124ca698 100644 --- a/tests-ui/utils/ezgraph.js +++ b/tests-ui/utils/ezgraph.js @@ -12,7 +12,7 @@ * @typedef { (...args: EzOutput[] | [...EzOutput[], Record]) => EzNode } EzNodeFactory */ -class EzConnection { +export class EzConnection { /** @type { app } */ app; /** @type { InstanceType } */ @@ -48,7 +48,7 @@ class EzConnection { } } -class EzSlot { +export class EzSlot { /** @type { EzNode } */ node; /** @type { number } */ @@ -64,7 +64,7 @@ class EzSlot { } } -class EzInput extends EzSlot { +export class EzInput extends EzSlot { /** @type { INodeInputSlot } */ input; @@ -83,7 +83,7 @@ class EzInput extends EzSlot { } } -class EzOutput extends EzSlot { +export class EzOutput extends EzSlot { /** @type { INodeOutputSlot } */ output; @@ -98,8 +98,9 @@ class EzOutput extends EzSlot { } get connections() { - return (this.node.node.outputs?.[this.index]?.links ?? []) - .map(l => new EzConnection(this.node.app, this.node.app.graph.links[l])); + return (this.node.node.outputs?.[this.index]?.links ?? []).map( + (l) => new EzConnection(this.node.app, this.node.app.graph.links[l]) + ); } /** @@ -123,18 +124,22 @@ class EzOutput extends EzSlot { } } -class EzNodeMenuItem { +export class EzNodeMenuItem { /** @type { EzNode } */ node; + /** @type { number } */ + index; /** @type { ContextMenuItem } */ item; /** * @param { EzNode } node + * @param { number } index * @param { ContextMenuItem } item */ - constructor(node, item) { + constructor(node, index, item) { this.node = node; + this.index = index; this.item = item; } @@ -147,18 +152,22 @@ class EzNodeMenuItem { } } -class EzWidget { +export class EzWidget { /** @type { EzNode } */ node; + /** @type { number } */ + index; /** @type { IWidget } */ widget; /** * @param { EzNode } node + * @param { number } index * @param { IWidget } widget */ - constructor(node, widget) { + constructor(node, index, widget) { this.node = node; + this.index = index; this.widget = widget; } @@ -176,10 +185,9 @@ class EzWidget { } getConvertedInput() { - if (!this.isConvertedToInput) - throw new Error(`Widget ${this.widget.name} is not converted to input.`); - - return this.node.inputs.find(inp => inp.input["widget"]?.name === this.widget.name); + if (!this.isConvertedToInput) throw new Error(`Widget ${this.widget.name} is not converted to input.`); + + return this.node.inputs.find((inp) => inp.input["widget"]?.name === this.widget.name); } convertToWidget() { @@ -195,7 +203,7 @@ class EzWidget { } } -class EzNode { +export class EzNode { /** @type { app } */ app; /** @type { LGNode } */ @@ -215,55 +223,68 @@ class EzNode { } get inputs() { - return this.#getSlotItems("inputs"); + return this.#makeLookupArray("inputs", "name", EzInput); } get outputs() { - return this.#getSlotItems("outputs"); + return this.#makeLookupArray("outputs", "name", EzOutput); } - /** @returns { Record } */ get widgets() { - return (this.node.widgets ?? []).reduce((p, w, i) => { - p[w.name ?? i] = new EzWidget(this, w); - return p; - }, {}); + return this.#makeLookupArray("widgets", "name", EzWidget); } get menu() { - const items = this.app.canvas.getNodeMenuOptions(this.node); - return items.reduce((p, w) => { - if(w?.content) { - p[w.content] = new EzNodeMenuItem(this, w); - } - return p; - }, {}); + return this.#makeLookupArray(() => this.app.canvas.getNodeMenuOptions(this.node), "content", EzNodeMenuItem); } select() { this.app.canvas.selectNode(this.node); } - /** - * @template { "inputs" | "outputs" } T - * @param { T } type - * @returns { Record & (type extends "inputs" ? EzInput [] : EzOutput[]) } - */ - #getSlotItems(type) { - // @ts-ignore : these items are correct - return (this.node[type] ?? []).reduce((p, s, i) => { - if(s.name in p) { - throw new Error(`Unable to store input ${s.name} on array as name conflicts.`); - } - ; + // /** + // * @template { "inputs" | "outputs" } T + // * @param { T } type + // * @returns { Record & (type extends "inputs" ? EzInput [] : EzOutput[]) } + // */ + // #getSlotItems(type) { + // // @ts-ignore : these items are correct + // return (this.node[type] ?? []).reduce((p, s, i) => { + // if (s.name in p) { + // throw new Error(`Unable to store input ${s.name} on array as name conflicts.`); + // } + // // @ts-ignore + // p.push((p[s.name] = new (type === "inputs" ? EzInput : EzOutput)(this, i, s))); + // return p; + // }, Object.assign([], { $: this })); + // } + + /** + * @template { { new(node: EzNode, index: number, obj: any): any } } T + * @param { "inputs" | "outputs" | "widgets" | (() => Array) } nodeProperty + * @param { string } nameProperty + * @param { T } ctor + * @returns { Record> & Array> } + */ + #makeLookupArray(nodeProperty, nameProperty, ctor) { + const items = typeof nodeProperty === "function" ? nodeProperty() : this.node[nodeProperty]; + // @ts-ignore + return (items ?? []).reduce((p, s, i) => { + if (!s) return p; + + const name = s[nameProperty]; // @ts-ignore - p.push(p[s.name] = new (type === "inputs" ? EzInput : EzOutput)(this, i, s)); + if (!name || name in p) { + throw new Error(`Unable to store ${nodeProperty} ${name} on array as name conflicts.`); + } + // @ts-ignore + p.push((p[name] = new ctor(this, i, s))); return p; - }, Object.assign([], {$: this})) + }, Object.assign([], { $: this })); } } -class EzGraph { +export class EzGraph { /** @type { app } */ app; @@ -373,8 +394,7 @@ export const Ez = { const inputs = ezNode.inputs; let slot = 0; - for (let i = 0; i < args.length; i++) { - const arg = args[i]; + for (const arg of args) { if (arg instanceof EzOutput) { arg.connectTo(inputs[slot++]); } else { diff --git a/tests-ui/utils/index.js b/tests-ui/utils/index.js index 33d0ac025..50399a7ef 100644 --- a/tests-ui/utils/index.js +++ b/tests-ui/utils/index.js @@ -12,3 +12,45 @@ export async function start(config = undefined) { await app.setup(); return Ez.graph(app, global["LiteGraph"], global["LGraphCanvas"]); } + +/** + * @param { ReturnType["graph"] } graph + * @param { (hasReloaded: boolean) => (Promise | void) } cb + */ +export async function checkBeforeAndAfterReload(graph, cb) { + await cb(false); + await graph.reload(); + await cb(true); +} + +/** + * @param { string } name + * @param { Record } input + * @returns { import("../../web/types/comfy").ComfyObjectInfo } + */ +export function makeNodeDef(name, input) { + const nodeDef = { + name, + category: "test", + output_name: [], + input: { + required: {} + }, + }; + for(const k in input) { + nodeDef.input.required[k] = typeof input[k] === "string" ? [input[k], {}] : [...input[k]]; + } + return nodeDef; +} + +/** +/** + * @template { any } T + * @param { T } x + * @returns { x is Exclude } + */ +export function assertNotNullOrUndefined(x) { + expect(x).not.toEqual(null); + expect(x).not.toEqual(undefined); + return true; +} \ No newline at end of file From b419cd29f9391e52625f647b0d25a64ee62ef5a0 Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:10:00 +0100 Subject: [PATCH 04/13] throw earlier to get less confusing error --- tests-ui/utils/ezgraph.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests-ui/utils/ezgraph.js b/tests-ui/utils/ezgraph.js index 1124ca698..0e81fd47b 100644 --- a/tests-ui/utils/ezgraph.js +++ b/tests-ui/utils/ezgraph.js @@ -107,6 +107,8 @@ export class EzOutput extends EzSlot { * @param { EzInput } input */ connectTo(input) { + if (!input) throw new Error("Invalid input"); + /** * @type { LG["LLink"] | null } */ From 4a8c36b4039eb8cd906c8485f2919d3f9f617b58 Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:10:09 +0100 Subject: [PATCH 05/13] support outputs --- tests-ui/utils/index.js | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/tests-ui/utils/index.js b/tests-ui/utils/index.js index 50399a7ef..01c58b21f 100644 --- a/tests-ui/utils/index.js +++ b/tests-ui/utils/index.js @@ -26,13 +26,16 @@ export async function checkBeforeAndAfterReload(graph, cb) { /** * @param { string } name * @param { Record } input - * @returns { import("../../web/types/comfy").ComfyObjectInfo } + * @param { (string | string[])[] | Record } output + * @returns { Record } */ -export function makeNodeDef(name, input) { +export function makeNodeDef(name, input, output = {}) { const nodeDef = { name, category: "test", + output: [], output_name: [], + output_is_list: [], input: { required: {} }, @@ -40,7 +43,19 @@ export function makeNodeDef(name, input) { for(const k in input) { nodeDef.input.required[k] = typeof input[k] === "string" ? [input[k], {}] : [...input[k]]; } - return nodeDef; + if(output instanceof Array) { + output = output.reduce((p, c) => { + p[c] = c; + return p; + }, {}) + } + for(const k in output) { + nodeDef.output.push(output[k]); + nodeDef.output_name.push(k); + nodeDef.output_is_list.push(false); + } + + return { [name]: nodeDef }; } /** From 36534e043a384661512becb534fbca52a28fc120 Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:10:22 +0100 Subject: [PATCH 06/13] more test --- tests-ui/tests/widgetInputs.test.js | 64 ++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/tests-ui/tests/widgetInputs.test.js b/tests-ui/tests/widgetInputs.test.js index 7fed1f28f..022e54926 100644 --- a/tests-ui/tests/widgetInputs.test.js +++ b/tests-ui/tests/widgetInputs.test.js @@ -68,9 +68,7 @@ describe("widget inputs", () => { ].forEach((c) => { test(`widget conversion + primitive works on ${c.name}`, async () => { const { ez, graph } = await start({ - mockNodeDefs: { - ["TestNode"]: makeNodeDef("TestNode", { [c.name]: [c.type, c.opt ?? {}] }), - }, + mockNodeDefs: makeNodeDef("TestNode", { [c.name]: [c.type, c.opt ?? {}] }), }); // Create test node and convert to input @@ -206,9 +204,7 @@ describe("widget inputs", () => { test("defaultInput widgets can be converted back to inputs", async () => { const { graph, ez } = await start({ - mockNodeDefs: { - ["TestNode"]: makeNodeDef("TestNode", { example: ["INT", { defaultInput: true }] }), - }, + mockNodeDefs: makeNodeDef("TestNode", { example: ["INT", { defaultInput: true }] }), }); // Create test node and ensure it starts as an input @@ -247,9 +243,7 @@ describe("widget inputs", () => { test("forceInput widgets can not be converted back to inputs", async () => { const { graph, ez } = await start({ - mockNodeDefs: { - ["TestNode"]: makeNodeDef("TestNode", { example: ["INT", { forceInput: true }] }), - }, + mockNodeDefs: makeNodeDef("TestNode", { example: ["INT", { forceInput: true }] }), }); // Create test node and ensure it starts as an input @@ -271,7 +265,55 @@ describe("widget inputs", () => { } }); - test("primitive combo cannot connect to non matching list", () => { - throw new Error("not implemented"); + test("primitive can connect to matching combos on converted widgets", async () => { + const { ez } = await start({ + mockNodeDefs: { + ...makeNodeDef("TestNode1", { example: [["A", "B", "C"], { forceInput: true }] }), + ...makeNodeDef("TestNode2", { example: [["A", "B", "C"], { forceInput: true }] }), + }, + }); + + const n1 = ez.TestNode1(); + const n2 = ez.TestNode2(); + const p = ez.PrimitiveNode(); + p.outputs[0].connectTo(n1.inputs[0]); + p.outputs[0].connectTo(n2.inputs[0]); + expect(p.outputs[0].connections).toHaveLength(2); + const valueWidget = p.widgets.value; + expect(valueWidget.widget.type).toBe("combo"); + expect(valueWidget.widget.options.values).toEqual(["A", "B", "C"]); + }); + + test("primitive can not connect to non matching combos on converted widgets", async () => { + const { ez } = await start({ + mockNodeDefs: { + ...makeNodeDef("TestNode1", { example: [["A", "B", "C"], { forceInput: true }] }), + ...makeNodeDef("TestNode2", { example: [["A", "B"], { forceInput: true }] }), + }, + }); + + const n1 = ez.TestNode1(); + const n2 = ez.TestNode2(); + const p = ez.PrimitiveNode(); + p.outputs[0].connectTo(n1.inputs[0]); + expect(() => p.outputs[0].connectTo(n2.inputs[0])).toThrow(); + expect(p.outputs[0].connections).toHaveLength(1); + }); + + test("combo output can not connect to non matching combos list input", async () => { + const { ez } = await start({ + mockNodeDefs: { + ...makeNodeDef("TestNode1", {}, [["A", "B"]]), + ...makeNodeDef("TestNode2", { example: [["A", "B"], { forceInput: true}] }), + ...makeNodeDef("TestNode3", { example: [["A", "B", "C"], { forceInput: true}] }), + }, + }); + + const n1 = ez.TestNode1(); + const n2 = ez.TestNode2(); + const n3 = ez.TestNode3(); + + n1.outputs[0].connectTo(n2.inputs[0]); + expect(() => n1.outputs[0].connectTo(n3.inputs[0])).toThrow(); }); }); From 1a24b4448fe25b4705372cae0e2b6118dec43465 Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:43:19 +0100 Subject: [PATCH 07/13] add ci action --- .github/workflows/test-ui.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .github/workflows/test-ui.yaml diff --git a/.github/workflows/test-ui.yaml b/.github/workflows/test-ui.yaml new file mode 100644 index 000000000..d68614111 --- /dev/null +++ b/.github/workflows/test-ui.yaml @@ -0,0 +1,17 @@ +name: Tests CI + +on: [push, pull_request] + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Test using Node.js + uses: actions/setup-node@v1 + with: + node-version: '12' + working-directory: ./tests-ui + run: | + npm install + npm test \ No newline at end of file From caf454fcc21a2055babc47decc2cedb2967eedd9 Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:44:04 +0100 Subject: [PATCH 08/13] use lts node --- .github/workflows/test-ui.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-ui.yaml b/.github/workflows/test-ui.yaml index d68614111..d81140808 100644 --- a/.github/workflows/test-ui.yaml +++ b/.github/workflows/test-ui.yaml @@ -10,7 +10,7 @@ jobs: - name: Test using Node.js uses: actions/setup-node@v1 with: - node-version: '12' + node-version: '18' working-directory: ./tests-ui run: | npm install From bc3fa627c7fe686a0d239fe84d2fbb73b0f1d152 Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:45:22 +0100 Subject: [PATCH 09/13] Fix? --- .github/workflows/test-ui.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test-ui.yaml b/.github/workflows/test-ui.yaml index d81140808..cca1d96ba 100644 --- a/.github/workflows/test-ui.yaml +++ b/.github/workflows/test-ui.yaml @@ -11,6 +11,7 @@ jobs: uses: actions/setup-node@v1 with: node-version: '18' + - name: Run tests working-directory: ./tests-ui run: | npm install From 017248f6ff91c7d155b90ed7b1677a632a64d82e Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:46:54 +0100 Subject: [PATCH 10/13] Prevent connecting non matching combos --- web/extensions/core/widgetInputs.js | 63 +++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 17 deletions(-) diff --git a/web/extensions/core/widgetInputs.js b/web/extensions/core/widgetInputs.js index bc0fc2bb0..84abd8b7d 100644 --- a/web/extensions/core/widgetInputs.js +++ b/web/extensions/core/widgetInputs.js @@ -100,6 +100,27 @@ function getWidgetType(config) { return { type }; } + +function isValidCombo(combo, obj) { + // New input isnt a combo + if (!(obj instanceof Array)) { + console.log(`connection rejected: tried to connect combo to ${obj}`); + return false; + } + // New imput combo has a different size + if (combo.length !== obj.length) { + console.log(`connection rejected: combo lists dont match`); + return false; + } + // New input combo has different elements + if (combo.find((v, i) => obj[i] !== v)) { + console.log(`connection rejected: combo lists dont match`); + return false; + } + + return true; +} + app.registerExtension({ name: "Comfy.WidgetInputs", async beforeRegisterNodeDef(nodeType, nodeData, app) { @@ -256,6 +277,28 @@ app.registerExtension({ return r; }; + + // Prevent connecting COMBO lists to converted inputs that dont match types + const onConnectInput = nodeType.prototype.onConnectInput; + nodeType.prototype.onConnectInput = function (targetSlot, type, output, originNode, originSlot) { + const v = onConnectInput?.(this, arguments); + // Not a combo, ignore + if (type !== "COMBO") return v; + // Primitive output, allow that to handle + if (originNode.outputs[originSlot].widget) return v; + + // Ensure target is also a combo + const targetCombo = this.inputs[targetSlot].widget?.[GET_CONFIG]?.()?.[0]; + if (!targetCombo || !(targetCombo instanceof Array)) return v; + + // Check they match + const originConfig = originNode.constructor?.nodeData?.output?.[originSlot]; + if (!originConfig || !isValidCombo(targetCombo, originConfig)) { + return false; + } + + return v; + }; }, registerCustomNodes() { class PrimitiveNode { @@ -315,7 +358,7 @@ app.registerExtension({ onAfterGraphConfigured() { if (this.outputs[0].links?.length && !this.widgets?.length) { - if(!this.#onFirstConnection()) return; + if (!this.#onFirstConnection()) return; // Populate widget values from config data if (this.widgets) { @@ -387,7 +430,7 @@ app.registerExtension({ } const config = widget[GET_CONFIG]?.(); - if(!config) return; + if (!config) return; const { type } = getWidgetType(config); // Update our output to restrict to the widget type @@ -500,21 +543,7 @@ app.registerExtension({ const config2 = input.widget[GET_CONFIG](); if (config1[0] instanceof Array) { - // New input isnt a combo - if (!(config2[0] instanceof Array)) { - console.log(`connection rejected: tried to connect combo to ${config2[0]}`); - return false; - } - // New imput combo has a different size - if (config1[0].length !== config2[0].length) { - console.log(`connection rejected: combo lists dont match`); - return false; - } - // New input combo has different elements - if (config1[0].find((v, i) => config2[0][i] !== v)) { - console.log(`connection rejected: combo lists dont match`); - return false; - } + if (!isValidCombo(config1[0], config2[0])) return false; } else if (config1[0] !== config2[0]) { // Types dont match console.log(`connection rejected: types dont match`, config1[0], config2[0]); From beb2a48ee12d50fcdd7ca3916a23f8e7634953a0 Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:51:33 +0100 Subject: [PATCH 11/13] update --- .github/workflows/test-ui.yaml | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/.github/workflows/test-ui.yaml b/.github/workflows/test-ui.yaml index cca1d96ba..1120972fd 100644 --- a/.github/workflows/test-ui.yaml +++ b/.github/workflows/test-ui.yaml @@ -6,13 +6,10 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - name: Test using Node.js - uses: actions/setup-node@v1 - with: - node-version: '18' - - name: Run tests - working-directory: ./tests-ui - run: | - npm install - npm test \ No newline at end of file + - uses: actions/checkout@v4 + - uses: actions/setup-node@v3 + with: + node-version: 18 + - name: Run Tests + run: npm test + working-directory: ./tests-ui From c8830e5f8726344257ce8e7801d6379cbd98a90b Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:52:58 +0100 Subject: [PATCH 12/13] accidently removed npm i --- .github/workflows/test-ui.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-ui.yaml b/.github/workflows/test-ui.yaml index 1120972fd..1470d110c 100644 --- a/.github/workflows/test-ui.yaml +++ b/.github/workflows/test-ui.yaml @@ -11,5 +11,7 @@ jobs: with: node-version: 18 - name: Run Tests - run: npm test + run: | + npm install + npm test working-directory: ./tests-ui From 9dfd61b48d6235a275ca60d59d1b47db537ed358 Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Mon, 16 Oct 2023 11:57:45 +0100 Subject: [PATCH 13/13] Disable logging extension --- tests-ui/globalSetup.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests-ui/globalSetup.js b/tests-ui/globalSetup.js index 4942f3fa7..b9d97f58a 100644 --- a/tests-ui/globalSetup.js +++ b/tests-ui/globalSetup.js @@ -9,4 +9,6 @@ module.exports = async function () { global.enableWebGLCanvas = nop; HTMLCanvasElement.prototype.getContext = nop; + + localStorage["Comfy.Settings.Comfy.Logging.Enabled"] = "false"; };