diff --git a/browser_tests/assets/missing/missing_media_bypassed.json b/browser_tests/assets/missing/missing_media_bypassed.json new file mode 100644 index 0000000000..4bb7267409 --- /dev/null +++ b/browser_tests/assets/missing/missing_media_bypassed.json @@ -0,0 +1,34 @@ +{ + "last_node_id": 10, + "last_link_id": 0, + "nodes": [ + { + "id": 10, + "type": "LoadImage", + "pos": [50, 200], + "size": [315, 314], + "flags": {}, + "order": 0, + "mode": 4, + "inputs": [], + "outputs": [ + { "name": "IMAGE", "type": "IMAGE", "links": null }, + { "name": "MASK", "type": "MASK", "links": null } + ], + "properties": { + "Node name for S&R": "LoadImage" + }, + "widgets_values": ["nonexistent_test_image_12345.png", "image"] + } + ], + "links": [], + "groups": [], + "config": {}, + "extra": { + "ds": { + "offset": [0, 0], + "scale": 1 + } + }, + "version": 0.4 +} diff --git a/browser_tests/assets/missing/missing_media_multiple.json b/browser_tests/assets/missing/missing_media_multiple.json index 46499deb0a..9b0fb1a297 100644 --- a/browser_tests/assets/missing/missing_media_multiple.json +++ b/browser_tests/assets/missing/missing_media_multiple.json @@ -5,7 +5,7 @@ { "id": 10, "type": "LoadImage", - "pos": [50, 50], + "pos": [50, 200], "size": [315, 314], "flags": {}, "order": 0, @@ -31,7 +31,7 @@ { "id": 11, "type": "LoadImage", - "pos": [450, 50], + "pos": [450, 200], "size": [315, 314], "flags": {}, "order": 1, diff --git a/browser_tests/assets/missing/missing_media_single.json b/browser_tests/assets/missing/missing_media_single.json index 197b08a5fc..da91c64bd1 100644 --- a/browser_tests/assets/missing/missing_media_single.json +++ b/browser_tests/assets/missing/missing_media_single.json @@ -5,7 +5,7 @@ { "id": 10, "type": "LoadImage", - "pos": [50, 50], + "pos": [50, 200], "size": [315, 314], "flags": {}, "order": 0, diff --git a/browser_tests/assets/missing/missing_models.json b/browser_tests/assets/missing/missing_models.json index c4a8d5a0d4..eef6a66ab3 100644 --- a/browser_tests/assets/missing/missing_models.json +++ b/browser_tests/assets/missing/missing_models.json @@ -1,7 +1,27 @@ { - "last_node_id": 0, + "last_node_id": 1, "last_link_id": 0, - "nodes": [], + "nodes": [ + { + "id": 1, + "type": "CheckpointLoaderSimple", + "pos": [256, 256], + "size": [315, 98], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { "name": "MODEL", "type": "MODEL", "links": null }, + { "name": "CLIP", "type": "CLIP", "links": null }, + { "name": "VAE", "type": "VAE", "links": null } + ], + "properties": { + "Node name for S&R": "CheckpointLoaderSimple" + }, + "widgets_values": ["fake_model.safetensors"] + } + ], "links": [], "groups": [], "config": {}, @@ -15,7 +35,7 @@ { "name": "fake_model.safetensors", "url": "http://localhost:8188/api/devtools/fake_model.safetensors", - "directory": "text_encoders" + "directory": "checkpoints" } ], "version": 0.4 diff --git a/browser_tests/assets/missing/missing_models_bypassed.json b/browser_tests/assets/missing/missing_models_bypassed.json new file mode 100644 index 0000000000..8ade47403c --- /dev/null +++ b/browser_tests/assets/missing/missing_models_bypassed.json @@ -0,0 +1,42 @@ +{ + "last_node_id": 1, + "last_link_id": 0, + "nodes": [ + { + "id": 1, + "type": "CheckpointLoaderSimple", + "pos": [256, 256], + "size": [315, 98], + "flags": {}, + "order": 0, + "mode": 4, + "inputs": [], + "outputs": [ + { "name": "MODEL", "type": "MODEL", "links": null }, + { "name": "CLIP", "type": "CLIP", "links": null }, + { "name": "VAE", "type": "VAE", "links": null } + ], + "properties": { + "Node name for S&R": "CheckpointLoaderSimple" + }, + "widgets_values": ["fake_model.safetensors"] + } + ], + "links": [], + "groups": [], + "config": {}, + "extra": { + "ds": { + "scale": 1, + "offset": [0, 0] + } + }, + "models": [ + { + "name": "fake_model.safetensors", + "url": "http://localhost:8188/api/devtools/fake_model.safetensors", + "directory": "checkpoints" + } + ], + "version": 0.4 +} diff --git a/browser_tests/assets/missing/missing_models_in_subgraph.json b/browser_tests/assets/missing/missing_models_in_subgraph.json new file mode 100644 index 0000000000..613fa77be8 --- /dev/null +++ b/browser_tests/assets/missing/missing_models_in_subgraph.json @@ -0,0 +1,141 @@ +{ + "id": "test-missing-models-in-subgraph", + "revision": 0, + "last_node_id": 2, + "last_link_id": 0, + "nodes": [ + { + "id": 1, + "type": "KSampler", + "pos": [100, 100], + "size": [270, 262], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { "name": "model", "type": "MODEL", "link": null }, + { "name": "positive", "type": "CONDITIONING", "link": null }, + { "name": "negative", "type": "CONDITIONING", "link": null }, + { "name": "latent_image", "type": "LATENT", "link": null } + ], + "outputs": [{ "name": "LATENT", "type": "LATENT", "links": [] }], + "properties": { + "Node name for S&R": "KSampler" + }, + "widgets_values": [0, "randomize", 20, 8, "euler", "simple", 1] + }, + { + "id": 2, + "type": "subgraph-with-missing-model", + "pos": [450, 100], + "size": [400, 200], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [{ "name": "model", "type": "MODEL", "link": null }], + "outputs": [{ "name": "MODEL", "type": "MODEL", "links": null }], + "properties": {}, + "widgets_values": [] + } + ], + "links": [], + "groups": [], + "definitions": { + "subgraphs": [ + { + "id": "subgraph-with-missing-model", + "version": 1, + "state": { + "lastGroupId": 0, + "lastNodeId": 1, + "lastLinkId": 2, + "lastRerouteId": 0 + }, + "revision": 0, + "config": {}, + "name": "Subgraph with Missing Model", + "inputNode": { + "id": -10, + "bounding": [100, 200, 120, 60] + }, + "outputNode": { + "id": -20, + "bounding": [500, 200, 120, 60] + }, + "inputs": [ + { + "id": "input1-id", + "name": "model", + "type": "MODEL", + "linkIds": [1], + "pos": { "0": 150, "1": 220 } + } + ], + "outputs": [ + { + "id": "output1-id", + "name": "MODEL", + "type": "MODEL", + "linkIds": [2], + "pos": { "0": 520, "1": 220 } + } + ], + "widgets": [], + "nodes": [ + { + "id": 1, + "type": "CheckpointLoaderSimple", + "pos": [250, 180], + "size": [315, 98], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { "name": "MODEL", "type": "MODEL", "links": [2] }, + { "name": "CLIP", "type": "CLIP", "links": null }, + { "name": "VAE", "type": "VAE", "links": null } + ], + "properties": { + "Node name for S&R": "CheckpointLoaderSimple" + }, + "widgets_values": ["fake_model.safetensors"] + } + ], + "links": [ + { + "id": 1, + "origin_id": -10, + "origin_slot": 0, + "target_id": 1, + "target_slot": 0, + "type": "MODEL" + }, + { + "id": 2, + "origin_id": 1, + "origin_slot": 0, + "target_id": -20, + "target_slot": 0, + "type": "MODEL" + } + ] + } + ] + }, + "config": {}, + "extra": { + "ds": { + "scale": 1, + "offset": [0, 0] + } + }, + "models": [ + { + "name": "fake_model.safetensors", + "url": "http://localhost:8188/api/devtools/fake_model.safetensors", + "directory": "checkpoints" + } + ], + "version": 0.4 +} diff --git a/browser_tests/assets/missing/missing_models_with_nodes.json b/browser_tests/assets/missing/missing_models_with_nodes.json index 9e24271bab..3f2a8de9d0 100644 --- a/browser_tests/assets/missing/missing_models_with_nodes.json +++ b/browser_tests/assets/missing/missing_models_with_nodes.json @@ -78,7 +78,7 @@ { "name": "fake_model.safetensors", "url": "http://localhost:8188/api/devtools/fake_model.safetensors", - "directory": "text_encoders" + "directory": "checkpoints" } ], "version": 0.4 diff --git a/browser_tests/fixtures/selectors.ts b/browser_tests/fixtures/selectors.ts index b5e92cd34c..3c3c662c46 100644 --- a/browser_tests/fixtures/selectors.ts +++ b/browser_tests/fixtures/selectors.ts @@ -83,7 +83,8 @@ export const TestIds = { bookmarksSection: 'node-library-bookmarks-section' }, propertiesPanel: { - root: 'properties-panel' + root: 'properties-panel', + errorsTab: 'panel-tab-errors' }, subgraphEditor: { toggle: 'subgraph-editor-toggle', diff --git a/browser_tests/tests/errorOverlay.spec.ts b/browser_tests/tests/errorOverlay.spec.ts index 75e315bb95..e998f1093a 100644 --- a/browser_tests/tests/errorOverlay.spec.ts +++ b/browser_tests/tests/errorOverlay.spec.ts @@ -5,6 +5,7 @@ import { comfyExpect as expect } from '@e2e/fixtures/ComfyPage' import { TestIds } from '@e2e/fixtures/selectors' +import { cleanupFakeModel } from '@e2e/tests/propertiesPanel/ErrorsTabHelper' test.describe('Error overlay', { tag: '@ui' }, () => { test.beforeEach(async ({ comfyPage }) => { @@ -47,16 +48,7 @@ test.describe('Error overlay', { tag: '@ui' }, () => { test('Should display "Show missing models" button for missing model errors', async ({ comfyPage }) => { - await expect - .poll(() => - comfyPage.page.evaluate(async (url: string) => { - const response = await fetch( - `${url}/api/devtools/cleanup_fake_model` - ) - return response.ok - }, comfyPage.url) - ) - .toBeTruthy() + await cleanupFakeModel(comfyPage) await comfyPage.workflow.loadWorkflow('missing/missing_models') @@ -117,6 +109,33 @@ test.describe('Error overlay', { tag: '@ui' }, () => { await comfyPage.keyboard.redo() await expect(errorOverlay).toBeHidden() }) + + test('Does not resurface error overlay when switching back to workflow with missing nodes', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting( + 'Comfy.Workflow.WorkflowTabsPosition', + 'Sidebar' + ) + await comfyPage.menu.workflowsTab.open() + + await comfyPage.workflow.loadWorkflow('missing/missing_nodes') + + const errorOverlay = getOverlay(comfyPage.page) + await expect(errorOverlay).toBeVisible() + + await errorOverlay + .getByTestId(TestIds.dialogs.errorOverlayDismiss) + .click() + await expect(errorOverlay).toBeHidden() + + await comfyPage.menu.workflowsTab.open() + await comfyPage.command.executeCommand('Comfy.NewBlankWorkflow') + + await comfyPage.menu.workflowsTab.switchToWorkflow('missing_nodes') + + await expect(errorOverlay).toBeHidden() + }) }) test.describe('See Errors flow', () => { diff --git a/browser_tests/tests/propertiesPanel/ErrorsTabHelper.ts b/browser_tests/tests/propertiesPanel/ErrorsTabHelper.ts index 7e67c6b8e0..0e76d2d7e2 100644 --- a/browser_tests/tests/propertiesPanel/ErrorsTabHelper.ts +++ b/browser_tests/tests/propertiesPanel/ErrorsTabHelper.ts @@ -2,8 +2,9 @@ import { expect } from '@playwright/test' import type { ComfyPage } from '@e2e/fixtures/ComfyPage' import { TestIds } from '@e2e/fixtures/selectors' +import { PropertiesPanelHelper } from '@e2e/tests/propertiesPanel/PropertiesPanelHelper' -export async function openErrorsTabViaSeeErrors( +export async function loadWorkflowAndOpenErrorsTab( comfyPage: ComfyPage, workflow: string ) { @@ -15,3 +16,30 @@ export async function openErrorsTabViaSeeErrors( await errorOverlay.getByTestId(TestIds.dialogs.errorOverlaySeeErrors).click() await expect(errorOverlay).toBeHidden() } + +export async function openErrorsTab(comfyPage: ComfyPage) { + const panel = new PropertiesPanelHelper(comfyPage.page) + await panel.open(comfyPage.actionbar.propertiesButton) + + const errorsTab = comfyPage.page.getByTestId( + TestIds.propertiesPanel.errorsTab + ) + await expect(errorsTab).toBeVisible() + await errorsTab.click() +} + +/** + * Remove the fake model file from the backend so it is detected as missing. + * Fixture URLs (e.g. http://localhost:8188/...) are not actually downloaded + * during tests — they only serve as metadata for the missing model UI. + */ +export async function cleanupFakeModel(comfyPage: ComfyPage) { + await expect + .poll(() => + comfyPage.page.evaluate(async (url: string) => { + const response = await fetch(`${url}/api/devtools/cleanup_fake_model`) + return response.ok + }, comfyPage.url) + ) + .toBeTruthy() +} diff --git a/browser_tests/tests/propertiesPanel/errorsTabMissingMedia.spec.ts b/browser_tests/tests/propertiesPanel/errorsTabMissingMedia.spec.ts index 5d367395ef..bf204133ba 100644 --- a/browser_tests/tests/propertiesPanel/errorsTabMissingMedia.spec.ts +++ b/browser_tests/tests/propertiesPanel/errorsTabMissingMedia.spec.ts @@ -3,7 +3,7 @@ import { expect } from '@playwright/test' import type { ComfyPage } from '@e2e/fixtures/ComfyPage' import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage' import { TestIds } from '@e2e/fixtures/selectors' -import { openErrorsTabViaSeeErrors } from '@e2e/tests/propertiesPanel/ErrorsTabHelper' +import { loadWorkflowAndOpenErrorsTab } from '@e2e/tests/propertiesPanel/ErrorsTabHelper' async function uploadFileViaDropzone(comfyPage: ComfyPage) { const dropzone = comfyPage.page.getByTestId( @@ -47,7 +47,10 @@ test.describe('Errors tab - Missing media', { tag: '@ui' }, () => { test.describe('Detection', () => { test('Shows missing media group in errors tab', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_media_single') + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/missing_media_single' + ) await expect( comfyPage.page.getByTestId(TestIds.dialogs.missingMediaGroup) @@ -57,7 +60,7 @@ test.describe('Errors tab - Missing media', { tag: '@ui' }, () => { test('Shows correct number of missing media rows', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors( + await loadWorkflowAndOpenErrorsTab( comfyPage, 'missing/missing_media_multiple' ) @@ -68,7 +71,10 @@ test.describe('Errors tab - Missing media', { tag: '@ui' }, () => { test('Shows upload dropzone and library select for each missing item', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_media_single') + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/missing_media_single' + ) await expect(getDropzone(comfyPage)).toBeVisible() await expect( @@ -81,7 +87,10 @@ test.describe('Errors tab - Missing media', { tag: '@ui' }, () => { test('Upload via file picker shows status card then allows confirm', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_media_single') + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/missing_media_single' + ) await uploadFileViaDropzone(comfyPage) await expect(getStatusCard(comfyPage)).toBeVisible() @@ -95,7 +104,10 @@ test.describe('Errors tab - Missing media', { tag: '@ui' }, () => { test('Selecting from library shows status card then allows confirm', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_media_single') + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/missing_media_single' + ) const librarySelect = comfyPage.page.getByTestId( TestIds.dialogs.missingMediaLibrarySelect @@ -122,7 +134,10 @@ test.describe('Errors tab - Missing media', { tag: '@ui' }, () => { test('Cancelling pending selection returns to upload/library UI', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_media_single') + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/missing_media_single' + ) await uploadFileViaDropzone(comfyPage) await expect(getStatusCard(comfyPage)).toBeVisible() @@ -141,7 +156,10 @@ test.describe('Errors tab - Missing media', { tag: '@ui' }, () => { test('Missing Inputs group disappears when all items are resolved', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_media_single') + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/missing_media_single' + ) await uploadFileViaDropzone(comfyPage) await confirmPendingSelection(comfyPage) @@ -155,7 +173,10 @@ test.describe('Errors tab - Missing media', { tag: '@ui' }, () => { test('Locate button navigates canvas to the missing media node', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_media_single') + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/missing_media_single' + ) const offsetBefore = await comfyPage.page.evaluate(() => { const canvas = window['app']?.canvas diff --git a/browser_tests/tests/propertiesPanel/errorsTabMissingModels.spec.ts b/browser_tests/tests/propertiesPanel/errorsTabMissingModels.spec.ts index cf3d06a9fc..c0e4a1f87d 100644 --- a/browser_tests/tests/propertiesPanel/errorsTabMissingModels.spec.ts +++ b/browser_tests/tests/propertiesPanel/errorsTabMissingModels.spec.ts @@ -6,7 +6,10 @@ import { interceptClipboardWrite, getClipboardText } from '@e2e/helpers/clipboardSpy' -import { openErrorsTabViaSeeErrors } from '@e2e/tests/propertiesPanel/ErrorsTabHelper' +import { + cleanupFakeModel, + loadWorkflowAndOpenErrorsTab +} from '@e2e/tests/propertiesPanel/ErrorsTabHelper' test.describe('Errors tab - Missing models', { tag: '@ui' }, () => { test.beforeEach(async ({ comfyPage }) => { @@ -15,20 +18,13 @@ test.describe('Errors tab - Missing models', { tag: '@ui' }, () => { 'Comfy.RightSidePanel.ShowErrorsTab', true ) - await expect - .poll(async () => { - return await comfyPage.page.evaluate(async (url: string) => { - const response = await fetch(`${url}/api/devtools/cleanup_fake_model`) - return response.ok - }, comfyPage.url) - }) - .toBeTruthy() + await cleanupFakeModel(comfyPage) }) test('Should show missing models group in errors tab', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_models') + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models') await expect( comfyPage.page.getByTestId(TestIds.dialogs.missingModelsGroup) @@ -38,7 +34,7 @@ test.describe('Errors tab - Missing models', { tag: '@ui' }, () => { test('Should display model name with referencing node count', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_models') + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models') const modelsGroup = comfyPage.page.getByTestId( TestIds.dialogs.missingModelsGroup @@ -49,7 +45,7 @@ test.describe('Errors tab - Missing models', { tag: '@ui' }, () => { test('Should expand model row to show referencing nodes', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors( + await loadWorkflowAndOpenErrorsTab( comfyPage, 'missing/missing_models_with_nodes' ) @@ -69,14 +65,14 @@ test.describe('Errors tab - Missing models', { tag: '@ui' }, () => { }) test('Should copy model name to clipboard', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_models') + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models') await interceptClipboardWrite(comfyPage.page) const copyButton = comfyPage.page.getByTestId( TestIds.dialogs.missingModelCopyName ) await expect(copyButton.first()).toBeVisible() - await copyButton.first().click() + await copyButton.first().dispatchEvent('click') const copiedText = await getClipboardText(comfyPage.page) expect(copiedText).toContain('fake_model.safetensors') @@ -86,7 +82,7 @@ test.describe('Errors tab - Missing models', { tag: '@ui' }, () => { test('Should show Copy URL button for non-asset models', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_models') + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models') const copyUrlButton = comfyPage.page.getByTestId( TestIds.dialogs.missingModelCopyUrl @@ -97,7 +93,7 @@ test.describe('Errors tab - Missing models', { tag: '@ui' }, () => { test('Should show Download button for downloadable models', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_models') + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models') const downloadButton = comfyPage.page.getByTestId( TestIds.dialogs.missingModelDownload diff --git a/browser_tests/tests/propertiesPanel/errorsTabMissingNodes.spec.ts b/browser_tests/tests/propertiesPanel/errorsTabMissingNodes.spec.ts index eecbce2223..77370c327f 100644 --- a/browser_tests/tests/propertiesPanel/errorsTabMissingNodes.spec.ts +++ b/browser_tests/tests/propertiesPanel/errorsTabMissingNodes.spec.ts @@ -2,7 +2,7 @@ import { expect } from '@playwright/test' import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage' import { TestIds } from '@e2e/fixtures/selectors' -import { openErrorsTabViaSeeErrors } from '@e2e/tests/propertiesPanel/ErrorsTabHelper' +import { loadWorkflowAndOpenErrorsTab } from '@e2e/tests/propertiesPanel/ErrorsTabHelper' test.describe('Errors tab - Missing nodes', { tag: '@ui' }, () => { test.beforeEach(async ({ comfyPage }) => { @@ -14,7 +14,7 @@ test.describe('Errors tab - Missing nodes', { tag: '@ui' }, () => { }) test('Should show MissingNodeCard in errors tab', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_nodes') + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_nodes') await expect( comfyPage.page.getByTestId(TestIds.dialogs.missingNodeCard) @@ -22,7 +22,7 @@ test.describe('Errors tab - Missing nodes', { tag: '@ui' }, () => { }) test('Should show missing node packs group', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors(comfyPage, 'missing/missing_nodes') + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_nodes') await expect( comfyPage.page.getByTestId(TestIds.dialogs.missingNodePacksGroup) @@ -32,7 +32,7 @@ test.describe('Errors tab - Missing nodes', { tag: '@ui' }, () => { test('Should expand pack group to reveal node type names', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors( + await loadWorkflowAndOpenErrorsTab( comfyPage, 'missing/missing_nodes_in_subgraph' ) @@ -52,7 +52,7 @@ test.describe('Errors tab - Missing nodes', { tag: '@ui' }, () => { }) test('Should collapse expanded pack group', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors( + await loadWorkflowAndOpenErrorsTab( comfyPage, 'missing/missing_nodes_in_subgraph' ) @@ -80,7 +80,7 @@ test.describe('Errors tab - Missing nodes', { tag: '@ui' }, () => { test('Locate node button is visible for expanded pack nodes', async ({ comfyPage }) => { - await openErrorsTabViaSeeErrors( + await loadWorkflowAndOpenErrorsTab( comfyPage, 'missing/missing_nodes_in_subgraph' ) diff --git a/browser_tests/tests/propertiesPanel/errorsTabModeAware.spec.ts b/browser_tests/tests/propertiesPanel/errorsTabModeAware.spec.ts new file mode 100644 index 0000000000..4e777f550d --- /dev/null +++ b/browser_tests/tests/propertiesPanel/errorsTabModeAware.spec.ts @@ -0,0 +1,519 @@ +import { expect } from '@playwright/test' + +import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage' +import { TestIds } from '@e2e/fixtures/selectors' +import { + cleanupFakeModel, + openErrorsTab, + loadWorkflowAndOpenErrorsTab +} from '@e2e/tests/propertiesPanel/ErrorsTabHelper' + +test.describe('Errors tab - Mode-aware errors', { tag: '@ui' }, () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top') + await comfyPage.settings.setSetting( + 'Comfy.RightSidePanel.ShowErrorsTab', + true + ) + }) + + test.describe('Missing nodes', () => { + test('Deleting a missing node removes its error from the errors tab', async ({ + comfyPage + }) => { + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_nodes') + + const missingNodeGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingNodePacksGroup + ) + await expect(missingNodeGroup).toBeVisible() + + const node = await comfyPage.nodeOps.getNodeRefById('1') + await node.delete() + + await expect(missingNodeGroup).toBeHidden() + }) + + test('Undo after bypass restores error without showing overlay', async ({ + comfyPage + }) => { + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_nodes') + + const missingNodeGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingNodePacksGroup + ) + const errorOverlay = comfyPage.page.getByTestId( + TestIds.dialogs.errorOverlay + ) + await expect(missingNodeGroup).toBeVisible() + + const node = await comfyPage.nodeOps.getNodeRefById('1') + await node.click('title') + await comfyPage.keyboard.bypass() + await expect.poll(() => node.isBypassed()).toBeTruthy() + await expect(missingNodeGroup).toBeHidden() + + await comfyPage.keyboard.undo() + await expect.poll(() => node.isBypassed()).toBeFalsy() + await expect(errorOverlay).toBeHidden() + await openErrorsTab(comfyPage) + await expect(missingNodeGroup).toBeVisible() + + await comfyPage.keyboard.redo() + await expect.poll(() => node.isBypassed()).toBeTruthy() + await expect(missingNodeGroup).toBeHidden() + }) + }) + + test.describe('Missing models', () => { + test.beforeEach(async ({ comfyPage }) => { + await cleanupFakeModel(comfyPage) + }) + + test.afterEach(async ({ comfyPage }) => { + await cleanupFakeModel(comfyPage) + }) + + test('Loading a workflow with all nodes bypassed shows no errors', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow('missing/missing_models_bypassed') + + const errorOverlay = comfyPage.page.getByTestId( + TestIds.dialogs.errorOverlay + ) + await expect(errorOverlay).toBeHidden() + + await comfyPage.actionbar.propertiesButton.click() + await expect( + comfyPage.page.getByTestId(TestIds.propertiesPanel.errorsTab) + ).toBeHidden() + }) + + test('Bypassing a node hides its error, un-bypassing restores it', async ({ + comfyPage + }) => { + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models') + + const missingModelGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingModelsGroup + ) + await expect(missingModelGroup).toBeVisible() + + const node = await comfyPage.nodeOps.getNodeRefById('1') + await node.click('title') + await comfyPage.keyboard.bypass() + await expect.poll(() => node.isBypassed()).toBeTruthy() + await expect(missingModelGroup).toBeHidden() + + await node.click('title') + await comfyPage.keyboard.bypass() + await expect.poll(() => node.isBypassed()).toBeFalsy() + await openErrorsTab(comfyPage) + await expect(missingModelGroup).toBeVisible() + }) + + test('Pasting a node with missing model increases referencing node count', async ({ + comfyPage + }) => { + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models') + + const missingModelGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingModelsGroup + ) + await expect(missingModelGroup).toBeVisible() + await expect(missingModelGroup).toContainText( + /fake_model\.safetensors\s*\(1\)/ + ) + + const node = await comfyPage.nodeOps.getNodeRefById('1') + await node.click('title') + await comfyPage.clipboard.copy() + await comfyPage.clipboard.paste() + + await expect.poll(() => comfyPage.nodeOps.getNodeCount()).toBe(2) + + await comfyPage.canvas.click() + await expect(missingModelGroup).toContainText( + /fake_model\.safetensors\s*\(2\)/ + ) + }) + + test('Pasting a bypassed node does not add a new error', async ({ + comfyPage + }) => { + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models') + + const missingModelGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingModelsGroup + ) + + const node = await comfyPage.nodeOps.getNodeRefById('1') + await node.click('title') + await comfyPage.keyboard.bypass() + await expect.poll(() => node.isBypassed()).toBeTruthy() + await expect(missingModelGroup).toBeHidden() + + await comfyPage.clipboard.copy() + await comfyPage.clipboard.paste() + + await expect.poll(() => comfyPage.nodeOps.getNodeCount()).toBe(2) + await expect(missingModelGroup).toBeHidden() + }) + + test('Deleting a node with missing model removes its error', async ({ + comfyPage + }) => { + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models') + + const missingModelGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingModelsGroup + ) + await expect(missingModelGroup).toBeVisible() + + const node = await comfyPage.nodeOps.getNodeRefById('1') + await node.delete() + + await expect(missingModelGroup).toBeHidden() + }) + + test('Undo after bypass restores error without showing overlay', async ({ + comfyPage + }) => { + await loadWorkflowAndOpenErrorsTab(comfyPage, 'missing/missing_models') + + const missingModelGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingModelsGroup + ) + const errorOverlay = comfyPage.page.getByTestId( + TestIds.dialogs.errorOverlay + ) + await expect(missingModelGroup).toBeVisible() + + const node = await comfyPage.nodeOps.getNodeRefById('1') + await node.click('title') + await comfyPage.keyboard.bypass() + await expect.poll(() => node.isBypassed()).toBeTruthy() + await expect(missingModelGroup).toBeHidden() + + await comfyPage.keyboard.undo() + await expect.poll(() => node.isBypassed()).toBeFalsy() + await expect(errorOverlay).toBeHidden() + await openErrorsTab(comfyPage) + await expect(missingModelGroup).toBeVisible() + + await comfyPage.keyboard.redo() + await expect.poll(() => node.isBypassed()).toBeTruthy() + await expect(missingModelGroup).toBeHidden() + }) + + test('Selecting a node filters errors tab to only that node', async ({ + comfyPage + }) => { + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/missing_models_with_nodes' + ) + + const missingModelGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingModelsGroup + ) + await expect(missingModelGroup).toContainText(/\(2\)/) + + const node1 = await comfyPage.nodeOps.getNodeRefById('1') + await node1.click('title') + await expect(missingModelGroup).toContainText(/\(1\)/) + + await comfyPage.canvas.click() + await expect(missingModelGroup).toContainText(/\(2\)/) + }) + }) + + test.describe('Missing media', () => { + test('Loading a workflow with all nodes bypassed shows no errors', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow('missing/missing_media_bypassed') + + const errorOverlay = comfyPage.page.getByTestId( + TestIds.dialogs.errorOverlay + ) + await expect(errorOverlay).toBeHidden() + + await comfyPage.actionbar.propertiesButton.click() + await expect( + comfyPage.page.getByTestId(TestIds.propertiesPanel.errorsTab) + ).toBeHidden() + }) + + test('Bypassing a node hides its error, un-bypassing restores it', async ({ + comfyPage + }) => { + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/missing_media_single' + ) + + const missingMediaGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingMediaGroup + ) + await expect(missingMediaGroup).toBeVisible() + + const node = await comfyPage.nodeOps.getNodeRefById('10') + await node.click('title') + await comfyPage.keyboard.bypass() + await expect.poll(() => node.isBypassed()).toBeTruthy() + await expect(missingMediaGroup).toBeHidden() + + await node.click('title') + await comfyPage.keyboard.bypass() + await expect.poll(() => node.isBypassed()).toBeFalsy() + await openErrorsTab(comfyPage) + await expect(missingMediaGroup).toBeVisible() + }) + + test('Pasting a bypassed node does not add a new error', async ({ + comfyPage + }) => { + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/missing_media_single' + ) + + const missingMediaGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingMediaGroup + ) + + const node = await comfyPage.nodeOps.getNodeRefById('10') + await node.click('title') + await comfyPage.keyboard.bypass() + await expect.poll(() => node.isBypassed()).toBeTruthy() + await expect(missingMediaGroup).toBeHidden() + + await comfyPage.clipboard.copy() + await comfyPage.clipboard.paste() + + await expect.poll(() => comfyPage.nodeOps.getNodeCount()).toBe(2) + await expect(missingMediaGroup).toBeHidden() + }) + + test('Selecting a node filters errors tab to only that node', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow('missing/missing_media_multiple') + + const errorOverlay = comfyPage.page.getByTestId( + TestIds.dialogs.errorOverlay + ) + await expect(errorOverlay).toBeVisible() + await errorOverlay + .getByTestId(TestIds.dialogs.errorOverlayDismiss) + .click() + + const mediaRows = comfyPage.page.getByTestId( + TestIds.dialogs.missingMediaRow + ) + + await openErrorsTab(comfyPage) + await expect(mediaRows).toHaveCount(2) + + const node = await comfyPage.nodeOps.getNodeRefById('10') + await node.click('title') + await expect(mediaRows).toHaveCount(1) + + await comfyPage.canvas.click({ position: { x: 400, y: 600 } }) + await expect(mediaRows).toHaveCount(2) + }) + }) + + test.describe('Subgraph', () => { + test.beforeEach(async ({ comfyPage }) => { + await cleanupFakeModel(comfyPage) + }) + + test.afterEach(async ({ comfyPage }) => { + await cleanupFakeModel(comfyPage) + }) + + test('Bypassing a subgraph hides interior errors, un-bypassing restores them', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow( + 'missing/missing_models_in_subgraph' + ) + + const errorOverlay = comfyPage.page.getByTestId( + TestIds.dialogs.errorOverlay + ) + await expect(errorOverlay).toBeVisible() + await errorOverlay + .getByTestId(TestIds.dialogs.errorOverlayDismiss) + .click() + + const missingModelGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingModelsGroup + ) + + const subgraphNode = await comfyPage.nodeOps.getNodeRefById('2') + const errorsTab = comfyPage.page.getByTestId( + TestIds.propertiesPanel.errorsTab + ) + + await comfyPage.keyboard.selectAll() + await comfyPage.keyboard.bypass() + await expect.poll(() => subgraphNode.isBypassed()).toBeTruthy() + + await comfyPage.actionbar.propertiesButton.click() + await expect(errorsTab).toBeHidden() + + await comfyPage.keyboard.selectAll() + await comfyPage.keyboard.bypass() + await expect.poll(() => subgraphNode.isBypassed()).toBeFalsy() + await openErrorsTab(comfyPage) + await expect(missingModelGroup).toBeVisible() + }) + + test('Deleting a node inside a subgraph removes its missing model error', async ({ + comfyPage + }) => { + // Regression: before the execId fix, onNodeRemoved fell back to the + // interior node's local id (e.g. "1") when node.graph was already + // null, so the error keyed under "2:1" was never removed. + await comfyPage.workflow.loadWorkflow( + 'missing/missing_models_in_subgraph' + ) + + const errorOverlay = comfyPage.page.getByTestId( + TestIds.dialogs.errorOverlay + ) + await expect(errorOverlay).toBeVisible() + await errorOverlay + .getByTestId(TestIds.dialogs.errorOverlayDismiss) + .click() + + const missingModelGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingModelsGroup + ) + await openErrorsTab(comfyPage) + await expect(missingModelGroup).toBeVisible() + + const subgraphNode = await comfyPage.nodeOps.getNodeRefById('2') + await subgraphNode.navigateIntoSubgraph() + + // Select-all + Delete: interior node IDs may be reassigned during + // subgraph configure when they collide with root-graph IDs, so + // looking up by static id can fail. + await comfyPage.keyboard.selectAll() + await comfyPage.page.keyboard.press('Delete') + + await expect(missingModelGroup).toBeHidden() + }) + + test('Deleting a node inside a subgraph removes its missing node-type error', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow('missing/missing_nodes_in_subgraph') + + const errorOverlay = comfyPage.page.getByTestId( + TestIds.dialogs.errorOverlay + ) + await expect(errorOverlay).toBeVisible() + await errorOverlay + .getByTestId(TestIds.dialogs.errorOverlayDismiss) + .click() + + const missingNodeGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingNodePacksGroup + ) + await openErrorsTab(comfyPage) + await expect(missingNodeGroup).toBeVisible() + + const subgraphNode = await comfyPage.nodeOps.getNodeRefById('2') + await subgraphNode.navigateIntoSubgraph() + + // Select-all + Delete: interior node IDs may be reassigned during + // subgraph configure when they collide with root-graph IDs, so + // looking up by static id can fail. + await comfyPage.keyboard.selectAll() + await comfyPage.page.keyboard.press('Delete') + + await expect(missingNodeGroup).toBeHidden() + }) + + test('Bypassing a node inside a subgraph hides its error, un-bypassing restores it', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow( + 'missing/missing_models_in_subgraph' + ) + + const errorOverlay = comfyPage.page.getByTestId( + TestIds.dialogs.errorOverlay + ) + await expect(errorOverlay).toBeVisible() + await errorOverlay + .getByTestId(TestIds.dialogs.errorOverlayDismiss) + .click() + + const missingModelGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingModelsGroup + ) + + const subgraphNode = await comfyPage.nodeOps.getNodeRefById('2') + await subgraphNode.navigateIntoSubgraph() + + await comfyPage.keyboard.selectAll() + await comfyPage.keyboard.bypass() + + const errorsTab = comfyPage.page.getByTestId( + TestIds.propertiesPanel.errorsTab + ) + await comfyPage.actionbar.propertiesButton.click() + await expect(errorsTab).toBeHidden() + + await comfyPage.keyboard.selectAll() + await comfyPage.keyboard.bypass() + await openErrorsTab(comfyPage) + await expect(missingModelGroup).toBeVisible() + }) + }) + + test.describe('Workflow switching', () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.settings.setSetting( + 'Comfy.Workflow.WorkflowTabsPosition', + 'Sidebar' + ) + await comfyPage.menu.workflowsTab.open() + }) + + test('Restores missing nodes in errors tab when switching back to workflow', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow('missing/missing_nodes') + + const errorOverlay = comfyPage.page.getByTestId( + TestIds.dialogs.errorOverlay + ) + await expect(errorOverlay).toBeVisible() + await errorOverlay + .getByTestId(TestIds.dialogs.errorOverlayDismiss) + .click() + + const missingNodeGroup = comfyPage.page.getByTestId( + TestIds.dialogs.missingNodePacksGroup + ) + + await openErrorsTab(comfyPage) + await expect(missingNodeGroup).toBeVisible() + + await comfyPage.menu.workflowsTab.open() + await comfyPage.command.executeCommand('Comfy.NewBlankWorkflow') + await expect(missingNodeGroup).toBeHidden() + + await comfyPage.menu.workflowsTab.switchToWorkflow('missing_nodes') + await openErrorsTab(comfyPage) + await expect(missingNodeGroup).toBeVisible() + }) + }) +}) diff --git a/browser_tests/tests/sidebar/workflows.spec.ts b/browser_tests/tests/sidebar/workflows.spec.ts index 603de6f904..f4df6fb65b 100644 --- a/browser_tests/tests/sidebar/workflows.spec.ts +++ b/browser_tests/tests/sidebar/workflows.spec.ts @@ -2,6 +2,7 @@ import { expect } from '@playwright/test' import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage' import { TestIds } from '@e2e/fixtures/selectors' +import { openErrorsTab } from '@e2e/tests/propertiesPanel/ErrorsTabHelper' test.describe('Workflows sidebar', () => { test.beforeEach(async ({ comfyPage }) => { @@ -249,7 +250,7 @@ test.describe('Workflows sidebar', () => { .toEqual('workflow1') }) - test('Reports missing nodes warning again when switching back to workflow', async ({ + test('Restores missing nodes errors silently when switching back to workflow', async ({ comfyPage }) => { await comfyPage.settings.setSetting( @@ -271,11 +272,17 @@ test.describe('Workflows sidebar', () => { await comfyPage.menu.workflowsTab.open() await comfyPage.command.executeCommand('Comfy.NewBlankWorkflow') - // Switch back to the missing_nodes workflow — overlay should reappear - // so users can install missing node packs without a page reload + // Switch back to the missing_nodes workflow — overlay should NOT + // reappear (silent restore), but errors tab should have content await comfyPage.menu.workflowsTab.switchToWorkflow('missing_nodes') - await expect(errorOverlay).toBeVisible() + await expect(errorOverlay).toBeHidden() + + // Errors tab should still show missing nodes after silent restore + await openErrorsTab(comfyPage) + await expect( + comfyPage.page.getByTestId(TestIds.dialogs.missingNodePacksGroup) + ).toBeVisible() }) test('Can close saved-workflows from the open workflows section', async ({ diff --git a/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts-snapshots/vue-node-bypassed-state-chromium-linux.png b/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts-snapshots/vue-node-bypassed-state-chromium-linux.png index 3873f31172..6a5eaccd0c 100644 Binary files a/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts-snapshots/vue-node-bypassed-state-chromium-linux.png and b/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts-snapshots/vue-node-bypassed-state-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/nodeStates/mute.spec.ts-snapshots/vue-node-muted-state-chromium-linux.png b/browser_tests/tests/vueNodes/nodeStates/mute.spec.ts-snapshots/vue-node-muted-state-chromium-linux.png index 73d4ec760a..45f8d4c46b 100644 Binary files a/browser_tests/tests/vueNodes/nodeStates/mute.spec.ts-snapshots/vue-node-muted-state-chromium-linux.png and b/browser_tests/tests/vueNodes/nodeStates/mute.spec.ts-snapshots/vue-node-muted-state-chromium-linux.png differ diff --git a/src/components/rightSidePanel/RightSidePanel.vue b/src/components/rightSidePanel/RightSidePanel.vue index 24bba7e6f3..7b562aa920 100644 --- a/src/components/rightSidePanel/RightSidePanel.vue +++ b/src/components/rightSidePanel/RightSidePanel.vue @@ -16,6 +16,7 @@ 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 { useMissingMediaStore } from '@/platform/missingMedia/missingMediaStore' import { useExecutionErrorStore } from '@/stores/executionErrorStore' import { useMissingNodesErrorStore } from '@/platform/nodeReplacement/missingNodesErrorStore' import { useRightSidePanelStore } from '@/stores/workspace/rightSidePanelStore' @@ -41,6 +42,7 @@ import TabErrors from './errors/TabErrors.vue' const canvasStore = useCanvasStore() const executionErrorStore = useExecutionErrorStore() const missingModelStore = useMissingModelStore() +const missingMediaStore = useMissingMediaStore() const missingNodesErrorStore = useMissingNodesErrorStore() const rightSidePanelStore = useRightSidePanelStore() const settingStore = useSettingStore() @@ -58,6 +60,7 @@ const activeMissingNodeGraphIds = computed>(() => { }) const { activeMissingModelGraphIds } = storeToRefs(missingModelStore) +const { activeMissingMediaGraphIds } = storeToRefs(missingMediaStore) const { findParentGroup } = useGraphHierarchy() @@ -142,13 +145,22 @@ const hasMissingModelSelected = computed( ) ) +const hasMissingMediaSelected = computed( + () => + hasSelection.value && + selectedNodes.value.some((node) => + activeMissingMediaGraphIds.value.has(String(node.id)) + ) +) + const hasRelevantErrors = computed(() => { if (!hasSelection.value) return hasAnyError.value return ( hasDirectNodeError.value || hasContainerInternalError.value || hasMissingNodeSelected.value || - hasMissingModelSelected.value + hasMissingModelSelected.value || + hasMissingMediaSelected.value ) }) @@ -287,11 +299,14 @@ function handleTitleCancel() { @cancel="handleTitleCancel" @click="isEditing = true" /> - + > +