wip: addressing PR feedback

n8n-io · Jan 29, 2025 · d4a68fb · d4a68fb
1 parent 091c0cc
commit d4a68fb
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 29 deletions.
diff --git a/packages/cli/src/databases/repositories/test-case-execution.repository.ee.ts b/packages/cli/src/databases/repositories/test-case-execution.repository.ee.ts
@@ -1,4 +1,5 @@
 import { Service } from '@n8n/di';
+import type { EntityManager } from '@n8n/typeorm';
 import { DataSource, In, Not, Repository } from '@n8n/typeorm';
 import type { DeepPartial } from '@n8n/typeorm/common/DeepPartial';
 
@@ -55,8 +56,12 @@ export class TestCaseExecutionRepository extends Repository<TestCaseExecution> {
 		testRunId: string,
 		pastExecutionId: string,
 		metrics: Record<string, number>,
+		trx?: EntityManager,
 	) {
-		return await this.update(
+		trx = trx ?? this.manager;
+
+		return await trx.update(
+			TestCaseExecution,
 			{ testRun: { id: testRunId }, pastExecutionId },
 			{
 				status: 'success',
@@ -66,8 +71,11 @@ export class TestCaseExecutionRepository extends Repository<TestCaseExecution> {
 		);
 	}
 
-	async markPendingAsCancelled(testRunId: string) {
-		return await this.update(
+	async markPendingAsCancelled(testRunId: string, trx?: EntityManager) {
+		trx = trx ?? this.manager;
+
+		return await trx.update(
+			TestCaseExecution,
 			{ testRun: { id: testRunId }, status: Not(In(['success', 'error', 'cancelled'])) },
 			{
 				status: 'cancelled',
@@ -76,8 +84,11 @@ export class TestCaseExecutionRepository extends Repository<TestCaseExecution> {
 		);
 	}
 
-	async markAsFailed(testRunId: string, pastExecutionId: string) {
-		return await this.update(
+	async markAsFailed(testRunId: string, pastExecutionId: string, trx?: EntityManager) {
+		trx = trx ?? this.manager;
+
+		return await trx.update(
+			TestCaseExecution,
 			{ testRun: { id: testRunId }, pastExecutionId },
 			{
 				status: 'error',

diff --git a/packages/cli/src/databases/repositories/test-run.repository.ee.ts b/packages/cli/src/databases/repositories/test-run.repository.ee.ts
@@ -1,5 +1,5 @@
 import { Service } from '@n8n/di';
-import type { FindManyOptions } from '@n8n/typeorm';
+import type { EntityManager, FindManyOptions } from '@n8n/typeorm';
 import { DataSource, Repository } from '@n8n/typeorm';
 
 import type { AggregatedTestRunMetrics } from '@/databases/entities/test-run.ee';
@@ -35,16 +35,19 @@ export class TestRunRepository extends Repository<TestRun> {
 		return await this.update(id, { status: 'completed', completedAt: new Date(), metrics });
 	}
 
-	async markAsCancelled(id: string) {
-		return await this.update(id, { status: 'cancelled' });
+	async markAsCancelled(id: string, trx?: EntityManager) {
+		trx = trx ?? this.manager;
+		return await trx.update(TestRun, id, { status: 'cancelled' });
 	}
 
-	async incrementPassed(id: string) {
-		return await this.increment({ id }, 'passedCases', 1);
+	async incrementPassed(id: string, trx?: EntityManager) {
+		trx = trx ?? this.manager;
+		return await trx.increment(TestRun, { id }, 'passedCases', 1);
 	}
 
-	async incrementFailed(id: string) {
-		return await this.increment({ id }, 'failedCases', 1);
+	async incrementFailed(id: string, trx?: EntityManager) {
+		trx = trx ?? this.manager;
+		return await trx.increment(TestRun, { id }, 'failedCases', 1);
 	}
 
 	async getMany(testDefinitionId: string, options: ListQuery.Options) {

diff --git a/packages/cli/src/evaluation.ee/test-runner/test-runner.service.ee.ts b/packages/cli/src/evaluation.ee/test-runner/test-runner.service.ee.ts
@@ -23,6 +23,7 @@ import { TestCaseExecutionRepository } from '@/databases/repositories/test-case-
 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 * as Db from '@/db';
 import { NodeTypes } from '@/node-types';
 import { getRunData } from '@/workflow-execute-additional-data';
 import { WorkflowRunner } from '@/workflow-runner';
@@ -383,30 +384,37 @@ export class TestRunnerService {
 					// Extract the output of the last node executed in the evaluation workflow
 					const addedMetrics = metrics.addResults(this.extractEvaluationResult(evalExecution));
 
-					if (evalExecution.data.resultData.error) {
-						await this.testRunRepository.incrementFailed(testRun.id);
-						await this.testCaseExecutionRepository.markAsFailed(testRun.id, pastExecutionId);
-					} else {
-						await this.testRunRepository.incrementPassed(testRun.id);
-						await this.testCaseExecutionRepository.markAsCompleted(
-							testRun.id,
-							pastExecutionId,
-							addedMetrics,
-						);
-					}
+					await Db.transaction(async (trx) => {
+						if (evalExecution.data.resultData.error) {
+							await this.testRunRepository.incrementFailed(testRun.id, trx);
+							await this.testCaseExecutionRepository.markAsFailed(testRun.id, pastExecutionId, trx);
+						} else {
+							await this.testRunRepository.incrementPassed(testRun.id, trx);
+							await this.testCaseExecutionRepository.markAsCompleted(
+								testRun.id,
+								pastExecutionId,
+								addedMetrics,
+								trx,
+							);
+						}
+					});
 				} catch (e) {
 					// In case of an unexpected error, increment the failed count and continue with the next test case
-					await this.testRunRepository.incrementFailed(testRun.id);
-					await this.testCaseExecutionRepository.markAsFailed(testRun.id, pastExecutionId);
+					await Db.transaction(async (trx) => {
+						await this.testRunRepository.incrementFailed(testRun.id, trx);
+						await this.testCaseExecutionRepository.markAsFailed(testRun.id, pastExecutionId, trx);
+					});
 
 					this.errorReporter.error(e);
 				}
 			}
 
 			// Mark the test run as completed or cancelled
 			if (abortSignal.aborted) {
-				await this.testRunRepository.markAsCancelled(testRun.id);
-				await this.testCaseExecutionRepository.markPendingAsCancelled(testRun.id);
+				await Db.transaction(async (trx) => {
+					await this.testRunRepository.markAsCancelled(testRun.id, trx);
+					await this.testCaseExecutionRepository.markPendingAsCancelled(testRun.id, trx);
+				});
 			} else {
 				const aggregatedMetrics = metrics.getAggregatedMetrics();
 				await this.testRunRepository.markAsCompleted(testRun.id, aggregatedMetrics);
@@ -420,8 +428,10 @@ export class TestRunnerService {
 					stoppedOn: e.extra?.executionId,
 				});
 
-				await this.testRunRepository.markAsCancelled(testRun.id);
-				await this.testCaseExecutionRepository.markPendingAsCancelled(testRun.id);
+				await Db.transaction(async (trx) => {
+					await this.testRunRepository.markAsCancelled(testRun.id, trx);
+					await this.testCaseExecutionRepository.markPendingAsCancelled(testRun.id, trx);
+				});
 			} else {
 				throw e;
 			}