Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(core): Change evaluation workflow input data format (no-changelog) #13109

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions packages/cli/src/evaluation.ee/test-definitions.controller.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,20 @@ export class TestDefinitionsController {

res.status(202).json({ success: true });
}

@Get('/:id/example-evaluation-input')
async exampleEvaluationInput(req: TestDefinitionsRequest.ExampleEvaluationInput) {
const { id: testDefinitionId } = req.params;
const { annotationTagId } = req.query;

const workflowIds = await getSharedWorkflowIds(req.user, ['workflow:read']);

const testDefinition = await this.testDefinitionService.findOne(testDefinitionId, workflowIds);
if (!testDefinition) throw new NotFoundError('Test definition not found');

return await this.testRunnerService.getExampleEvaluationInputData(
testDefinition,
annotationTagId,
);
}
}
7 changes: 7 additions & 0 deletions packages/cli/src/evaluation.ee/test-definitions.types.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ export declare namespace TestDefinitionsRequest {
type Delete = AuthenticatedRequest<RouteParams.TestId>;

type Run = AuthenticatedRequest<RouteParams.TestId>;

type ExampleEvaluationInput = AuthenticatedRequest<
RouteParams.TestId,
{},
{},
{ annotationTagId: string }
>;
}

// ----------------------------------
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { readFileSync } from 'fs';
import { mock } from 'jest-mock-extended';
import path from 'path';

import type { TestCaseRunMetadata } from '@/evaluation.ee/test-runner/test-runner.service.ee';
import { formatTestCaseExecutionInputData } from '@/evaluation.ee/test-runner/utils.ee';

const wfUnderTestJson = JSON.parse(
readFileSync(path.join(__dirname, './mock-data/workflow.under-test.json'), { encoding: 'utf-8' }),
);

const executionDataJson = JSON.parse(
readFileSync(path.join(__dirname, './mock-data/execution-data.json'), { encoding: 'utf-8' }),
);

describe('formatTestCaseExecutionInputData', () => {
test('should format the test case execution input data correctly', () => {
const data = formatTestCaseExecutionInputData(
executionDataJson.resultData.runData,
wfUnderTestJson,
executionDataJson.resultData.runData,
wfUnderTestJson,
mock<TestCaseRunMetadata>({
pastExecutionId: 'exec-id',
highlightedData: [],
annotation: {
vote: 'up',
tags: [{ id: 'tag-id', name: 'tag-name' }],
},
}),
);

// Check data have all expected properties
expect(data.json).toMatchObject({
originalExecution: expect.anything(),
newExecution: expect.anything(),
annotations: expect.anything(),
});

// Check original execution contains all the expected nodes
expect(data.json.originalExecution).toHaveProperty('72256d90-3a67-4e29-b032-47df4e5768af');
expect(data.json.originalExecution).toHaveProperty('319f29bc-1dd4-4122-b223-c584752151a4');
expect(data.json.originalExecution).toHaveProperty('d2474215-63af-40a4-a51e-0ea30d762621');

// Check format of specific node data
expect(data.json.originalExecution).toMatchObject({
'72256d90-3a67-4e29-b032-47df4e5768af': {
nodeName: 'When clicking ‘Test workflow’',
runs: [
{
executionTime: 0,
rootNode: true,
output: {
main: [
[
{
query: 'First item',
},
{
query: 'Second item',
},
{
query: 'Third item',
},
],
],
},
},
],
},
});

// Check annotations
expect(data).toMatchObject({
json: {
annotations: {
vote: 'up',
tags: [{ id: 'tag-id', name: 'tag-name' }],
highlightedData: {},
},
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,29 @@ const executionDataMultipleTriggersJson2 = JSON.parse(

const executionMocks = [
mock<ExecutionEntity>({
id: 'some-execution-id',
id: 'past-execution-id',
workflowId: 'workflow-under-test-id',
status: 'success',
executionData: {
data: stringify(executionDataJson),
workflowData: wfUnderTestJson,
},
metadata: [
{
key: 'testRunId',
value: 'test-run-id',
},
],
}),
mock<ExecutionEntity>({
id: 'some-execution-id-2',
id: 'past-execution-id-2',
workflowId: 'workflow-under-test-id',
status: 'success',
executionData: {
data: stringify(executionDataRenamedNodesJson),
workflowData: wfUnderTestRenamedNodesJson,
},
metadata: [],
}),
];

Expand Down Expand Up @@ -179,10 +186,10 @@ describe('TestRunnerService', () => {
executionsQbMock.getMany.mockResolvedValueOnce(executionMocks);
executionRepository.createQueryBuilder.mockReturnValueOnce(executionsQbMock);
executionRepository.findOne
.calledWith(expect.objectContaining({ where: { id: 'some-execution-id' } }))
.calledWith(expect.objectContaining({ where: { id: 'past-execution-id' } }))
.mockResolvedValueOnce(executionMocks[0]);
executionRepository.findOne
.calledWith(expect.objectContaining({ where: { id: 'some-execution-id-2' } }))
.calledWith(expect.objectContaining({ where: { id: 'past-execution-id-2' } }))
.mockResolvedValueOnce(executionMocks[1]);

testRunRepository.createTestRun.mockResolvedValue(mock<TestRun>({ id: 'test-run-id' }));
Expand Down Expand Up @@ -242,20 +249,20 @@ describe('TestRunnerService', () => {
...wfEvaluationJson,
});

workflowRunner.run.mockResolvedValue('test-execution-id');
workflowRunner.run.mockResolvedValue('some-execution-id');

await testRunnerService.runTest(
mock<User>(),
mock<TestDefinition>({
workflowId: 'workflow-under-test-id',
evaluationWorkflowId: 'evaluation-workflow-id',
mockedNodes: [],
mockedNodes: [{ id: '72256d90-3a67-4e29-b032-47df4e5768af' }],
}),
);

expect(executionRepository.createQueryBuilder).toHaveBeenCalledTimes(1);
expect(executionRepository.findOne).toHaveBeenCalledTimes(2);
expect(workflowRunner.run).toHaveBeenCalledTimes(2);
expect(workflowRunner.run).toHaveBeenCalled();
});

test('should run both workflow under test and evaluation workflow', async () => {
Expand Down Expand Up @@ -676,6 +683,47 @@ describe('TestRunnerService', () => {
});
});

test('should properly run test when nodes were renamed', async () => {
const testRunnerService = new TestRunnerService(
logger,
telemetry,
workflowRepository,
workflowRunner,
executionRepository,
activeExecutions,
testRunRepository,
testCaseExecutionRepository,
testMetricRepository,
mockNodeTypes,
errorReporter,
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
id: 'workflow-under-test-id',
...wfUnderTestJson,
});

workflowRepository.findById.calledWith('evaluation-workflow-id').mockResolvedValueOnce({
id: 'evaluation-workflow-id',
...wfEvaluationJson,
});

workflowRunner.run.mockResolvedValue('test-execution-id');

await testRunnerService.runTest(
mock<User>(),
mock<TestDefinition>({
workflowId: 'workflow-under-test-id',
evaluationWorkflowId: 'evaluation-workflow-id',
mockedNodes: [{ id: '72256d90-3a67-4e29-b032-47df4e5768af' }],
}),
);

expect(executionRepository.createQueryBuilder).toHaveBeenCalledTimes(1);
expect(executionRepository.findOne).toHaveBeenCalledTimes(2);
expect(workflowRunner.run).toHaveBeenCalledTimes(2);
});

test('should properly choose trigger when it was renamed', async () => {
const testRunnerService = new TestRunnerService(
logger,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { ExecutionCancelledError, NodeConnectionType, Workflow } from 'n8n-workf
import type {
IDataObject,
IRun,
IRunData,
IRunExecutionData,
IWorkflowBase,
IWorkflowExecutionDataProcess,
Expand All @@ -30,15 +29,21 @@ import { getRunData } from '@/workflow-execute-additional-data';
import { WorkflowRunner } from '@/workflow-runner';

import { EvaluationMetrics } from './evaluation-metrics.ee';
import { createPinData, getPastExecutionTriggerNode } from './utils.ee';
import {
createPinData,
formatTestCaseExecutionInputData,
getPastExecutionTriggerNode,
} from './utils.ee';

interface TestRunMetadata {
export interface TestRunMetadata {
testRunId: string;
userId: string;
}

interface TestCaseRunMetadata extends TestRunMetadata {
export interface TestCaseRunMetadata extends TestRunMetadata {
pastExecutionId: string;
annotation: ExecutionEntity['annotation'];
highlightedData: ExecutionEntity['metadata'];
}

/**
Expand Down Expand Up @@ -197,8 +202,7 @@ export class TestRunnerService {
*/
private async runTestCaseEvaluation(
evaluationWorkflow: IWorkflowBase,
expectedData: IRunData,
actualData: IRunData,
evaluationInputData: any,
abortSignal: AbortSignal,
metadata: TestCaseRunMetadata,
) {
Expand All @@ -207,15 +211,6 @@ export class TestRunnerService {
return;
}

// Prepare the evaluation wf input data.
// Provide both the expected data and the actual data
const evaluationInputData = {
json: {
originalExecution: expectedData,
newExecution: actualData,
},
};

// Prepare the data to run the evaluation workflow
const data = await getRunData(evaluationWorkflow, [evaluationInputData]);
data.executionMode = 'integrated';
Expand Down Expand Up @@ -374,7 +369,7 @@ export class TestRunnerService {
// Fetch past execution with data
const pastExecution = await this.executionRepository.findOne({
where: { id: pastExecutionId },
relations: ['executionData', 'metadata'],
relations: ['executionData', 'metadata', 'annotation', 'annotation.tags'],
});
assert(pastExecution, 'Execution not found');

Expand All @@ -383,6 +378,8 @@ export class TestRunnerService {
const testCaseMetadata = {
...testRunMetadata,
pastExecutionId,
highlightedData: pastExecution.metadata,
annotation: pastExecution.annotation,
};

// Run the test case and wait for it to finish
Expand Down Expand Up @@ -418,11 +415,18 @@ export class TestRunnerService {
// Get the original runData from the test case execution data
const originalRunData = executionData.resultData.runData;

const evaluationInputData = formatTestCaseExecutionInputData(
originalRunData,
pastExecution.executionData.workflowData,
testCaseRunData,
workflow,
testCaseMetadata,
);

// Run the evaluation workflow with the original and new run data
const evalExecution = await this.runTestCaseEvaluation(
evaluationWorkflow,
originalRunData,
testCaseRunData,
evaluationInputData,
abortSignal,
testCaseMetadata,
);
Expand Down Expand Up @@ -555,4 +559,61 @@ export class TestRunnerService {
});
}
}

/**
* Returns the example evaluation WF input for the test definition.
* It uses the latest execution of a workflow under test as a source and formats it
* the same way as the evaluation input would be formatted.
* We explicitly provide annotation tag here (and DO NOT use the one from DB), because the test definition
* might not be saved to the DB with the updated annotation tag at the moment we need to get the example data.
*/
async getExampleEvaluationInputData(test: TestDefinition, annotationTagId: string) {
// Select the id of latest execution with the annotation tag and workflow ID of the test
const lastPastExecution: Pick<ExecutionEntity, 'id'> | null = await this.executionRepository
.createQueryBuilder('execution')
.select('execution.id')
.leftJoin('execution.annotation', 'annotation')
.leftJoin('annotation.tags', 'annotationTag')
.where('annotationTag.id = :tagId', { tagId: annotationTagId })
.andWhere('execution.workflowId = :workflowId', { workflowId: test.workflowId })
.orderBy('execution.createdAt', 'DESC')
.getOne();

if (lastPastExecution === null) {
return null;
}

// Fetch past execution with data
const pastExecution = await this.executionRepository.findOne({
where: {
id: lastPastExecution.id,
},
relations: ['executionData', 'metadata', 'annotation', 'annotation.tags'],
});
assert(pastExecution, 'Execution not found');

const executionData = parse(pastExecution.executionData.data) as IRunExecutionData;

const sampleTestCaseMetadata = {
testRunId: 'sample-test-run-id',
userId: 'sample-user-id',
pastExecutionId: lastPastExecution.id,
highlightedData: pastExecution.metadata,
annotation: pastExecution.annotation,
};

// Get the original runData from the test case execution data
const originalRunData = executionData.resultData.runData;

// We use the same execution data for the original and new run data format example
const evaluationInputData = formatTestCaseExecutionInputData(
originalRunData,
pastExecution.executionData.workflowData,
originalRunData,
pastExecution.executionData.workflowData,
sampleTestCaseMetadata,
);

return evaluationInputData.json;
}
}
Loading
Loading