From d531bc34c47cc11a36ba9909c147f06efa89c3a2 Mon Sep 17 00:00:00 2001 From: Chenlei Hu Date: Mon, 25 Nov 2024 18:59:27 -0800 Subject: [PATCH] Make ChangeTracker detect changes in workflow.extra (Except ds) (#1692) * Compare workflow.extra content * Add tests --- browser_tests/changeTracker.spec.ts | 76 +++++++++++++---------------- browser_tests/fixtures/ComfyPage.ts | 23 +++++++++ src/scripts/changeTracker.ts | 7 ++- 3 files changed, 61 insertions(+), 45 deletions(-) diff --git a/browser_tests/changeTracker.spec.ts b/browser_tests/changeTracker.spec.ts index 188526540..e21a105c0 100644 --- a/browser_tests/changeTracker.spec.ts +++ b/browser_tests/changeTracker.spec.ts @@ -3,9 +3,6 @@ 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(() => { @@ -26,64 +23,41 @@ test.describe('Change Tracker', () => { }) test('Can undo multiple operations', async ({ comfyPage }) => { - function isModified() { - return comfyPage.page.evaluate(async () => { - return !!(window['app'].extensionManager as WorkspaceStore).workflow - .activeWorkflow?.isModified - }) - } - - function getUndoQueueSize() { - return comfyPage.page.evaluate(() => { - 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 as WorkspaceStore) - .workflow.activeWorkflow - return workflow?.changeTracker.redoQueue.length - }) - } - expect(await getUndoQueueSize()).toBe(0) - expect(await getRedoQueueSize()).toBe(0) + expect(await comfyPage.getUndoQueueSize()).toBe(0) + expect(await comfyPage.getRedoQueueSize()).toBe(0) // Save, confirm no errors & workflow modified flag removed await comfyPage.menu.topbar.saveWorkflow('undo-redo-test') expect(await comfyPage.getToastErrorCount()).toBe(0) - expect(await isModified()).toBe(false) - - expect(await getUndoQueueSize()).toBe(0) - expect(await getRedoQueueSize()).toBe(0) + expect(await comfyPage.isCurrentWorkflowModified()).toBe(false) + expect(await comfyPage.getUndoQueueSize()).toBe(0) + expect(await comfyPage.getRedoQueueSize()).toBe(0) const node = (await comfyPage.getFirstNodeRef())! await node.click('title') await node.click('collapse') await expect(node).toBeCollapsed() - expect(await isModified()).toBe(true) - expect(await getUndoQueueSize()).toBe(1) - expect(await getRedoQueueSize()).toBe(0) + expect(await comfyPage.isCurrentWorkflowModified()).toBe(true) + expect(await comfyPage.getUndoQueueSize()).toBe(1) + expect(await comfyPage.getRedoQueueSize()).toBe(0) await comfyPage.ctrlB() await expect(node).toBeBypassed() - expect(await isModified()).toBe(true) - expect(await getUndoQueueSize()).toBe(2) - expect(await getRedoQueueSize()).toBe(0) + expect(await comfyPage.isCurrentWorkflowModified()).toBe(true) + expect(await comfyPage.getUndoQueueSize()).toBe(2) + expect(await comfyPage.getRedoQueueSize()).toBe(0) await comfyPage.ctrlZ() await expect(node).not.toBeBypassed() - expect(await isModified()).toBe(true) - expect(await getUndoQueueSize()).toBe(1) - expect(await getRedoQueueSize()).toBe(1) + expect(await comfyPage.isCurrentWorkflowModified()).toBe(true) + expect(await comfyPage.getUndoQueueSize()).toBe(1) + expect(await comfyPage.getRedoQueueSize()).toBe(1) await comfyPage.ctrlZ() await expect(node).not.toBeCollapsed() - expect(await isModified()).toBe(false) - expect(await getUndoQueueSize()).toBe(0) - expect(await getRedoQueueSize()).toBe(2) + expect(await comfyPage.isCurrentWorkflowModified()).toBe(false) + expect(await comfyPage.getUndoQueueSize()).toBe(0) + expect(await comfyPage.getRedoQueueSize()).toBe(2) }) }) @@ -174,4 +148,20 @@ test.describe('Change Tracker', () => { await expect(node).toBePinned() await expect(node).toBeCollapsed() }) + + test('Can detect changes in workflow.extra', async ({ comfyPage }) => { + expect(await comfyPage.getUndoQueueSize()).toBe(0) + await comfyPage.page.evaluate(() => { + window['app'].graph.extra.foo = 'bar' + }) + // Click empty space to trigger a change detection. + await comfyPage.clickEmptySpace() + expect(await comfyPage.getUndoQueueSize()).toBe(1) + }) + + test('Ignores changes in workflow.ds', async ({ comfyPage }) => { + expect(await comfyPage.getUndoQueueSize()).toBe(0) + await comfyPage.pan({ x: 10, y: 10 }) + expect(await comfyPage.getUndoQueueSize()).toBe(0) + }) }) diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index abf5feebb..ce7ca8c2d 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -17,8 +17,11 @@ import { import { Topbar } from './components/Topbar' import { NodeReference } from './utils/litegraphUtils' import type { Position, Size } from './types' +import type { useWorkspaceStore } from '../../src/stores/workspaceStore' import { SettingDialog } from './components/SettingDialog' +type WorkspaceStore = ReturnType + class ComfyMenu { public readonly sideToolbar: Locator public readonly themeToggleButton: Locator @@ -788,6 +791,26 @@ export class ComfyPage { async moveMouseToEmptyArea() { await this.page.mouse.move(10, 10) } + async getUndoQueueSize() { + return this.page.evaluate(() => { + const workflow = (window['app'].extensionManager as WorkspaceStore) + .workflow.activeWorkflow + return workflow?.changeTracker.undoQueue.length + }) + } + async getRedoQueueSize() { + return this.page.evaluate(() => { + const workflow = (window['app'].extensionManager as WorkspaceStore) + .workflow.activeWorkflow + return workflow?.changeTracker.redoQueue.length + }) + } + async isCurrentWorkflowModified() { + return this.page.evaluate(() => { + return (window['app'].extensionManager as WorkspaceStore).workflow + .activeWorkflow?.isModified + }) + } } export const comfyPageFixture = base.extend<{ comfyPage: ComfyPage }>({ diff --git a/src/scripts/changeTracker.ts b/src/scripts/changeTracker.ts index 181e4fd50..17ad40ebd 100644 --- a/src/scripts/changeTracker.ts +++ b/src/scripts/changeTracker.ts @@ -388,8 +388,11 @@ export class ChangeTracker { return false } - // Reroutes (schema 0.4) - if (!_.isEqual(a.extra?.reroutes, b.extra?.reroutes)) return false + // Compare extra properties ignoring ds + if ( + !_.isEqual(_.omit(a.extra ?? {}, ['ds']), _.omit(b.extra ?? {}, ['ds'])) + ) + return false // Compare other properties normally for (const key of ['links', 'reroutes', 'groups']) {