Comfy-Org/ComfyUI_frontend
1
0
Fork 0
mirror of https://github.com/Comfy-Org/ComfyUI_frontend.git synced 2026-05-05 21:54:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

test: expand Image Compare E2E and stabilize widget selectors (#11196)

Browse Source 
## Summary

This PR extends **Image Compare** browser tests, adds **stable
`data-testid` hooks** on `WidgetImageCompare.vue`, and aligns slider
interactions with the **same viewport element** used by
`useMouseInElement`, with **layout polling** to reduce flakes.

## What this PR does

- [x] Adds **`data-testid="image-compare-viewport"`** on the compare
area root (the `containerRef` div) so E2E targets the real slider hit
region instead of a long Tailwind class chain or the `<img>` box alone.
- [x] Adds **`data-testid="image-compare-empty"`** on the no-images
branch so the empty state can be asserted **without hard-coded English**
UI text.
- [x] Adds a **smoke** test that the widget **shows both images and the
drag handle** after value injection, with **`waitForImagesLoaded`** (no
extra full-node screenshot to avoid duplicating the default-50 golden).
- [x] Extends slider coverage to **clamp at both edges** (**0%** and
**~100%**) using the viewport locator and **`expect.poll`** until
**`boundingBox()`** is valid before reading coordinates.
- [x] Updates **`moveToPercentage`** to **`expect.poll`** until the
target locator has a **non-empty layout box** before moving the mouse.
- [x] Routes **hover**, **preserve position**, and **25% / 75%
screenshot** mouse moves through **`image-compare-viewport`**
(consistent with `containerRef`).
- [x] Adds an E2E assertion that the **workflow ImageCompare node** size
is **at least 400×350** (matches the widget workflow asset).
- [x] Hardens the **broken image** case: **`page.on('pageerror')`** /
**`page.off`** in **`finally`**, **`http://127.0.0.1:1/...`** URLs for
fast failure, **`expect.soft`** on key UI invariants, and a hard
assertion that **no page errors** were recorded.
- [x] Extends the **large batch (20×20)** test to **page to the last
index** and assert **counters `20 / 20`**, **previous enabled**, and
**next disabled** on both sides.
- [x] Renames the clamp test title to use an **ASCII hyphen** (`0-100%`)
for easier grepping.

## Out of scope (unchanged in this PR)

- [ ] Replacing **`setImageCompareValue`**’s **`page.evaluate`** setup
with a full UI-driven path (would be a larger follow-up).

## Suggested title

`test: expand Image Compare E2E and stabilize widget selectors`

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Primarily test and selector-hook changes; low production risk, with
only minor DOM attribute additions that could affect external test
tooling if relied upon.
> 
> **Overview**
> Improves Image Compare Playwright coverage and reduces flakiness by
driving slider interactions through a new stable
`data-testid="image-compare-viewport"` hook and polling for a valid
layout box before mouse moves.
> 
> Updates assertions to avoid localized text (new
`data-testid="image-compare-empty"`), adds smoke coverage that
images/handle render after value injection, validates slider clamping at
both 0% and ~100%, and extends screenshot tests to use the same viewport
target.
> 
> Hardens edge-case tests by ensuring broken image loads don’t raise
uncaught `pageerror`s, adds a minimum node size assertion, and extends
large-batch navigation checks through the final index and
disabled/enabled nav states.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
ce3f7fbf8c. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11196-test-expand-Image-Compare-E2E-and-stabilize-widget-selectors-3416d73d3650814d8d2bf207943d6205)
by [Unito](https://www.unito.io)
This commit is contained in:
Kelly Yang
2026-04-14 18:53:13 -07:00
committed by GitHub
parent 121ceda66d
commit 66e8d570e7
2 changed files with 164 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

197
browser_tests/tests/imageCompare.spec.ts
View File

@@ -46,8 +46,16 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
containerLocator: Locator,
percentage: number
) {
await expect
.poll(async () => {
const b = await containerLocator.boundingBox()
return b !== null && b.width > 0 && b.height > 0
})
.toBe(true)
const box = await containerLocator.boundingBox()
if (!box) throw new Error('Container not found')
if (!box || box.width <= 0 || box.height <= 0) {
throw new Error('Slider move target has no layout box')
}
await page.mouse.move(
box.x + box.width * (percentage / 100),
box.y + box.height / 2
@@ -90,12 +98,34 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
const node = comfyPage.vueNodes.getNodeLocator('1')
await expect(node).toBeVisible()
await expect(node).toContainText('No images to compare')
await expect(node.getByTestId('image-compare-empty')).toBeVisible()
await expect(node.locator('img')).toHaveCount(0)
await expect(node.getByRole('presentation')).toHaveCount(0)
}
)
test(
'Widget displays images and handle after value is set',
{ tag: '@smoke' },
async ({ comfyPage }) => {
const beforeUrl = createTestImageDataUrl('Before', '#c00')
const afterUrl = createTestImageDataUrl('After', '#00c')
await setImageCompareValue(comfyPage, {
beforeImages: [beforeUrl],
afterImages: [afterUrl]
})
const node = comfyPage.vueNodes.getNodeLocator('1')
const beforeImg = node.locator('img[alt="Before image"]')
const afterImg = node.locator('img[alt="After image"]')
await expect(beforeImg).toBeVisible()
await expect(afterImg).toBeVisible()
await expect(node.getByRole('presentation')).toBeVisible()
await waitForImagesLoaded(node)
}
)
// ---------------------------------------------------------------------------
// Slider defaults
// ---------------------------------------------------------------------------
@@ -153,10 +183,12 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
const handle = node.getByRole('presentation')
const beforeImg = node.locator('img[alt="Before image"]')
const afterImg = node.locator('img[alt="After image"]')
const viewport = node.getByTestId('image-compare-viewport')
await expect(afterImg).toBeVisible()
await expect(viewport).toBeVisible()
// Left edge: sliderPosition ≈ 5 → clip-path inset right ≈ 95%
await moveToPercentage(comfyPage.page, afterImg, 5)
await moveToPercentage(comfyPage.page, viewport, 5)
await expect
.poll(() => getClipPathInsetRightPercent(beforeImg))
.toBeGreaterThan(90)
@@ -167,7 +199,7 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
.toBeLessThan(10)
// Right edge: sliderPosition ≈ 95 → clip-path inset right ≈ 5%
await moveToPercentage(comfyPage.page, afterImg, 95)
await moveToPercentage(comfyPage.page, viewport, 95)
await expect
.poll(() => getClipPathInsetRightPercent(beforeImg))
.toBeLessThan(10)
@@ -192,9 +224,11 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
const node = comfyPage.vueNodes.getNodeLocator('1')
const handle = node.getByRole('presentation')
const afterImg = node.locator('img[alt="After image"]')
const viewport = node.getByTestId('image-compare-viewport')
await expect(afterImg).toBeVisible()
await expect(viewport).toBeVisible()
await moveToPercentage(comfyPage.page, afterImg, 30)
await moveToPercentage(comfyPage.page, viewport, 30)
// Wait for Vue to commit the slider update
await expect
.poll(() =>
@@ -215,7 +249,7 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
.toBeCloseTo(positionWhileInside, 0)
})
test('Slider clamps to 0% at left edge of container', async ({
test('Slider position clamps to 0-100% range at container edges', async ({
comfyPage
}) => {
const beforeUrl = createTestImageDataUrl('Before', '#c00')
@@ -227,17 +261,35 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
const node = comfyPage.vueNodes.getNodeLocator('1')
const handle = node.getByRole('presentation')
const afterImg = node.locator('img[alt="After image"]')
await expect(afterImg).toBeVisible()
const compareArea = node.getByTestId('image-compare-viewport')
await expect(compareArea).toBeVisible()
const box = await afterImg.boundingBox()
if (!box) throw new Error('Container not found')
await expect
.poll(async () => {
const b = await compareArea.boundingBox()
return b !== null && b.width > 0 && b.height > 0
})
.toBe(true)
const box = await compareArea.boundingBox()
if (!box || box.width <= 0 || box.height <= 0) {
throw new Error('Compare viewport layout not ready')
}
// Move to the leftmost pixel (elementX = 0 → sliderPosition = 0)
await comfyPage.page.mouse.move(box.x, box.y + box.height / 2)
await expect
.poll(() => handle.evaluate((el) => (el as HTMLElement).style.left))
.toBe('0%')
await comfyPage.page.mouse.move(
box.x + box.width - 0.5,
box.y + box.height / 2
)
await expect
.poll(() =>
handle.evaluate((el) => parseFloat((el as HTMLElement).style.left))
)
.toBeCloseTo(100, 0)
})
// ---------------------------------------------------------------------------
@@ -402,6 +454,31 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
)
})
// ---------------------------------------------------------------------------
// Node sizing
// ---------------------------------------------------------------------------
test('ImageCompare node enforces minimum size', async ({ comfyPage }) => {
const size = await comfyPage.page.evaluate(() => {
const graphNode = window.app!.graph.getNodeById(1)
if (!graphNode?.size) return null
return { width: graphNode.size[0], height: graphNode.size[1] }
})
expect(
size,
'ImageCompare node id 1 must exist in loaded workflow graph'
).not.toBeNull()
if (size === null) return
expect(
size.width,
'ImageCompare node minimum width'
).toBeGreaterThanOrEqual(400)
expect(
size.height,
'ImageCompare node minimum height'
).toBeGreaterThanOrEqual(350)
})
// ---------------------------------------------------------------------------
// Visual regression screenshots
// ---------------------------------------------------------------------------
@@ -423,9 +500,10 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
const node = comfyPage.vueNodes.getNodeLocator('1')
const beforeImg = node.locator('img[alt="Before image"]')
const afterImg = node.locator('img[alt="After image"]')
const viewport = node.getByTestId('image-compare-viewport')
await waitForImagesLoaded(node)
await moveToPercentage(comfyPage.page, afterImg, pct)
await expect(viewport).toBeVisible()
await moveToPercentage(comfyPage.page, viewport, pct)
await expect
.poll(() => getClipPathInsetRightPercent(beforeImg))
.toBeGreaterThan(expectedClipMin)
@@ -442,30 +520,56 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
// Edge cases
// ---------------------------------------------------------------------------
test('Widget remains stable with broken image URLs', async ({
test('Widget handles image load failure gracefully', async ({
comfyPage
}) => {
await setImageCompareValue(comfyPage, {
beforeImages: ['https://example.invalid/broken.png'],
afterImages: ['https://example.invalid/broken2.png']
})
const brokenBefore = 'http://127.0.0.1:1/broken.png'
const brokenAfter = 'http://127.0.0.1:1/broken2.png'
const node = comfyPage.vueNodes.getNodeLocator('1')
await expect(node.locator('img')).toHaveCount(2)
await expect(node.getByRole('presentation')).toBeVisible()
const pageErrors: Error[] = []
const onPageError = (err: Error) => {
pageErrors.push(err)
}
comfyPage.page.on('pageerror', onPageError)
await expect
.poll(() =>
node.evaluate((el) => {
const imgs = el.querySelectorAll('img')
let errors = 0
imgs.forEach((img) => {
if (img.complete && img.naturalWidth === 0 && img.src) errors++
try {
await setImageCompareValue(comfyPage, {
beforeImages: [brokenBefore],
afterImages: [brokenAfter]
})
const node = comfyPage.vueNodes.getNodeLocator('1')
await expect.soft(node, 'ImageCompare node stays on canvas').toBeVisible()
await expect
.soft(node.locator('img'), 'Broken URLs still render img elements')
.toHaveCount(2)
await expect
.soft(
node.getByRole('presentation'),
'Compare slider remains for failed network loads'
)
.toBeVisible()
await expect
.poll(() =>
node.evaluate((el) => {
const imgs = el.querySelectorAll('img')
let errors = 0
imgs.forEach((img) => {
if (img.complete && img.naturalWidth === 0 && img.src) errors++
})
return errors
})
return errors
})
)
.toBe(2)
)
.toBe(2)
expect(
pageErrors,
'Image load failures must not surface as uncaught page errors'
).toHaveLength(0)
} finally {
comfyPage.page.off('pageerror', onPageError)
}
})
test('Rapid value updates show latest images and reset batch index', async ({
@@ -540,7 +644,9 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
await expect(node.locator('img[alt="Custom after"]')).toBeVisible()
})
test('Large batch sizes show correct counter', async ({ comfyPage }) => {
test('Large batch sizes show correct counter and end navigation state', async ({
comfyPage
}) => {
const images = Array.from({ length: 20 }, (_, i) =>
createTestImageDataUrl(String(i + 1), '#c00')
)
@@ -550,11 +656,24 @@ test.describe('Image Compare', { tag: ['@widget', '@vue-nodes'] }, () => {
})
const node = comfyPage.vueNodes.getNodeLocator('1')
await expect(
node.getByTestId('before-batch').getByTestId('batch-counter')
).toHaveText('1 / 20')
await expect(
node.getByTestId('after-batch').getByTestId('batch-counter')
).toHaveText('1 / 20')
const beforeBatch = node.getByTestId('before-batch')
const afterBatch = node.getByTestId('after-batch')
await expect(beforeBatch.getByTestId('batch-counter')).toHaveText('1 / 20')
await expect(afterBatch.getByTestId('batch-counter')).toHaveText('1 / 20')
const beforeNext = beforeBatch.getByTestId('batch-next')
const afterNext = afterBatch.getByTestId('batch-next')
for (let i = 0; i < 19; i++) {
await beforeNext.click()
await afterNext.click()
}
await expect(beforeBatch.getByTestId('batch-counter')).toHaveText('20 / 20')
await expect(afterBatch.getByTestId('batch-counter')).toHaveText('20 / 20')
await expect(beforeBatch.getByTestId('batch-prev')).toBeEnabled()
await expect(afterBatch.getByTestId('batch-prev')).toBeEnabled()
await expect(beforeNext).toBeDisabled()
await expect(afterNext).toBeDisabled()
})
})