mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-05 13:41:59 +00:00
test: migrate 132 test files from @vue/test-utils to @testing-library/vue (#10965)
## Summary Migrate 132 test files from `@vue/test-utils` (VTU) to `@testing-library/vue` (VTL) with `@testing-library/user-event`, adopting user-centric behavioral testing patterns across the codebase. ## Changes - **What**: Systematic migration of component/unit tests from VTU's `mount`/`wrapper` API to VTL's `render`/`screen`/`userEvent` API across 132 files in `src/` - **Breaking**: None — test-only changes, no production code affected ### Migration breakdown | Batch | Files | Description | |-------|-------|-------------| | 1 | 19 | Simple render/assert tests | | 2A | 16 | Interactive tests with user events | | 2B-1 | 14 | Interactive tests (continued) | | 2B-2 | 32 | Interactive tests (continued) | | 3A–3E | 51 | Complex tests (stores, composables, heavy mocking) | | Lint fix | 7 | `await` on `fireEvent` calls for `no-floating-promises` | | Review fixes | 15 | Address CodeRabbit feedback (3 rounds) | ### Review feedback addressed - Removed class-based assertions (`text-ellipsis`, `pr-3`, `.pi-save`, `.skeleton`, `.bg-black\/15`, Tailwind utilities) in favor of behavioral/accessible queries - Added null guards before `querySelector` casts - Added `expect(roots).toHaveLength(N)` guards before indexed NodeList access - Wrapped fake timer tests in `try/finally` for guaranteed cleanup - Split double-render tests into focused single-render tests - Replaced CSS class selectors with `screen.getByText`/`screen.getByRole` queries - Updated stubs to use semantic `role`/`aria-label` instead of CSS classes - Consolidated redundant edge-case tests - Removed manual `document.body.appendChild` in favor of VTL container management - Used distinct mock return values to verify command wiring ### VTU holdouts (2 files) These files intentionally retain `@vue/test-utils` because their components use `<script setup>` without `defineExpose`, making internal computed properties and methods inaccessible via VTL: 1. **`NodeWidgets.test.ts`** — partial VTU for `vm.processedWidgets` 2. **`WidgetSelectDropdown.test.ts`** — full VTU for heavy `wrapper.vm.*` access ## Follow-up Deferred items (`ComponentProps` typing, camelCase listener props) tracked in #10966. ## Review Focus - Test correctness: all migrated tests preserve original behavioral coverage - VTL idioms: proper use of `screen` queries, `userEvent`, and accessibility-based selectors - The 2 VTU holdout files are intentional, not oversights ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10965-test-migrate-132-test-files-from-vue-test-utils-to-testing-library-vue-33c6d73d36508199a6a7e513cf5d8296) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Christian Byrne <cbyrne@comfy.org>
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import { flushPromises, mount } from '@vue/test-utils'
|
||||
import { render, screen, waitFor } from '@testing-library/vue'
|
||||
import userEvent from '@testing-library/user-event'
|
||||
import { createPinia, setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
@@ -60,6 +61,7 @@ vi.mock('@/components/widget/layout/BaseModalLayout.vue', () => ({
|
||||
emits: ['close'],
|
||||
template: `
|
||||
<div data-testid="base-modal-layout">
|
||||
<span data-testid="modal-title">{{ contentTitle }}</span>
|
||||
<div v-if="$slots.leftPanel" data-testid="left-panel">
|
||||
<slot name="leftPanel" />
|
||||
</div>
|
||||
@@ -87,15 +89,27 @@ vi.mock('@/components/widget/panel/LeftSidePanel.vue', () => ({
|
||||
<div v-if="$slots['header-title']" data-testid="header-title">
|
||||
<slot name="header-title" />
|
||||
</div>
|
||||
<button
|
||||
v-for="item in navItems"
|
||||
:key="item.id"
|
||||
@click="$emit('update:modelValue', item.id)"
|
||||
:data-testid="'nav-item-' + item.id"
|
||||
:class="{ active: modelValue === item.id }"
|
||||
>
|
||||
{{ item.label }}
|
||||
</button>
|
||||
<template v-for="item in navItems" :key="item.id || item.title">
|
||||
<button
|
||||
v-if="item.id"
|
||||
@click="$emit('update:modelValue', item.id)"
|
||||
:data-testid="'nav-item-' + item.id"
|
||||
:class="{ active: modelValue === item.id }"
|
||||
>
|
||||
{{ item.label }}
|
||||
</button>
|
||||
<template v-else-if="item.items">
|
||||
<button
|
||||
v-for="child in item.items"
|
||||
:key="child.id"
|
||||
@click="$emit('update:modelValue', child.id)"
|
||||
:data-testid="'nav-item-' + child.id"
|
||||
:class="{ active: modelValue === child.id }"
|
||||
>
|
||||
{{ child.label }}
|
||||
</button>
|
||||
</template>
|
||||
</template>
|
||||
</div>
|
||||
`
|
||||
}
|
||||
@@ -151,6 +165,9 @@ vi.mock('vue-i18n', () => ({
|
||||
})
|
||||
}))
|
||||
|
||||
const flushPromises = () =>
|
||||
new Promise<void>((resolve) => setTimeout(resolve, 0))
|
||||
|
||||
describe('AssetBrowserModal', () => {
|
||||
const createTestAsset = (
|
||||
id: string,
|
||||
@@ -173,11 +190,11 @@ describe('AssetBrowserModal', () => {
|
||||
}
|
||||
})
|
||||
|
||||
const createWrapper = (props: Record<string, unknown>) => {
|
||||
function renderModal(props: Record<string, unknown>) {
|
||||
const pinia = createPinia()
|
||||
setActivePinia(pinia)
|
||||
|
||||
return mount(AssetBrowserModal, {
|
||||
return render(AssetBrowserModal, {
|
||||
props,
|
||||
global: {
|
||||
plugins: [pinia],
|
||||
@@ -207,14 +224,16 @@ describe('AssetBrowserModal', () => {
|
||||
]
|
||||
mockAssetsByKey.set('CheckpointLoaderSimple', assets)
|
||||
|
||||
const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' })
|
||||
renderModal({ nodeType: 'CheckpointLoaderSimple' })
|
||||
await flushPromises()
|
||||
|
||||
const assetGrid = wrapper.findComponent({ name: 'AssetGrid' })
|
||||
const gridAssets = assetGrid.props('assets') as AssetItem[]
|
||||
|
||||
expect(gridAssets).toHaveLength(2)
|
||||
expect(gridAssets[0].id).toBe('asset1')
|
||||
expect(screen.getByTestId('asset-asset1')).toBeDefined()
|
||||
expect(screen.getByTestId('asset-asset2')).toBeDefined()
|
||||
/* eslint-disable testing-library/no-node-access */
|
||||
expect(
|
||||
screen.getByTestId('asset-grid').querySelectorAll('.asset-card')
|
||||
).toHaveLength(2)
|
||||
/* eslint-enable testing-library/no-node-access */
|
||||
})
|
||||
|
||||
it('passes category-filtered assets to AssetFilterBar', async () => {
|
||||
@@ -224,23 +243,22 @@ describe('AssetBrowserModal', () => {
|
||||
]
|
||||
mockAssetsByKey.set('CheckpointLoaderSimple', assets)
|
||||
|
||||
const wrapper = createWrapper({
|
||||
renderModal({
|
||||
nodeType: 'CheckpointLoaderSimple',
|
||||
showLeftPanel: true
|
||||
})
|
||||
await flushPromises()
|
||||
|
||||
const filterBar = wrapper.findComponent({ name: 'AssetFilterBar' })
|
||||
const filterBarAssets = filterBar.props('assets') as AssetItem[]
|
||||
|
||||
expect(filterBarAssets).toHaveLength(2)
|
||||
expect(screen.getByTestId('asset-filter-bar').textContent).toContain(
|
||||
'2 assets'
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('Data fetching', () => {
|
||||
it('triggers store refresh for node type on mount', async () => {
|
||||
const store = useAssetsStore()
|
||||
createWrapper({ nodeType: 'CheckpointLoaderSimple' })
|
||||
renderModal({ nodeType: 'CheckpointLoaderSimple' })
|
||||
await flushPromises()
|
||||
|
||||
expect(store.updateModelsForNodeType).toHaveBeenCalledWith(
|
||||
@@ -252,18 +270,17 @@ describe('AssetBrowserModal', () => {
|
||||
const assets = [createTestAsset('asset1', 'Cached Model', 'checkpoints')]
|
||||
mockAssetsByKey.set('CheckpointLoaderSimple', assets)
|
||||
|
||||
const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' })
|
||||
renderModal({ nodeType: 'CheckpointLoaderSimple' })
|
||||
|
||||
const assetGrid = wrapper.findComponent({ name: 'AssetGrid' })
|
||||
const gridAssets = assetGrid.props('assets') as AssetItem[]
|
||||
|
||||
expect(gridAssets).toHaveLength(1)
|
||||
expect(gridAssets[0].name).toBe('Cached Model')
|
||||
expect(screen.getByTestId('asset-asset1')).toBeDefined()
|
||||
expect(screen.getByTestId('asset-asset1').textContent).toContain(
|
||||
'Cached Model'
|
||||
)
|
||||
})
|
||||
|
||||
it('triggers store refresh for asset type (tag) on mount', async () => {
|
||||
const store = useAssetsStore()
|
||||
createWrapper({ assetType: 'models' })
|
||||
renderModal({ assetType: 'models' })
|
||||
await flushPromises()
|
||||
|
||||
expect(store.updateModelsForTag).toHaveBeenCalledWith('models')
|
||||
@@ -273,116 +290,133 @@ describe('AssetBrowserModal', () => {
|
||||
const assets = [createTestAsset('asset1', 'Tagged Model', 'models')]
|
||||
mockAssetsByKey.set('tag:models', assets)
|
||||
|
||||
const wrapper = createWrapper({ assetType: 'models' })
|
||||
renderModal({ assetType: 'models' })
|
||||
await flushPromises()
|
||||
|
||||
const assetGrid = wrapper.findComponent({ name: 'AssetGrid' })
|
||||
const gridAssets = assetGrid.props('assets') as AssetItem[]
|
||||
|
||||
expect(gridAssets).toHaveLength(1)
|
||||
expect(gridAssets[0].name).toBe('Tagged Model')
|
||||
expect(screen.getByTestId('asset-asset1')).toBeDefined()
|
||||
expect(screen.getByTestId('asset-asset1').textContent).toContain(
|
||||
'Tagged Model'
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('Asset Selection', () => {
|
||||
it('emits asset-select event when asset is selected', async () => {
|
||||
const user = userEvent.setup()
|
||||
const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')]
|
||||
mockAssetsByKey.set('CheckpointLoaderSimple', assets)
|
||||
|
||||
const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' })
|
||||
const onAssetSelect = vi.fn()
|
||||
renderModal({
|
||||
nodeType: 'CheckpointLoaderSimple',
|
||||
'onAsset-select': onAssetSelect
|
||||
})
|
||||
await flushPromises()
|
||||
|
||||
const assetGrid = wrapper.findComponent({ name: 'AssetGrid' })
|
||||
await assetGrid.vm.$emit('asset-select', assets[0])
|
||||
await user.click(screen.getByTestId('asset-asset1'))
|
||||
|
||||
expect(wrapper.emitted('asset-select')).toEqual([[assets[0]]])
|
||||
expect(onAssetSelect).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: assets[0].id, name: assets[0].name })
|
||||
)
|
||||
})
|
||||
|
||||
it('executes onSelect callback when provided', async () => {
|
||||
const user = userEvent.setup()
|
||||
const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')]
|
||||
mockAssetsByKey.set('CheckpointLoaderSimple', assets)
|
||||
|
||||
const onSelect = vi.fn()
|
||||
const wrapper = createWrapper({
|
||||
renderModal({
|
||||
nodeType: 'CheckpointLoaderSimple',
|
||||
onSelect
|
||||
})
|
||||
await flushPromises()
|
||||
|
||||
const assetGrid = wrapper.findComponent({ name: 'AssetGrid' })
|
||||
await assetGrid.vm.$emit('asset-select', assets[0])
|
||||
await user.click(screen.getByTestId('asset-asset1'))
|
||||
|
||||
expect(onSelect).toHaveBeenCalledWith(assets[0])
|
||||
expect(onSelect).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: assets[0].id, name: assets[0].name })
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('Left Panel Conditional Logic', () => {
|
||||
it('hides left panel by default when showLeftPanel is undefined', async () => {
|
||||
const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' })
|
||||
renderModal({ nodeType: 'CheckpointLoaderSimple' })
|
||||
await flushPromises()
|
||||
|
||||
const leftPanel = wrapper.find('[data-testid="left-panel"]')
|
||||
expect(leftPanel.exists()).toBe(false)
|
||||
expect(screen.queryByTestId('left-panel')).toBeNull()
|
||||
})
|
||||
|
||||
it('shows left panel when showLeftPanel prop is explicitly true', async () => {
|
||||
const wrapper = createWrapper({
|
||||
renderModal({
|
||||
nodeType: 'CheckpointLoaderSimple',
|
||||
showLeftPanel: true
|
||||
})
|
||||
await flushPromises()
|
||||
|
||||
const leftPanel = wrapper.find('[data-testid="left-panel"]')
|
||||
expect(leftPanel.exists()).toBe(true)
|
||||
expect(screen.getByTestId('left-panel')).toBeDefined()
|
||||
})
|
||||
|
||||
it('hides left panel when showLeftPanel is false', async () => {
|
||||
renderModal({
|
||||
nodeType: 'CheckpointLoaderSimple',
|
||||
showLeftPanel: false
|
||||
})
|
||||
await flushPromises()
|
||||
|
||||
expect(screen.queryByTestId('left-panel')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('Filter Options Reactivity', () => {
|
||||
it('updates filter options when category changes', async () => {
|
||||
const user = userEvent.setup()
|
||||
const assets = [
|
||||
createTestAsset('asset1', 'Model A', 'checkpoints'),
|
||||
createTestAsset('asset2', 'Model B', 'loras')
|
||||
]
|
||||
mockAssetsByKey.set('CheckpointLoaderSimple', assets)
|
||||
|
||||
const wrapper = createWrapper({
|
||||
renderModal({
|
||||
nodeType: 'CheckpointLoaderSimple',
|
||||
showLeftPanel: true
|
||||
})
|
||||
await flushPromises()
|
||||
|
||||
const filterBar = wrapper.findComponent({ name: 'AssetFilterBar' })
|
||||
expect(filterBar.props('assets')).toHaveLength(2)
|
||||
expect(screen.getByTestId('asset-filter-bar').textContent).toContain(
|
||||
'2 assets'
|
||||
)
|
||||
|
||||
const leftPanel = wrapper.findComponent({ name: 'LeftSidePanel' })
|
||||
await leftPanel.vm.$emit('update:modelValue', 'loras')
|
||||
await wrapper.vm.$nextTick()
|
||||
await user.click(screen.getByTestId('nav-item-loras'))
|
||||
|
||||
expect(filterBar.props('assets')).toHaveLength(1)
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('asset-filter-bar').textContent).toContain(
|
||||
'1 assets'
|
||||
)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('Title Management', () => {
|
||||
it('passes custom title to BaseModalLayout when title prop provided', async () => {
|
||||
const wrapper = createWrapper({
|
||||
renderModal({
|
||||
nodeType: 'CheckpointLoaderSimple',
|
||||
title: 'Custom Title'
|
||||
})
|
||||
await flushPromises()
|
||||
|
||||
const layout = wrapper.findComponent({ name: 'BaseModalLayout' })
|
||||
expect(layout.props('contentTitle')).toBe('Custom Title')
|
||||
expect(screen.getByTestId('modal-title').textContent).toBe('Custom Title')
|
||||
})
|
||||
|
||||
it('passes computed contentTitle to BaseModalLayout when no title prop', async () => {
|
||||
const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')]
|
||||
mockAssetsByKey.set('CheckpointLoaderSimple', assets)
|
||||
|
||||
const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' })
|
||||
renderModal({ nodeType: 'CheckpointLoaderSimple' })
|
||||
await flushPromises()
|
||||
|
||||
const layout = wrapper.findComponent({ name: 'BaseModalLayout' })
|
||||
expect(layout.props('contentTitle')).toBe(
|
||||
expect(screen.getByTestId('modal-title').textContent).toBe(
|
||||
'assetBrowser.allCategory:{"category":"Checkpoints"}'
|
||||
)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user