Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion browser_tests/fixtures/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,15 @@ export const TestIds = {
settingsTabAbout: 'settings-tab-about',
confirm: 'confirm-dialog',
errorOverlay: 'error-overlay',
errorOverlaySeeErrors: 'error-overlay-see-errors',
runtimeErrorPanel: 'runtime-error-panel',
missingNodeCard: 'missing-node-card',
errorCardFindOnGithub: 'error-card-find-on-github',
errorCardCopy: 'error-card-copy',
about: 'about-panel',
whatsNewSection: 'whats-new-section'
whatsNewSection: 'whats-new-section',
missingNodePacksGroup: 'error-group-missing-node',
missingModelsGroup: 'error-group-missing-model'
},
keybindings: {
presetMenu: 'keybinding-preset-menu'
Expand Down Expand Up @@ -76,6 +81,10 @@ export const TestIds = {
},
user: {
currentUserIndicator: 'current-user-indicator'
},
errors: {
imageLoadError: 'error-loading-image',
videoLoadError: 'error-loading-video'
}
} as const

Expand All @@ -101,3 +110,4 @@ export type TestIdValue =
(id: string) => string
>
| (typeof TestIds.user)[keyof typeof TestIds.user]
| (typeof TestIds.errors)[keyof typeof TestIds.errors]
34 changes: 20 additions & 14 deletions browser_tests/tests/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ test.describe('Missing nodes in Error Overlay', { tag: '@ui' }, () => {
)
await expect(errorOverlay).toBeVisible()

const missingNodesTitle = comfyPage.page.getByText(/Missing Node Packs/)
const missingNodesTitle = errorOverlay.getByText(/Missing Node Packs/)
await expect(missingNodesTitle).toBeVisible()
})

Expand All @@ -42,11 +42,13 @@ test.describe('Missing nodes in Error Overlay', { tag: '@ui' }, () => {
)
await expect(errorOverlay).toBeVisible()

const missingNodesTitle = comfyPage.page.getByText(/Missing Node Packs/)
const missingNodesTitle = errorOverlay.getByText(/Missing Node Packs/)
await expect(missingNodesTitle).toBeVisible()

// Click "See Errors" to open the errors tab and verify subgraph node content
await errorOverlay.getByRole('button', { name: 'See Errors' }).click()
await errorOverlay
.getByTestId(TestIds.dialogs.errorOverlaySeeErrors)
.click()
await expect(errorOverlay).not.toBeVisible()

const missingNodeCard = comfyPage.page.getByTestId(
Expand Down Expand Up @@ -75,7 +77,9 @@ test.describe('Missing nodes in Error Overlay', { tag: '@ui' }, () => {
await expect(errorOverlay).toBeVisible()

// Click "See Errors" to open the right side panel errors tab
await errorOverlay.getByRole('button', { name: 'See Errors' }).click()
await errorOverlay
.getByTestId(TestIds.dialogs.errorOverlaySeeErrors)
.click()
await expect(errorOverlay).not.toBeVisible()

// Verify MissingNodeCard is rendered in the errors tab
Expand Down Expand Up @@ -165,17 +169,19 @@ test.describe('Error actions in Errors Tab', { tag: '@ui' }, () => {
TestIds.dialogs.errorOverlay
)
await expect(errorOverlay).toBeVisible()
await errorOverlay.getByRole('button', { name: 'See Errors' }).click()
await errorOverlay
.getByTestId(TestIds.dialogs.errorOverlaySeeErrors)
.click()
await expect(errorOverlay).not.toBeVisible()

// Verify Find on GitHub button is present in the error card
const findOnGithubButton = comfyPage.page.getByRole('button', {
name: 'Find on GitHub'
})
const findOnGithubButton = comfyPage.page.getByTestId(
TestIds.dialogs.errorCardFindOnGithub
)
await expect(findOnGithubButton).toBeVisible()

// Verify Copy button is present in the error card
const copyButton = comfyPage.page.getByRole('button', { name: 'Copy' })
const copyButton = comfyPage.page.getByTestId(TestIds.dialogs.errorCardCopy)
await expect(copyButton).toBeVisible()
})
})
Expand Down Expand Up @@ -204,7 +210,7 @@ test.describe('Missing models in Error Tab', () => {
)
await expect(errorOverlay).toBeVisible()

const missingModelsTitle = comfyPage.page.getByText(/Missing Models/)
const missingModelsTitle = errorOverlay.getByText(/Missing Models/)
await expect(missingModelsTitle).toBeVisible()
})

Expand All @@ -220,7 +226,7 @@ test.describe('Missing models in Error Tab', () => {
)
await expect(errorOverlay).toBeVisible()

const missingModelsTitle = comfyPage.page.getByText(/Missing Models/)
const missingModelsTitle = errorOverlay.getByText(/Missing Models/)
await expect(missingModelsTitle).toBeVisible()
})

Expand All @@ -231,13 +237,13 @@ test.describe('Missing models in Error Tab', () => {
'missing/model_metadata_widget_mismatch'
)

const missingModelsTitle = comfyPage.page.getByText(/Missing Models/)
await expect(missingModelsTitle).not.toBeVisible()

const errorOverlay = comfyPage.page.getByTestId(
TestIds.dialogs.errorOverlay
)
await expect(errorOverlay).not.toBeVisible()

const missingModelsTitle = errorOverlay.getByText(/Missing Models/)
await expect(missingModelsTitle).not.toBeVisible()
})

// Flaky test after parallelization
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
comfyExpect as expect,
comfyPageFixture as test
} from '../../../../fixtures/ComfyPage'
import { TestIds } from '../../../../fixtures/selectors'

test.describe('Vue Upload Widgets', () => {
test.beforeEach(async ({ comfyPage }) => {
Expand All @@ -19,10 +20,14 @@ test.describe('Vue Upload Widgets', () => {
).not.toBeVisible()

await expect
.poll(() => comfyPage.page.getByText('Error loading image').count())
.poll(() =>
comfyPage.page.getByTestId(TestIds.errors.imageLoadError).count()
)
.toBeGreaterThan(0)
await expect
.poll(() => comfyPage.page.getByText('Error loading video').count())
.poll(() =>
comfyPage.page.getByTestId(TestIds.errors.videoLoadError).count()
)
.toBeGreaterThan(0)
})
})
5 changes: 1 addition & 4 deletions src/components/dialog/content/error/FindIssueButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ const props = defineProps<{

const queryString = computed(() => props.errorMessage + ' is:issue')

/**
* Open GitHub issues search and track telemetry.
*/
const openGitHubIssues = () => {
function openGitHubIssues() {
useTelemetry()?.trackUiButtonClicked({
button_id: 'error_dialog_find_existing_issues_clicked'
})
Expand Down
7 changes: 6 additions & 1 deletion src/components/error/ErrorOverlay.vue
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,12 @@
<Button variant="muted-textonly" size="unset" @click="dismiss">
{{ t('g.dismiss') }}
</Button>
<Button variant="secondary" size="lg" @click="seeErrors">
<Button
variant="secondary"
size="lg"
data-testid="error-overlay-see-errors"
@click="seeErrors"
>
{{
appMode ? t('linearMode.error.goto') : t('errorOverlay.seeErrors')
}}
Expand Down
16 changes: 14 additions & 2 deletions src/components/rightSidePanel/RightSidePanel.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ import TabList from '@/components/tab/TabList.vue'
import Button from '@/components/ui/button/Button.vue'
import { useGraphHierarchy } from '@/composables/graph/useGraphHierarchy'
import { st } from '@/i18n'
import { app } from '@/scripts/app'
import { getActiveGraphNodeIds } from '@/utils/graphTraversalUtil'
import { SubgraphNode } from '@/lib/litegraph/src/litegraph'
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
import { useSettingStore } from '@/platform/settings/settingStore'
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
import { useMissingModelStore } from '@/platform/missingModel/missingModelStore'
import { useExecutionErrorStore } from '@/stores/executionErrorStore'
import { useMissingNodesErrorStore } from '@/platform/nodeReplacement/missingNodesErrorStore'
import { useRightSidePanelStore } from '@/stores/workspace/rightSidePanelStore'
import type { RightSidePanelTab } from '@/stores/workspace/rightSidePanelStore'
import { resolveNodeDisplayName } from '@/utils/nodeTitleUtil'
Expand All @@ -38,12 +41,21 @@ import TabErrors from './errors/TabErrors.vue'
const canvasStore = useCanvasStore()
const executionErrorStore = useExecutionErrorStore()
const missingModelStore = useMissingModelStore()
const missingNodesErrorStore = useMissingNodesErrorStore()
const rightSidePanelStore = useRightSidePanelStore()
const settingStore = useSettingStore()
const { t } = useI18n()

const { hasAnyError, allErrorExecutionIds, activeMissingNodeGraphIds } =
storeToRefs(executionErrorStore)
const { hasAnyError, allErrorExecutionIds } = storeToRefs(executionErrorStore)

const activeMissingNodeGraphIds = computed<Set<string>>(() => {
if (!app.isGraphReady) return new Set()
return getActiveGraphNodeIds(
app.rootGraph,
canvasStore.currentGraph ?? app.rootGraph,
missingNodesErrorStore.missingAncestorExecutionIds
)
})

const { activeMissingModelGraphIds } = storeToRefs(missingModelStore)

Expand Down
5 changes: 5 additions & 0 deletions src/components/rightSidePanel/errors/ErrorNodeCard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@ describe('ErrorNodeCard.vue', () => {

// Report is still generated with fallback log message
expect(mockGenerateErrorReport).toHaveBeenCalledOnce()
expect(mockGenerateErrorReport).toHaveBeenCalledWith(
expect.objectContaining({
serverLogs: 'Failed to retrieve server logs'
})
)
expect(wrapper.text()).toContain('ComfyUI Error Report')
})

Expand Down
29 changes: 5 additions & 24 deletions src/components/rightSidePanel/errors/ErrorNodeCard.vue
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
variant="secondary"
size="sm"
class="h-8 w-2/3 justify-center gap-1 rounded-lg text-xs"
data-testid="error-card-find-on-github"
@click="handleCheckGithub(error)"
>
{{ t('g.findOnGithub') }}
Expand All @@ -99,6 +100,7 @@
variant="secondary"
size="sm"
class="h-8 w-1/3 justify-center gap-1 rounded-lg text-xs"
data-testid="error-card-copy"
@click="handleCopyError(idx)"
>
{{ t('g.copy') }}
Expand All @@ -125,12 +127,10 @@
import { useI18n } from 'vue-i18n'

import Button from '@/components/ui/button/Button.vue'
import { useExternalLink } from '@/composables/useExternalLink'
import { useTelemetry } from '@/platform/telemetry'
import { useCommandStore } from '@/stores/commandStore'
import { cn } from '@/utils/tailwindUtil'

import type { ErrorCardData, ErrorItem } from './types'
import { useErrorActions } from './useErrorActions'
import { useErrorReport } from './useErrorReport'

const {
Expand All @@ -154,10 +154,8 @@ const emit = defineEmits<{
}>()

const { t } = useI18n()
const telemetry = useTelemetry()
const { staticUrls } = useExternalLink()
const commandStore = useCommandStore()
const { displayedDetailsMap } = useErrorReport(() => card)
const { findOnGitHub, contactSupport: handleGetHelp } = useErrorActions()

function handleLocateNode() {
if (card.nodeId) {
Expand All @@ -178,23 +176,6 @@ function handleCopyError(idx: number) {
}

function handleCheckGithub(error: ErrorItem) {
telemetry?.trackUiButtonClicked({
button_id: 'error_tab_find_existing_issues_clicked'
})
const query = encodeURIComponent(error.message + ' is:issue')
window.open(
`${staticUrls.githubIssues}?q=${query}`,
'_blank',
'noopener,noreferrer'
)
}

function handleGetHelp() {
telemetry?.trackHelpResourceClicked({
resource_type: 'help_feedback',
is_external: true,
source: 'error_dialog'
})
commandStore.execute('Comfy.ContactSupport')
findOnGitHub(error.message)
}
</script>
16 changes: 8 additions & 8 deletions src/components/rightSidePanel/errors/MissingNodeCard.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { mount } from '@vue/test-utils'
import { createTestingPinia } from '@pinia/testing'
import PrimeVue from 'primevue/config'
import { ref } from 'vue'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { createI18n } from 'vue-i18n'

Expand Down Expand Up @@ -42,23 +40,25 @@ vi.mock('@/stores/systemStatsStore', () => ({
})
}))

const mockApplyChanges = vi.fn()
const mockIsRestarting = ref(false)
const mockApplyChanges = vi.hoisted(() => vi.fn())
const mockIsRestarting = vi.hoisted(() => ({ value: false }))
vi.mock('@/workbench/extensions/manager/composables/useApplyChanges', () => ({
useApplyChanges: () => ({
isRestarting: mockIsRestarting,
get isRestarting() {
return mockIsRestarting.value
},
applyChanges: mockApplyChanges
})
}))

const mockIsPackInstalled = vi.fn(() => false)
const mockIsPackInstalled = vi.hoisted(() => vi.fn(() => false))
vi.mock('@/workbench/extensions/manager/stores/comfyManagerStore', () => ({
useComfyManagerStore: () => ({
isPackInstalled: mockIsPackInstalled
})
}))

const mockShouldShowManagerButtons = { value: false }
const mockShouldShowManagerButtons = vi.hoisted(() => ({ value: false }))
vi.mock('@/workbench/extensions/manager/composables/useManagerState', () => ({
useManagerState: () => ({
shouldShowManagerButtons: mockShouldShowManagerButtons
Expand Down Expand Up @@ -128,7 +128,7 @@ function mountCard(
...props
},
global: {
plugins: [createTestingPinia({ createSpy: vi.fn }), PrimeVue, i18n],
plugins: [createTestingPinia({ createSpy: vi.fn }), i18n],
stubs: {
DotSpinner: { template: '<span role="status" aria-label="loading" />' }
}
Expand Down
13 changes: 7 additions & 6 deletions src/components/rightSidePanel/errors/TabErrors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,9 @@ describe('TabErrors.vue', () => {
}
})

// Find the copy button by text (rendered inside ErrorNodeCard)
const copyButton = wrapper
.findAll('button')
.find((btn) => btn.text().includes('Copy'))
expect(copyButton).toBeTruthy()
await copyButton!.trigger('click')
const copyButton = wrapper.find('[data-testid="error-card-copy"]')
expect(copyButton.exists()).toBe(true)
await copyButton.trigger('click')

expect(mockCopy).toHaveBeenCalledWith('Test message\n\nTest details')
})
Expand Down Expand Up @@ -245,5 +242,9 @@ describe('TabErrors.vue', () => {
// Should render in the dedicated runtime error panel, not inside accordion
const runtimePanel = wrapper.find('[data-testid="runtime-error-panel"]')
expect(runtimePanel.exists()).toBe(true)
// Verify the error message appears exactly once (not duplicated in accordion)
expect(
wrapper.text().match(/RuntimeError: Out of memory/g) ?? []
).toHaveLength(1)
})
})
Loading
Loading