Skip to content

Commit

Permalink
fix(core): Node mocking for evaluation executions (no-changelog) (n8n…
Browse files Browse the repository at this point in the history
  • Loading branch information
burivuhster authored Jan 13, 2025
1 parent 865fc21 commit dcd7feb
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type { WorkflowRepository } from '@/databases/repositories/workflow.repos
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
import { NodeTypes } from '@/node-types';
import type { WorkflowRunner } from '@/workflow-runner';
import { mockInstance } from '@test/mocking';
import { mockInstance, mockLogger } from '@test/mocking';
import { mockNodeTypesData } from '@test-integration/utils/node-types-data';

import { TestRunnerService } from '../test-runner.service.ee';
Expand Down Expand Up @@ -129,6 +129,9 @@ function mockEvaluationExecutionData(metrics: Record<string, GenericValue>) {
});
}

const errorReporter = mock<ErrorReporter>();
const logger = mockLogger();

describe('TestRunnerService', () => {
const executionRepository = mock<ExecutionRepository>();
const workflowRepository = mock<WorkflowRepository>();
Expand Down Expand Up @@ -176,29 +179,31 @@ describe('TestRunnerService', () => {

test('should create an instance of TestRunnerService', async () => {
const testRunnerService = new TestRunnerService(
logger,
workflowRepository,
workflowRunner,
executionRepository,
activeExecutions,
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
errorReporter,
);

expect(testRunnerService).toBeInstanceOf(TestRunnerService);
});

test('should create and run test cases from past executions', async () => {
const testRunnerService = new TestRunnerService(
logger,
workflowRepository,
workflowRunner,
executionRepository,
activeExecutions,
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
errorReporter,
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -229,14 +234,15 @@ describe('TestRunnerService', () => {

test('should run both workflow under test and evaluation workflow', async () => {
const testRunnerService = new TestRunnerService(
logger,
workflowRepository,
workflowRunner,
executionRepository,
activeExecutions,
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
errorReporter,
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -330,14 +336,15 @@ describe('TestRunnerService', () => {

test('should properly count passed and failed executions', async () => {
const testRunnerService = new TestRunnerService(
logger,
workflowRepository,
workflowRunner,
executionRepository,
activeExecutions,
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
errorReporter,
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -388,14 +395,15 @@ describe('TestRunnerService', () => {

test('should properly count failed test executions', async () => {
const testRunnerService = new TestRunnerService(
logger,
workflowRepository,
workflowRunner,
executionRepository,
activeExecutions,
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
errorReporter,
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -442,14 +450,15 @@ describe('TestRunnerService', () => {

test('should properly count failed evaluations', async () => {
const testRunnerService = new TestRunnerService(
logger,
workflowRepository,
workflowRunner,
executionRepository,
activeExecutions,
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
errorReporter,
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -500,14 +509,15 @@ describe('TestRunnerService', () => {

test('should specify correct start nodes when running workflow under test', async () => {
const testRunnerService = new TestRunnerService(
logger,
workflowRepository,
workflowRunner,
executionRepository,
activeExecutions,
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
errorReporter,
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -574,14 +584,15 @@ describe('TestRunnerService', () => {

test('should properly choose trigger and start nodes', async () => {
const testRunnerService = new TestRunnerService(
logger,
workflowRepository,
workflowRunner,
executionRepository,
activeExecutions,
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
errorReporter,
);

const startNodesData = (testRunnerService as any).getStartNodesData(
Expand All @@ -599,14 +610,15 @@ describe('TestRunnerService', () => {

test('should properly choose trigger and start nodes 2', async () => {
const testRunnerService = new TestRunnerService(
logger,
workflowRepository,
workflowRunner,
executionRepository,
activeExecutions,
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
errorReporter,
);

const startNodesData = (testRunnerService as any).getStartNodesData(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Service } from '@n8n/di';
import { parse } from 'flatted';
import { ErrorReporter } from 'n8n-core';
import { ErrorReporter, Logger } from 'n8n-core';
import { NodeConnectionType, Workflow } from 'n8n-workflow';
import type {
IDataObject,
Expand Down Expand Up @@ -39,6 +39,7 @@ import { createPinData, getPastExecutionTriggerNode } from './utils.ee';
@Service()
export class TestRunnerService {
constructor(
private readonly logger: Logger,
private readonly workflowRepository: WorkflowRepository,
private readonly workflowRunner: WorkflowRunner,
private readonly executionRepository: ExecutionRepository,
Expand Down Expand Up @@ -115,8 +116,9 @@ export class TestRunnerService {
executionMode: 'evaluation',
runData: {},
pinData,
workflowData: workflow,
workflowData: { ...workflow, pinData },
userId,
partialExecutionVersion: '1',
};

// Trigger the workflow under test with mocked data
Expand Down Expand Up @@ -203,6 +205,8 @@ export class TestRunnerService {
* Creates a new test run for the given test definition.
*/
async runTest(user: User, test: TestDefinition): Promise<void> {
this.logger.debug('Starting new test run', { testId: test.id });

const workflow = await this.workflowRepository.findById(test.workflowId);
assert(workflow, 'Workflow not found');

Expand All @@ -227,6 +231,8 @@ export class TestRunnerService {
.andWhere('execution.workflowId = :workflowId', { workflowId: test.workflowId })
.getMany();

this.logger.debug('Found past executions', { count: pastExecutions.length });

// Get the metrics to collect from the evaluation workflow
const testMetricNames = await this.getTestMetricNames(test.id);

Expand All @@ -238,6 +244,8 @@ export class TestRunnerService {
const metrics = new EvaluationMetrics(testMetricNames);

for (const { id: pastExecutionId } of pastExecutions) {
this.logger.debug('Running test case', { pastExecutionId });

try {
// Fetch past execution with data
const pastExecution = await this.executionRepository.findOne({
Expand All @@ -257,6 +265,8 @@ export class TestRunnerService {
user.id,
);

this.logger.debug('Test case execution finished', { pastExecutionId });

// In case of a permission check issue, the test case execution will be undefined.
// Skip them, increment the failed count and continue with the next test case
if (!testCaseExecution) {
Expand All @@ -279,6 +289,8 @@ export class TestRunnerService {
);
assert(evalExecution);

this.logger.debug('Evaluation execution finished', { pastExecutionId });

metrics.addResults(this.extractEvaluationResult(evalExecution));

if (evalExecution.data.resultData.error) {
Expand All @@ -297,5 +309,7 @@ export class TestRunnerService {
const aggregatedMetrics = metrics.getAggregatedMetrics();

await this.testRunRepository.markAsCompleted(testRun.id, aggregatedMetrics);

this.logger.debug('Test run finished', { testId: test.id });
}
}
6 changes: 5 additions & 1 deletion packages/cli/src/manual-execution.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ export class ManualExecutionService {
},
};

const workflowExecute = new WorkflowExecute(additionalData, 'manual', executionData);
const workflowExecute = new WorkflowExecute(
additionalData,
data.executionMode,
executionData,
);
return workflowExecute.processRunExecutionData(workflow);
} else if (
data.runData === undefined ||
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/webhooks/webhook-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ export async function executeWebhook(
}

let pinData: IPinData | undefined;
const usePinData = executionMode === 'manual';
const usePinData = ['manual', 'evaluation'].includes(executionMode);
if (usePinData) {
pinData = workflowData.pinData;
runExecutionData.resultData.pinData = pinData;
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/workflow-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ export class WorkflowRunner {
}

let pinData: IPinData | undefined;
if (data.executionMode === 'manual') {
if (['manual', 'evaluation'].includes(data.executionMode)) {
pinData = data.pinData ?? data.workflowData.pinData;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/editor-ui/src/composables/useCanvasOperations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1923,7 +1923,7 @@ export function useCanvasOperations({ router }: { router: ReturnType<typeof useR

workflowsStore.setWorkflowExecutionData(data);

if (data.mode !== 'manual') {
if (!['manual', 'evaluation'].includes(data.mode)) {
workflowsStore.setWorkflowPinData({});
}

Expand Down
3 changes: 2 additions & 1 deletion packages/editor-ui/src/views/NodeView.v2.vue
Original file line number Diff line number Diff line change
Expand Up @@ -1385,7 +1385,8 @@ async function onPostMessageReceived(messageEvent: MessageEvent) {
try {
// If this NodeView is used in preview mode (in iframe) it will not have access to the main app store
// so everything it needs has to be sent using post messages and passed down to child components
isProductionExecutionPreview.value = json.executionMode !== 'manual';
isProductionExecutionPreview.value =
json.executionMode !== 'manual' && json.executionMode !== 'evaluation';
await onOpenExecution(json.executionId);
canOpenNDV.value = json.canOpenNDV ?? true;
Expand Down

0 comments on commit dcd7feb

Please sign in to comment.