From 66e8d570e79cd3550d6637b2b29ada3affd1af25 Mon Sep 17 00:00:00 2001 From: Kelly Yang <124ykl@gmail.com> Date: Tue, 14 Apr 2026 18:53:13 -0700 Subject: [PATCH] test: expand Image Compare E2E and stabilize widget selectors (#11196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 `` 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` --- > [!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. > > Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ce3f7fbf8cdf4189d4e3553ed52b5dee465a8a28. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot). ┆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) --- browser_tests/tests/imageCompare.spec.ts | 197 ++++++++++++++---- .../widgets/components/WidgetImageCompare.vue | 7 +- 2 files changed, 164 insertions(+), 40 deletions(-) diff --git a/browser_tests/tests/imageCompare.spec.ts b/browser_tests/tests/imageCompare.spec.ts index ce8538daa1..3afbbdee55 100644 --- a/browser_tests/tests/imageCompare.spec.ts +++ b/browser_tests/tests/imageCompare.spec.ts @@ -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() }) }) diff --git a/src/renderer/extensions/vueNodes/widgets/components/WidgetImageCompare.vue b/src/renderer/extensions/vueNodes/widgets/components/WidgetImageCompare.vue index 0f99067431..d4a3f46c97 100644 --- a/src/renderer/extensions/vueNodes/widgets/components/WidgetImageCompare.vue +++ b/src/renderer/extensions/vueNodes/widgets/components/WidgetImageCompare.vue @@ -26,6 +26,7 @@
-
+
{{ $t('imageCompare.noImages') }}