Skip to content

Commit 9cc09cd

Browse files
AustinMrozDrJKL
AustinMroz
and
DrJKL
authored
Add additional subgraph test fixtures and tests (#11806)
- Adds functions to SubgraphHelper to perform widget promotion by standard user means - Right Click -> Promote - Properties Panel - Adds new slot fixture code that works with simple `locator.dragTo` operations. - Adds multiple subgraph tests with a focus on historically difficult operations. - Fixes a bug where the litegraph `node.selected` state would not be unset when switching graphs. This made it so 'Selecting a node -> leaving subgraph -> re-enter subgraph -> right click on node' would fail to select the node because it is marked as already selected. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11806-Add-helper-functions-for-widget-promotion-3536d73d365081f58dd9cd730c1a91a9) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org>
1 parent de1c1ee commit 9cc09cd

12 files changed

Lines changed: 406 additions & 47 deletions

File tree

‎browser_tests/fixtures/VueNodeHelpers.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,4 +246,18 @@ export class VueNodeHelpers {
246246
position: { x: box.width / 2, y: box.height * 0.75 }
247247
})
248248
}
249+
async isSlotConnected(slot: Locator) {
250+
const key = await slot.getByTestId('slot-dot').getAttribute('data-slot-key')
251+
if (!key) return false
252+
253+
return await this.page.evaluate((key) => {
254+
const [nodeId, type, slotId] = key.split('-')
255+
const node = app?.canvas?.graph?.getNodeById(nodeId)
256+
if (!node) return false
257+
258+
return type === 'in'
259+
? node.inputs[Number(slotId)]?.link !== null
260+
: !!node.outputs[Number(slotId)].links?.length
261+
}, key)
262+
}
249263
}

‎browser_tests/fixtures/components/ContextMenu.ts‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@ export class ContextMenu {
66
public readonly litegraphMenu: Locator
77
public readonly litegraphContextMenu: Locator
88
public readonly menuItems: Locator
9+
protected readonly anyMenu: Locator
910

1011
constructor(public readonly page: Page) {
1112
this.primeVueMenu = page.locator('.p-contextmenu, .p-menu')
1213
this.litegraphMenu = page.locator('.litemenu')
1314
this.litegraphContextMenu = page.locator('.litecontextmenu')
1415
this.menuItems = page.locator('.p-menuitem, .litemenu-entry')
16+
this.anyMenu = this.primeVueMenu
17+
.or(this.litegraphMenu)
18+
.or(this.litegraphContextMenu)
1519
}
1620

1721
async clickMenuItem(name: string): Promise<void> {
@@ -36,16 +40,7 @@ export class ContextMenu {
3640
}
3741

3842
async isVisible(): Promise<boolean> {
39-
const primeVueVisible = await this.primeVueMenu
40-
.isVisible()
41-
.catch(() => false)
42-
const litegraphVisible = await this.litegraphMenu
43-
.isVisible()
44-
.catch(() => false)
45-
const litegraphContextVisible = await this.litegraphContextMenu
46-
.isVisible()
47-
.catch(() => false)
48-
return primeVueVisible || litegraphVisible || litegraphContextVisible
43+
return await this.anyMenu.isVisible()
4944
}
5045

5146
async assertHasItems(items: string[]): Promise<void> {
@@ -58,7 +53,7 @@ export class ContextMenu {
5853

5954
async openFor(locator: Locator): Promise<this> {
6055
await locator.click({ button: 'right' })
61-
await expect.poll(() => this.isVisible()).toBe(true)
56+
await expect(this.anyMenu).toBeVisible()
6257
return this
6358
}
6459

‎browser_tests/fixtures/components/SubgraphEditor.ts‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import type { Locator } from '@playwright/test'
2+
3+
import { comfyExpect as expect } from '@e2e/fixtures/ComfyPage'
4+
import type { ComfyPage } from '@e2e/fixtures/ComfyPage'
5+
import { TestIds } from '@e2e/fixtures/selectors'
6+
import { dragByIndex } from '@e2e/fixtures/utils/dragAndDrop'
7+
import { VueNodeFixture } from '@e2e/fixtures/utils/vueNodeFixtures'
8+
9+
export class SubgraphEditor {
10+
public readonly root: Locator
11+
public readonly promotionItems: Locator
12+
13+
constructor(protected readonly comfyPage: ComfyPage) {
14+
this.root = this.comfyPage.menu.propertiesPanel.root
15+
this.promotionItems = this.root.getByTestId(
16+
TestIds.subgraphEditor.widgetItem
17+
)
18+
}
19+
20+
async open(subgraphNode: Locator) {
21+
await new VueNodeFixture(subgraphNode).select()
22+
const menu = await this.comfyPage.contextMenu.openFor(subgraphNode)
23+
await menu.clickMenuItemExact('Edit Subgraph Widgets')
24+
await expect(this.root, 'Open Properties Panel').toBeVisible()
25+
}
26+
27+
resolveItem(options: {
28+
nodeName?: string
29+
nodeId?: string
30+
widgetName: string
31+
}): Locator {
32+
const nodeItems =
33+
options.nodeId !== undefined
34+
? this.comfyPage.page.locator(`[data-nodeid="${options.nodeId}"]`)
35+
: options.nodeName !== undefined
36+
? this.promotionItems.filter({
37+
has: this.comfyPage.page
38+
.getByTestId(TestIds.subgraphEditor.nodeName)
39+
.filter({ hasText: options.nodeName })
40+
})
41+
: this.promotionItems
42+
43+
return nodeItems.filter({
44+
has: this.comfyPage.page
45+
.getByTestId(TestIds.subgraphEditor.widgetLabel)
46+
.filter({ hasText: options.widgetName })
47+
})
48+
}
49+
50+
getToggleButton(item: Locator) {
51+
return item.getByTestId(TestIds.subgraphEditor.widgetToggle)
52+
}
53+
54+
async togglePromotionOnItem(item: Locator, toState?: boolean) {
55+
const toggleIcon = item.getByTestId(TestIds.subgraphEditor.iconEye)
56+
if (toState !== undefined) {
57+
const expectedIcon = `icon-[lucide--eye${toState ? '-off' : ''}]`
58+
await expect(toggleIcon).toContainClass(expectedIcon)
59+
}
60+
await toggleIcon.click()
61+
}
62+
63+
async togglePromotion(
64+
subgraphNode: Locator,
65+
options: {
66+
nodeName?: string
67+
nodeId?: string
68+
widgetName: string
69+
toState?: boolean
70+
}
71+
) {
72+
await this.open(subgraphNode)
73+
74+
const item = this.resolveItem(options)
75+
await this.togglePromotionOnItem(item, options.toState)
76+
}
77+
async dragItem(fromIndex: number, toIndex: number) {
78+
await dragByIndex(this.promotionItems, fromIndex, toIndex)
79+
await this.comfyPage.nextFrame()
80+
}
81+
}

‎browser_tests/fixtures/helpers/BuilderSelectHelper.ts‎

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,34 +2,7 @@ import type { Locator, Page } from '@playwright/test'
22

33
import type { ComfyPage } from '@e2e/fixtures/ComfyPage'
44
import { TestIds } from '@e2e/fixtures/selectors'
5-
6-
/**
7-
* Drag an element from one index to another within a list of locators.
8-
* Uses mousedown/mousemove/mouseup to trigger the DraggableList library.
9-
*
10-
* DraggableList toggles position when the dragged item's center crosses
11-
* past an idle item's center. To reliably land at the target position,
12-
* we overshoot slightly past the target's far edge.
13-
*/
14-
async function dragByIndex(items: Locator, fromIndex: number, toIndex: number) {
15-
const fromBox = await items.nth(fromIndex).boundingBox()
16-
const toBox = await items.nth(toIndex).boundingBox()
17-
if (!fromBox || !toBox) throw new Error('Item not visible for drag')
18-
19-
const draggingDown = toIndex > fromIndex
20-
const targetY = draggingDown
21-
? toBox.y + toBox.height * 0.9
22-
: toBox.y + toBox.height * 0.1
23-
24-
const page = items.page()
25-
await page.mouse.move(
26-
fromBox.x + fromBox.width / 2,
27-
fromBox.y + fromBox.height / 2
28-
)
29-
await page.mouse.down()
30-
await page.mouse.move(toBox.x + toBox.width / 2, targetY, { steps: 10 })
31-
await page.mouse.up()
32-
}
5+
import { dragByIndex } from '@e2e/fixtures/utils/dragAndDrop'
336

347
export class BuilderSelectHelper {
358
/** All IoItem locators in the current step sidebar. */

‎browser_tests/fixtures/helpers/SubgraphHelper.ts‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,17 @@ import type { ComfyWorkflow } from '@/platform/workflow/management/stores/comfyW
99
import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema'
1010

1111
import type { ComfyPage } from '@e2e/fixtures/ComfyPage'
12+
import { SubgraphEditor } from '@e2e/fixtures/components/SubgraphEditor'
1213
import { TestIds } from '@e2e/fixtures/selectors'
1314
import type { NodeReference } from '@e2e/fixtures/utils/litegraphUtils'
1415
import { SubgraphSlotReference } from '@e2e/fixtures/utils/litegraphUtils'
1516

1617
export class SubgraphHelper {
17-
constructor(private readonly comfyPage: ComfyPage) {}
18+
public readonly editor: SubgraphEditor
19+
20+
constructor(private readonly comfyPage: ComfyPage) {
21+
this.editor = new SubgraphEditor(comfyPage)
22+
}
1823

1924
private get page(): Page {
2025
return this.comfyPage.page
@@ -327,6 +332,23 @@ export class SubgraphHelper {
327332
await this.comfyPage.nextFrame()
328333
}
329334

335+
async promoteWidget(nodeLocator: Locator, widgetName: string): Promise<void> {
336+
const widget = nodeLocator.getByLabel(widgetName, { exact: true })
337+
await this.comfyPage.contextMenu
338+
.openFor(widget)
339+
.then((m) => m.clickMenuItemExact(`Promote Widget: ${widgetName}`))
340+
}
341+
342+
async unpromoteWidget(
343+
nodeLocator: Locator,
344+
widgetName: string
345+
): Promise<void> {
346+
const widget = nodeLocator.getByLabel(widgetName, { exact: true })
347+
await this.comfyPage.contextMenu
348+
.openFor(widget)
349+
.then((m) => m.clickMenuItemExact(`Un-Promote Widget: ${widgetName}`))
350+
}
351+
330352
async isInSubgraph(): Promise<boolean> {
331353
return this.page.evaluate(() => {
332354
const graph = window.app!.canvas.graph

‎browser_tests/fixtures/selectors.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,16 @@ export const TestIds = {
104104
errorsTab: 'panel-tab-errors'
105105
},
106106
subgraphEditor: {
107-
toggle: 'subgraph-editor-toggle',
108-
shownSection: 'subgraph-editor-shown-section',
109107
hiddenSection: 'subgraph-editor-hidden-section',
110-
widgetToggle: 'subgraph-widget-toggle',
111-
widgetLabel: 'subgraph-widget-label',
112-
iconLink: 'icon-link',
113108
iconEye: 'icon-eye',
114-
widgetActionsMenuButton: 'widget-actions-menu-button'
109+
iconLink: 'icon-link',
110+
nodeName: 'subgraph-widget-node-name',
111+
shownSection: 'subgraph-editor-shown-section',
112+
toggle: 'subgraph-editor-toggle',
113+
widgetActionsMenuButton: 'widget-actions-menu-button',
114+
widgetItem: 'subgraph-widget-item',
115+
widgetLabel: 'subgraph-widget-label',
116+
widgetToggle: 'subgraph-widget-toggle'
115117
},
116118
node: {
117119
titleInput: 'node-title-input',

‎browser_tests/fixtures/utils/dragAndDrop.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import type { Locator } from '@playwright/test'
2+
3+
/**
4+
* Drag an element from one index to another within a list of locators.
5+
* Uses mousedown/mousemove/mouseup to trigger the DraggableList library.
6+
*
7+
* DraggableList toggles position when the dragged item's center crosses
8+
* past an idle item's center. To reliably land at the target position,
9+
* we overshoot slightly past the target's far edge.
10+
*/
11+
export async function dragByIndex(
12+
items: Locator,
13+
fromIndex: number,
14+
toIndex: number
15+
) {
16+
const fromBox = await items.nth(fromIndex).boundingBox()
17+
const toBox = await items.nth(toIndex).boundingBox()
18+
if (!fromBox || !toBox) throw new Error('Item not visible for drag')
19+
20+
const draggingDown = toIndex > fromIndex
21+
const targetY = draggingDown
22+
? toBox.y + toBox.height * 0.9
23+
: toBox.y + toBox.height * 0.1
24+
25+
const page = items.page()
26+
await page.mouse.move(
27+
fromBox.x + fromBox.width / 2,
28+
fromBox.y + fromBox.height / 2
29+
)
30+
await page.mouse.down()
31+
await page.mouse.move(toBox.x + toBox.width / 2, targetY, { steps: 10 })
32+
await page.mouse.up()
33+
}

‎browser_tests/fixtures/utils/vueNodeFixtures.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export class VueNodeFixture {
1515
public readonly root: Locator
1616
public readonly widgets: Locator
1717
public readonly imagePreview: Locator
18+
public readonly content: Locator
1819

1920
constructor(private readonly locator: Locator) {
2021
this.header = locator.locator('[data-testid^="node-header-"]')
@@ -27,6 +28,7 @@ export class VueNodeFixture {
2728
this.root = locator
2829
this.widgets = this.locator.locator('.lg-node-widget')
2930
this.imagePreview = locator.locator('.image-preview')
31+
this.content = locator.locator('.lg-node-content')
3032
}
3133

3234
async getTitle(): Promise<string> {
@@ -39,6 +41,10 @@ export class VueNodeFixture {
3941
await this.titleEditor.setTitle(value)
4042
}
4143

44+
async select() {
45+
await this.header.click()
46+
}
47+
4248
async toggleCollapse(): Promise<void> {
4349
await this.collapseButton.click()
4450
}
@@ -60,4 +66,15 @@ export class VueNodeFixture {
6066
boundingBox(): ReturnType<Locator['boundingBox']> {
6167
return this.locator.boundingBox()
6268
}
69+
70+
getSlot(nameOrLocator: string | Locator) {
71+
const slotLocators = this.root
72+
.getByTestId('node-widget')
73+
.or(this.root.locator('.lg-slot'))
74+
const filteredLocator =
75+
typeof nameOrLocator === 'string'
76+
? slotLocators.filter({ hasText: nameOrLocator })
77+
: slotLocators.filter({ has: nameOrLocator })
78+
return filteredLocator.getByTestId('slot-dot').locator('..')
79+
}
6380
}

0 commit comments

Comments
 (0)