From 2b267b1c05e8da6ba3e9cf098fb3b89fac0c5bd6 Mon Sep 17 00:00:00 2001 From: Eugene Date: Tue, 17 Dec 2024 09:44:20 +0100 Subject: [PATCH] chore: Switch to the new partial execution logic in test runner (no-changelog) (#12140) --- .../__tests__/test-runner.service.ee.test.ts | 150 ++++++++++++++++++ .../test-runner/test-runner.service.ee.ts | 53 ++++++- .../src/evaluation/test-runner/utils.ee.ts | 4 +- .../deduplication-helper.test.ts | 33 +--- .../integration/permission-checker.test.ts | 33 +--- .../shared/utils/node-types-data.ts | 33 ++++ 6 files changed, 235 insertions(+), 71 deletions(-) create mode 100644 packages/cli/test/integration/shared/utils/node-types-data.ts diff --git a/packages/cli/src/evaluation/test-runner/__tests__/test-runner.service.ee.test.ts b/packages/cli/src/evaluation/test-runner/__tests__/test-runner.service.ee.test.ts index e03fb8dc47..bc0b70a27d 100644 --- a/packages/cli/src/evaluation/test-runner/__tests__/test-runner.service.ee.test.ts +++ b/packages/cli/src/evaluation/test-runner/__tests__/test-runner.service.ee.test.ts @@ -15,7 +15,11 @@ import type { ExecutionRepository } from '@/databases/repositories/execution.rep import type { TestMetricRepository } from '@/databases/repositories/test-metric.repository.ee'; import type { TestRunRepository } from '@/databases/repositories/test-run.repository.ee'; import type { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; +import { NodeTypes } from '@/node-types'; import type { WorkflowRunner } from '@/workflow-runner'; +import { mockInstance } from '@test/mocking'; +import { mockNodeTypesData } from '@test-integration/utils/node-types-data'; import { TestRunnerService } from '../test-runner.service.ee'; @@ -27,10 +31,28 @@ const wfEvaluationJson = JSON.parse( readFileSync(path.join(__dirname, './mock-data/workflow.evaluation.json'), { encoding: 'utf-8' }), ); +const wfMultipleTriggersJson = JSON.parse( + readFileSync(path.join(__dirname, './mock-data/workflow.multiple-triggers.json'), { + encoding: 'utf-8', + }), +); + const executionDataJson = JSON.parse( readFileSync(path.join(__dirname, './mock-data/execution-data.json'), { encoding: 'utf-8' }), ); +const executionDataMultipleTriggersJson = JSON.parse( + readFileSync(path.join(__dirname, './mock-data/execution-data.multiple-triggers.json'), { + encoding: 'utf-8', + }), +); + +const executionDataMultipleTriggersJson2 = JSON.parse( + readFileSync(path.join(__dirname, './mock-data/execution-data.multiple-triggers-2.json'), { + encoding: 'utf-8', + }), +); + const executionMocks = [ mock({ id: 'some-execution-id', @@ -93,6 +115,11 @@ describe('TestRunnerService', () => { const testRunRepository = mock(); const testMetricRepository = mock(); + const mockNodeTypes = mockInstance(NodeTypes); + mockInstance(LoadNodesAndCredentials, { + loadedNodes: mockNodeTypesData(['manualTrigger', 'set', 'if', 'code']), + }); + beforeEach(() => { const executionsQbMock = mockDeep>({ fallbackMockImplementation: jest.fn().mockReturnThis(), @@ -131,6 +158,7 @@ describe('TestRunnerService', () => { activeExecutions, testRunRepository, testMetricRepository, + mockNodeTypes, ); expect(testRunnerService).toBeInstanceOf(TestRunnerService); @@ -144,6 +172,7 @@ describe('TestRunnerService', () => { activeExecutions, testRunRepository, testMetricRepository, + mockNodeTypes, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -180,6 +209,7 @@ describe('TestRunnerService', () => { activeExecutions, testRunRepository, testMetricRepository, + mockNodeTypes, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -267,4 +297,124 @@ describe('TestRunnerService', () => { metric2: 0, }); }); + + test('should specify correct start nodes when running workflow under test', async () => { + const testRunnerService = new TestRunnerService( + workflowRepository, + workflowRunner, + executionRepository, + activeExecutions, + testRunRepository, + testMetricRepository, + mockNodeTypes, + ); + + 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.mockResolvedValueOnce('some-execution-id'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-2'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-3'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-4'); + + // Mock executions of workflow under test + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id') + .mockResolvedValue(mockExecutionData()); + + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-3') + .mockResolvedValue(mockExecutionData()); + + // Mock executions of evaluation workflow + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-2') + .mockResolvedValue(mockEvaluationExecutionData({ metric1: 1, metric2: 0 })); + + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-4') + .mockResolvedValue(mockEvaluationExecutionData({ metric1: 0.5 })); + + await testRunnerService.runTest( + mock(), + mock({ + workflowId: 'workflow-under-test-id', + evaluationWorkflowId: 'evaluation-workflow-id', + }), + ); + + expect(workflowRunner.run).toHaveBeenCalledTimes(4); + + // Check workflow under test was executed + expect(workflowRunner.run).toHaveBeenCalledWith( + expect.objectContaining({ + executionMode: 'evaluation', + pinData: { + 'When clicking ‘Test workflow’': + executionDataJson.resultData.runData['When clicking ‘Test workflow’'][0].data.main[0], + }, + workflowData: expect.objectContaining({ + id: 'workflow-under-test-id', + }), + triggerToStartFrom: expect.objectContaining({ + name: 'When clicking ‘Test workflow’', + }), + }), + ); + }); + + test('should properly choose trigger and start nodes', async () => { + const testRunnerService = new TestRunnerService( + workflowRepository, + workflowRunner, + executionRepository, + activeExecutions, + testRunRepository, + testMetricRepository, + mockNodeTypes, + ); + + const startNodesData = (testRunnerService as any).getStartNodesData( + wfMultipleTriggersJson, + executionDataMultipleTriggersJson, + ); + + expect(startNodesData).toEqual({ + startNodes: expect.arrayContaining([expect.objectContaining({ name: 'NoOp' })]), + triggerToStartFrom: expect.objectContaining({ + name: 'When clicking ‘Test workflow’', + }), + }); + }); + + test('should properly choose trigger and start nodes 2', async () => { + const testRunnerService = new TestRunnerService( + workflowRepository, + workflowRunner, + executionRepository, + activeExecutions, + testRunRepository, + testMetricRepository, + mockNodeTypes, + ); + + const startNodesData = (testRunnerService as any).getStartNodesData( + wfMultipleTriggersJson, + executionDataMultipleTriggersJson2, + ); + + expect(startNodesData).toEqual({ + startNodes: expect.arrayContaining([expect.objectContaining({ name: 'NoOp' })]), + triggerToStartFrom: expect.objectContaining({ + name: 'When chat message received', + }), + }); + }); }); diff --git a/packages/cli/src/evaluation/test-runner/test-runner.service.ee.ts b/packages/cli/src/evaluation/test-runner/test-runner.service.ee.ts index 11903581cd..36781d6839 100644 --- a/packages/cli/src/evaluation/test-runner/test-runner.service.ee.ts +++ b/packages/cli/src/evaluation/test-runner/test-runner.service.ee.ts @@ -6,6 +6,7 @@ import type { IRunExecutionData, IWorkflowExecutionDataProcess, } from 'n8n-workflow'; +import { NodeConnectionType, Workflow } from 'n8n-workflow'; import assert from 'node:assert'; import { Service } from 'typedi'; @@ -18,6 +19,7 @@ import { ExecutionRepository } from '@/databases/repositories/execution.reposito import { TestMetricRepository } from '@/databases/repositories/test-metric.repository.ee'; import { TestRunRepository } from '@/databases/repositories/test-run.repository.ee'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { NodeTypes } from '@/node-types'; import { getRunData } from '@/workflow-execute-additional-data'; import { WorkflowRunner } from '@/workflow-runner'; @@ -41,8 +43,50 @@ export class TestRunnerService { private readonly activeExecutions: ActiveExecutions, private readonly testRunRepository: TestRunRepository, private readonly testMetricRepository: TestMetricRepository, + private readonly nodeTypes: NodeTypes, ) {} + /** + * Prepares the start nodes and trigger node data props for the `workflowRunner.run` method input. + */ + private getStartNodesData( + workflow: WorkflowEntity, + pastExecutionData: IRunExecutionData, + ): Pick { + // Create a new workflow instance to use the helper functions (getChildNodes) + const workflowInstance = new Workflow({ + nodes: workflow.nodes, + connections: workflow.connections, + active: false, + nodeTypes: this.nodeTypes, + }); + + // Determine the trigger node of the past execution + const pastExecutionTriggerNode = getPastExecutionTriggerNode(pastExecutionData); + assert(pastExecutionTriggerNode, 'Could not find the trigger node of the past execution'); + + const triggerNodeData = pastExecutionData.resultData.runData[pastExecutionTriggerNode][0]; + assert(triggerNodeData, 'Trigger node data not found'); + + const triggerToStartFrom = { + name: pastExecutionTriggerNode, + data: triggerNodeData, + }; + + // Start nodes are the nodes that are connected to the trigger node + const startNodes = workflowInstance + .getChildNodes(pastExecutionTriggerNode, NodeConnectionType.Main, 1) + .map((nodeName) => ({ + name: nodeName, + sourceData: { previousNode: pastExecutionTriggerNode }, + })); + + return { + startNodes, + triggerToStartFrom, + }; + } + /** * Runs a test case with the given pin data. * Waits for the workflow under test to finish execution. @@ -56,20 +100,13 @@ export class TestRunnerService { // Create pin data from the past execution data const pinData = createPinData(workflow, mockedNodes, pastExecutionData); - // Determine the start node of the past execution - const pastExecutionStartNode = getPastExecutionTriggerNode(pastExecutionData); - // Prepare the data to run the workflow const data: IWorkflowExecutionDataProcess = { - destinationNode: pastExecutionData.startData?.destinationNode, - startNodes: pastExecutionStartNode - ? [{ name: pastExecutionStartNode, sourceData: null }] - : undefined, + ...this.getStartNodesData(workflow, pastExecutionData), executionMode: 'evaluation', runData: {}, pinData, workflowData: workflow, - partialExecutionVersion: '-1', userId, }; diff --git a/packages/cli/src/evaluation/test-runner/utils.ee.ts b/packages/cli/src/evaluation/test-runner/utils.ee.ts index e608ad6b4a..9cb516b430 100644 --- a/packages/cli/src/evaluation/test-runner/utils.ee.ts +++ b/packages/cli/src/evaluation/test-runner/utils.ee.ts @@ -31,8 +31,8 @@ export function createPinData( } /** - * Returns the start node of the past execution. - * The start node is the node that has no source and has run data. + * Returns the trigger node of the past execution. + * The trigger node is the node that has no source and has run data. */ export function getPastExecutionTriggerNode(executionData: IRunExecutionData) { return Object.keys(executionData.resultData.runData).find((nodeName) => { diff --git a/packages/cli/test/integration/deduplication/deduplication-helper.test.ts b/packages/cli/test/integration/deduplication/deduplication-helper.test.ts index 2859bb363c..467e6e0924 100644 --- a/packages/cli/test/integration/deduplication/deduplication-helper.test.ts +++ b/packages/cli/test/integration/deduplication/deduplication-helper.test.ts @@ -1,5 +1,5 @@ import { DataDeduplicationService } from 'n8n-core'; -import type { ICheckProcessedContextData, INodeTypeData } from 'n8n-workflow'; +import type { ICheckProcessedContextData } from 'n8n-workflow'; import type { IDeduplicationOutput, INode, DeduplicationItemTypes } from 'n8n-workflow'; import { Workflow } from 'n8n-workflow'; @@ -8,6 +8,7 @@ import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { NodeTypes } from '@/node-types'; import { mockInstance } from '@test/mocking'; import { createWorkflow } from '@test-integration/db/workflows'; +import { mockNodeTypesData } from '@test-integration/utils/node-types-data'; import * as testDb from '../shared/test-db'; @@ -22,35 +23,7 @@ mockInstance(LoadNodesAndCredentials, { credentials: {}, }, }); -function mockNodeTypesData( - nodeNames: string[], - options?: { - addTrigger?: boolean; - }, -) { - return nodeNames.reduce((acc, nodeName) => { - return ( - (acc[`n8n-nodes-base.${nodeName}`] = { - sourcePath: '', - type: { - description: { - displayName: nodeName, - name: nodeName, - group: [], - description: '', - version: 1, - defaults: {}, - inputs: [], - outputs: [], - properties: [], - }, - trigger: options?.addTrigger ? async () => undefined : undefined, - }, - }), - acc - ); - }, {}); -} + const node: INode = { id: 'uuid-1234', parameters: {}, diff --git a/packages/cli/test/integration/permission-checker.test.ts b/packages/cli/test/integration/permission-checker.test.ts index e746ec29f7..1a0963b685 100644 --- a/packages/cli/test/integration/permission-checker.test.ts +++ b/packages/cli/test/integration/permission-checker.test.ts @@ -1,4 +1,4 @@ -import type { INode, INodeTypeData } from 'n8n-workflow'; +import type { INode } from 'n8n-workflow'; import { randomInt } from 'n8n-workflow'; import { Container } from 'typedi'; import { v4 as uuid } from 'uuid'; @@ -14,6 +14,7 @@ import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { NodeTypes } from '@/node-types'; import { OwnershipService } from '@/services/ownership.service'; import { PermissionChecker } from '@/user-management/permission-checker'; +import { mockNodeTypesData } from '@test-integration/utils/node-types-data'; import { affixRoleToSaveCredential } from './shared/db/credentials'; import { getPersonalProject } from './shared/db/projects'; @@ -25,36 +26,6 @@ import { mockInstance } from '../shared/mocking'; const ownershipService = mockInstance(OwnershipService); -function mockNodeTypesData( - nodeNames: string[], - options?: { - addTrigger?: boolean; - }, -) { - return nodeNames.reduce((acc, nodeName) => { - return ( - (acc[`n8n-nodes-base.${nodeName}`] = { - sourcePath: '', - type: { - description: { - displayName: nodeName, - name: nodeName, - group: [], - description: '', - version: 1, - defaults: {}, - inputs: [], - outputs: [], - properties: [], - }, - trigger: options?.addTrigger ? async () => undefined : undefined, - }, - }), - acc - ); - }, {}); -} - const createWorkflow = async (nodes: INode[], workflowOwner?: User): Promise => { const workflowDetails = { id: randomInt(1, 10).toString(), diff --git a/packages/cli/test/integration/shared/utils/node-types-data.ts b/packages/cli/test/integration/shared/utils/node-types-data.ts new file mode 100644 index 0000000000..35178de207 --- /dev/null +++ b/packages/cli/test/integration/shared/utils/node-types-data.ts @@ -0,0 +1,33 @@ +import type { INodeTypeData } from 'n8n-workflow'; + +export function mockNodeTypesData( + nodeNames: string[], + options?: { + addTrigger?: boolean; + }, +) { + return nodeNames.reduce((acc, nodeName) => { + const fullName = nodeName.indexOf('.') === -1 ? `n8n-nodes-base.${nodeName}` : nodeName; + + return ( + (acc[fullName] = { + sourcePath: '', + type: { + description: { + displayName: nodeName, + name: nodeName, + group: [], + description: '', + version: 1, + defaults: {}, + inputs: [], + outputs: [], + properties: [], + }, + trigger: options?.addTrigger ? async () => undefined : undefined, + }, + }), + acc + ); + }, {}); +}