Skip to content

Commit 2775f61

Browse files
OlegIvanivOlegIvaniv
authored
fix(editor): Fix workflow initilisation for test definition routes & add unit tests (#12507)
1 parent c28f302 commit 2775f61

File tree

11 files changed

+302
-83
lines changed

11 files changed

+302
-83
lines changed

Diff for: ‎packages/editor-ui/src/api/testDefinition.ee.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export interface TestDefinitionRecord {
1111
updatedAt?: string;
1212
createdAt?: string;
1313
annotationTag?: string | null;
14-
mockedNodes?: Array<{ name: string }>;
14+
mockedNodes?: Array<{ name: string; id: string }>;
1515
}
1616

1717
interface CreateTestDefinitionParams {
@@ -25,7 +25,7 @@ export interface UpdateTestDefinitionParams {
2525
evaluationWorkflowId?: string | null;
2626
annotationTagId?: string | null;
2727
description?: string | null;
28-
mockedNodes?: Array<{ name: string }>;
28+
mockedNodes?: Array<{ name: string; id: string }>;
2929
}
3030

3131
export interface UpdateTestResponse {

Diff for: ‎packages/editor-ui/src/components/TestDefinition/EditDefinition/EvaluationStep.vue

+9-30
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,26 @@
22
import { useI18n } from '@/composables/useI18n';
33
import { ElCollapseTransition } from 'element-plus';
44
import { ref, nextTick } from 'vue';
5-
import N8nTooltip from 'n8n-design-system/components/N8nTooltip';
65
76
interface EvaluationStep {
87
title: string;
98
warning?: boolean;
109
small?: boolean;
1110
expanded?: boolean;
12-
tooltip?: string;
11+
description?: string;
1312
}
1413
1514
const props = withDefaults(defineProps<EvaluationStep>(), {
1615
description: '',
1716
warning: false,
1817
small: false,
1918
expanded: true,
20-
tooltip: '',
2119
});
2220
2321
const locale = useI18n();
2422
const isExpanded = ref(props.expanded);
2523
const contentRef = ref<HTMLElement | null>(null);
2624
const containerRef = ref<HTMLElement | null>(null);
27-
const isTooltipVisible = ref(false);
2825
2926
const toggleExpand = async () => {
3027
isExpanded.value = !isExpanded.value;
@@ -35,14 +32,6 @@ const toggleExpand = async () => {
3532
}
3633
}
3734
};
38-
39-
const showTooltip = () => {
40-
isTooltipVisible.value = true;
41-
};
42-
43-
const hideTooltip = () => {
44-
isTooltipVisible.value = false;
45-
};
4635
</script>
4736

4837
<template>
@@ -51,16 +40,7 @@ const hideTooltip = () => {
5140
:class="[$style.evaluationStep, small && $style.small]"
5241
data-test-id="evaluation-step"
5342
>
54-
<N8nTooltip :disabled="!tooltip" placement="right" :offset="25" :visible="isTooltipVisible">
55-
<template #content>
56-
{{ tooltip }}
57-
</template>
58-
<!-- This empty div is needed to ensure the tooltip trigger area spans the full width of the step.
59-
Without it, the tooltip would only show when hovering over the content div, which is narrower.
60-
The contentPlaceholder creates an invisible full-width area that can trigger the tooltip. -->
61-
<div :class="$style.contentPlaceholder"></div>
62-
</N8nTooltip>
63-
<div :class="$style.content" @mouseenter="showTooltip" @mouseleave="hideTooltip">
43+
<div :class="$style.content">
6444
<div :class="$style.header">
6545
<div :class="[$style.icon, warning && $style.warning]">
6646
<slot name="icon" />
@@ -83,6 +63,7 @@ const hideTooltip = () => {
8363
<font-awesome-icon :icon="isExpanded ? 'angle-down' : 'angle-right'" size="lg" />
8464
</button>
8565
</div>
66+
<div v-if="description" :class="$style.description">{{ description }}</div>
8667
<ElCollapseTransition v-if="$slots.cardContent">
8768
<div v-show="isExpanded" :class="$style.cardContentWrapper">
8869
<div ref="contentRef" :class="$style.cardContent" data-test-id="evaluation-step-content">
@@ -111,14 +92,6 @@ const hideTooltip = () => {
11192
width: 80%;
11293
}
11394
}
114-
.contentPlaceholder {
115-
position: absolute;
116-
top: 0;
117-
left: 0;
118-
width: 100%;
119-
height: 100%;
120-
z-index: -1;
121-
}
12295
.icon {
12396
display: flex;
12497
align-items: center;
@@ -173,4 +146,10 @@ const hideTooltip = () => {
173146
.cardContentWrapper {
174147
height: max-content;
175148
}
149+
150+
.description {
151+
font-size: var(--font-size-2xs);
152+
color: var(--color-text-light);
153+
line-height: 1rem;
154+
}
176155
</style>

Diff for: ‎packages/editor-ui/src/components/TestDefinition/EditDefinition/NodesPinning.vue

+7-13
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ const eventBus = createEventBus<CanvasEventBusEvents>();
2222
const style = useCssModule();
2323
const uuid = crypto.randomUUID();
2424
const props = defineProps<{
25-
modelValue: Array<{ name: string }>;
25+
modelValue: Array<{ name: string; id: string }>;
2626
}>();
2727
2828
const emit = defineEmits<{
29-
'update:modelValue': [value: Array<{ name: string }>];
29+
'update:modelValue': [value: Array<{ name: string; id: string }>];
3030
}>();
3131
3232
const isLoading = ref(true);
@@ -84,14 +84,7 @@ function disableAllNodes() {
8484
const ids = mappedNodes.value.map((node) => node.id);
8585
updateNodeClasses(ids, false);
8686
87-
const pinnedNodes = props.modelValue
88-
.map((node) => {
89-
const matchedNode = mappedNodes.value.find(
90-
(mappedNode) => mappedNode?.data?.name === node.name,
91-
);
92-
return matchedNode?.id ?? null;
93-
})
94-
.filter((n) => n !== null);
87+
const pinnedNodes = props.modelValue.map((node) => node.id).filter((id) => id !== null);
9588
9689
if (pinnedNodes.length > 0) {
9790
updateNodeClasses(pinnedNodes, true);
@@ -101,10 +94,10 @@ function onPinButtonClick(data: CanvasNodeData) {
10194
const nodeName = getNodeNameById(data.id);
10295
if (!nodeName) return;
10396
104-
const isPinned = props.modelValue.some((node) => node.name === nodeName);
97+
const isPinned = props.modelValue.some((node) => node.id === data.id);
10598
const updatedNodes = isPinned
106-
? props.modelValue.filter((node) => node.name !== nodeName)
107-
: [...props.modelValue, { name: nodeName }];
99+
? props.modelValue.filter((node) => node.id !== data.id)
100+
: [...props.modelValue, { name: nodeName, id: data.id }];
108101
109102
emit('update:modelValue', updatedNodes);
110103
updateNodeClasses([data.id], !isPinned);
@@ -145,6 +138,7 @@ onMounted(loadData);
145138
size="large"
146139
icon="thumbtack"
147140
:class="$style.pinButton"
141+
data-test-id="node-pin-button"
148142
@click="onPinButtonClick(data)"
149143
/>
150144
</N8nTooltip>

Diff for: ‎packages/editor-ui/src/components/TestDefinition/EditDefinition/tests/NodesPinning.test.ts

+143
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
import { waitFor } from '@testing-library/vue';
2+
import { createPinia, setActivePinia } from 'pinia';
3+
import { createTestingPinia } from '@pinia/testing';
4+
import NodesPinning from '../NodesPinning.vue';
5+
import { createComponentRenderer } from '@/__tests__/render';
6+
import { useWorkflowsStore } from '@/stores/workflows.store';
7+
import { useNodeTypesStore } from '@/stores/nodeTypes.store';
8+
9+
import {
10+
createTestNode,
11+
createTestWorkflow,
12+
createTestWorkflowObject,
13+
mockNodeTypeDescription,
14+
} from '@/__tests__/mocks';
15+
import { mockedStore } from '@/__tests__/utils';
16+
import { NodeConnectionType } from 'n8n-workflow';
17+
import { SET_NODE_TYPE } from '@/constants';
18+
19+
vi.mock('vue-router', () => {
20+
const push = vi.fn();
21+
return {
22+
useRouter: () => ({
23+
push,
24+
}),
25+
useRoute: () => ({
26+
params: {
27+
name: 'test-workflow',
28+
testId: 'test-123',
29+
},
30+
}),
31+
RouterLink: {
32+
template: '<a><slot /></a>',
33+
},
34+
};
35+
});
36+
37+
const renderComponent = createComponentRenderer(NodesPinning, {
38+
props: {
39+
modelValue: [{ id: '1', name: 'Node 1' }],
40+
},
41+
global: {
42+
plugins: [createTestingPinia()],
43+
},
44+
});
45+
46+
describe('NodesPinning', () => {
47+
const workflowsStore = mockedStore(useWorkflowsStore);
48+
const nodes = [
49+
createTestNode({ id: '1', name: 'Node 1', type: SET_NODE_TYPE }),
50+
createTestNode({ id: '2', name: 'Node 2', type: SET_NODE_TYPE }),
51+
];
52+
53+
const nodeTypesStore = mockedStore(useNodeTypesStore);
54+
const nodeTypeDescription = mockNodeTypeDescription({
55+
name: SET_NODE_TYPE,
56+
inputs: [NodeConnectionType.Main],
57+
outputs: [NodeConnectionType.Main],
58+
});
59+
nodeTypesStore.nodeTypes = {
60+
node: { 1: nodeTypeDescription },
61+
};
62+
63+
nodeTypesStore.getNodeType = vi.fn().mockReturnValue(nodeTypeDescription);
64+
const workflow = createTestWorkflow({
65+
id: 'test-workflow',
66+
name: 'Test Workflow',
67+
nodes,
68+
connections: {},
69+
});
70+
71+
const workflowObject = createTestWorkflowObject(workflow);
72+
73+
workflowsStore.getWorkflowById = vi.fn().mockReturnValue(workflow);
74+
workflowsStore.getCurrentWorkflow = vi.fn().mockReturnValue(workflowObject);
75+
beforeEach(() => {
76+
const pinia = createPinia();
77+
setActivePinia(pinia);
78+
79+
nodeTypesStore.setNodeTypes([nodeTypeDescription]);
80+
});
81+
82+
afterEach(() => {
83+
vi.clearAllMocks();
84+
});
85+
86+
it('should render workflow nodes', async () => {
87+
const { container } = renderComponent();
88+
89+
await waitFor(() => {
90+
expect(container.querySelectorAll('.vue-flow__node')).toHaveLength(2);
91+
});
92+
93+
expect(container.querySelector('[data-node-name="Node 1"]')).toBeInTheDocument();
94+
expect(container.querySelector('[data-node-name="Node 2"]')).toBeInTheDocument();
95+
});
96+
97+
it('should update node classes when pinning/unpinning nodes', async () => {
98+
const { container } = renderComponent();
99+
100+
await waitFor(() => {
101+
expect(container.querySelector('[data-node-name="Node 1"]')).toBeInTheDocument();
102+
});
103+
104+
await waitFor(() => {
105+
expect(container.querySelector('[data-node-name="Node 1"]')).toHaveClass(
106+
'canvasNode pinnedNode',
107+
);
108+
expect(container.querySelector('[data-node-name="Node 2"]')).toHaveClass(
109+
'canvasNode notPinnedNode',
110+
);
111+
});
112+
});
113+
114+
it('should emit update:modelValue when pinning nodes', async () => {
115+
const { container, emitted, getAllByTestId } = renderComponent();
116+
117+
await waitFor(() => {
118+
expect(container.querySelector('[data-node-name="Node 1"]')).toBeInTheDocument();
119+
});
120+
const pinButton = getAllByTestId('node-pin-button')[1];
121+
pinButton?.click();
122+
123+
expect(emitted('update:modelValue')).toBeTruthy();
124+
expect(emitted('update:modelValue')[0]).toEqual([
125+
[
126+
{ id: '1', name: 'Node 1' },
127+
{ id: '2', name: 'Node 2' },
128+
],
129+
]);
130+
});
131+
it('should emit update:modelValue when unpinning nodes', async () => {
132+
const { container, emitted, getAllByTestId } = renderComponent();
133+
134+
await waitFor(() => {
135+
expect(container.querySelector('[data-node-name="Node 1"]')).toBeInTheDocument();
136+
});
137+
const pinButton = getAllByTestId('node-pin-button')[0];
138+
pinButton?.click();
139+
140+
expect(emitted('update:modelValue')).toBeTruthy();
141+
expect(emitted('update:modelValue')[0]).toEqual([[]]);
142+
});
143+
});

Diff for: ‎packages/editor-ui/src/components/TestDefinition/types.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export interface EvaluationFormState extends EditableFormState {
1616
description: string;
1717
evaluationWorkflow: INodeParameterResourceLocator;
1818
metrics: TestMetricRecord[];
19-
mockedNodes: Array<{ name: string }>;
19+
mockedNodes: Array<{ name: string; id: string }>;
2020
}
2121

2222
export interface TestExecution {

Diff for: ‎packages/editor-ui/src/plugins/i18n/locales/en.json

+11-12
Original file line numberDiff line numberDiff line change
@@ -2786,20 +2786,19 @@
27862786
"testDefinition.edit.testSaved": "Test saved",
27872787
"testDefinition.edit.testSaveFailed": "Failed to save test",
27882788
"testDefinition.edit.description": "Description",
2789-
"testDefinition.edit.description.tooltip": "Add details about what this test evaluates and what success looks like",
2789+
"testDefinition.edit.description.description": "Add details about what this test evaluates and what success looks like",
27902790
"testDefinition.edit.tagName": "Tag name",
27912791
"testDefinition.edit.step.intro": "When running a test",
2792-
"testDefinition.edit.step.executions": "Fetch past executions | Fetch {count} past execution | Fetch {count} past executions",
2793-
"testDefinition.edit.step.executions.tooltip": "Select which tagged executions to use as test cases. Each execution will be replayed to compare performance",
2794-
"testDefinition.edit.step.nodes": "Mock nodes",
2795-
"testDefinition.edit.step.mockedNodes": "No nodes mocked | {count} node mocked | {count} nodes mocked",
2796-
"testDefinition.edit.step.nodes.tooltip": "Replace specific nodes with test data to isolate what you're testing",
2797-
"testDefinition.edit.step.reRunExecutions": "Re-run executions",
2798-
"testDefinition.edit.step.reRunExecutions.tooltip": "Each test case will be re-run using the current workflow version",
2799-
"testDefinition.edit.step.compareExecutions": "Compare each past and new execution",
2800-
"testDefinition.edit.step.compareExecutions.tooltip": "Select which workflow to use for running the comparison tests",
2801-
"testDefinition.edit.step.metrics": "Summarise metrics",
2802-
"testDefinition.edit.step.metrics.tooltip": "Define which output fields to track and compare between test runs",
2792+
"testDefinition.edit.step.executions": "1. Fetch N past executions tagge | Fetch {count} past execution tagged | Fetch {count} past executions tagged",
2793+
"testDefinition.edit.step.executions.description": "Use a tag to select past executions for use as test cases in evaluation. The trigger data from each of these past executions will be used as input to run your workflow. The outputs of past executions are used as benchmark and compared against to check whether performance has changed based on logic and metrics that you define below.",
2794+
"testDefinition.edit.step.mockedNodes": "2. Mock N nodes |2. Mock {count} node |2. Mock {count} nodes",
2795+
"testDefinition.edit.step.nodes.description": "Mocked nodes have their data replayed rather than being re-executed. Do this to avoid calling external services, save time executing, and isolate what you are evaluating. If a node is mocked, the tagged past execution's output data for that node is used in the evaluation instead.",
2796+
"testDefinition.edit.step.reRunExecutions": "3. Simulation",
2797+
"testDefinition.edit.step.reRunExecutions.description": "Each past execution is re-run using the latest version of the workflow being tested. Outputs from both the evaluated and comparison data will be passed to evaluation logic.",
2798+
"testDefinition.edit.step.compareExecutions": "4. Compare each past and new execution",
2799+
"testDefinition.edit.step.compareExecutions.description": "Each past execution is compared with its new equivalent to check for changes in performance. This is done using a separate evaluation workflow: it receives the two execution versions as input, and outputs metrics based on logic that you define.",
2800+
"testDefinition.edit.step.metrics": "5. Summarise metrics",
2801+
"testDefinition.edit.step.metrics.description": "The names of metrics fields returned by the evaluation workflow (defined above). If included in this section, they are displayed in the test run results and averaged to give a score for the entire test run.",
28032802
"testDefinition.edit.step.collapse": "Collapse",
28042803
"testDefinition.edit.step.expand": "Expand",
28052804
"testDefinition.edit.selectNodes": "Select nodes",

0 commit comments

Comments
 (0)