test: address image crop E2E review feedback

Align Image Crop tests with reviewer feedback by improving polling stability, ratio-lock coverage, and realistic image-load failure simulation. Also export and document MIN_CROP_SIZE for shared test assertions and improve loading-state accessibility.
This commit is contained in:
Kelly Yang
2026-04-13 22:59:14 -07:00
parent ec6d393f0d
commit d32981f2e9
4 changed files with 346 additions and 192 deletions

View File

@@ -1,5 +1,7 @@
import type { Locator } from '@playwright/test'
import { MIN_CROP_SIZE } from '@/composables/useImageCrop'
import {
comfyExpect as expect,
comfyPageFixture as test
@@ -56,12 +58,39 @@ async function setCropBounds(
await comfyPage.nextFrame()
}
async function waitForImageNaturalSize(
img: Locator,
options?: { timeout?: number }
) {
await expect
.poll(
() =>
img.evaluate(
(el: HTMLImageElement) => el.naturalWidth > 0 && el.naturalHeight > 0
),
{
message: 'image naturalWidth and naturalHeight should be ready',
...(options?.timeout != null ? { timeout: options.timeout } : {})
}
)
.toBe(true)
}
async function dragOnLocator(
comfyPage: ComfyPage,
target: Locator,
deltaClientX: number,
deltaClientY: number
) {
await expect
.poll(
async () => {
const b = await target.boundingBox()
return b !== null && b.width > 0 && b.height > 0
},
{ message: 'drag target should have a laid-out bounding box' }
)
.toBe(true)
const box = await target.boundingBox()
if (!box) throw new Error('drag target has no bounding box')
const x0 = box.x + box.width / 2
@@ -102,13 +131,21 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
{ tag: '@smoke' },
async ({ comfyPage }) => {
const node = comfyPage.vueNodes.getNodeLocator('1')
await expect(node).toBeVisible()
await expect(node, 'image crop node should render').toBeVisible()
await expect.soft(node.getByTestId('crop-empty-icon')).toBeVisible()
await expect.soft(node).toContainText('No input image connected')
await expect.soft(node.getByTestId('crop-overlay')).toHaveCount(0)
await expect.soft(node.locator('img')).toHaveCount(0)
await expect.soft(node.getByTestId('crop-resize-right')).toBeHidden()
await expect
.soft(node.getByTestId('crop-empty-icon'), 'empty state icon')
.toBeVisible()
await expect
.soft(node, 'empty state copy')
.toContainText('No input image connected')
await expect
.soft(node.getByTestId('crop-overlay'), 'no overlay without image')
.toHaveCount(0)
await expect.soft(node.locator('img'), 'no preview img').toHaveCount(0)
await expect
.soft(node.getByTestId('crop-resize-right'), 'no resize handles')
.toBeHidden()
}
)
@@ -117,13 +154,17 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
{ tag: '@smoke' },
async ({ comfyPage }) => {
const node = comfyPage.vueNodes.getNodeLocator('1')
await expect(node).toBeVisible()
await expect(node, 'image crop node should render').toBeVisible()
await expect(node.getByText('Ratio')).toBeVisible()
await expect(node.getByText('Ratio'), 'ratio label').toBeVisible()
await expect(
node.locator('button:has(.icon-\\[lucide--lock-open\\])')
node.locator('button:has(.icon-\\[lucide--lock-open\\])'),
'ratio unlock button'
).toBeVisible()
await expect(node.locator('input')).toHaveCount(4)
await expect(
node.locator('input'),
'bounding box numeric inputs'
).toHaveCount(4)
}
)
@@ -132,7 +173,7 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
{ tag: '@screenshot' },
async ({ comfyPage }) => {
const node = comfyPage.vueNodes.getNodeLocator('1')
await expect(node).toBeVisible()
await expect(node, 'image crop node should render').toBeVisible()
await comfyPage.nextFrame()
await comfyPage.nextFrame()
await expect(node).toHaveScreenshot('image-crop-empty-state.png', {
@@ -151,7 +192,11 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
const empty = node.getByTestId('crop-empty-state')
await dragOnLocator(comfyPage, empty, 40, 30)
await expect.poll(() => getCropValue(comfyPage, 1)).toStrictEqual(before)
await expect
.poll(() => getCropValue(comfyPage, 1), {
message: 'empty-state drag should not mutate crop value'
})
.toStrictEqual(before)
})
})
@@ -164,7 +209,8 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
await comfyPage.vueNodes.waitForNodes()
await comfyPage.runButton.click()
await expect(
comfyPage.vueNodes.getNodeLocator('2').locator('img')
comfyPage.vueNodes.getNodeLocator('2').locator('img'),
'source image preview should appear after run'
).toBeVisible({ timeout: 30_000 })
})
@@ -175,15 +221,17 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
const node = comfyPage.vueNodes.getNodeLocator('2')
const img = node.locator('img')
await expect
.poll(() => img.evaluate((el: HTMLImageElement) => el.naturalWidth))
.toBeGreaterThan(0)
await waitForImageNaturalSize(img)
await expect(node.getByTestId('crop-overlay')).toBeVisible()
await expect(
node.getByTestId('crop-overlay'),
'crop overlay should show when image is ready'
).toBeVisible()
await expect(
node
.locator('[data-testid^="crop-resize-"]')
.filter({ visible: true })
.filter({ visible: true }),
'unlocked ratio should expose eight handles'
).toHaveCount(8)
await comfyPage.nextFrame()
@@ -200,6 +248,15 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
async ({ comfyPage }) => {
const node = comfyPage.vueNodes.getNodeLocator('2')
const cropBox = node.getByTestId('crop-overlay')
await expect
.poll(
async () => {
const b = await cropBox.boundingBox()
return b !== null && b.width > 0 && b.height > 0 ? b : null
},
{ message: 'crop overlay should have a stable bounding box' }
)
.not.toBeNull()
const box = await cropBox.boundingBox()
if (!box) throw new Error('Crop box not found')
@@ -238,33 +295,23 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
return (
!!v &&
v.x > valueBefore.x &&
v.y > valueBefore.y &&
v.width === valueBefore.width &&
v.height === valueBefore.height
)
if (
!v ||
v.x <= valueBefore.x ||
v.y <= valueBefore.y ||
v.width !== valueBefore.width ||
v.height !== valueBefore.height
) {
return null
}
return v
},
{ timeout: 5000 }
{
timeout: 5000,
message: 'crop drag should increase x/y without changing size'
}
)
.toBe(true)
const valueAfter = await getCropValue(comfyPage, 2)
expect(
valueAfter,
'crop value should exist after drag'
).not.toBeNull()
expect(
valueAfter!.x,
'crop X should increase after drag'
).toBeGreaterThan(valueBefore.x)
expect(
valueAfter!.y,
'crop Y should increase after drag'
).toBeGreaterThan(valueBefore.y)
expect(valueAfter!.width).toBe(valueBefore.width)
expect(valueAfter!.height).toBe(valueBefore.height)
.not.toBeNull()
await expect(node).toHaveScreenshot('image-crop-after-drag.png', {
maxDiffPixelRatio: 0.05
@@ -277,13 +324,11 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
}) => {
const node = comfyPage.vueNodes.getNodeLocator('2')
const img = node.locator('img')
await expect
.poll(() => img.evaluate((el: HTMLImageElement) => el.naturalWidth))
.toBeGreaterThan(0)
const nw = await img.evaluate((el: HTMLImageElement) => el.naturalWidth)
const nh = await img.evaluate(
(el: HTMLImageElement) => el.naturalHeight
)
await waitForImageNaturalSize(img)
const { nw, nh } = await img.evaluate((el: HTMLImageElement) => ({
nw: el.naturalWidth,
nh: el.naturalHeight
}))
await setCropBounds(comfyPage, 2, {
x: nw - 100,
@@ -296,17 +341,23 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
await dragOnLocator(comfyPage, cropBox, 400, 200)
await expect
.poll(async () => {
const v = await getCropValue(comfyPage, 2)
return v ? v.x + v.width : 0
})
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
return v ? v.x + v.width : 0
},
{ message: 'crop drag should clamp to image right edge' }
)
.toBeLessThanOrEqual(nw)
await expect
.poll(async () => {
const v = await getCropValue(comfyPage, 2)
return v ? v.y + v.height : 0
})
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
return v ? v.y + v.height : 0
},
{ message: 'crop drag should clamp to image bottom edge' }
)
.toBeLessThanOrEqual(nh)
})
@@ -326,10 +377,14 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
await dragOnLocator(comfyPage, cropBox, -400, -350)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.x ?? -1)
.poll(async () => (await getCropValue(comfyPage, 2))?.x ?? -1, {
message: 'crop drag should clamp x to the image'
})
.toBeGreaterThanOrEqual(0)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.y ?? -1)
.poll(async () => (await getCropValue(comfyPage, 2))?.y ?? -1, {
message: 'crop drag should clamp y to the image'
})
.toBeGreaterThanOrEqual(0)
})
@@ -351,13 +406,20 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
await dragOnLocator(comfyPage, handle, 55, 0)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.width ?? 0)
.toBeGreaterThan(before.width)
const after = await getCropValue(comfyPage, 2)
expect(after?.x).toBe(before.x)
expect(after?.y).toBe(before.y)
expect(after?.height).toBe(before.height)
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
if (!v) return false
return (
v.width > before.width &&
v.x === before.x &&
v.y === before.y &&
v.height === before.height
)
},
{ message: 'right-edge resize should grow width only' }
)
.toBe(true)
})
test('Resize from left edge decreases x and increases width', async ({
@@ -374,40 +436,28 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
const before = await getCropValue(comfyPage, 2)
if (!before) throw new Error('missing crop')
const handle = node.getByTestId('crop-resize-left')
const box = await handle.boundingBox()
if (!box) throw new Error('left handle missing')
const x0 = box.x + box.width / 2
const y0 = box.y + box.height / 2
await handle.dispatchEvent('pointerdown', {
...POINTER_OPTS,
clientX: x0,
clientY: y0
})
await comfyPage.nextFrame()
await handle.dispatchEvent('pointermove', {
...POINTER_OPTS,
clientX: x0 - 50,
clientY: y0
})
await comfyPage.nextFrame()
await handle.dispatchEvent('pointerup', {
...POINTER_OPTS,
clientX: x0 - 50,
clientY: y0
})
await comfyPage.nextFrame()
await dragOnLocator(
comfyPage,
node.getByTestId('crop-resize-left'),
-50,
0
)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.x ?? 999)
.toBeLessThan(before.x)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.width ?? 0)
.toBeGreaterThan(before.width)
const after = await getCropValue(comfyPage, 2)
expect(after?.y).toBe(before.y)
expect(after?.height).toBe(before.height)
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
if (!v) return false
return (
v.x < before.x &&
v.width > before.width &&
v.y === before.y &&
v.height === before.height
)
},
{ message: 'left-edge resize should move x and grow width only' }
)
.toBe(true)
})
test('Resize from bottom edge increases height only', async ({
@@ -432,13 +482,20 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.height ?? 0)
.toBeGreaterThan(before.height)
const after = await getCropValue(comfyPage, 2)
expect(after?.x).toBe(before.x)
expect(after?.y).toBe(before.y)
expect(after?.width).toBe(before.width)
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
if (!v) return false
return (
v.height > before.height &&
v.x === before.x &&
v.y === before.y &&
v.width === before.width
)
},
{ message: 'bottom-edge resize should grow height only' }
)
.toBe(true)
})
test('Resize from top edge decreases y and increases height', async ({
@@ -455,40 +512,28 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
const before = await getCropValue(comfyPage, 2)
if (!before) throw new Error('missing crop')
const handle = node.getByTestId('crop-resize-top')
const box = await handle.boundingBox()
if (!box) throw new Error('top handle missing')
const x0 = box.x + box.width / 2
const y0 = box.y + box.height / 2
await handle.dispatchEvent('pointerdown', {
...POINTER_OPTS,
clientX: x0,
clientY: y0
})
await comfyPage.nextFrame()
await handle.dispatchEvent('pointermove', {
...POINTER_OPTS,
clientX: x0,
clientY: y0 - 45
})
await comfyPage.nextFrame()
await handle.dispatchEvent('pointerup', {
...POINTER_OPTS,
clientX: x0,
clientY: y0 - 45
})
await comfyPage.nextFrame()
await dragOnLocator(
comfyPage,
node.getByTestId('crop-resize-top'),
0,
-45
)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.y ?? 999)
.toBeLessThan(before.y)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.height ?? 0)
.toBeGreaterThan(before.height)
const after = await getCropValue(comfyPage, 2)
expect(after?.x).toBe(before.x)
expect(after?.width).toBe(before.width)
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
if (!v) return false
return (
v.y < before.y &&
v.height > before.height &&
v.x === before.x &&
v.width === before.width
)
},
{ message: 'top-edge resize should move y and grow height only' }
)
.toBe(true)
})
test(
@@ -514,15 +559,20 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.width ?? 0)
.toBeGreaterThan(before.width)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.height ?? 0)
.toBeGreaterThan(before.height)
const after = await getCropValue(comfyPage, 2)
expect(after?.x).toBe(before.x)
expect(after?.y).toBe(before.y)
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
if (!v) return false
return (
v.width > before.width &&
v.height > before.height &&
v.x === before.x &&
v.y === before.y
)
},
{ message: 'SE corner resize should grow width and height' }
)
.toBe(true)
await comfyPage.nextFrame()
await comfyPage.nextFrame()
@@ -555,17 +605,20 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.x ?? 999)
.toBeLessThan(before.x)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.y ?? 999)
.toBeLessThan(before.y)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.width ?? 0)
.toBeGreaterThan(before.width)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.height ?? 0)
.toBeGreaterThan(before.height)
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
if (!v) return false
return (
v.x < before.x &&
v.y < before.y &&
v.width > before.width &&
v.height > before.height
)
},
{ message: 'NW corner resize should move origin and grow box' }
)
.toBe(true)
await comfyPage.nextFrame()
await comfyPage.nextFrame()
@@ -592,19 +645,27 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.width ?? 0)
.toBeGreaterThanOrEqual(16)
.poll(async () => (await getCropValue(comfyPage, 2))?.width ?? 0, {
message: 'crop width should respect minimum size'
})
.toBeGreaterThanOrEqual(MIN_CROP_SIZE)
await expect
.poll(async () => (await getCropValue(comfyPage, 2))?.height ?? 0)
.toBeGreaterThanOrEqual(16)
.poll(async () => (await getCropValue(comfyPage, 2))?.height ?? 0, {
message: 'crop height should respect minimum size'
})
.toBeGreaterThanOrEqual(MIN_CROP_SIZE)
})
test('Resize clamps to image boundaries on the right edge', async ({
test('Resize clamps to image boundaries on the right and bottom edges', async ({
comfyPage
}) => {
const node = comfyPage.vueNodes.getNodeLocator('2')
const img = node.locator('img')
const nw = await img.evaluate((el: HTMLImageElement) => el.naturalWidth)
await waitForImageNaturalSize(img)
const { nw, nh } = await img.evaluate((el: HTMLImageElement) => ({
nw: el.naturalWidth,
nh: el.naturalHeight
}))
await setCropBounds(comfyPage, 2, {
x: nw - 120,
@@ -621,11 +682,33 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
)
await expect
.poll(async () => {
const v = await getCropValue(comfyPage, 2)
return v ? v.x + v.width : 0
})
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
return v ? v.x + v.width : 0
},
{ message: 'right-edge resize should not extend past image width' }
)
.toBeLessThanOrEqual(nw)
await dragOnLocator(
comfyPage,
node.getByTestId('crop-resize-bottom'),
0,
400
)
await expect
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
return v ? v.y + v.height : 0
},
{
message: 'bottom-edge resize should not extend past image height'
}
)
.toBeLessThanOrEqual(nh)
})
test(
@@ -636,7 +719,8 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
await expect(
node
.locator('[data-testid^="crop-resize-"]')
.filter({ visible: true })
.filter({ visible: true }),
'unlocked ratio should expose edge and corner handles'
).toHaveCount(8)
await comfyPage.nextFrame()
@@ -647,23 +731,90 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
}
)
test('Four corner resize handles are visible when aspect ratio is locked', async ({
comfyPage
}) => {
const node = comfyPage.vueNodes.getNodeLocator('2')
await node.getByRole('button', { name: 'Lock aspect ratio' }).click()
await expect(
node
.locator('[data-testid^="crop-resize-"]')
.filter({ visible: true }),
'locked ratio should only show corner handles'
).toHaveCount(4)
})
test('Resize with locked aspect ratio keeps width and height proportional', async ({
comfyPage
}) => {
const node = comfyPage.vueNodes.getNodeLocator('2')
await setCropBounds(comfyPage, 2, {
x: 70,
y: 80,
width: 160,
height: 120
})
await node.getByRole('button', { name: 'Lock aspect ratio' }).click()
const before = await getCropValue(comfyPage, 2)
if (!before) throw new Error('missing crop')
const ratio = before.width / before.height
await dragOnLocator(
comfyPage,
node.getByTestId('crop-resize-se'),
48,
36
)
await expect
.poll(
async () => {
const v = await getCropValue(comfyPage, 2)
if (!v || v.width <= before.width || v.height <= before.height) {
return null
}
const r = v.width / v.height
if (Math.abs(r - ratio) > 0.06) return null
return v
},
{ message: 'locked ratio resize should preserve aspect ratio' }
)
.not.toBeNull()
})
test('Broken image URL resets widget to empty state', async ({
comfyPage
}) => {
const node = comfyPage.vueNodes.getNodeLocator('2')
const img = node.locator('img')
await expect
.poll(() => img.evaluate((el: HTMLImageElement) => el.naturalWidth))
.toBeGreaterThan(0)
await waitForImageNaturalSize(img)
await img.evaluate((el: HTMLImageElement) => {
el.src = 'http://127.0.0.1:9/__e2e_image_crop_invalid__'
let failExamplePng = false
await comfyPage.page.route('**/api/view**', async (route) => {
const u = route.request().url()
if (failExamplePng && u.includes('example.png')) {
await route.abort('failed')
return
}
await route.continue()
})
await expect(node.getByTestId('crop-empty-state')).toBeVisible({
timeout: 15_000
})
await expect(node.getByTestId('crop-overlay')).toHaveCount(0)
try {
failExamplePng = true
const runDone = comfyPage.runButton.click()
await expect(
node.getByTestId('crop-empty-state'),
'failed view fetch should show empty state'
).toBeVisible({ timeout: 15_000 })
await expect(
node.getByTestId('crop-overlay'),
'crop overlay should hide when image fails'
).toHaveCount(0)
await runDone
} finally {
await comfyPage.page.unroute('**/api/view**')
}
})
}
)
@@ -697,22 +848,19 @@ test.describe('Image Crop', { tag: '@widget' }, () => {
await comfyPage.vueNodes.waitForNodes()
const node = comfyPage.vueNodes.getNodeLocator('2')
const runDone = comfyPage.runButton.click()
await expect(node.getByText('Loading...')).toBeVisible({
timeout: 10_000
})
await expect(
node.getByText('Loading...'),
'delayed view should show loading overlay'
).toBeVisible({ timeout: 10_000 })
await runDone
const img = node.locator('img')
await expect
.poll(
() => img.evaluate((el: HTMLImageElement) => el.naturalWidth),
{
timeout: 30_000
}
)
.toBeGreaterThan(0)
await waitForImageNaturalSize(img, { timeout: 30_000 })
await expect(node.getByText('Loading...')).toBeHidden()
await expect(
node.getByText('Loading...'),
'loading overlay should hide after delayed image loads'
).toBeHidden()
} finally {
await comfyPage.page.unroute('**/api/view**')
}

View File

@@ -36,6 +36,7 @@
<div
v-if="isLoading"
aria-live="polite"
class="absolute inset-0 z-10 flex size-full items-center justify-center bg-node-component-surface/90"
>
<span class="text-sm">{{ $t('imageCrop.loading') }}</span>

View File

@@ -7,6 +7,10 @@ describe('imageCropLoadingAfterUrlChange', () => {
expect(imageCropLoadingAfterUrlChange(null, 'https://a/b.png')).toBe(false)
})
it('keeps loading off when url stays null', () => {
expect(imageCropLoadingAfterUrlChange(null, null)).toBe(false)
})
it('starts loading when url changes to a new string', () => {
expect(imageCropLoadingAfterUrlChange('https://b', 'https://a')).toBe(true)
})

View File

@@ -19,7 +19,8 @@ type ResizeDirection =
const HANDLE_SIZE = 8
const CORNER_SIZE = 10
const MIN_CROP_SIZE = 16
/** Minimum crop width/height in source image pixel space. */
export const MIN_CROP_SIZE = 16
const CROP_BOX_BORDER = 2
/**