diff --git a/browser_tests/changeTracker.spec.ts b/browser_tests/changeTracker.spec.ts index 087016d3b0..abaa48f4a7 100644 --- a/browser_tests/changeTracker.spec.ts +++ b/browser_tests/changeTracker.spec.ts @@ -3,6 +3,9 @@ import { comfyPageFixture as test, comfyExpect as expect } from './fixtures/ComfyPage' +import type { useWorkspaceStore } from '../src/stores/workspaceStore' + +type WorkspaceStore = ReturnType async function beforeChange(comfyPage: ComfyPage) { await comfyPage.page.evaluate(() => { @@ -19,38 +22,40 @@ test.describe('Change Tracker', () => { test.describe('Undo/Redo', () => { test.beforeEach(async ({ comfyPage }) => { await comfyPage.setSetting('Comfy.UseNewMenu', 'Top') + await comfyPage.setupWorkflowsDirectory({}) }) test('Can undo multiple operations', async ({ comfyPage }) => { function isModified() { return comfyPage.page.evaluate(async () => { - return window['app'].extensionManager.workflow.activeWorkflow - .isModified + return !!(window['app'].extensionManager as WorkspaceStore).workflow + .activeWorkflow?.isModified }) } function getUndoQueueSize() { return comfyPage.page.evaluate(() => { - const workflow = - window['app'].extensionManager.workflow.activeWorkflow - return workflow.changeTracker.undoQueue.length + const workflow = (window['app'].extensionManager as WorkspaceStore) + .workflow.activeWorkflow + return workflow?.changeTracker.undoQueue.length }) } function getRedoQueueSize() { return comfyPage.page.evaluate(() => { - const workflow = - window['app'].extensionManager.workflow.activeWorkflow - return workflow.changeTracker.redoQueue.length + const workflow = (window['app'].extensionManager as WorkspaceStore) + .workflow.activeWorkflow + return workflow?.changeTracker.redoQueue.length }) } expect(await getUndoQueueSize()).toBe(0) expect(await getRedoQueueSize()).toBe(0) + // Save, confirm no errors & workflow modified flag removed await comfyPage.menu.topbar.saveWorkflow('undo-redo-test') - await comfyPage.page.waitForFunction( - () => !window['app'].extensionManager.workflow.activeWorkflow.isModified - ) + expect(await comfyPage.getToastErrorCount()).toBe(0) + expect(await isModified()).toBe(false) + // TODO(huchenlei): Investigate why saving the workflow is causing the // undo queue to be triggered. expect(await getUndoQueueSize()).toBe(1) @@ -75,13 +80,11 @@ test.describe('Change Tracker', () => { expect(await getUndoQueueSize()).toBe(2) expect(await getRedoQueueSize()).toBe(1) - // TODO(huchenlei): Following assertion is flaky. - // Seems like ctrlZ() is not triggered correctly. - // await comfyPage.ctrlZ() - // await expect(node).not.toBeCollapsed() - // expect(await isModified()).toBe(false) - // expect(await getUndoQueueSize()).toBe(1) - // expect(await getRedoQueueSize()).toBe(2) + await comfyPage.ctrlZ() + await expect(node).not.toBeCollapsed() + expect(await isModified()).toBe(false) + expect(await getUndoQueueSize()).toBe(1) + expect(await getRedoQueueSize()).toBe(2) }) }) @@ -109,7 +112,7 @@ test.describe('Change Tracker', () => { await expect(node).not.toBeCollapsed() // Run again, but within a change transaction - beforeChange(comfyPage) + await beforeChange(comfyPage) await node.click('collapse') await comfyPage.ctrlB() @@ -117,7 +120,7 @@ test.describe('Change Tracker', () => { await expect(node).toBeBypassed() // End transaction - afterChange(comfyPage) + await afterChange(comfyPage) // Ensure undo reverts both changes await comfyPage.ctrlZ() @@ -125,7 +128,7 @@ test.describe('Change Tracker', () => { await expect(node).not.toBeCollapsed() }) - test('Can group multiple transaction calls into a single one', async ({ + test('Can nest multiple change transactions without adding undo steps', async ({ comfyPage }) => { const node = (await comfyPage.getFirstNodeRef())! diff --git a/browser_tests/copyPaste.spec.ts b/browser_tests/copyPaste.spec.ts index 50ee5dc7bc..9e9c127c5c 100644 --- a/browser_tests/copyPaste.spec.ts +++ b/browser_tests/copyPaste.spec.ts @@ -15,9 +15,9 @@ test.describe('Copy Paste', () => { await textBox.click() const originalString = await textBox.inputValue() await textBox.selectText() - await comfyPage.ctrlC() - await comfyPage.ctrlV() - await comfyPage.ctrlV() + await comfyPage.ctrlC(null) + await comfyPage.ctrlV(null) + await comfyPage.ctrlV(null) const resultString = await textBox.inputValue() expect(resultString).toBe(originalString + originalString) }) @@ -31,7 +31,7 @@ test.describe('Copy Paste', () => { y: 643 } }) - await comfyPage.ctrlC() + await comfyPage.ctrlC(null) // KSampler's seed await comfyPage.canvas.click({ position: { @@ -39,7 +39,7 @@ test.describe('Copy Paste', () => { y: 281 } }) - await comfyPage.ctrlV() + await comfyPage.ctrlV(null) await comfyPage.page.keyboard.press('Enter') await expect(comfyPage.canvas).toHaveScreenshot('copied-widget-value.png') }) @@ -51,14 +51,14 @@ test.describe('Copy Paste', () => { comfyPage }) => { await comfyPage.clickEmptyLatentNode() - await comfyPage.ctrlC() + await comfyPage.ctrlC(null) const textBox = comfyPage.widgetTextBox await textBox.click() await textBox.inputValue() await textBox.selectText() - await comfyPage.ctrlC() - await comfyPage.ctrlV() - await comfyPage.ctrlV() + await comfyPage.ctrlC(null) + await comfyPage.ctrlV(null) + await comfyPage.ctrlV(null) await expect(comfyPage.canvas).toHaveScreenshot( 'paste-in-text-area-with-node-previously-copied.png' ) @@ -69,10 +69,10 @@ test.describe('Copy Paste', () => { await textBox.click() await textBox.inputValue() await textBox.selectText() - await comfyPage.ctrlC() + await comfyPage.ctrlC(null) // Unfocus textbox. await comfyPage.page.mouse.click(10, 10) - await comfyPage.ctrlV() + await comfyPage.ctrlV(null) await expect(comfyPage.canvas).toHaveScreenshot('no-node-copied.png') }) diff --git a/browser_tests/dialog.spec.ts b/browser_tests/dialog.spec.ts index 7a9dedd446..28f101ede6 100644 --- a/browser_tests/dialog.spec.ts +++ b/browser_tests/dialog.spec.ts @@ -36,6 +36,7 @@ test.describe('Execution error', () => { }) => { await comfyPage.loadWorkflow('execution_error') await comfyPage.queueButton.click() + await comfyPage.nextFrame() // Wait for the element with the .comfy-execution-error selector to be visible const executionError = comfyPage.page.locator('.comfy-error-report') diff --git a/browser_tests/extensionAPI.spec.ts b/browser_tests/extensionAPI.spec.ts index 6114efe2d4..ce075c5ddd 100644 --- a/browser_tests/extensionAPI.spec.ts +++ b/browser_tests/extensionAPI.spec.ts @@ -48,7 +48,7 @@ test.describe('Topbar commands', () => { }) }) - const menuItem: Locator = await comfyPage.menu.topbar.getMenuItem('ext') + const menuItem = comfyPage.menu.topbar.getMenuItem('ext') expect(await menuItem.count()).toBe(0) }) diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index 0df1a6d194..3679ca5bda 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -349,6 +349,12 @@ export class ComfyPage { await this.nextFrame() } + async getToastErrorCount() { + return await this.page + .locator('.p-toast-message.p-toast-message-error') + .count() + } + async getVisibleToastCount() { return await this.page.locator('.p-toast:visible').count() } @@ -550,7 +556,7 @@ export class ComfyPage { } async doubleClickCanvas() { - await this.page.mouse.dblclick(10, 10) + await this.page.mouse.dblclick(10, 10, { delay: 5 }) await this.nextFrame() } @@ -586,43 +592,42 @@ export class ComfyPage { await this.nextFrame() } - async ctrlSend(keyToPress: string) { - await this.page.keyboard.down('Control') - await this.page.keyboard.press(keyToPress) - await this.page.keyboard.up('Control') + async ctrlSend(keyToPress: string, locator: Locator | null = this.canvas) { + const target = locator ?? this.page.keyboard + await target.press(`Control+${keyToPress}`) await this.nextFrame() } - async ctrlA() { - await this.ctrlSend('KeyA') + async ctrlA(locator?: Locator | null) { + await this.ctrlSend('KeyA', locator) } - async ctrlB() { - await this.ctrlSend('KeyB') + async ctrlB(locator?: Locator | null) { + await this.ctrlSend('KeyB', locator) } - async ctrlC() { - await this.ctrlSend('KeyC') + async ctrlC(locator?: Locator | null) { + await this.ctrlSend('KeyC', locator) } - async ctrlV() { - await this.ctrlSend('KeyV') + async ctrlV(locator?: Locator | null) { + await this.ctrlSend('KeyV', locator) } - async ctrlZ() { - await this.ctrlSend('KeyZ') + async ctrlZ(locator?: Locator | null) { + await this.ctrlSend('KeyZ', locator) } - async ctrlY() { - await this.ctrlSend('KeyY') + async ctrlY(locator?: Locator | null) { + await this.ctrlSend('KeyY', locator) } - async ctrlArrowUp() { - await this.ctrlSend('ArrowUp') + async ctrlArrowUp(locator?: Locator | null) { + await this.ctrlSend('ArrowUp', locator) } - async ctrlArrowDown() { - await this.ctrlSend('ArrowDown') + async ctrlArrowDown(locator?: Locator | null) { + await this.ctrlSend('ArrowDown', locator) } async closeMenu() { diff --git a/browser_tests/fixtures/components/Topbar.ts b/browser_tests/fixtures/components/Topbar.ts index bc64fea1d7..b57461e59b 100644 --- a/browser_tests/fixtures/components/Topbar.ts +++ b/browser_tests/fixtures/components/Topbar.ts @@ -13,35 +13,46 @@ export class Topbar { await this.page.locator('.p-menubar-mobile .p-menubar-button').click() } - async getMenuItem(itemLabel: string): Promise { + getMenuItem(itemLabel: string): Locator { return this.page.locator(`.p-menubar-item-label:text-is("${itemLabel}")`) } - async getWorkflowTab(tabName: string): Promise { + getWorkflowTab(tabName: string): Locator { return this.page .locator(`.workflow-tabs .workflow-label:has-text("${tabName}")`) .locator('..') } async closeWorkflowTab(tabName: string) { - const tab = await this.getWorkflowTab(tabName) + const tab = this.getWorkflowTab(tabName) await tab.locator('.close-button').click({ force: true }) } - async saveWorkflow(workflowName: string) { - await this.triggerTopbarCommand(['Workflow', 'Save']) - await this.page.locator('.p-dialog-content input').fill(workflowName) - await this.page.keyboard.press('Enter') - // Wait for the dialog to close. - await this.page.waitForTimeout(300) + getSaveDialog(): Locator { + return this.page.locator('.p-dialog-content input') } - async saveWorkflowAs(workflowName: string) { - await this.triggerTopbarCommand(['Workflow', 'Save As']) - await this.page.locator('.p-dialog-content input').fill(workflowName) + saveWorkflow(workflowName: string): Promise { + return this._saveWorkflow(workflowName, 'Save') + } + + saveWorkflowAs(workflowName: string): Promise { + return this._saveWorkflow(workflowName, 'Save As') + } + + async _saveWorkflow(workflowName: string, command: 'Save' | 'Save As') { + await this.triggerTopbarCommand(['Workflow', command]) + await this.getSaveDialog().fill(workflowName) await this.page.keyboard.press('Enter') + + // Wait for workflow service to finish saving + await this.page.waitForFunction( + () => !window['app'].extensionManager.workflow.isBusy, + undefined, + { timeout: 3000 } + ) // Wait for the dialog to close. - await this.page.waitForTimeout(300) + await this.getSaveDialog().waitFor({ state: 'hidden', timeout: 500 }) } async triggerTopbarCommand(path: string[]) { diff --git a/browser_tests/interaction.spec.ts b/browser_tests/interaction.spec.ts index f4cd61bc1a..f4710a99ac 100644 --- a/browser_tests/interaction.spec.ts +++ b/browser_tests/interaction.spec.ts @@ -285,7 +285,8 @@ test.describe('Node Interaction', () => { position: { x: 50, y: 10 - } + }, + delay: 5 }) await comfyPage.page.keyboard.type('Hello World') await comfyPage.page.keyboard.press('Enter') @@ -300,7 +301,8 @@ test.describe('Node Interaction', () => { position: { x: 50, y: 50 - } + }, + delay: 5 }) expect(await comfyPage.page.locator('.node-title-editor').count()).toBe(0) }) @@ -352,7 +354,8 @@ test.describe('Group Interaction', () => { position: { x: 50, y: 10 - } + }, + delay: 5 }) await comfyPage.page.keyboard.type('Hello World') await comfyPage.page.keyboard.press('Enter') @@ -504,7 +507,7 @@ test.describe('Widget Interaction', () => { await expect(textBox).toHaveValue('') await textBox.fill('Hello World') await expect(textBox).toHaveValue('Hello World') - await comfyPage.ctrlZ() + await comfyPage.ctrlZ(null) await expect(textBox).toHaveValue('') }) @@ -515,9 +518,9 @@ test.describe('Widget Interaction', () => { await textBox.fill('1girl') await expect(textBox).toHaveValue('1girl') await textBox.selectText() - await comfyPage.ctrlArrowUp() + await comfyPage.ctrlArrowUp(null) await expect(textBox).toHaveValue('(1girl:1.05)') - await comfyPage.ctrlZ() + await comfyPage.ctrlZ(null) await expect(textBox).toHaveValue('1girl') }) }) diff --git a/browser_tests/menu.spec.ts b/browser_tests/menu.spec.ts index 083a4a4a58..7349df3cd7 100644 --- a/browser_tests/menu.spec.ts +++ b/browser_tests/menu.spec.ts @@ -504,6 +504,7 @@ test.describe('Menu', () => { await comfyPage.menu.topbar.saveWorkflow(workflowName) expect(await comfyPage.menu.topbar.getTabNames()).toEqual([workflowName]) await comfyPage.menu.topbar.closeWorkflowTab(workflowName) + await comfyPage.nextFrame() expect(await comfyPage.menu.topbar.getTabNames()).toEqual([ 'Unsaved Workflow' ]) @@ -525,8 +526,7 @@ test.describe('Menu', () => { }) test('Displays keybinding next to item', async ({ comfyPage }) => { - const workflowMenuItem = - await comfyPage.menu.topbar.getMenuItem('Workflow') + const workflowMenuItem = comfyPage.menu.topbar.getMenuItem('Workflow') await workflowMenuItem.click() const exportTag = comfyPage.page.locator('.keybinding-tag', { hasText: 'Ctrl + s' diff --git a/browser_tests/nodeSearchBox.spec.ts b/browser_tests/nodeSearchBox.spec.ts index b3bc562653..426e770cc7 100644 --- a/browser_tests/nodeSearchBox.spec.ts +++ b/browser_tests/nodeSearchBox.spec.ts @@ -15,7 +15,7 @@ test.describe('Node search box', () => { test(`Can trigger on group body double click`, async ({ comfyPage }) => { await comfyPage.loadWorkflow('single_group_only') - await comfyPage.page.mouse.dblclick(50, 50) + await comfyPage.page.mouse.dblclick(50, 50, { delay: 5 }) await comfyPage.nextFrame() await expect(comfyPage.searchBox.input).toHaveCount(1) }) diff --git a/src/stores/workflowStore.ts b/src/stores/workflowStore.ts index b757dd37fc..d9f6aa361a 100644 --- a/src/stores/workflowStore.ts +++ b/src/stores/workflowStore.ts @@ -284,32 +284,45 @@ export const useWorkflowStore = defineStore('workflow', () => { workflows.value.filter((workflow) => workflow.isModified) ) + /** A filesystem operation is currently in progress (e.g. save, rename, delete) */ + const isBusy = ref(false) + const renameWorkflow = async (workflow: ComfyWorkflow, newPath: string) => { - // Capture all needed values upfront - const oldPath = workflow.path - const wasBookmarked = bookmarkStore.isBookmarked(oldPath) - - const openIndex = detachWorkflow(workflow) - // Perform the actual rename operation first + isBusy.value = true try { - await workflow.rename(newPath) - } finally { - attachWorkflow(workflow, openIndex) - } + // Capture all needed values upfront + const oldPath = workflow.path + const wasBookmarked = bookmarkStore.isBookmarked(oldPath) - // Update bookmarks - if (wasBookmarked) { - bookmarkStore.setBookmarked(oldPath, false) - bookmarkStore.setBookmarked(newPath, true) + const openIndex = detachWorkflow(workflow) + // Perform the actual rename operation first + try { + await workflow.rename(newPath) + } finally { + attachWorkflow(workflow, openIndex) + } + + // Update bookmarks + if (wasBookmarked) { + bookmarkStore.setBookmarked(oldPath, false) + bookmarkStore.setBookmarked(newPath, true) + } + } finally { + isBusy.value = false } } const deleteWorkflow = async (workflow: ComfyWorkflow) => { - await workflow.delete() - if (bookmarkStore.isBookmarked(workflow.path)) { - bookmarkStore.setBookmarked(workflow.path, false) + isBusy.value = true + try { + await workflow.delete() + if (bookmarkStore.isBookmarked(workflow.path)) { + bookmarkStore.setBookmarked(workflow.path, false) + } + delete workflowLookup.value[workflow.path] + } finally { + isBusy.value = false } - delete workflowLookup.value[workflow.path] } /** @@ -317,12 +330,17 @@ export const useWorkflowStore = defineStore('workflow', () => { * @param workflow The workflow to save. */ const saveWorkflow = async (workflow: ComfyWorkflow) => { - // Detach the workflow and re-attach to force refresh the tree objects. - const openIndex = detachWorkflow(workflow) + isBusy.value = true try { - await workflow.save() + // Detach the workflow and re-attach to force refresh the tree objects. + const openIndex = detachWorkflow(workflow) + try { + await workflow.save() + } finally { + attachWorkflow(workflow, openIndex) + } } finally { - attachWorkflow(workflow, openIndex) + isBusy.value = false } } @@ -333,6 +351,7 @@ export const useWorkflowStore = defineStore('workflow', () => { openedWorkflowIndexShift, openWorkflow, isOpen, + isBusy, closeWorkflow, createTemporary, renameWorkflow,