Skip to content

Commit c30f528

Browse files
[refactor] adjust Vue node fixtures to not be coupled to Litegraph (#6033)
## Summary Changes the Vue node test fixture to not rely on Litegraph internal objects (which should eventually be fully decoupled from Vue nodes) and instead interact with nodes using black-box approach that emulates user actions (preferred appraoch for e2e tests). ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6033-refactor-adjust-Vue-node-fixtures-to-not-be-coupled-to-Litegraph-28a6d73d3650817b8152d27dc4fe0017) by [Unito](https://www.unito.io)
1 parent 0497421 commit c30f528

File tree

4 files changed

+94
-159
lines changed

4 files changed

+94
-159
lines changed

browser_tests/fixtures/VueNodeHelpers.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
*/
44
import type { Locator, Page } from '@playwright/test'
55

6+
import { VueNodeFixture } from './utils/vueNodeFixtures'
7+
68
export class VueNodeHelpers {
79
constructor(private page: Page) {}
810

@@ -106,6 +108,24 @@ export class VueNodeHelpers {
106108
await this.page.keyboard.press('Backspace')
107109
}
108110

111+
/**
112+
* Return a DOM-focused VueNodeFixture for the first node matching the title.
113+
* Resolves the node id up front so subsequent interactions survive title changes.
114+
*/
115+
async getFixtureByTitle(title: string): Promise<VueNodeFixture> {
116+
const node = this.getNodeByTitle(title).first()
117+
await node.waitFor({ state: 'visible' })
118+
119+
const nodeId = await node.evaluate((el) => el.getAttribute('data-node-id'))
120+
if (!nodeId) {
121+
throw new Error(
122+
`Vue node titled "${title}" is missing its data-node-id attribute`
123+
)
124+
}
125+
126+
return new VueNodeFixture(this.getNodeLocator(nodeId))
127+
}
128+
109129
/**
110130
* Wait for Vue nodes to be rendered
111131
*/
Lines changed: 37 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -1,131 +1,66 @@
1-
import type { Locator, Page } from '@playwright/test'
1+
import { expect } from '@playwright/test'
2+
import type { Locator } from '@playwright/test'
23

3-
import type { NodeReference } from './litegraphUtils'
4-
5-
/**
6-
* VueNodeFixture provides Vue-specific testing utilities for interacting with
7-
* Vue node components. It bridges the gap between litegraph node references
8-
* and Vue UI components.
9-
*/
4+
/** DOM-centric helper for a single Vue-rendered node on the canvas. */
105
export class VueNodeFixture {
11-
constructor(
12-
private readonly nodeRef: NodeReference,
13-
private readonly page: Page
14-
) {}
6+
constructor(private readonly locator: Locator) {}
157

16-
/**
17-
* Get the node's header element using data-testid
18-
*/
19-
async getHeader(): Promise<Locator> {
20-
const nodeId = this.nodeRef.id
21-
return this.page.locator(`[data-testid="node-header-${nodeId}"]`)
8+
get header(): Locator {
9+
return this.locator.locator('[data-testid^="node-header-"]')
2210
}
2311

24-
/**
25-
* Get the node's title element
26-
*/
27-
async getTitleElement(): Promise<Locator> {
28-
const header = await this.getHeader()
29-
return header.locator('[data-testid="node-title"]')
12+
get title(): Locator {
13+
return this.locator.locator('[data-testid="node-title"]')
3014
}
3115

32-
/**
33-
* Get the current title text
34-
*/
35-
async getTitle(): Promise<string> {
36-
const titleElement = await this.getTitleElement()
37-
return (await titleElement.textContent()) || ''
16+
get titleInput(): Locator {
17+
return this.locator.locator('[data-testid="node-title-input"]')
3818
}
3919

40-
/**
41-
* Set a new title by double-clicking and entering text
42-
*/
43-
async setTitle(newTitle: string): Promise<void> {
44-
const titleElement = await this.getTitleElement()
45-
await titleElement.dblclick()
46-
47-
const input = (await this.getHeader()).locator(
48-
'[data-testid="node-title-input"]'
49-
)
50-
await input.fill(newTitle)
51-
await input.press('Enter')
20+
get body(): Locator {
21+
return this.locator.locator('[data-testid^="node-body-"]')
5222
}
5323

54-
/**
55-
* Cancel title editing
56-
*/
57-
async cancelTitleEdit(): Promise<void> {
58-
const titleElement = await this.getTitleElement()
59-
await titleElement.dblclick()
60-
61-
const input = (await this.getHeader()).locator(
62-
'[data-testid="node-title-input"]'
63-
)
64-
await input.press('Escape')
24+
get collapseButton(): Locator {
25+
return this.locator.locator('[data-testid="node-collapse-button"]')
6526
}
6627

67-
/**
68-
* Check if the title is currently being edited
69-
*/
70-
async isEditingTitle(): Promise<boolean> {
71-
const header = await this.getHeader()
72-
const input = header.locator('[data-testid="node-title-input"]')
73-
return await input.isVisible()
28+
get collapseIcon(): Locator {
29+
return this.collapseButton.locator('i')
7430
}
7531

76-
/**
77-
* Get the collapse/expand button
78-
*/
79-
async getCollapseButton(): Promise<Locator> {
80-
const header = await this.getHeader()
81-
return header.locator('[data-testid="node-collapse-button"]')
32+
get root(): Locator {
33+
return this.locator
8234
}
8335

84-
/**
85-
* Toggle the node's collapsed state
86-
*/
87-
async toggleCollapse(): Promise<void> {
88-
const button = await this.getCollapseButton()
89-
await button.click()
36+
async getTitle(): Promise<string> {
37+
return (await this.title.textContent()) ?? ''
9038
}
9139

92-
/**
93-
* Get the collapse icon element
94-
*/
95-
async getCollapseIcon(): Promise<Locator> {
96-
const button = await this.getCollapseButton()
97-
return button.locator('i')
40+
async setTitle(value: string): Promise<void> {
41+
await this.header.dblclick()
42+
const input = this.titleInput
43+
await expect(input).toBeVisible()
44+
await input.fill(value)
45+
await input.press('Enter')
9846
}
9947

100-
/**
101-
* Get the collapse icon's CSS classes
102-
*/
103-
async getCollapseIconClass(): Promise<string> {
104-
const icon = await this.getCollapseIcon()
105-
return (await icon.getAttribute('class')) || ''
48+
async cancelTitleEdit(): Promise<void> {
49+
await this.header.dblclick()
50+
const input = this.titleInput
51+
await expect(input).toBeVisible()
52+
await input.press('Escape')
10653
}
10754

108-
/**
109-
* Check if the collapse button is visible
110-
*/
111-
async isCollapseButtonVisible(): Promise<boolean> {
112-
const button = await this.getCollapseButton()
113-
return await button.isVisible()
55+
async toggleCollapse(): Promise<void> {
56+
await this.collapseButton.click()
11457
}
11558

116-
/**
117-
* Get the node's body/content element
118-
*/
119-
async getBody(): Promise<Locator> {
120-
const nodeId = this.nodeRef.id
121-
return this.page.locator(`[data-testid="node-body-${nodeId}"]`)
59+
async getCollapseIconClass(): Promise<string> {
60+
return (await this.collapseIcon.getAttribute('class')) ?? ''
12261
}
12362

124-
/**
125-
* Check if the node body is visible (not collapsed)
126-
*/
127-
async isBodyVisible(): Promise<boolean> {
128-
const body = await this.getBody()
129-
return await body.isVisible()
63+
boundingBox(): ReturnType<Locator['boundingBox']> {
64+
return this.locator.boundingBox()
13065
}
13166
}

browser_tests/tests/vueNodes/interactions/node/rename.spec.ts

Lines changed: 13 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,70 +2,46 @@ import {
22
comfyExpect as expect,
33
comfyPageFixture as test
44
} from '../../../../fixtures/ComfyPage'
5-
import { VueNodeFixture } from '../../../../fixtures/utils/vueNodeFixtures'
65

76
test.describe('Vue Nodes Renaming', () => {
87
test.beforeEach(async ({ comfyPage }) => {
98
await comfyPage.setSetting('Comfy.Graph.CanvasMenu', false)
109
await comfyPage.setSetting('Comfy.VueNodes.Enabled', true)
1110
await comfyPage.setup()
11+
await comfyPage.vueNodes.waitForNodes()
1212
})
1313

1414
test('should display node title', async ({ comfyPage }) => {
15-
// Get the KSampler node from the default workflow
16-
const nodes = await comfyPage.getNodeRefsByType('KSampler')
17-
expect(nodes.length).toBeGreaterThanOrEqual(1)
18-
19-
const node = nodes[0]
20-
const vueNode = new VueNodeFixture(node, comfyPage.page)
21-
22-
const title = await vueNode.getTitle()
23-
expect(title).toBe('KSampler')
24-
25-
// Verify title is visible in the header
26-
const header = await vueNode.getHeader()
27-
await expect(header).toContainText('KSampler')
15+
const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
16+
await expect(vueNode.header).toContainText('KSampler')
2817
})
2918

3019
test('should allow title renaming by double clicking on the node header', async ({
3120
comfyPage
3221
}) => {
33-
const nodes = await comfyPage.getNodeRefsByType('KSampler')
34-
const node = nodes[0]
35-
const vueNode = new VueNodeFixture(node, comfyPage.page)
36-
22+
const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
3723
// Test renaming with Enter
3824
await vueNode.setTitle('My Custom Sampler')
39-
const newTitle = await vueNode.getTitle()
40-
expect(newTitle).toBe('My Custom Sampler')
41-
42-
// Verify the title is displayed
43-
const header = await vueNode.getHeader()
44-
await expect(header).toContainText('My Custom Sampler')
25+
await expect(await vueNode.getTitle()).toBe('My Custom Sampler')
26+
await expect(vueNode.header).toContainText('My Custom Sampler')
4527

4628
// Test cancel with Escape
47-
const titleElement = await vueNode.getTitleElement()
48-
await titleElement.dblclick()
29+
await vueNode.title.dblclick()
4930
await comfyPage.nextFrame()
50-
51-
// Type a different value but cancel
52-
const input = (await vueNode.getHeader()).locator(
53-
'[data-testid="node-title-input"]'
54-
)
55-
await input.fill('This Should Be Cancelled')
56-
await input.press('Escape')
31+
await vueNode.titleInput.fill('This Should Be Cancelled')
32+
await vueNode.titleInput.press('Escape')
5733
await comfyPage.nextFrame()
5834

5935
// Title should remain as the previously saved value
60-
const titleAfterCancel = await vueNode.getTitle()
61-
expect(titleAfterCancel).toBe('My Custom Sampler')
36+
await expect(await vueNode.getTitle()).toBe('My Custom Sampler')
6237
})
6338

6439
test('Double click node body does not trigger edit', async ({
6540
comfyPage
6641
}) => {
67-
const loadCheckpointNode =
68-
comfyPage.vueNodes.getNodeByTitle('Load Checkpoint')
42+
const loadCheckpointNode = comfyPage.vueNodes
43+
.getNodeByTitle('Load Checkpoint')
44+
.first()
6945
const nodeBbox = await loadCheckpointNode.boundingBox()
7046
if (!nodeBbox) throw new Error('Node not found')
7147
await loadCheckpointNode.dblclick()

browser_tests/tests/vueNodes/nodeStates/collapse.spec.ts

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,51 +2,57 @@ import {
22
comfyExpect as expect,
33
comfyPageFixture as test
44
} from '../../../fixtures/ComfyPage'
5-
import { VueNodeFixture } from '../../../fixtures/utils/vueNodeFixtures'
65

76
test.describe('Vue Node Collapse', () => {
87
test.beforeEach(async ({ comfyPage }) => {
98
await comfyPage.setSetting('Comfy.Graph.CanvasMenu', false)
109
await comfyPage.setSetting('Comfy.EnableTooltips', true)
1110
await comfyPage.setSetting('Comfy.VueNodes.Enabled', true)
1211
await comfyPage.setup()
12+
await comfyPage.vueNodes.waitForNodes()
1313
})
1414

1515
test('should allow collapsing node with collapse icon', async ({
1616
comfyPage
1717
}) => {
18-
// Get the KSampler node from the default workflow
19-
const nodes = await comfyPage.getNodeRefsByType('KSampler')
20-
const node = nodes[0]
21-
const vueNode = new VueNodeFixture(node, comfyPage.page)
18+
const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
19+
await expect(vueNode.root).toBeVisible()
2220

2321
// Initially should not be collapsed
24-
expect(await node.isCollapsed()).toBe(false)
25-
const body = await vueNode.getBody()
22+
const body = vueNode.body
2623
await expect(body).toBeVisible()
24+
const expandedBoundingBox = await vueNode.boundingBox()
25+
if (!expandedBoundingBox)
26+
throw new Error('Failed to get node bounding box before collapse')
2727

2828
// Collapse the node
2929
await vueNode.toggleCollapse()
30-
expect(await node.isCollapsed()).toBe(true)
30+
await comfyPage.nextFrame()
3131

3232
// Verify node content is hidden
33-
const collapsedSize = await node.getSize()
3433
await expect(body).not.toBeVisible()
34+
const collapsedBoundingBox = await vueNode.boundingBox()
35+
if (!collapsedBoundingBox)
36+
throw new Error('Failed to get node bounding box after collapse')
37+
expect(collapsedBoundingBox.height).toBeLessThan(expandedBoundingBox.height)
3538

3639
// Expand again
3740
await vueNode.toggleCollapse()
38-
expect(await node.isCollapsed()).toBe(false)
41+
await comfyPage.nextFrame()
3942
await expect(body).toBeVisible()
4043

4144
// Size should be restored
42-
const expandedSize = await node.getSize()
43-
expect(expandedSize.height).toBeGreaterThanOrEqual(collapsedSize.height)
45+
const expandedBoundingBoxAfter = await vueNode.boundingBox()
46+
if (!expandedBoundingBoxAfter)
47+
throw new Error('Failed to get node bounding box after expand')
48+
expect(expandedBoundingBoxAfter.height).toBeGreaterThanOrEqual(
49+
collapsedBoundingBox.height
50+
)
4451
})
4552

4653
test('should show collapse/expand icon state', async ({ comfyPage }) => {
47-
const nodes = await comfyPage.getNodeRefsByType('KSampler')
48-
const node = nodes[0]
49-
const vueNode = new VueNodeFixture(node, comfyPage.page)
54+
const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
55+
await expect(vueNode.root).toBeVisible()
5056

5157
// Check initial expanded state icon
5258
let iconClass = await vueNode.getCollapseIconClass()
@@ -66,9 +72,8 @@ test.describe('Vue Node Collapse', () => {
6672
test('should preserve title when collapsing/expanding', async ({
6773
comfyPage
6874
}) => {
69-
const nodes = await comfyPage.getNodeRefsByType('KSampler')
70-
const node = nodes[0]
71-
const vueNode = new VueNodeFixture(node, comfyPage.page)
75+
const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
76+
await expect(vueNode.root).toBeVisible()
7277

7378
// Set custom title
7479
await vueNode.setTitle('Test Sampler')
@@ -83,7 +88,6 @@ test.describe('Vue Node Collapse', () => {
8388
expect(await vueNode.getTitle()).toBe('Test Sampler')
8489

8590
// Verify title is still displayed
86-
const header = await vueNode.getHeader()
87-
await expect(header).toContainText('Test Sampler')
91+
await expect(vueNode.header).toContainText('Test Sampler')
8892
})
8993
})

0 commit comments

Comments
 (0)