Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions browser_tests/fixtures/helpers/SubgraphHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,49 @@ export class SubgraphHelper {
return id
}

async getBoundaryLinkSnapshot() {
return this.page.evaluate(() => {
const graph = window.app!.graph!
const host = graph.nodes.find((node) => node.isSubgraphNode())
if (!host) {
return {
rootLinks: ['no subgraph node'],
incompatibleHostInputLinks: ['no subgraph node'],
incompatibleHostOutputLinks: ['no subgraph node']
}
}

const hostId = host.id
function label(id: string | number) {
return id === hostId ? 'HOST' : String(id)
}

const links = [...graph.links.values()]
return {
rootLinks: links
.map(
(link) =>
`${label(link.origin_id)}:${link.origin_slot}->${label(link.target_id)}:${link.target_slot}`
)
.sort(),
incompatibleHostInputLinks: links
.filter((link) => link.target_id === host.id)
.filter((link) => host.inputs[link.target_slot]?.type !== link.type)
.map(
(link) =>
`${link.type} link landed on slot ${link.target_slot} typed ${host.inputs[link.target_slot]?.type}`
),
incompatibleHostOutputLinks: links
.filter((link) => link.origin_id === host.id)
.filter((link) => host.outputs[link.origin_slot]?.type !== link.type)
.map(
(link) =>
`${link.type} link left slot ${link.origin_slot} typed ${host.outputs[link.origin_slot]?.type}`
)
}
})
}

async serializeAndReload(): Promise<void> {
const serialized = await this.page.evaluate(() =>
window.app!.graph!.serialize()
Expand Down
14 changes: 14 additions & 0 deletions browser_tests/fixtures/utils/litegraphUtils.ts

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixture is pretty clunky. It's sole caller is a @vue-nodes test, but instead of leveraging this

await expect(node.header).toBeInViewport({ ratio: 1 })

It's implemented as a slow .poll() over the position of the not actually rendered litegraph node.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You were right, and it's done in 7a03b0c — the fixture is gone and the step now uses exactly what you suggested:

await comfyPage.command.executeCommand('Comfy.Canvas.FitView')
const vaeDecode = await comfyPage.vueNodes.getFixtureByTitle('VAE Decode')
await expect(vaeDecode.header).toBeInViewport({ ratio: 1 })

litegraphUtils.ts is no longer touched at all; the diff is down to two files, +102/-0. Leaving this thread for you to resolve rather than closing it myself.

Re-assigning to you — your approval was auto-dismissed by the pushes that followed it, not withdrawn.

Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,20 @@ export class NodeReference {
const nodeSize = await this.getSize()
return { x: nodePos.x + nodeSize.width / 2, y: nodePos.y - 15 }
}
async waitForTitleInView(): Promise<void> {
const canvas = await this.comfyPage.canvas.boundingBox()
if (!canvas) throw new Error('Canvas bounding box not available')

let previousX = Number.NaN
await expect
.poll(async () => {
const { x } = await this.getTitlePosition()
const settledInView = x === previousX && x < canvas.width
previousX = x
return settledInView
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
})
.toBe(true)
}
async dragBy(
delta: Position,
options?: {
Expand Down
59 changes: 59 additions & 0 deletions browser_tests/tests/subgraph/subgraphCreationBoundaryLinks.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { expect } from '@playwright/test'

import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage'

const SUBGRAPH_LINKS_EXPECTED = {
rootLinks: [
'4:0->HOST:0',
'4:1->6:0',
'4:1->7:0',
'4:2->HOST:4',
'5:0->HOST:3',
'6:0->HOST:1',
'7:0->HOST:2',
'HOST:0->9:0'
],
incompatibleHostInputLinks: [],
incompatibleHostOutputLinks: []
} as const

test(
'Subgraph creation rewires boundary links to compatible slots across reload',
{ tag: ['@slow', '@subgraph', '@vue-nodes'] },
async ({ comfyPage }) => {
await test.step('Select both nodes in the default workflow', async () => {
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Disabled')
await comfyPage.workflow.loadWorkflow('default')

// VAE Decode sits past the right edge of the 1280px canvas at the default
// view, so clicking its title silently misses and the selection is left
// holding KSampler alone.
await comfyPage.command.executeCommand('Comfy.Canvas.FitView')
const vaeDecode = await comfyPage.nodeOps.getNodeRefById('8')
await vaeDecode.waitForTitleInView()

await comfyPage.nodeOps.selectNodes(['KSampler', 'VAE Decode'])
expect(
await comfyPage.nodeOps.getSelectedNodeIds(),
'both nodes must be selected, or the conversion under test is not the one being asserted'
).toEqual(['3', '8'])
})

await test.step('Convert and verify the boundary links', async () => {
const ksampler = await comfyPage.nodeOps.getNodeRefById('3')
await ksampler.convertToSubgraph()

await expect
.poll(() => comfyPage.subgraph.getBoundaryLinkSnapshot())
.toEqual(SUBGRAPH_LINKS_EXPECTED)
})

await test.step('Reload and verify the boundary links', async () => {
await comfyPage.subgraph.serializeAndReload()

await expect
.poll(() => comfyPage.subgraph.getBoundaryLinkSnapshot())
.toEqual(SUBGRAPH_LINKS_EXPECTED)
})
}
)
Loading