From c2949ace552e49bce215c68582fb5dd620b7f3a8 Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Wed, 15 Oct 2025 11:21:45 +0200 Subject: [PATCH] test(editor): Improve NDV v2 test performance and coverage (#20789) --- .../editor-ui/src/components/NDVHeader.vue | 8 +- .../src/components/NodeDetailsViewV2.test.ts | 410 +++++++++++------- .../src/components/NodeDetailsViewV2.vue | 7 +- 3 files changed, 260 insertions(+), 165 deletions(-) diff --git a/packages/frontend/editor-ui/src/components/NDVHeader.vue b/packages/frontend/editor-ui/src/components/NDVHeader.vue index db61639b576..86e6cf72888 100644 --- a/packages/frontend/editor-ui/src/components/NDVHeader.vue +++ b/packages/frontend/editor-ui/src/components/NDVHeader.vue @@ -57,7 +57,13 @@ function onRename(newNodeName: string) { - + diff --git a/packages/frontend/editor-ui/src/components/NodeDetailsViewV2.test.ts b/packages/frontend/editor-ui/src/components/NodeDetailsViewV2.test.ts index 6f8531b69e5..92fc053456e 100644 --- a/packages/frontend/editor-ui/src/components/NodeDetailsViewV2.test.ts +++ b/packages/frontend/editor-ui/src/components/NodeDetailsViewV2.test.ts @@ -1,236 +1,190 @@ -import { createPinia, setActivePinia } from 'pinia'; +import { createTestingPinia } from '@pinia/testing'; +import { setActivePinia } from 'pinia'; import { waitFor, fireEvent } from '@testing-library/vue'; +import userEvent from '@testing-library/user-event'; import NodeDetailsViewV2 from '@/components/NodeDetailsViewV2.vue'; -import { VIEWS } from '@/constants'; -import { useSettingsStore } from '@/stores/settings.store'; -import { useUsersStore } from '@/features/users/users.store'; +import { MANUAL_TRIGGER_NODE_TYPE, SET_NODE_TYPE, STICKY_NODE_TYPE, VIEWS } from '@/constants'; import { useNDVStore } from '@/stores/ndv.store'; import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { useWorkflowsStore } from '@/stores/workflows.store'; import { createComponentRenderer } from '@/__tests__/render'; -import { setupServer } from '@/__tests__/server'; import { + createTestNode, createTestWorkflow, createTestWorkflowObject, defaultNodeDescriptions, - mockNodes, } from '@/__tests__/mocks'; import type { Workflow } from 'n8n-workflow'; -vi.mock('vue-router', () => { - return { - useRouter: () => ({}), - useRoute: () => ({ meta: {} }), - RouterLink: vi.fn(), - }; -}); +vi.mock('vue-router', () => ({ + useRouter: () => ({}), + useRoute: () => ({ meta: {} }), + RouterLink: vi.fn(), +})); + +const setupStore = (nodes: Array>) => { + const pinia = createTestingPinia({ + stubActions: false, + }); + setActivePinia(pinia); -async function createPiniaStore( - { activeNodeName }: { activeNodeName: string | null } = { activeNodeName: null }, -) { const workflow = createTestWorkflow({ connections: {}, active: true, - nodes: mockNodes, + nodes, }); - const pinia = createPinia(); - setActivePinia(pinia); - const workflowsStore = useWorkflowsStore(); const nodeTypesStore = useNodeTypesStore(); - const ndvStore = useNDVStore(); nodeTypesStore.setNodeTypes(defaultNodeDescriptions); workflowsStore.workflow = workflow; workflowsStore.workflowObject = createTestWorkflowObject(workflow); - workflowsStore.nodeMetadata = mockNodes.reduce( + workflowsStore.nodeMetadata = nodes.reduce( (acc, node) => ({ ...acc, [node.name]: { pristine: true } }), {}, ); - if (activeNodeName) { - ndvStore.setActiveNodeName(activeNodeName, 'other'); - } else { - ndvStore.unsetActiveNodeName(); - } - - await useSettingsStore().getSettings(); - await useUsersStore().loginWithCookie(); - return { pinia, workflowObject: workflowsStore.workflowObject as Workflow, }; -} +}; describe('NodeDetailsViewV2', () => { - let server: ReturnType; + let pinia: ReturnType; + let workflowObject: Workflow; + const manualTriggerNode = createTestNode({ + name: 'Manual Trigger', + type: MANUAL_TRIGGER_NODE_TYPE, + }); + const setNode = createTestNode({ name: 'Set', type: SET_NODE_TYPE }); + const stickyNode = createTestNode({ name: 'Sticky', type: STICKY_NODE_TYPE }); - beforeAll(() => { + const renderComponent = (props: { readOnly?: boolean; activeNodeName?: string | null } = {}) => { + const { activeNodeName = null, ...componentProps } = props; + + const ndvStore = useNDVStore(); + if (activeNodeName) { + ndvStore.setActiveNodeName(activeNodeName, 'other'); + } else { + ndvStore.unsetActiveNodeName(); + } + + const render = createComponentRenderer(NodeDetailsViewV2, { + props: { + workflowObject, + ...componentProps, + }, + global: { + mocks: { + $route: { + name: VIEWS.WORKFLOW, + }, + }, + stubs: { + InputPanel: { template: '
' }, + OutputPanel: { template: '
' }, + TriggerPanel: { + template: '
', + emits: ['execute', 'activate'], + }, + NodeSettings: { template: '
' }, + }, + }, + }); + + return render({ pinia }); + }; + + beforeEach(() => { HTMLDialogElement.prototype.show = vi.fn(); - server = setupServer(); + HTMLDialogElement.prototype.close = vi.fn(); }); afterEach(() => { vi.clearAllMocks(); }); - afterAll(() => { - server.shutdown(); - }); - - test('should render correctly', async () => { - const { pinia, workflowObject } = await createPiniaStore({ activeNodeName: 'Manual Trigger' }); - - const renderComponent = createComponentRenderer(NodeDetailsViewV2, { - props: { - workflowObject, - }, - global: { - mocks: { - $route: { - name: VIEWS.WORKFLOW, - }, - }, - }, + describe('rendering', () => { + beforeEach(() => { + const store = setupStore([manualTriggerNode, setNode, stickyNode]); + pinia = store.pinia; + workflowObject = store.workflowObject; }); - const { getByTestId } = renderComponent({ - pinia, + test('should not render when no node is active', () => { + const { queryByTestId } = renderComponent({ activeNodeName: null }); + expect(queryByTestId('ndv')).not.toBeInTheDocument(); }); - await waitFor(() => expect(getByTestId('ndv')).toBeInTheDocument()); - }); - - test('should not render for stickies', async () => { - const { pinia, workflowObject } = await createPiniaStore({ activeNodeName: 'Sticky' }); - - const renderComponent = createComponentRenderer(NodeDetailsViewV2, { - props: { - workflowObject, - }, - global: { - mocks: { - $route: { - name: VIEWS.WORKFLOW, - }, - }, - }, + test('should not render for sticky nodes', () => { + const { queryByTestId } = renderComponent({ activeNodeName: 'Sticky' }); + expect(queryByTestId('ndv')).not.toBeInTheDocument(); }); - const { queryByTestId } = renderComponent({ - pinia, - }); - - expect(queryByTestId('ndv')).not.toBeInTheDocument(); - }); - - describe('keyboard listener', () => { - test('should register and unregister keydown listener based on modal open state', async () => { - const addEventListenerSpy = vi.spyOn(document, 'addEventListener'); - const removeEventListenerSpy = vi.spyOn(document, 'removeEventListener'); - - const { pinia, workflowObject } = await createPiniaStore({ - activeNodeName: 'Manual Trigger', - }); - - const renderComponent = createComponentRenderer(NodeDetailsViewV2, { - props: { - workflowObject, - }, - global: { - mocks: { - $route: { - name: VIEWS.WORKFLOW, - }, - }, - }, - }); - - const { getByTestId, unmount } = renderComponent({ - pinia, - }); - + test('should render for regular nodes', async () => { + const { getByTestId } = renderComponent({ activeNodeName: 'Set' }); await waitFor(() => expect(getByTestId('ndv')).toBeInTheDocument()); + }); - expect(addEventListenerSpy).toHaveBeenCalledWith('keydown', expect.any(Function), true); - expect(removeEventListenerSpy).not.toHaveBeenCalledWith( - 'keydown', - expect.any(Function), - true, - ); + test('should render trigger panel for trigger nodes', async () => { + const { getByTestId, queryByTestId } = renderComponent({ activeNodeName: 'Manual Trigger' }); - unmount(); + await waitFor(() => { + expect(getByTestId('ndv')).toBeInTheDocument(); + expect(getByTestId('trigger-panel')).toBeInTheDocument(); + expect(queryByTestId('input-panel')).not.toBeInTheDocument(); + }); + }); - expect(removeEventListenerSpy).toHaveBeenCalledWith('keydown', expect.any(Function), true); + test('should render input panel for non-trigger nodes', async () => { + const { getByTestId, queryByTestId } = renderComponent({ activeNodeName: 'Set' }); + + await waitFor(() => { + expect(getByTestId('ndv')).toBeInTheDocument(); + expect(getByTestId('input-panel')).toBeInTheDocument(); + expect(queryByTestId('trigger-panel')).not.toBeInTheDocument(); + }); + }); + }); + + describe('keyboard shortcuts', () => { + beforeEach(() => { + const store = setupStore([manualTriggerNode, setNode, stickyNode]); + pinia = store.pinia; + workflowObject = store.workflowObject; + }); + + test('should register keydown listener on mount', async () => { + const addEventListenerSpy = vi.spyOn(document, 'addEventListener'); + + renderComponent({ activeNodeName: 'Manual Trigger' }); + + await waitFor(() => { + expect(addEventListenerSpy).toHaveBeenCalledWith('keydown', expect.any(Function), true); + }); addEventListenerSpy.mockRestore(); - removeEventListenerSpy.mockRestore(); }); test('should unregister keydown listener on unmount', async () => { - const { pinia, workflowObject } = await createPiniaStore(); - const ndvStore = useNDVStore(); + const removeEventListenerSpy = vi.spyOn(document, 'removeEventListener'); - const renderComponent = createComponentRenderer(NodeDetailsViewV2, { - props: { - workflowObject, - }, - global: { - mocks: { - $route: { - name: VIEWS.WORKFLOW, - }, - }, - }, - }); - - const { getByTestId, unmount } = renderComponent({ - pinia, - }); - - ndvStore.setActiveNodeName('Manual Trigger', 'other'); + const { getByTestId, unmount } = renderComponent({ activeNodeName: 'Manual Trigger' }); await waitFor(() => expect(getByTestId('ndv')).toBeInTheDocument()); - const removeEventListenerSpy = vi.spyOn(document, 'removeEventListener'); - expect(removeEventListenerSpy).not.toHaveBeenCalledWith( - 'keydown', - expect.any(Function), - true, - ); - unmount(); expect(removeEventListenerSpy).toHaveBeenCalledWith('keydown', expect.any(Function), true); - removeEventListenerSpy.mockRestore(); }); - test("should emit 'saveKeyboardShortcut' when save shortcut keybind is pressed", async () => { - const { pinia, workflowObject } = await createPiniaStore({ - activeNodeName: 'Manual Trigger', - }); - - const renderComponent = createComponentRenderer(NodeDetailsViewV2, { - props: { - workflowObject, - }, - global: { - mocks: { - $route: { - name: VIEWS.WORKFLOW, - }, - }, - }, - }); - - const { getByTestId, emitted } = renderComponent({ - pinia, - }); + test('should emit saveKeyboardShortcut when Ctrl+S is pressed', async () => { + const { getByTestId, emitted } = renderComponent({ activeNodeName: 'Manual Trigger' }); await waitFor(() => expect(getByTestId('ndv')).toBeInTheDocument()); @@ -242,6 +196,136 @@ describe('NodeDetailsViewV2', () => { }); expect(emitted().saveKeyboardShortcut).toBeTruthy(); + expect(emitted().saveKeyboardShortcut).toHaveLength(1); + }); + + test('should not emit saveKeyboardShortcut in readOnly mode', async () => { + const { getByTestId, emitted } = renderComponent({ + activeNodeName: 'Manual Trigger', + readOnly: true, + }); + + await waitFor(() => expect(getByTestId('ndv')).toBeInTheDocument()); + + await fireEvent.keyDown(getByTestId('ndv'), { + key: 's', + ctrlKey: true, + bubbles: true, + cancelable: true, + }); + + expect(emitted().saveKeyboardShortcut).toBeFalsy(); + }); + + test('should not emit saveKeyboardShortcut when Ctrl is not pressed', async () => { + const { getByTestId, emitted } = renderComponent({ activeNodeName: 'Manual Trigger' }); + + await waitFor(() => expect(getByTestId('ndv')).toBeInTheDocument()); + + await fireEvent.keyDown(getByTestId('ndv'), { + key: 's', + bubbles: true, + cancelable: true, + }); + + expect(emitted().saveKeyboardShortcut).toBeFalsy(); + }); + }); + + describe('lifecycle', () => { + beforeEach(() => { + const store = setupStore([manualTriggerNode, setNode, stickyNode]); + pinia = store.pinia; + workflowObject = store.workflowObject; + }); + + test('should open dialog on mount', async () => { + const showSpy = vi.spyOn(HTMLDialogElement.prototype, 'show'); + + renderComponent({ activeNodeName: 'Manual Trigger' }); + + await waitFor(() => { + expect(showSpy).toHaveBeenCalled(); + }); + + showSpy.mockRestore(); + }); + + test('should handle dynamic node activation', async () => { + const { getByTestId, queryByTestId } = renderComponent({ activeNodeName: null }); + + // Initially no NDV + expect(queryByTestId('ndv')).not.toBeInTheDocument(); + + // Activate a node + const ndvStore = useNDVStore(); + ndvStore.setActiveNodeName('Set', 'other'); + + // NDV should appear + await waitFor(() => { + expect(getByTestId('ndv')).toBeInTheDocument(); + }); + }); + }); + + describe('user interactions', () => { + beforeEach(() => { + const store = setupStore([manualTriggerNode, setNode, stickyNode]); + pinia = store.pinia; + workflowObject = store.workflowObject; + }); + + test('should close NDV when close button is clicked', async () => { + const user = userEvent.setup(); + const { getByRole, getByTestId } = renderComponent({ activeNodeName: 'Manual Trigger' }); + + await waitFor(() => expect(getByRole('dialog')).toBeInTheDocument()); + + const closeButton = getByTestId('ndv-close-button'); + await user.click(closeButton); + + const ndvStore = useNDVStore(); + await waitFor(() => { + expect(ndvStore.activeNodeName).toBeNull(); + }); + }); + + test('should close NDV when backdrop is clicked', async () => { + const user = userEvent.setup(); + const { getByRole, getByTestId } = renderComponent({ activeNodeName: 'Manual Trigger' }); + + await waitFor(() => expect(getByRole('dialog')).toBeInTheDocument()); + + const backdrop = getByTestId('ndv-backdrop'); + await user.click(backdrop); + + const ndvStore = useNDVStore(); + await waitFor(() => { + expect(ndvStore.activeNodeName).toBeNull(); + }); + }); + + test('should emit renameNode when node name is edited', async () => { + const user = userEvent.setup(); + const { emitted, getByRole, getByTestId } = renderComponent({ + activeNodeName: 'Manual Trigger', + }); + + await waitFor(() => expect(getByRole('dialog')).toBeInTheDocument()); + + const editableArea = getByTestId('inline-editable-area'); + await user.click(editableArea); + + const input = getByTestId('inline-edit-input'); + expect(input).toHaveValue('Manual Trigger'); + + await user.clear(input); + await user.type(input, 'Renamed Trigger'); + await user.tab(); + + await waitFor(() => { + expect(emitted().renameNode).toEqual([['Renamed Trigger']]); + }); }); }); }); diff --git a/packages/frontend/editor-ui/src/components/NodeDetailsViewV2.vue b/packages/frontend/editor-ui/src/components/NodeDetailsViewV2.vue index 9f672dca616..75bab338fc8 100644 --- a/packages/frontend/editor-ui/src/components/NodeDetailsViewV2.vue +++ b/packages/frontend/editor-ui/src/components/NodeDetailsViewV2.vue @@ -717,7 +717,12 @@ onBeforeUnmount(() => {