From c30f528d11369001f5ba393c96f655883bd84ebf Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Sun, 12 Oct 2025 19:56:42 -0700 Subject: [PATCH] [refactor] adjust Vue node fixtures to not be coupled to Litegraph (#6033) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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) --- browser_tests/fixtures/VueNodeHelpers.ts | 20 +++ .../fixtures/utils/vueNodeFixtures.ts | 147 +++++------------- .../vueNodes/interactions/node/rename.spec.ts | 50 ++---- .../vueNodes/nodeStates/collapse.spec.ts | 44 +++--- 4 files changed, 98 insertions(+), 163 deletions(-) diff --git a/browser_tests/fixtures/VueNodeHelpers.ts b/browser_tests/fixtures/VueNodeHelpers.ts index 64c03b156..86d715bfd 100644 --- a/browser_tests/fixtures/VueNodeHelpers.ts +++ b/browser_tests/fixtures/VueNodeHelpers.ts @@ -3,6 +3,8 @@ */ import type { Locator, Page } from '@playwright/test' +import { VueNodeFixture } from './utils/vueNodeFixtures' + export class VueNodeHelpers { constructor(private page: Page) {} @@ -106,6 +108,24 @@ export class VueNodeHelpers { await this.page.keyboard.press('Backspace') } + /** + * Return a DOM-focused VueNodeFixture for the first node matching the title. + * Resolves the node id up front so subsequent interactions survive title changes. + */ + async getFixtureByTitle(title: string): Promise { + const node = this.getNodeByTitle(title).first() + await node.waitFor({ state: 'visible' }) + + const nodeId = await node.evaluate((el) => el.getAttribute('data-node-id')) + if (!nodeId) { + throw new Error( + `Vue node titled "${title}" is missing its data-node-id attribute` + ) + } + + return new VueNodeFixture(this.getNodeLocator(nodeId)) + } + /** * Wait for Vue nodes to be rendered */ diff --git a/browser_tests/fixtures/utils/vueNodeFixtures.ts b/browser_tests/fixtures/utils/vueNodeFixtures.ts index 5c4541b92..fca464405 100644 --- a/browser_tests/fixtures/utils/vueNodeFixtures.ts +++ b/browser_tests/fixtures/utils/vueNodeFixtures.ts @@ -1,131 +1,66 @@ -import type { Locator, Page } from '@playwright/test' +import { expect } from '@playwright/test' +import type { Locator } from '@playwright/test' -import type { NodeReference } from './litegraphUtils' - -/** - * VueNodeFixture provides Vue-specific testing utilities for interacting with - * Vue node components. It bridges the gap between litegraph node references - * and Vue UI components. - */ +/** DOM-centric helper for a single Vue-rendered node on the canvas. */ export class VueNodeFixture { - constructor( - private readonly nodeRef: NodeReference, - private readonly page: Page - ) {} + constructor(private readonly locator: Locator) {} - /** - * Get the node's header element using data-testid - */ - async getHeader(): Promise { - const nodeId = this.nodeRef.id - return this.page.locator(`[data-testid="node-header-${nodeId}"]`) + get header(): Locator { + return this.locator.locator('[data-testid^="node-header-"]') } - /** - * Get the node's title element - */ - async getTitleElement(): Promise { - const header = await this.getHeader() - return header.locator('[data-testid="node-title"]') + get title(): Locator { + return this.locator.locator('[data-testid="node-title"]') + } + + get titleInput(): Locator { + return this.locator.locator('[data-testid="node-title-input"]') + } + + get body(): Locator { + return this.locator.locator('[data-testid^="node-body-"]') + } + + get collapseButton(): Locator { + return this.locator.locator('[data-testid="node-collapse-button"]') + } + + get collapseIcon(): Locator { + return this.collapseButton.locator('i') + } + + get root(): Locator { + return this.locator } - /** - * Get the current title text - */ async getTitle(): Promise { - const titleElement = await this.getTitleElement() - return (await titleElement.textContent()) || '' + return (await this.title.textContent()) ?? '' } - /** - * Set a new title by double-clicking and entering text - */ - async setTitle(newTitle: string): Promise { - const titleElement = await this.getTitleElement() - await titleElement.dblclick() - - const input = (await this.getHeader()).locator( - '[data-testid="node-title-input"]' - ) - await input.fill(newTitle) + async setTitle(value: string): Promise { + await this.header.dblclick() + const input = this.titleInput + await expect(input).toBeVisible() + await input.fill(value) await input.press('Enter') } - /** - * Cancel title editing - */ async cancelTitleEdit(): Promise { - const titleElement = await this.getTitleElement() - await titleElement.dblclick() - - const input = (await this.getHeader()).locator( - '[data-testid="node-title-input"]' - ) + await this.header.dblclick() + const input = this.titleInput + await expect(input).toBeVisible() await input.press('Escape') } - /** - * Check if the title is currently being edited - */ - async isEditingTitle(): Promise { - const header = await this.getHeader() - const input = header.locator('[data-testid="node-title-input"]') - return await input.isVisible() - } - - /** - * Get the collapse/expand button - */ - async getCollapseButton(): Promise { - const header = await this.getHeader() - return header.locator('[data-testid="node-collapse-button"]') - } - - /** - * Toggle the node's collapsed state - */ async toggleCollapse(): Promise { - const button = await this.getCollapseButton() - await button.click() + await this.collapseButton.click() } - /** - * Get the collapse icon element - */ - async getCollapseIcon(): Promise { - const button = await this.getCollapseButton() - return button.locator('i') - } - - /** - * Get the collapse icon's CSS classes - */ async getCollapseIconClass(): Promise { - const icon = await this.getCollapseIcon() - return (await icon.getAttribute('class')) || '' + return (await this.collapseIcon.getAttribute('class')) ?? '' } - /** - * Check if the collapse button is visible - */ - async isCollapseButtonVisible(): Promise { - const button = await this.getCollapseButton() - return await button.isVisible() - } - - /** - * Get the node's body/content element - */ - async getBody(): Promise { - const nodeId = this.nodeRef.id - return this.page.locator(`[data-testid="node-body-${nodeId}"]`) - } - - /** - * Check if the node body is visible (not collapsed) - */ - async isBodyVisible(): Promise { - const body = await this.getBody() - return await body.isVisible() + boundingBox(): ReturnType { + return this.locator.boundingBox() } } diff --git a/browser_tests/tests/vueNodes/interactions/node/rename.spec.ts b/browser_tests/tests/vueNodes/interactions/node/rename.spec.ts index 3984989e1..5cd6b2fea 100644 --- a/browser_tests/tests/vueNodes/interactions/node/rename.spec.ts +++ b/browser_tests/tests/vueNodes/interactions/node/rename.spec.ts @@ -2,70 +2,46 @@ import { comfyExpect as expect, comfyPageFixture as test } from '../../../../fixtures/ComfyPage' -import { VueNodeFixture } from '../../../../fixtures/utils/vueNodeFixtures' test.describe('Vue Nodes Renaming', () => { test.beforeEach(async ({ comfyPage }) => { await comfyPage.setSetting('Comfy.Graph.CanvasMenu', false) await comfyPage.setSetting('Comfy.VueNodes.Enabled', true) await comfyPage.setup() + await comfyPage.vueNodes.waitForNodes() }) test('should display node title', async ({ comfyPage }) => { - // Get the KSampler node from the default workflow - const nodes = await comfyPage.getNodeRefsByType('KSampler') - expect(nodes.length).toBeGreaterThanOrEqual(1) - - const node = nodes[0] - const vueNode = new VueNodeFixture(node, comfyPage.page) - - const title = await vueNode.getTitle() - expect(title).toBe('KSampler') - - // Verify title is visible in the header - const header = await vueNode.getHeader() - await expect(header).toContainText('KSampler') + const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler') + await expect(vueNode.header).toContainText('KSampler') }) test('should allow title renaming by double clicking on the node header', async ({ comfyPage }) => { - const nodes = await comfyPage.getNodeRefsByType('KSampler') - const node = nodes[0] - const vueNode = new VueNodeFixture(node, comfyPage.page) - + const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler') // Test renaming with Enter await vueNode.setTitle('My Custom Sampler') - const newTitle = await vueNode.getTitle() - expect(newTitle).toBe('My Custom Sampler') - - // Verify the title is displayed - const header = await vueNode.getHeader() - await expect(header).toContainText('My Custom Sampler') + await expect(await vueNode.getTitle()).toBe('My Custom Sampler') + await expect(vueNode.header).toContainText('My Custom Sampler') // Test cancel with Escape - const titleElement = await vueNode.getTitleElement() - await titleElement.dblclick() + await vueNode.title.dblclick() await comfyPage.nextFrame() - - // Type a different value but cancel - const input = (await vueNode.getHeader()).locator( - '[data-testid="node-title-input"]' - ) - await input.fill('This Should Be Cancelled') - await input.press('Escape') + await vueNode.titleInput.fill('This Should Be Cancelled') + await vueNode.titleInput.press('Escape') await comfyPage.nextFrame() // Title should remain as the previously saved value - const titleAfterCancel = await vueNode.getTitle() - expect(titleAfterCancel).toBe('My Custom Sampler') + await expect(await vueNode.getTitle()).toBe('My Custom Sampler') }) test('Double click node body does not trigger edit', async ({ comfyPage }) => { - const loadCheckpointNode = - comfyPage.vueNodes.getNodeByTitle('Load Checkpoint') + const loadCheckpointNode = comfyPage.vueNodes + .getNodeByTitle('Load Checkpoint') + .first() const nodeBbox = await loadCheckpointNode.boundingBox() if (!nodeBbox) throw new Error('Node not found') await loadCheckpointNode.dblclick() diff --git a/browser_tests/tests/vueNodes/nodeStates/collapse.spec.ts b/browser_tests/tests/vueNodes/nodeStates/collapse.spec.ts index fb5fc3c17..8e8e22995 100644 --- a/browser_tests/tests/vueNodes/nodeStates/collapse.spec.ts +++ b/browser_tests/tests/vueNodes/nodeStates/collapse.spec.ts @@ -2,7 +2,6 @@ import { comfyExpect as expect, comfyPageFixture as test } from '../../../fixtures/ComfyPage' -import { VueNodeFixture } from '../../../fixtures/utils/vueNodeFixtures' test.describe('Vue Node Collapse', () => { test.beforeEach(async ({ comfyPage }) => { @@ -10,43 +9,50 @@ test.describe('Vue Node Collapse', () => { await comfyPage.setSetting('Comfy.EnableTooltips', true) await comfyPage.setSetting('Comfy.VueNodes.Enabled', true) await comfyPage.setup() + await comfyPage.vueNodes.waitForNodes() }) test('should allow collapsing node with collapse icon', async ({ comfyPage }) => { - // Get the KSampler node from the default workflow - const nodes = await comfyPage.getNodeRefsByType('KSampler') - const node = nodes[0] - const vueNode = new VueNodeFixture(node, comfyPage.page) + const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler') + await expect(vueNode.root).toBeVisible() // Initially should not be collapsed - expect(await node.isCollapsed()).toBe(false) - const body = await vueNode.getBody() + const body = vueNode.body await expect(body).toBeVisible() + const expandedBoundingBox = await vueNode.boundingBox() + if (!expandedBoundingBox) + throw new Error('Failed to get node bounding box before collapse') // Collapse the node await vueNode.toggleCollapse() - expect(await node.isCollapsed()).toBe(true) + await comfyPage.nextFrame() // Verify node content is hidden - const collapsedSize = await node.getSize() await expect(body).not.toBeVisible() + const collapsedBoundingBox = await vueNode.boundingBox() + if (!collapsedBoundingBox) + throw new Error('Failed to get node bounding box after collapse') + expect(collapsedBoundingBox.height).toBeLessThan(expandedBoundingBox.height) // Expand again await vueNode.toggleCollapse() - expect(await node.isCollapsed()).toBe(false) + await comfyPage.nextFrame() await expect(body).toBeVisible() // Size should be restored - const expandedSize = await node.getSize() - expect(expandedSize.height).toBeGreaterThanOrEqual(collapsedSize.height) + const expandedBoundingBoxAfter = await vueNode.boundingBox() + if (!expandedBoundingBoxAfter) + throw new Error('Failed to get node bounding box after expand') + expect(expandedBoundingBoxAfter.height).toBeGreaterThanOrEqual( + collapsedBoundingBox.height + ) }) test('should show collapse/expand icon state', async ({ comfyPage }) => { - const nodes = await comfyPage.getNodeRefsByType('KSampler') - const node = nodes[0] - const vueNode = new VueNodeFixture(node, comfyPage.page) + const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler') + await expect(vueNode.root).toBeVisible() // Check initial expanded state icon let iconClass = await vueNode.getCollapseIconClass() @@ -66,9 +72,8 @@ test.describe('Vue Node Collapse', () => { test('should preserve title when collapsing/expanding', async ({ comfyPage }) => { - const nodes = await comfyPage.getNodeRefsByType('KSampler') - const node = nodes[0] - const vueNode = new VueNodeFixture(node, comfyPage.page) + const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler') + await expect(vueNode.root).toBeVisible() // Set custom title await vueNode.setTitle('Test Sampler') @@ -83,7 +88,6 @@ test.describe('Vue Node Collapse', () => { expect(await vueNode.getTitle()).toBe('Test Sampler') // Verify title is still displayed - const header = await vueNode.getHeader() - await expect(header).toContainText('Test Sampler') + await expect(vueNode.header).toContainText('Test Sampler') }) })