fix(editor): Fix workflow initilisation for test definition routes & add unit tests (#12507)

This commit is contained in:
oleg 2025-01-08 16:07:46 +01:00 committed by GitHub
parent c28f302c2f
commit 2775f617ae
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 302 additions and 83 deletions

View file

@ -11,7 +11,7 @@ export interface TestDefinitionRecord {
updatedAt?: string;
createdAt?: string;
annotationTag?: string | null;
mockedNodes?: Array<{ name: string }>;
mockedNodes?: Array<{ name: string; id: string }>;
}
interface CreateTestDefinitionParams {
@ -25,7 +25,7 @@ export interface UpdateTestDefinitionParams {
evaluationWorkflowId?: string | null;
annotationTagId?: string | null;
description?: string | null;
mockedNodes?: Array<{ name: string }>;
mockedNodes?: Array<{ name: string; id: string }>;
}
export interface UpdateTestResponse {

View file

@ -2,14 +2,13 @@
import { useI18n } from '@/composables/useI18n';
import { ElCollapseTransition } from 'element-plus';
import { ref, nextTick } from 'vue';
import N8nTooltip from 'n8n-design-system/components/N8nTooltip';
interface EvaluationStep {
title: string;
warning?: boolean;
small?: boolean;
expanded?: boolean;
tooltip?: string;
description?: string;
}
const props = withDefaults(defineProps<EvaluationStep>(), {
@ -17,14 +16,12 @@ const props = withDefaults(defineProps<EvaluationStep>(), {
warning: false,
small: false,
expanded: true,
tooltip: '',
});
const locale = useI18n();
const isExpanded = ref(props.expanded);
const contentRef = ref<HTMLElement | null>(null);
const containerRef = ref<HTMLElement | null>(null);
const isTooltipVisible = ref(false);
const toggleExpand = async () => {
isExpanded.value = !isExpanded.value;
@ -35,14 +32,6 @@ const toggleExpand = async () => {
}
}
};
const showTooltip = () => {
isTooltipVisible.value = true;
};
const hideTooltip = () => {
isTooltipVisible.value = false;
};
</script>
<template>
@ -51,16 +40,7 @@ const hideTooltip = () => {
:class="[$style.evaluationStep, small && $style.small]"
data-test-id="evaluation-step"
>
<N8nTooltip :disabled="!tooltip" placement="right" :offset="25" :visible="isTooltipVisible">
<template #content>
{{ tooltip }}
</template>
<!-- This empty div is needed to ensure the tooltip trigger area spans the full width of the step.
Without it, the tooltip would only show when hovering over the content div, which is narrower.
The contentPlaceholder creates an invisible full-width area that can trigger the tooltip. -->
<div :class="$style.contentPlaceholder"></div>
</N8nTooltip>
<div :class="$style.content" @mouseenter="showTooltip" @mouseleave="hideTooltip">
<div :class="$style.content">
<div :class="$style.header">
<div :class="[$style.icon, warning && $style.warning]">
<slot name="icon" />
@ -83,6 +63,7 @@ const hideTooltip = () => {
<font-awesome-icon :icon="isExpanded ? 'angle-down' : 'angle-right'" size="lg" />
</button>
</div>
<div v-if="description" :class="$style.description">{{ description }}</div>
<ElCollapseTransition v-if="$slots.cardContent">
<div v-show="isExpanded" :class="$style.cardContentWrapper">
<div ref="contentRef" :class="$style.cardContent" data-test-id="evaluation-step-content">
@ -111,14 +92,6 @@ const hideTooltip = () => {
width: 80%;
}
}
.contentPlaceholder {
position: absolute;
top: 0;
left: 0;
width: 100%;
height: 100%;
z-index: -1;
}
.icon {
display: flex;
align-items: center;
@ -173,4 +146,10 @@ const hideTooltip = () => {
.cardContentWrapper {
height: max-content;
}
.description {
font-size: var(--font-size-2xs);
color: var(--color-text-light);
line-height: 1rem;
}
</style>

View file

@ -22,11 +22,11 @@ const eventBus = createEventBus<CanvasEventBusEvents>();
const style = useCssModule();
const uuid = crypto.randomUUID();
const props = defineProps<{
modelValue: Array<{ name: string }>;
modelValue: Array<{ name: string; id: string }>;
}>();
const emit = defineEmits<{
'update:modelValue': [value: Array<{ name: string }>];
'update:modelValue': [value: Array<{ name: string; id: string }>];
}>();
const isLoading = ref(true);
@ -84,14 +84,7 @@ function disableAllNodes() {
const ids = mappedNodes.value.map((node) => node.id);
updateNodeClasses(ids, false);
const pinnedNodes = props.modelValue
.map((node) => {
const matchedNode = mappedNodes.value.find(
(mappedNode) => mappedNode?.data?.name === node.name,
);
return matchedNode?.id ?? null;
})
.filter((n) => n !== null);
const pinnedNodes = props.modelValue.map((node) => node.id).filter((id) => id !== null);
if (pinnedNodes.length > 0) {
updateNodeClasses(pinnedNodes, true);
@ -101,10 +94,10 @@ function onPinButtonClick(data: CanvasNodeData) {
const nodeName = getNodeNameById(data.id);
if (!nodeName) return;
const isPinned = props.modelValue.some((node) => node.name === nodeName);
const isPinned = props.modelValue.some((node) => node.id === data.id);
const updatedNodes = isPinned
? props.modelValue.filter((node) => node.name !== nodeName)
: [...props.modelValue, { name: nodeName }];
? props.modelValue.filter((node) => node.id !== data.id)
: [...props.modelValue, { name: nodeName, id: data.id }];
emit('update:modelValue', updatedNodes);
updateNodeClasses([data.id], !isPinned);
@ -145,6 +138,7 @@ onMounted(loadData);
size="large"
icon="thumbtack"
:class="$style.pinButton"
data-test-id="node-pin-button"
@click="onPinButtonClick(data)"
/>
</N8nTooltip>

View file

@ -0,0 +1,143 @@
import { waitFor } from '@testing-library/vue';
import { createPinia, setActivePinia } from 'pinia';
import { createTestingPinia } from '@pinia/testing';
import NodesPinning from '../NodesPinning.vue';
import { createComponentRenderer } from '@/__tests__/render';
import { useWorkflowsStore } from '@/stores/workflows.store';
import { useNodeTypesStore } from '@/stores/nodeTypes.store';
import {
createTestNode,
createTestWorkflow,
createTestWorkflowObject,
mockNodeTypeDescription,
} from '@/__tests__/mocks';
import { mockedStore } from '@/__tests__/utils';
import { NodeConnectionType } from 'n8n-workflow';
import { SET_NODE_TYPE } from '@/constants';
vi.mock('vue-router', () => {
const push = vi.fn();
return {
useRouter: () => ({
push,
}),
useRoute: () => ({
params: {
name: 'test-workflow',
testId: 'test-123',
},
}),
RouterLink: {
template: '<a><slot /></a>',
},
};
});
const renderComponent = createComponentRenderer(NodesPinning, {
props: {
modelValue: [{ id: '1', name: 'Node 1' }],
},
global: {
plugins: [createTestingPinia()],
},
});
describe('NodesPinning', () => {
const workflowsStore = mockedStore(useWorkflowsStore);
const nodes = [
createTestNode({ id: '1', name: 'Node 1', type: SET_NODE_TYPE }),
createTestNode({ id: '2', name: 'Node 2', type: SET_NODE_TYPE }),
];
const nodeTypesStore = mockedStore(useNodeTypesStore);
const nodeTypeDescription = mockNodeTypeDescription({
name: SET_NODE_TYPE,
inputs: [NodeConnectionType.Main],
outputs: [NodeConnectionType.Main],
});
nodeTypesStore.nodeTypes = {
node: { 1: nodeTypeDescription },
};
nodeTypesStore.getNodeType = vi.fn().mockReturnValue(nodeTypeDescription);
const workflow = createTestWorkflow({
id: 'test-workflow',
name: 'Test Workflow',
nodes,
connections: {},
});
const workflowObject = createTestWorkflowObject(workflow);
workflowsStore.getWorkflowById = vi.fn().mockReturnValue(workflow);
workflowsStore.getCurrentWorkflow = vi.fn().mockReturnValue(workflowObject);
beforeEach(() => {
const pinia = createPinia();
setActivePinia(pinia);
nodeTypesStore.setNodeTypes([nodeTypeDescription]);
});
afterEach(() => {
vi.clearAllMocks();
});
it('should render workflow nodes', async () => {
const { container } = renderComponent();
await waitFor(() => {
expect(container.querySelectorAll('.vue-flow__node')).toHaveLength(2);
});
expect(container.querySelector('[data-node-name="Node 1"]')).toBeInTheDocument();
expect(container.querySelector('[data-node-name="Node 2"]')).toBeInTheDocument();
});
it('should update node classes when pinning/unpinning nodes', async () => {
const { container } = renderComponent();
await waitFor(() => {
expect(container.querySelector('[data-node-name="Node 1"]')).toBeInTheDocument();
});
await waitFor(() => {
expect(container.querySelector('[data-node-name="Node 1"]')).toHaveClass(
'canvasNode pinnedNode',
);
expect(container.querySelector('[data-node-name="Node 2"]')).toHaveClass(
'canvasNode notPinnedNode',
);
});
});
it('should emit update:modelValue when pinning nodes', async () => {
const { container, emitted, getAllByTestId } = renderComponent();
await waitFor(() => {
expect(container.querySelector('[data-node-name="Node 1"]')).toBeInTheDocument();
});
const pinButton = getAllByTestId('node-pin-button')[1];
pinButton?.click();
expect(emitted('update:modelValue')).toBeTruthy();
expect(emitted('update:modelValue')[0]).toEqual([
[
{ id: '1', name: 'Node 1' },
{ id: '2', name: 'Node 2' },
],
]);
});
it('should emit update:modelValue when unpinning nodes', async () => {
const { container, emitted, getAllByTestId } = renderComponent();
await waitFor(() => {
expect(container.querySelector('[data-node-name="Node 1"]')).toBeInTheDocument();
});
const pinButton = getAllByTestId('node-pin-button')[0];
pinButton?.click();
expect(emitted('update:modelValue')).toBeTruthy();
expect(emitted('update:modelValue')[0]).toEqual([[]]);
});
});

View file

@ -16,7 +16,7 @@ export interface EvaluationFormState extends EditableFormState {
description: string;
evaluationWorkflow: INodeParameterResourceLocator;
metrics: TestMetricRecord[];
mockedNodes: Array<{ name: string }>;
mockedNodes: Array<{ name: string; id: string }>;
}
export interface TestExecution {

View file

@ -2786,20 +2786,19 @@
"testDefinition.edit.testSaved": "Test saved",
"testDefinition.edit.testSaveFailed": "Failed to save test",
"testDefinition.edit.description": "Description",
"testDefinition.edit.description.tooltip": "Add details about what this test evaluates and what success looks like",
"testDefinition.edit.description.description": "Add details about what this test evaluates and what success looks like",
"testDefinition.edit.tagName": "Tag name",
"testDefinition.edit.step.intro": "When running a test",
"testDefinition.edit.step.executions": "Fetch past executions | Fetch {count} past execution | Fetch {count} past executions",
"testDefinition.edit.step.executions.tooltip": "Select which tagged executions to use as test cases. Each execution will be replayed to compare performance",
"testDefinition.edit.step.nodes": "Mock nodes",
"testDefinition.edit.step.mockedNodes": "No nodes mocked | {count} node mocked | {count} nodes mocked",
"testDefinition.edit.step.nodes.tooltip": "Replace specific nodes with test data to isolate what you're testing",
"testDefinition.edit.step.reRunExecutions": "Re-run executions",
"testDefinition.edit.step.reRunExecutions.tooltip": "Each test case will be re-run using the current workflow version",
"testDefinition.edit.step.compareExecutions": "Compare each past and new execution",
"testDefinition.edit.step.compareExecutions.tooltip": "Select which workflow to use for running the comparison tests",
"testDefinition.edit.step.metrics": "Summarise metrics",
"testDefinition.edit.step.metrics.tooltip": "Define which output fields to track and compare between test runs",
"testDefinition.edit.step.executions": "1. Fetch N past executions tagge | Fetch {count} past execution tagged | Fetch {count} past executions tagged",
"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.",
"testDefinition.edit.step.mockedNodes": "2. Mock N nodes |2. Mock {count} node |2. Mock {count} nodes",
"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.",
"testDefinition.edit.step.reRunExecutions": "3. Simulation",
"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.",
"testDefinition.edit.step.compareExecutions": "4. Compare each past and new execution",
"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.",
"testDefinition.edit.step.metrics": "5. Summarise metrics",
"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.",
"testDefinition.edit.step.collapse": "Collapse",
"testDefinition.edit.step.expand": "Expand",
"testDefinition.edit.selectNodes": "Select nodes",

View file

@ -63,6 +63,8 @@ const TestDefinitionListView = async () =>
await import('./views/TestDefinition/TestDefinitionListView.vue');
const TestDefinitionEditView = async () =>
await import('./views/TestDefinition/TestDefinitionEditView.vue');
const TestDefinitionRootView = async () =>
await import('./views/TestDefinition/TestDefinitionRootView.vue');
function getTemplatesRedirect(defaultRedirect: VIEWS[keyof VIEWS]): { name: string } | false {
const settingsStore = useSettingsStore();
@ -257,6 +259,11 @@ export const routes: RouteRecordRaw[] = [
},
{
path: '/workflow/:name/evaluation',
components: {
default: TestDefinitionRootView,
header: MainHeader,
sidebar: MainSidebar,
},
meta: {
keepWorkflowAlive: true,
middleware: ['authenticated'],
@ -267,8 +274,6 @@ export const routes: RouteRecordRaw[] = [
name: VIEWS.TEST_DEFINITION,
components: {
default: TestDefinitionListView,
header: MainHeader,
sidebar: MainSidebar,
},
meta: {
keepWorkflowAlive: true,
@ -280,8 +285,6 @@ export const routes: RouteRecordRaw[] = [
name: VIEWS.NEW_TEST_DEFINITION,
components: {
default: TestDefinitionEditView,
header: MainHeader,
sidebar: MainSidebar,
},
meta: {
keepWorkflowAlive: true,
@ -293,8 +296,6 @@ export const routes: RouteRecordRaw[] = [
name: VIEWS.TEST_DEFINITION_EDIT,
components: {
default: TestDefinitionEditView,
header: MainHeader,
sidebar: MainSidebar,
},
meta: {
keepWorkflowAlive: true,
@ -306,8 +307,6 @@ export const routes: RouteRecordRaw[] = [
name: VIEWS.TEST_DEFINITION_RUNS,
components: {
default: TestDefinitionRunsListView,
header: MainHeader,
sidebar: MainSidebar,
},
meta: {
keepWorkflowAlive: true,
@ -319,8 +318,6 @@ export const routes: RouteRecordRaw[] = [
name: VIEWS.TEST_DEFINITION_RUNS_DETAIL,
components: {
default: TestDefinitionRunDetailView,
header: MainHeader,
sidebar: MainSidebar,
},
meta: {
keepWorkflowAlive: true,

View file

@ -180,7 +180,6 @@ watch(
:class="$style.step"
:title="locale.baseText('testDefinition.edit.description')"
:expanded="false"
:tooltip="locale.baseText('testDefinition.edit.description.tooltip')"
>
<template #icon><font-awesome-icon icon="thumbtack" size="lg" /></template>
<template #cardContent>
@ -200,7 +199,7 @@ watch(
adjustToNumber: tagUsageCount,
})
"
:tooltip="locale.baseText('testDefinition.edit.step.executions.tooltip')"
:description="locale.baseText('testDefinition.edit.step.executions.description')"
>
<template #icon><font-awesome-icon icon="history" size="lg" /></template>
<template #cardContent>
@ -232,7 +231,7 @@ watch(
"
:small="true"
:expanded="true"
:tooltip="locale.baseText('testDefinition.edit.step.nodes.tooltip')"
:description="locale.baseText('testDefinition.edit.step.nodes.description')"
>
<template #icon><font-awesome-icon icon="thumbtack" size="lg" /></template>
<template #cardContent>
@ -251,7 +250,7 @@ watch(
:class="$style.step"
:title="locale.baseText('testDefinition.edit.step.reRunExecutions')"
:small="true"
:tooltip="locale.baseText('testDefinition.edit.step.reRunExecutions.tooltip')"
:description="locale.baseText('testDefinition.edit.step.reRunExecutions.description')"
>
<template #icon><font-awesome-icon icon="redo" size="lg" /></template>
</EvaluationStep>
@ -260,7 +259,7 @@ watch(
<EvaluationStep
:class="$style.step"
:title="locale.baseText('testDefinition.edit.step.compareExecutions')"
:tooltip="locale.baseText('testDefinition.edit.step.compareExecutions.tooltip')"
:description="locale.baseText('testDefinition.edit.step.compareExecutions.description')"
>
<template #icon><font-awesome-icon icon="equals" size="lg" /></template>
<template #cardContent>
@ -275,7 +274,7 @@ watch(
<EvaluationStep
:class="$style.step"
:title="locale.baseText('testDefinition.edit.step.metrics')"
:tooltip="locale.baseText('testDefinition.edit.step.metrics.tooltip')"
:description="locale.baseText('testDefinition.edit.step.metrics.description')"
>
<template #icon><font-awesome-icon icon="chart-bar" size="lg" /></template>
<template #cardContent>
@ -339,12 +338,13 @@ watch(
<style module lang="scss">
.container {
--evaluation-edit-panel-width: 35rem;
width: 100%;
height: 100%;
overflow: hidden;
padding: var(--spacing-s);
display: grid;
grid-template-columns: minmax(auto, 24rem) 1fr;
grid-template-columns: minmax(auto, var(--evaluation-edit-panel-width)) 1fr;
gap: var(--spacing-2xl);
}
@ -371,7 +371,7 @@ watch(
margin-bottom: var(--spacing-xl);
}
.panelBlock {
max-width: var(--evaluation-edit-panel-width, 24rem);
max-width: var(--evaluation-edit-panel-width, 35rem);
display: grid;
overflow-y: auto;
min-height: 0;
@ -396,7 +396,7 @@ watch(
justify-self: center;
}
.evaluationArrows {
--arrow-height: 13.8rem;
--arrow-height: 22rem;
display: flex;
justify-content: space-between;
width: 100%;

View file

@ -0,0 +1,25 @@
<script setup lang="ts">
import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers';
import { useWorkflowsStore } from '@/stores/workflows.store';
import { useRouter } from 'vue-router';
import { onMounted } from 'vue';
const router = useRouter();
const workflowHelpers = useWorkflowHelpers({ router });
const workflowStore = useWorkflowsStore();
async function initWorkflow() {
const workflowId = router.currentRoute.value.params.name as string;
const isAlreadyInitialized = workflowStore.workflow.id === workflowId;
if (isAlreadyInitialized) return;
const workflow = await workflowStore.fetchWorkflow(workflowId);
void workflowHelpers.initState(workflow);
}
onMounted(initWorkflow);
</script>
<template>
<router-view />
</template>

View file

@ -11,7 +11,6 @@ import { useExecutionsStore } from '@/stores/executions.store';
import { get } from 'lodash-es';
import type { ExecutionSummaryWithScopes } from '@/Interface';
import { VIEWS } from '@/constants';
import { useWorkflowsStore } from '@/stores/workflows.store';
import { useToast } from '@/composables/useToast';
interface TestCase extends ExecutionSummaryWithScopes {
@ -22,7 +21,6 @@ const router = useRouter();
const toast = useToast();
const testDefinitionStore = useTestDefinitionStore();
const executionsStore = useExecutionsStore();
const workflowStore = useWorkflowsStore();
const locale = useI18n();
const isLoading = ref(true);
@ -33,9 +31,6 @@ const testId = computed(() => router.currentRoute.value.params.testId as string)
const run = computed(() => testDefinitionStore.testRunsById[runId.value]);
const test = computed(() => testDefinitionStore.testDefinitionsById[testId.value]);
const workflow = computed(
() => workflowStore.workflowsById[test.value?.evaluationWorkflowId ?? ''],
);
const filteredTestCases = computed(() => {
return testCases.value;
});
@ -51,7 +46,7 @@ const columns = computed(
name: VIEWS.EXECUTION_PREVIEW,
params: { name: row.workflowId, executionId: row.id },
}),
formatter: (row: TestCase) => `[${row.id}] ${workflow.value?.name}`,
formatter: (row: TestCase) => `[${row.id}] ${row.workflowName}`,
openInNewTab: true,
},
{

View file

@ -0,0 +1,87 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { createPinia, setActivePinia } from 'pinia';
import { createTestingPinia } from '@pinia/testing';
import { createComponentRenderer } from '@/__tests__/render';
import TestDefinitionRootView from '../TestDefinitionRootView.vue';
import { useRouter } from 'vue-router';
import { useWorkflowsStore } from '@/stores/workflows.store';
import { mockedStore } from '@/__tests__/utils';
import type { IWorkflowDb } from '@/Interface';
import * as workflowsApi from '@/api/workflows';
vi.mock('vue-router');
vi.mock('@/api/workflows');
describe('TestDefinitionRootView', () => {
const renderComponent = createComponentRenderer(TestDefinitionRootView);
const mockWorkflow: IWorkflowDb = {
id: 'different-id',
name: 'Test Workflow',
active: false,
createdAt: Date.now(),
updatedAt: Date.now(),
nodes: [],
connections: {},
settings: {
executionOrder: 'v1',
},
tags: [],
pinData: {},
versionId: '',
usedCredentials: [],
};
beforeEach(() => {
setActivePinia(createPinia());
vi.mocked(useRouter).mockReturnValue({
currentRoute: {
value: {
params: {
name: 'workflow123',
},
},
},
} as unknown as ReturnType<typeof useRouter>);
vi.mocked(workflowsApi.getWorkflow).mockResolvedValue({
...mockWorkflow,
id: 'workflow123',
});
});
it('should initialize workflow on mount if not already initialized', async () => {
const pinia = createTestingPinia();
const workflowsStore = mockedStore(useWorkflowsStore);
workflowsStore.workflow = mockWorkflow;
const newWorkflow = {
...mockWorkflow,
id: 'workflow123',
};
workflowsStore.fetchWorkflow.mockResolvedValue(newWorkflow);
renderComponent({ pinia });
expect(workflowsStore.fetchWorkflow).toHaveBeenCalledWith('workflow123');
});
it('should not initialize workflow if already loaded', async () => {
const pinia = createTestingPinia();
const workflowsStore = mockedStore(useWorkflowsStore);
workflowsStore.workflow = {
...mockWorkflow,
id: 'workflow123',
};
renderComponent({ pinia });
expect(workflowsStore.fetchWorkflow).not.toHaveBeenCalled();
});
it('should render router view', () => {
const { container } = renderComponent();
expect(container.querySelector('router-view')).toBeTruthy();
});
});