feat(core): Automatically extract evaluation metrics (no-changelog) (#14051)

This commit is contained in:
oleg
2025-03-25 15:43:19 +01:00
committed by GitHub
parent 22e6569f7e
commit 53e11b19ad
28 changed files with 460 additions and 1286 deletions

View File

@@ -1,70 +0,0 @@
<script setup lang="ts">
import { useTemplateRef, nextTick } from 'vue';
import type { TestMetricRecord } from '@/api/testDefinition.ee';
import { useI18n } from '@/composables/useI18n';
import { N8nInput, N8nButton, N8nIconButton } from '@n8n/design-system';
export interface MetricsInputProps {
modelValue: Array<Partial<TestMetricRecord>>;
}
const props = defineProps<MetricsInputProps>();
const emit = defineEmits<{
'update:modelValue': [value: MetricsInputProps['modelValue']];
deleteMetric: [metric: TestMetricRecord];
}>();
const locale = useI18n();
const metricsRefs = useTemplateRef<Array<InstanceType<typeof N8nInput>>>('metric');
function addNewMetric() {
emit('update:modelValue', [...props.modelValue, { name: '' }]);
void nextTick(() => metricsRefs.value?.at(-1)?.focus());
}
function updateMetric(index: number, name: string) {
const newMetrics = [...props.modelValue];
newMetrics[index].name = name;
emit('update:modelValue', newMetrics);
}
function onDeleteMetric(metric: Partial<TestMetricRecord>, index: number) {
if (!metric.id) {
const newMetrics = [...props.modelValue];
newMetrics.splice(index, 1);
emit('update:modelValue', newMetrics);
} else {
emit('deleteMetric', metric as TestMetricRecord);
}
}
</script>
<template>
<div>
<div
v-for="(metric, index) in modelValue"
:key="index"
:class="$style.metricItem"
class="mb-xs"
>
<N8nInput
ref="metric"
data-test-id="evaluation-metric-item"
:model-value="metric.name"
:placeholder="locale.baseText('testDefinition.edit.metricsPlaceholder')"
@update:model-value="(value: string) => updateMetric(index, value)"
/>
<N8nIconButton icon="trash" type="secondary" text @click="onDeleteMetric(metric, index)" />
</div>
<N8nButton
type="secondary"
:label="locale.baseText('testDefinition.edit.metricsNew')"
@click="addNewMetric"
/>
</div>
</template>
<style module lang="scss">
.metricItem {
display: flex;
align-items: center;
}
</style>

View File

@@ -1,8 +1,6 @@
<script setup lang="ts">
import type { TestMetricRecord } from '@/api/testDefinition.ee';
import BlockArrow from '@/components/TestDefinition/EditDefinition/BlockArrow.vue';
import EvaluationStep from '@/components/TestDefinition/EditDefinition/EvaluationStep.vue';
import MetricsInput from '@/components/TestDefinition/EditDefinition/MetricsInput.vue';
import NodesPinning from '@/components/TestDefinition/EditDefinition/NodesPinning.vue';
import WorkflowSelector from '@/components/TestDefinition/EditDefinition/WorkflowSelector.vue';
import type { EditableFormState, EvaluationFormState } from '@/components/TestDefinition/types';
@@ -27,7 +25,6 @@ const props = defineProps<{
}>();
const emit = defineEmits<{
openPinningModal: [];
deleteMetric: [metric: TestMetricRecord];
openExecutionsViewForTag: [];
renameTag: [tag: string];
evaluationWorkflowCreated: [workflowId: string];
@@ -64,7 +61,6 @@ const evaluationWorkflow = defineModel<EvaluationFormState['evaluationWorkflow']
'evaluationWorkflow',
{ required: true },
);
const metrics = defineModel<EvaluationFormState['metrics']>('metrics', { required: true });
const mockedNodes = defineModel<EvaluationFormState['mockedNodes']>('mockedNodes', {
required: true,
});
@@ -177,25 +173,6 @@ function openExecutionsView() {
/>
</template>
</EvaluationStep>
<BlockArrow class="mt-5xs mb-5xs" />
<!-- Metrics -->
<EvaluationStep
:title="locale.baseText('testDefinition.edit.step.metrics')"
:issues="getFieldIssues('metrics')"
:description="locale.baseText('testDefinition.edit.step.metrics.description')"
:tooltip="locale.baseText('testDefinition.edit.step.metrics.tooltip')"
:external-tooltip="!hasRuns"
>
<template #cardContent>
<MetricsInput
v-model="metrics"
:class="{ 'has-issues': getFieldIssues('metrics').length > 0 }"
class="mt-xs"
@delete-metric="(metric) => emit('deleteMetric', metric)"
/>
</template>
</EvaluationStep>
</div>
<Modal
width="calc(100% - (48px * 2))"

View File

@@ -36,7 +36,6 @@ export function useTestDefinitionForm() {
value: '',
__rl: true,
},
metrics: [],
mockedNodes: [],
});
@@ -62,8 +61,6 @@ export function useTestDefinitionForm() {
const testDefinition = evaluationsStore.testDefinitionsById[testId];
if (testDefinition) {
const metrics = await evaluationsStore.fetchMetrics(testId);
state.value.description = {
value: testDefinition.description ?? '',
isEditing: false,
@@ -84,7 +81,6 @@ export function useTestDefinitionForm() {
value: testDefinition.evaluationWorkflowId ?? '',
__rl: true,
};
state.value.metrics = metrics;
state.value.mockedNodes = testDefinition.mockedNodes ?? [];
evaluationsStore.updateRunFieldIssues(testDefinition.id);
}
@@ -110,37 +106,6 @@ export function useTestDefinitionForm() {
}
};
const deleteMetric = async (metricId: string, testId: string) => {
await evaluationsStore.deleteMetric({ id: metricId, testDefinitionId: testId });
state.value.metrics = state.value.metrics.filter((metric) => metric.id !== metricId);
};
/**
* This method would perform unnecessary updates on the BE
* it's a performance degradation candidate if metrics reach certain amount
*/
const updateMetrics = async (testId: string) => {
const promises = state.value.metrics.map(async (metric) => {
if (!metric.name) return;
if (!metric.id) {
const createdMetric = await evaluationsStore.createMetric({
name: metric.name,
testDefinitionId: testId,
});
metric.id = createdMetric.id;
} else {
await evaluationsStore.updateMetric({
name: metric.name,
id: metric.id,
testDefinitionId: testId,
});
}
});
isSaving.value = true;
await Promise.all(promises);
isSaving.value = false;
};
const updateTest = async (testId: string) => {
if (isSaving.value) return;
@@ -230,8 +195,6 @@ export function useTestDefinitionForm() {
state,
fields,
isSaving: computed(() => isSaving.value),
deleteMetric,
updateMetrics,
loadTestData,
createTest,
updateTest,

View File

@@ -27,8 +27,6 @@ const errorTooltipMap: Record<string, BaseTextKey> = {
FAILED_TO_EXECUTE_EVALUATION_WORKFLOW: 'testDefinition.runDetail.error.evaluationFailed',
FAILED_TO_EXECUTE_WORKFLOW: 'testDefinition.runDetail.error.executionFailed',
TRIGGER_NO_LONGER_EXISTS: 'testDefinition.runDetail.error.triggerNoLongerExists',
METRICS_MISSING: 'testDefinition.runDetail.error.metricsMissing',
UNKNOWN_METRICS: 'testDefinition.runDetail.error.unknownMetrics',
INVALID_METRICS: 'testDefinition.runDetail.error.invalidMetrics',
// Test run errors

View File

@@ -1,142 +0,0 @@
import { describe, it, expect, beforeEach } from 'vitest';
import { createComponentRenderer } from '@/__tests__/render';
import MetricsInput from '../EditDefinition/MetricsInput.vue';
import userEvent from '@testing-library/user-event';
const renderComponent = createComponentRenderer(MetricsInput);
describe('MetricsInput', () => {
let props: { modelValue: Array<{ id?: string; name: string }> };
beforeEach(() => {
props = {
modelValue: [
{ name: 'Metric 1', id: 'metric-1' },
{ name: 'Metric 2', id: 'metric-2' },
],
};
});
it('should render correctly with initial metrics', () => {
const { getAllByPlaceholderText } = renderComponent({ props });
const inputs = getAllByPlaceholderText('e.g. latency');
expect(inputs).toHaveLength(2);
expect(inputs[0]).toHaveValue('Metric 1');
expect(inputs[1]).toHaveValue('Metric 2');
});
it('should update a metric when typing in the input', async () => {
const { getAllByPlaceholderText, emitted } = renderComponent({
props: {
modelValue: [{ name: '' }],
},
});
const inputs = getAllByPlaceholderText('e.g. latency');
await userEvent.type(inputs[0], 'Updated Metric 1');
// Every character typed triggers an update event. Let's check the last emission.
const allEmits = emitted('update:modelValue');
expect(allEmits).toBeTruthy();
// The last emission should contain the fully updated name
const lastEmission = allEmits[allEmits.length - 1];
expect(lastEmission).toEqual([[{ name: 'Updated Metric 1' }]]);
});
it('should render correctly with no initial metrics', () => {
props.modelValue = [];
const { queryAllByRole, getByText } = renderComponent({ props });
const inputs = queryAllByRole('textbox');
expect(inputs).toHaveLength(0);
expect(getByText('New metric')).toBeInTheDocument();
});
it('should handle adding multiple metrics', async () => {
const { getByText, emitted } = renderComponent({ props });
const addButton = getByText('New metric');
await userEvent.click(addButton);
await userEvent.click(addButton);
await userEvent.click(addButton);
// Each click adds a new metric
const updateEvents = emitted('update:modelValue');
expect(updateEvents).toHaveLength(3);
// Check the structure of one of the emissions
// Initial: [{ name: 'Metric 1' }, { name: 'Metric 2' }]
// After first click: [{ name: 'Metric 1' }, { name: 'Metric 2' }, { name: '' }]
expect(updateEvents[0]).toEqual([[...props.modelValue, { name: '' }]]);
});
it('should emit "deleteMetric" event when a delete button is clicked', async () => {
const { getAllByRole, emitted } = renderComponent({ props });
// Each metric row has a delete button, identified by "button"
const deleteButtons = getAllByRole('button', { name: '' });
expect(deleteButtons).toHaveLength(props.modelValue.length);
// Click on the delete button for the second metric
await userEvent.click(deleteButtons[1]);
expect(emitted('deleteMetric')).toBeTruthy();
expect(emitted('deleteMetric')[0]).toEqual([props.modelValue[1]]);
});
it('should emit multiple update events as the user types and reflect the final name correctly', async () => {
const { getAllByPlaceholderText, emitted } = renderComponent({
props: {
modelValue: [{ name: '' }],
},
});
const inputs = getAllByPlaceholderText('e.g. latency');
await userEvent.type(inputs[0], 'ABC');
const allEmits = emitted('update:modelValue');
expect(allEmits).toBeTruthy();
// Each character typed should emit a new value
expect(allEmits.length).toBe(3);
expect(allEmits[2]).toEqual([[{ name: 'ABC' }]]);
});
it('should not break if metrics are empty and still allow adding a new metric', async () => {
props.modelValue = [];
const { queryAllByRole, getByText, emitted } = renderComponent({ props });
// No metrics initially
const inputs = queryAllByRole('textbox');
expect(inputs).toHaveLength(0);
const addButton = getByText('New metric');
await userEvent.click(addButton);
const updates = emitted('update:modelValue');
expect(updates).toBeTruthy();
expect(updates[0]).toEqual([[{ name: '' }]]);
// After adding one metric, we should now have an input
const { getAllByPlaceholderText } = renderComponent({
props: { modelValue: [{ name: '' }] },
});
const updatedInputs = getAllByPlaceholderText('e.g. latency');
expect(updatedInputs).toHaveLength(1);
});
it('should handle deleting the first metric and still display remaining metrics correctly', async () => {
const { getAllByPlaceholderText, getAllByRole, rerender, emitted } = renderComponent({
props,
});
const inputs = getAllByPlaceholderText('e.g. latency');
expect(inputs).toHaveLength(2);
const deleteButtons = getAllByRole('button', { name: '' });
await userEvent.click(deleteButtons[0]);
expect(emitted('deleteMetric')).toBeTruthy();
expect(emitted('deleteMetric')[0]).toEqual([props.modelValue[0]]);
await rerender({ modelValue: [{ name: 'Metric 2' }] });
const updatedInputs = getAllByPlaceholderText('e.g. latency');
expect(updatedInputs).toHaveLength(1);
expect(updatedInputs[0]).toHaveValue('Metric 2');
});
});

View File

@@ -48,20 +48,12 @@ describe('useTestDefinitionForm', () => {
expect(state.value.description.value).toBe('');
expect(state.value.name.value).toContain('My Test');
expect(state.value.tags.value).toEqual([]);
expect(state.value.metrics).toEqual([]);
expect(state.value.evaluationWorkflow.value).toBe('');
});
it('should load test data', async () => {
const { loadTestData, state } = useTestDefinitionForm();
const fetchSpy = vi.spyOn(useTestDefinitionStore(), 'fetchAll');
const fetchMetricsSpy = vi.spyOn(useTestDefinitionStore(), 'fetchMetrics').mockResolvedValue([
{
id: 'metric1',
name: 'Metric 1',
testDefinitionId: TEST_DEF_A.id,
},
]);
const evaluationsStore = mockedStore(useTestDefinitionStore);
evaluationsStore.testDefinitionsById = {
@@ -71,14 +63,10 @@ describe('useTestDefinitionForm', () => {
await loadTestData(TEST_DEF_A.id, '123');
expect(fetchSpy).toBeCalled();
expect(fetchMetricsSpy).toBeCalledWith(TEST_DEF_A.id);
expect(state.value.name.value).toEqual(TEST_DEF_A.name);
expect(state.value.description.value).toEqual(TEST_DEF_A.description);
expect(state.value.tags.value).toEqual([TEST_DEF_A.annotationTagId]);
expect(state.value.evaluationWorkflow.value).toEqual(TEST_DEF_A.evaluationWorkflowId);
expect(state.value.metrics).toEqual([
{ id: 'metric1', name: 'Metric 1', testDefinitionId: TEST_DEF_A.id },
]);
});
it('should gracefully handle loadTestData when no test definition found', async () => {
@@ -94,7 +82,6 @@ describe('useTestDefinitionForm', () => {
expect(state.value.description.value).toBe('');
expect(state.value.name.value).toContain('My Test');
expect(state.value.tags.value).toEqual([]);
expect(state.value.metrics).toEqual([]);
});
it('should handle errors while loading test data', async () => {
@@ -176,68 +163,6 @@ describe('useTestDefinitionForm', () => {
expect(updateSpy).toBeCalled();
});
it('should delete a metric', async () => {
const { state, deleteMetric } = useTestDefinitionForm();
const evaluationsStore = mockedStore(useTestDefinitionStore);
const deleteMetricSpy = vi.spyOn(evaluationsStore, 'deleteMetric');
state.value.metrics = [
{
id: 'metric1',
name: 'Metric 1',
testDefinitionId: '1',
},
{
id: 'metric2',
name: 'Metric 2',
testDefinitionId: '1',
},
];
await deleteMetric('metric1', TEST_DEF_A.id);
expect(deleteMetricSpy).toBeCalledWith({ id: 'metric1', testDefinitionId: TEST_DEF_A.id });
expect(state.value.metrics).toEqual([
{ id: 'metric2', name: 'Metric 2', testDefinitionId: '1' },
]);
});
it('should update metrics', async () => {
const { state, updateMetrics } = useTestDefinitionForm();
const evaluationsStore = mockedStore(useTestDefinitionStore);
const updateMetricSpy = vi.spyOn(evaluationsStore, 'updateMetric');
const createMetricSpy = vi
.spyOn(evaluationsStore, 'createMetric')
.mockResolvedValue({ id: 'metric_new', name: 'Metric 2', testDefinitionId: TEST_DEF_A.id });
state.value.metrics = [
{
id: 'metric1',
name: 'Metric 1',
testDefinitionId: TEST_DEF_A.id,
},
{
id: '',
name: 'Metric 2',
testDefinitionId: TEST_DEF_A.id,
}, // New metric that needs creation
];
await updateMetrics(TEST_DEF_A.id);
expect(createMetricSpy).toHaveBeenCalledWith({
name: 'Metric 2',
testDefinitionId: TEST_DEF_A.id,
});
expect(updateMetricSpy).toHaveBeenCalledWith({
name: 'Metric 1',
id: 'metric1',
testDefinitionId: TEST_DEF_A.id,
});
expect(state.value.metrics).toEqual([
{ id: 'metric1', name: 'Metric 1', testDefinitionId: TEST_DEF_A.id },
{ id: 'metric_new', name: 'Metric 2', testDefinitionId: TEST_DEF_A.id },
]);
});
it('should start editing a field', () => {
const { state, startEditing } = useTestDefinitionForm();

View File

@@ -1,4 +1,3 @@
import type { TestMetricRecord } from '@/api/testDefinition.ee';
import type { INodeParameterResourceLocator } from 'n8n-workflow';
export interface EditableField<T = string> {
@@ -14,6 +13,5 @@ export interface EditableFormState {
export interface EvaluationFormState extends EditableFormState {
evaluationWorkflow: INodeParameterResourceLocator;
metrics: TestMetricRecord[];
mockedNodes: Array<{ name: string; id: string }>;
}