Skip to content

Commit 5a9db74

Browse files
committed
fix: apply code review feedback
- Remove redundant text-warning-background class on inner span - Add error overlay visibility assertion for widget mismatch test - Fix hasValidDirectory to check for non-empty array - Improve test isolation with explicit mockIsCloud reset - Extract makeModel helper to module level to reduce duplication - Add cross-reference comments for extension constants
1 parent 5812343 commit 5a9db74

File tree

6 files changed

+34
-57
lines changed

6 files changed

+34
-57
lines changed

browser_tests/tests/dialog.spec.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,9 @@ test.describe('Missing models in Error Tab', () => {
135135

136136
const missingModelsTitle = comfyPage.page.getByText(/Missing Models/)
137137
await expect(missingModelsTitle).not.toBeVisible()
138+
139+
const errorOverlay = comfyPage.page.locator('[data-testid="error-overlay"]')
140+
await expect(errorOverlay).not.toBeVisible()
138141
})
139142

140143
// Flaky test after parallelization

src/components/rightSidePanel/errors/useErrorGroups.test.ts

Lines changed: 20 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,26 @@ function makeMissingNodeType(
8686
}
8787
}
8888

89+
function makeModel(
90+
name: string,
91+
opts: {
92+
nodeId?: string | number
93+
widgetName?: string
94+
directory?: string
95+
isAssetSupported?: boolean
96+
} = {}
97+
) {
98+
return {
99+
name,
100+
nodeId: opts.nodeId ?? '1',
101+
nodeType: 'CheckpointLoaderSimple',
102+
widgetName: opts.widgetName ?? 'ckpt_name',
103+
isAssetSupported: opts.isAssetSupported ?? false,
104+
isMissing: true as const,
105+
directory: opts.directory
106+
}
107+
}
108+
89109
function createErrorGroups() {
90110
const store = useExecutionErrorStore()
91111
const searchQuery = ref('')
@@ -524,26 +544,6 @@ describe('useErrorGroups', () => {
524544
})
525545

526546
describe('missingModelGroups', () => {
527-
function makeModel(
528-
name: string,
529-
opts: {
530-
nodeId?: string | number
531-
widgetName?: string
532-
directory?: string
533-
isAssetSupported?: boolean
534-
} = {}
535-
) {
536-
return {
537-
name,
538-
nodeId: opts.nodeId ?? '1',
539-
nodeType: 'CheckpointLoaderSimple',
540-
widgetName: opts.widgetName ?? 'ckpt_name',
541-
isAssetSupported: opts.isAssetSupported ?? false,
542-
isMissing: true as const,
543-
directory: opts.directory
544-
}
545-
}
546-
547547
it('returns empty array when no missing models', () => {
548548
const { groups } = createErrorGroups()
549549
expect(groups.missingModelGroups.value).toEqual([])
@@ -696,26 +696,6 @@ describe('useErrorGroups', () => {
696696
mockIsCloud.value = false
697697
})
698698

699-
function makeModel(
700-
name: string,
701-
opts: {
702-
nodeId?: string | number
703-
widgetName?: string
704-
directory?: string
705-
isAssetSupported?: boolean
706-
} = {}
707-
) {
708-
return {
709-
name,
710-
nodeId: opts.nodeId ?? '1',
711-
nodeType: 'CheckpointLoaderSimple',
712-
widgetName: opts.widgetName ?? 'ckpt_name',
713-
isAssetSupported: opts.isAssetSupported ?? false,
714-
isMissing: true as const,
715-
directory: opts.directory
716-
}
717-
}
718-
719699
it('puts unsupported models into UNSUPPORTED group in Cloud', async () => {
720700
const { store, groups } = createErrorGroups()
721701
store.surfaceMissingModels([

src/platform/missingModel/components/MissingModelCard.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ function mountCard(
9898
}
9999

100100
describe('MissingModelCard', () => {
101+
beforeEach(() => {
102+
mockIsCloud.value = true
103+
})
104+
101105
describe('Rendering & Props', () => {
102106
it('renders directory name in category header', () => {
103107
const wrapper = mountCard({

src/platform/missingModel/components/MissingModelCard.vue

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@
1616
: 'text-destructive-background-hover'
1717
"
1818
>
19-
<span
20-
v-if="isCloud && !group.isAssetSupported"
21-
class="text-warning-background"
22-
>
19+
<span v-if="isCloud && !group.isAssetSupported">
2320
{{ t('rightSidePanel.missingModels.importNotSupported') }}
2421
({{ group.models.length }})
2522
</span>

src/platform/missingModel/missingModelDownload.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const WHITE_LISTED_URLS: ReadonlySet<string> = new Set([
2525
'https://github.com/xinntao/Real-ESRGAN/releases/download/v0.1.0/RealESRGAN_x4plus.pth'
2626
])
2727

28-
export interface ModelWithUrl {
28+
interface ModelWithUrl {
2929
name: string
3030
url: string
3131
directory: string
@@ -40,12 +40,6 @@ export function isModelDownloadable(model: ModelWithUrl): boolean {
4040
return true
4141
}
4242

43-
export function hasValidDirectory(
44-
model: ModelWithUrl,
45-
paths: Record<string, string[]>
46-
): boolean {
47-
return !!paths[model.directory]
48-
}
4943

5044
export function downloadModel(
5145
model: ModelWithUrl,
@@ -104,7 +98,7 @@ async function fetchCivitaiMetadata(url: string): Promise<ModelMetadata> {
10498

10599
const data: CivitaiModelVersionResponse = await res.json()
106100
const matchingFile = data.files?.find(
107-
(file) => file.downloadUrl && file.downloadUrl.startsWith(url)
101+
(file) => file.downloadUrl.startsWith(url) && file.downloadUrl
108102
)
109103
const fileSize = matchingFile?.sizeKB ? matchingFile.sizeKB * 1024 : null
110104
return { fileSize, gatedRepoUrl: null }
@@ -128,8 +122,10 @@ async function fetchHeadMetadata(url: string): Promise<ModelMetadata> {
128122
return { fileSize: null, gatedRepoUrl: null }
129123
}
130124
const size = response.headers.get('content-length')
125+
const parsedSize = size ? parseInt(size, 10) : null
131126
return {
132-
fileSize: size ? parseInt(size, 10) : null,
127+
fileSize:
128+
parsedSize !== null && !Number.isNaN(parsedSize) ? parsedSize : null,
133129
gatedRepoUrl: null
134130
}
135131
} catch {

src/platform/missingModel/missingModelScan.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,7 @@ export const MODEL_FILE_EXTENSIONS = new Set([
4646

4747
export function isModelFileName(name: string): boolean {
4848
const lower = name.toLowerCase()
49-
for (const ext of MODEL_FILE_EXTENSIONS) {
50-
if (lower.endsWith(ext)) return true
51-
}
52-
return false
49+
return Array.from(MODEL_FILE_EXTENSIONS).some((ext) => lower.endsWith(ext))
5350
}
5451

5552
function resolveComboOptions(widget: IComboWidget): string[] {

0 commit comments

Comments
 (0)