From 11f9212eda1ed83dc68ec3a60e3825be9fcc7230 Mon Sep 17 00:00:00 2001 From: Eugene Date: Wed, 27 Nov 2024 14:33:28 +0100 Subject: [PATCH] chore: Add test run entity (no-changelog) (#11832) --- packages/cli/src/databases/entities/index.ts | 2 + .../cli/src/databases/entities/test-run.ee.ts | 38 +++++++++++++++++++ .../1732549866705-CreateTestRunTable.ts | 27 +++++++++++++ .../src/databases/migrations/mysqldb/index.ts | 2 + .../databases/migrations/postgresdb/index.ts | 2 + .../src/databases/migrations/sqlite/index.ts | 2 + .../repositories/test-run.repository.ee.ts | 29 ++++++++++++++ .../__tests__/test-runner.service.ee.test.ts | 20 ++++++++++ .../test-runner/test-runner.service.ee.ts | 19 ++++++++-- 9 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 packages/cli/src/databases/entities/test-run.ee.ts create mode 100644 packages/cli/src/databases/migrations/common/1732549866705-CreateTestRunTable.ts create mode 100644 packages/cli/src/databases/repositories/test-run.repository.ee.ts diff --git a/packages/cli/src/databases/entities/index.ts b/packages/cli/src/databases/entities/index.ts index 3f445e835ae10..e6a1dedb3fe9c 100644 --- a/packages/cli/src/databases/entities/index.ts +++ b/packages/cli/src/databases/entities/index.ts @@ -22,6 +22,7 @@ import { SharedWorkflow } from './shared-workflow'; import { TagEntity } from './tag-entity'; import { TestDefinition } from './test-definition.ee'; import { TestMetric } from './test-metric.ee'; +import { TestRun } from './test-run.ee'; import { User } from './user'; import { Variables } from './variables'; import { WebhookEntity } from './webhook-entity'; @@ -62,4 +63,5 @@ export const entities = { ProcessedData, TestDefinition, TestMetric, + TestRun, }; diff --git a/packages/cli/src/databases/entities/test-run.ee.ts b/packages/cli/src/databases/entities/test-run.ee.ts new file mode 100644 index 0000000000000..ab5f041f11204 --- /dev/null +++ b/packages/cli/src/databases/entities/test-run.ee.ts @@ -0,0 +1,38 @@ +import { Column, Entity, Index, ManyToOne, RelationId } from '@n8n/typeorm'; + +import { + datetimeColumnType, + jsonColumnType, + WithTimestampsAndStringId, +} from '@/databases/entities/abstract-entity'; +import { TestDefinition } from '@/databases/entities/test-definition.ee'; + +type TestRunStatus = 'new' | 'running' | 'completed' | 'error'; + +export type AggregatedTestRunMetrics = Record; + +/** + * Entity representing a Test Run. + * It stores info about a specific run of a test, combining the test definition with the status and collected metrics + */ +@Entity() +@Index(['testDefinition']) +export class TestRun extends WithTimestampsAndStringId { + @ManyToOne('TestDefinition', 'runs') + testDefinition: TestDefinition; + + @RelationId((testRun: TestRun) => testRun.testDefinition) + testDefinitionId: string; + + @Column('varchar') + status: TestRunStatus; + + @Column({ type: datetimeColumnType, nullable: true }) + runAt: Date | null; + + @Column({ type: datetimeColumnType, nullable: true }) + completedAt: Date | null; + + @Column(jsonColumnType, { nullable: true }) + metrics: AggregatedTestRunMetrics; +} diff --git a/packages/cli/src/databases/migrations/common/1732549866705-CreateTestRunTable.ts b/packages/cli/src/databases/migrations/common/1732549866705-CreateTestRunTable.ts new file mode 100644 index 0000000000000..e9dc0ba823cc5 --- /dev/null +++ b/packages/cli/src/databases/migrations/common/1732549866705-CreateTestRunTable.ts @@ -0,0 +1,27 @@ +import type { MigrationContext, ReversibleMigration } from '@/databases/types'; + +const testRunTableName = 'test_run'; + +export class CreateTestRun1732549866705 implements ReversibleMigration { + async up({ schemaBuilder: { createTable, column } }: MigrationContext) { + await createTable(testRunTableName) + .withColumns( + column('id').varchar(36).primary.notNull, + column('testDefinitionId').varchar(36).notNull, + column('status').varchar().notNull, + column('runAt').timestamp(), + column('completedAt').timestamp(), + column('metrics').json, + ) + .withIndexOn('testDefinitionId') + .withForeignKey('testDefinitionId', { + tableName: 'test_definition', + columnName: 'id', + onDelete: 'CASCADE', + }).withTimestamps; + } + + async down({ schemaBuilder: { dropTable } }: MigrationContext) { + await dropTable(testRunTableName); + } +} diff --git a/packages/cli/src/databases/migrations/mysqldb/index.ts b/packages/cli/src/databases/migrations/mysqldb/index.ts index 534c4404fa9d2..d962042333404 100644 --- a/packages/cli/src/databases/migrations/mysqldb/index.ts +++ b/packages/cli/src/databases/migrations/mysqldb/index.ts @@ -72,6 +72,7 @@ import { UpdateProcessedDataValueColumnToText1729607673464 } from '../common/172 import { CreateTestDefinitionTable1730386903556 } from '../common/1730386903556-CreateTestDefinitionTable'; import { AddDescriptionToTestDefinition1731404028106 } from '../common/1731404028106-AddDescriptionToTestDefinition'; import { CreateTestMetricTable1732271325258 } from '../common/1732271325258-CreateTestMetricTable'; +import { CreateTestRun1732549866705 } from '../common/1732549866705-CreateTestRunTable'; export const mysqlMigrations: Migration[] = [ InitialMigration1588157391238, @@ -146,4 +147,5 @@ export const mysqlMigrations: Migration[] = [ AddDescriptionToTestDefinition1731404028106, MigrateTestDefinitionKeyToString1731582748663, CreateTestMetricTable1732271325258, + CreateTestRun1732549866705, ]; diff --git a/packages/cli/src/databases/migrations/postgresdb/index.ts b/packages/cli/src/databases/migrations/postgresdb/index.ts index 8a771490d97b9..012b18e31d0f9 100644 --- a/packages/cli/src/databases/migrations/postgresdb/index.ts +++ b/packages/cli/src/databases/migrations/postgresdb/index.ts @@ -72,6 +72,7 @@ import { UpdateProcessedDataValueColumnToText1729607673464 } from '../common/172 import { CreateTestDefinitionTable1730386903556 } from '../common/1730386903556-CreateTestDefinitionTable'; import { AddDescriptionToTestDefinition1731404028106 } from '../common/1731404028106-AddDescriptionToTestDefinition'; import { CreateTestMetricTable1732271325258 } from '../common/1732271325258-CreateTestMetricTable'; +import { CreateTestRun1732549866705 } from '../common/1732549866705-CreateTestRunTable'; export const postgresMigrations: Migration[] = [ InitialMigration1587669153312, @@ -146,4 +147,5 @@ export const postgresMigrations: Migration[] = [ AddDescriptionToTestDefinition1731404028106, MigrateTestDefinitionKeyToString1731582748663, CreateTestMetricTable1732271325258, + CreateTestRun1732549866705, ]; diff --git a/packages/cli/src/databases/migrations/sqlite/index.ts b/packages/cli/src/databases/migrations/sqlite/index.ts index 80e5d8491d9d5..7c8fcbf86f489 100644 --- a/packages/cli/src/databases/migrations/sqlite/index.ts +++ b/packages/cli/src/databases/migrations/sqlite/index.ts @@ -69,6 +69,7 @@ import { SeparateExecutionCreationFromStart1727427440136 } from '../common/17274 import { UpdateProcessedDataValueColumnToText1729607673464 } from '../common/1729607673464-UpdateProcessedDataValueColumnToText'; import { CreateTestDefinitionTable1730386903556 } from '../common/1730386903556-CreateTestDefinitionTable'; import { CreateTestMetricTable1732271325258 } from '../common/1732271325258-CreateTestMetricTable'; +import { CreateTestRun1732549866705 } from '../common/1732549866705-CreateTestRunTable'; const sqliteMigrations: Migration[] = [ InitialMigration1588102412422, @@ -140,6 +141,7 @@ const sqliteMigrations: Migration[] = [ AddDescriptionToTestDefinition1731404028106, MigrateTestDefinitionKeyToString1731582748663, CreateTestMetricTable1732271325258, + CreateTestRun1732549866705, ]; export { sqliteMigrations }; diff --git a/packages/cli/src/databases/repositories/test-run.repository.ee.ts b/packages/cli/src/databases/repositories/test-run.repository.ee.ts new file mode 100644 index 0000000000000..e8e2a1a5eff03 --- /dev/null +++ b/packages/cli/src/databases/repositories/test-run.repository.ee.ts @@ -0,0 +1,29 @@ +import { DataSource, Repository } from '@n8n/typeorm'; +import { Service } from 'typedi'; + +import type { AggregatedTestRunMetrics } from '@/databases/entities/test-run.ee'; +import { TestRun } from '@/databases/entities/test-run.ee'; + +@Service() +export class TestRunRepository extends Repository { + constructor(dataSource: DataSource) { + super(TestRun, dataSource.manager); + } + + public async createTestRun(testDefinitionId: string) { + const testRun = this.create({ + status: 'new', + testDefinition: { id: testDefinitionId }, + }); + + return await this.save(testRun); + } + + public async markAsRunning(id: string) { + return await this.update(id, { status: 'running', runAt: new Date() }); + } + + public async markAsCompleted(id: string, metrics: AggregatedTestRunMetrics) { + return await this.update(id, { status: 'completed', completedAt: new Date(), metrics }); + } +} 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 f0d92ed450111..5c8f8e958b778 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 @@ -8,8 +8,10 @@ import path from 'path'; import type { ActiveExecutions } from '@/active-executions'; import type { ExecutionEntity } from '@/databases/entities/execution-entity'; import type { TestDefinition } from '@/databases/entities/test-definition.ee'; +import type { TestRun } from '@/databases/entities/test-run.ee'; import type { User } from '@/databases/entities/user'; import type { ExecutionRepository } from '@/databases/repositories/execution.repository'; +import type { TestRunRepository } from '@/databases/repositories/test-run.repository.ee'; import type { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import type { WorkflowRunner } from '@/workflow-runner'; @@ -61,6 +63,7 @@ describe('TestRunnerService', () => { const workflowRepository = mock(); const workflowRunner = mock(); const activeExecutions = mock(); + const testRunRepository = mock(); beforeEach(() => { const executionsQbMock = mockDeep>({ @@ -75,11 +78,16 @@ describe('TestRunnerService', () => { executionRepository.findOne .calledWith(expect.objectContaining({ where: { id: 'some-execution-id-2' } })) .mockResolvedValueOnce(executionMocks[1]); + + testRunRepository.createTestRun.mockResolvedValue(mock({ id: 'test-run-id' })); }); afterEach(() => { activeExecutions.getPostExecutePromise.mockClear(); workflowRunner.run.mockClear(); + testRunRepository.createTestRun.mockClear(); + testRunRepository.markAsRunning.mockClear(); + testRunRepository.markAsCompleted.mockClear(); }); test('should create an instance of TestRunnerService', async () => { @@ -88,6 +96,7 @@ describe('TestRunnerService', () => { workflowRunner, executionRepository, activeExecutions, + testRunRepository, ); expect(testRunnerService).toBeInstanceOf(TestRunnerService); @@ -99,6 +108,7 @@ describe('TestRunnerService', () => { workflowRunner, executionRepository, activeExecutions, + testRunRepository, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -132,6 +142,7 @@ describe('TestRunnerService', () => { workflowRunner, executionRepository, activeExecutions, + testRunRepository, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -207,5 +218,14 @@ describe('TestRunnerService', () => { }), }), ); + + // Check Test Run status was updated correctly + expect(testRunRepository.createTestRun).toHaveBeenCalledTimes(1); + expect(testRunRepository.markAsRunning).toHaveBeenCalledTimes(1); + expect(testRunRepository.markAsRunning).toHaveBeenCalledWith('test-run-id'); + expect(testRunRepository.markAsCompleted).toHaveBeenCalledTimes(1); + expect(testRunRepository.markAsCompleted).toHaveBeenCalledWith('test-run-id', { + success: false, + }); }); }); 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 f75b3f44a83e1..e717742b42f76 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 @@ -15,6 +15,7 @@ import type { TestDefinition } from '@/databases/entities/test-definition.ee'; import type { User } from '@/databases/entities/user'; import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; +import { TestRunRepository } from '@/databases/repositories/test-run.repository.ee'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import type { IExecutionResponse } from '@/interfaces'; import { getRunData } from '@/workflow-execute-additional-data'; @@ -37,6 +38,7 @@ export class TestRunnerService { private readonly workflowRunner: WorkflowRunner, private readonly executionRepository: ExecutionRepository, private readonly activeExecutions: ActiveExecutions, + private readonly testRunRepository: TestRunRepository, ) {} /** @@ -144,6 +146,10 @@ export class TestRunnerService { const evaluationWorkflow = await this.workflowRepository.findById(test.evaluationWorkflowId); assert(evaluationWorkflow, 'Evaluation workflow not found'); + // 0. Create new Test Run + const testRun = await this.testRunRepository.createTestRun(test.id); + assert(testRun, 'Unable to create a test run'); + // 1. Make test cases from previous executions // Select executions with the annotation tag and workflow ID of the test. @@ -160,7 +166,12 @@ export class TestRunnerService { // 2. Run over all the test cases + await this.testRunRepository.markAsRunning(testRun.id); + + const metrics = []; + for (const { id: pastExecutionId } of pastExecutions) { + // Fetch past execution with data const pastExecution = await this.executionRepository.findOne({ where: { id: pastExecutionId }, relations: ['executionData', 'metadata'], @@ -194,11 +205,13 @@ export class TestRunnerService { assert(evalExecution); // Extract the output of the last node executed in the evaluation workflow - this.extractEvaluationResult(evalExecution); - - // TODO: collect metrics + metrics.push(this.extractEvaluationResult(evalExecution)); } // TODO: 3. Aggregate the results + // Now we just set success to true if all the test cases passed + const aggregatedMetrics = { success: metrics.every((metric) => metric.success) }; + + await this.testRunRepository.markAsCompleted(testRun.id, aggregatedMetrics); } }