From d4a68fb4ca27f2ef96633f0f5c7023379c651e02 Mon Sep 17 00:00:00 2001 From: Eugene Molodkin Date: Wed, 29 Jan 2025 16:31:42 +0100 Subject: [PATCH] wip: addressing PR feedback --- .../test-case-execution.repository.ee.ts | 21 ++++++--- .../repositories/test-run.repository.ee.ts | 17 ++++--- .../test-runner/test-runner.service.ee.ts | 44 ++++++++++++------- 3 files changed, 53 insertions(+), 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 index 7002ee1aa6e4d..651578bf44658 100644 --- 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 { testRunId: string, pastExecutionId: string, metrics: Record, + 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 { ); } - 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 { ); } - 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 index 728f0bc46493d..1f2039aceea36 100644 --- 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 { 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 index 137a3808c2fbd..6b84a02374974 100644 --- 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,21 +384,26 @@ 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); } @@ -405,8 +411,10 @@ export class TestRunnerService { // 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; }