Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Save the number of successful, failed and total executions for Test Run (no-changelog) #12131

Merged
merged 25 commits into from
Jan 7, 2025
Merged

feat: Save the number of successful, failed and total executions for Test Run (no-changelog) #12131

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f5d3f0a
chore: Save the number of successful, failed and total executions for…
burivuhster Dec 6, 2024
af4b2f6
fix: tests
burivuhster Dec 10, 2024
513b6b0
fix: type error
burivuhster Dec 11, 2024
aa3d9ba
wip: add tests for pass/fail count
burivuhster Dec 11, 2024
665937a
wip: add tests for pass/fail count
burivuhster Dec 11, 2024
58cb56e
wip: Rewrite migrations to raw SQL, add column constraints
burivuhster Dec 18, 2024
a145733
wip: Change comments to JSDoc
burivuhster Dec 18, 2024
ebcdffe
wip: Report unexpected errors during the test run to Sentry
burivuhster Dec 18, 2024
a4af1e9
Merge branch 'master' into ai-505-backend-i-want-to-know-how-many-tes…
burivuhster Dec 18, 2024
4ec0032
wip: Fix tests
burivuhster Dec 18, 2024
c858eef
wip: Fix linter issue
burivuhster Dec 18, 2024
a13e5f7
wip: addressing PR feedback
burivuhster Dec 18, 2024
d0405de
wip: Add constraint for the stat columns in cancelled test run state
burivuhster Dec 19, 2024
52166c4
wip: Switch to ErrorReporter
burivuhster Dec 19, 2024
b400107
wip: Fix linter issue
burivuhster Dec 19, 2024
b6b19d7
wip: update check constraint for error state
burivuhster Dec 20, 2024
75a5a72
Merge branch 'master' into ai-505-backend-i-want-to-know-how-many-tes…
burivuhster Dec 20, 2024
e54dd55
chore: update migration timestamp after merge
burivuhster Dec 20, 2024
6c21fb3
Merge branch 'master' into ai-505-backend-i-want-to-know-how-many-tes…
burivuhster Jan 6, 2025
25dcbfb
wip: change db migration timestamp after merge
burivuhster Jan 6, 2025
9587e0e
Merge branch 'master' into ai-505-backend-i-want-to-know-how-many-tes…
burivuhster Jan 7, 2025
76565f7
wip: fix formatting issue
burivuhster Jan 7, 2025
9e2cb58
wip: fix lint issue
burivuhster Jan 7, 2025
446d46e
Merge branch 'master' into ai-505-backend-i-want-to-know-how-many-tes…
burivuhster Jan 7, 2025
c10da5b
wip: fix formatting issue
burivuhster Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions ...1733494526213-AddStatsColumnsToTestRun.ts → ...1736172058779-AddStatsColumnsToTestRun.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,18 @@ import type { MigrationContext, ReversibleMigration } from '@/databases/types';

const columns = ['totalCases', 'passedCases', 'failedCases'] as const;

export class AddStatsColumnsToTestRun1733494526213 implements ReversibleMigration {
export class AddStatsColumnsToTestRun1736172058779 implements ReversibleMigration {
async up({ escape, runQuery }: MigrationContext) {
const tableName = escape.tableName('test_run');
const columnNames = columns.map((name) => escape.columnName(name));

// Values can be NULL only if the test run is new, otherwise they must be non-negative integers
// Values can be NULL only if the test run is new, otherwise they must be non-negative integers.
// Test run might be cancelled or interrupted by unexpected error at any moment, so values can be either NULL or non-negative integers.
for (const name of columnNames) {
await runQuery(`ALTER TABLE ${tableName} ADD COLUMN ${name} INT CHECK(
CASE
WHEN status = 'new' THEN ${name} IS NULL
WHEN status in ('cancelled', 'error') THEN ${name} IS NULL OR ${name} >= 0
ELSE ${name} >= 0
END
)`);
Expand Down
7 changes: 5 additions & 2 deletions packages/cli/src/databases/migrations/mysqldb/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ import { AddDescriptionToTestDefinition1731404028106 } from '../common/173140402
import { CreateTestMetricTable1732271325258 } from '../common/1732271325258-CreateTestMetricTable';
import { CreateTestRun1732549866705 } from '../common/1732549866705-CreateTestRunTable';
import { AddMockedNodesColumnToTestDefinition1733133775640 } from '../common/1733133775640-AddMockedNodesColumnToTestDefinition';
import { AddStatsColumnsToTestRun1733494526213 } from '../common/1733494526213-AddStatsColumnsToTestRun';
import { AddManagedColumnToCredentialsTable1734479635324 } from '../common/1734479635324-AddManagedColumnToCredentialsTable';
import { AddStatsColumnsToTestRun1736172058779 } from '../common/1736172058779-AddStatsColumnsToTestRun';

export const mysqlMigrations: Migration[] = [
InitialMigration1588157391238,
Expand Down Expand Up @@ -152,5 +153,7 @@ export const mysqlMigrations: Migration[] = [
CreateTestMetricTable1732271325258,
CreateTestRun1732549866705,
AddMockedNodesColumnToTestDefinition1733133775640,
AddStatsColumnsToTestRun1733494526213,
AddManagedColumnToCredentialsTable1734479635324,
AddProjectIcons1729607673469,
AddStatsColumnsToTestRun1736172058779,
];
7 changes: 5 additions & 2 deletions packages/cli/src/databases/migrations/postgresdb/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ import { AddDescriptionToTestDefinition1731404028106 } from '../common/173140402
import { CreateTestMetricTable1732271325258 } from '../common/1732271325258-CreateTestMetricTable';
import { CreateTestRun1732549866705 } from '../common/1732549866705-CreateTestRunTable';
import { AddMockedNodesColumnToTestDefinition1733133775640 } from '../common/1733133775640-AddMockedNodesColumnToTestDefinition';
import { AddStatsColumnsToTestRun1733494526213 } from '../common/1733494526213-AddStatsColumnsToTestRun';
import { AddManagedColumnToCredentialsTable1734479635324 } from '../common/1734479635324-AddManagedColumnToCredentialsTable';
import { AddStatsColumnsToTestRun1736172058779 } from '../common/1736172058779-AddStatsColumnsToTestRun';

export const postgresMigrations: Migration[] = [
InitialMigration1587669153312,
Expand Down Expand Up @@ -152,5 +153,7 @@ export const postgresMigrations: Migration[] = [
CreateTestMetricTable1732271325258,
CreateTestRun1732549866705,
AddMockedNodesColumnToTestDefinition1733133775640,
AddStatsColumnsToTestRun1733494526213,
AddManagedColumnToCredentialsTable1734479635324,
AddProjectIcons1729607673469,
AddStatsColumnsToTestRun1736172058779,
];
7 changes: 5 additions & 2 deletions packages/cli/src/databases/migrations/sqlite/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ import { CreateTestDefinitionTable1730386903556 } from '../common/1730386903556-
import { CreateTestMetricTable1732271325258 } from '../common/1732271325258-CreateTestMetricTable';
import { CreateTestRun1732549866705 } from '../common/1732549866705-CreateTestRunTable';
import { AddMockedNodesColumnToTestDefinition1733133775640 } from '../common/1733133775640-AddMockedNodesColumnToTestDefinition';
import { AddStatsColumnsToTestRun1733494526213 } from '../common/1733494526213-AddStatsColumnsToTestRun';
import { AddManagedColumnToCredentialsTable1734479635324 } from '../common/1734479635324-AddManagedColumnToCredentialsTable';
import { AddStatsColumnsToTestRun1736172058779 } from '../common/1736172058779-AddStatsColumnsToTestRun';

const sqliteMigrations: Migration[] = [
InitialMigration1588102412422,
Expand Down Expand Up @@ -146,7 +147,9 @@ const sqliteMigrations: Migration[] = [
CreateTestMetricTable1732271325258,
CreateTestRun1732549866705,
AddMockedNodesColumnToTestDefinition1733133775640,
AddStatsColumnsToTestRun1733494526213,
AddManagedColumnToCredentialsTable1734479635324,
AddProjectIcons1729607673469,
AddStatsColumnsToTestRun1736172058779,
];

export { sqliteMigrations };
8 changes: 4 additions & 4 deletions packages/cli/src/databases/repositories/test-run.repository.ee.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class TestRunRepository extends Repository<TestRun> {
return await this.save(testRun);
}

public async markAsRunning(id: string, totalCases: number) {
async markAsRunning(id: string, totalCases: number) {
return await this.update(id, {
status: 'running',
runAt: new Date(),
Expand All @@ -35,15 +35,15 @@ export class TestRunRepository extends Repository<TestRun> {
return await this.update(id, { status: 'completed', completedAt: new Date(), metrics });
}

public async incrementPassed(id: string) {
async incrementPassed(id: string) {
return await this.increment({ id }, 'passedCases', 1);
}

public async incrementFailed(id: string) {
async incrementFailed(id: string) {
return await this.increment({ id }, 'failedCases', 1);
}

public async getMany(testDefinitionId: string, options: ListQuery.Options) {
async getMany(testDefinitionId: string, options: ListQuery.Options) {
const findManyOptions: FindManyOptions<TestRun> = {
where: { testDefinition: { id: testDefinitionId } },
order: { createdAt: 'DESC' },
Expand Down
10 changes: 10 additions & 0 deletions packages/cli/src/evaluation.ee/test-runner/__tests__/test-runner.service.ee.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { SelectQueryBuilder } from '@n8n/typeorm';
import { stringify } from 'flatted';
import { readFileSync } from 'fs';
import { mock, mockDeep } from 'jest-mock-extended';
import type { ErrorReporter } from 'n8n-core';
import type { ExecutionError, GenericValue, IRun } from 'n8n-workflow';
import path from 'path';

Expand Down Expand Up @@ -182,6 +183,7 @@ describe('TestRunnerService', () => {
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
);

expect(testRunnerService).toBeInstanceOf(TestRunnerService);
Expand All @@ -196,6 +198,7 @@ describe('TestRunnerService', () => {
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -233,6 +236,7 @@ describe('TestRunnerService', () => {
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -333,6 +337,7 @@ describe('TestRunnerService', () => {
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -390,6 +395,7 @@ describe('TestRunnerService', () => {
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -443,6 +449,7 @@ describe('TestRunnerService', () => {
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -500,6 +507,7 @@ describe('TestRunnerService', () => {
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
);

workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({
Expand Down Expand Up @@ -573,6 +581,7 @@ describe('TestRunnerService', () => {
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
);

const startNodesData = (testRunnerService as any).getStartNodesData(
Expand All @@ -597,6 +606,7 @@ describe('TestRunnerService', () => {
testRunRepository,
testMetricRepository,
mockNodeTypes,
mock<ErrorReporter>(),
);

const startNodesData = (testRunnerService as any).getStartNodesData(
Expand Down
8 changes: 5 additions & 3 deletions packages/cli/src/evaluation.ee/test-runner/test-runner.service.ee.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { captureException } from '@sentry/node';
import { Service } from '@n8n/di';
import { parse } from 'flatted';
import { ErrorReporter } from 'n8n-core';
import { NodeConnectionType, Workflow } from 'n8n-workflow';
import type {
IDataObject,
Expand Down Expand Up @@ -45,6 +46,7 @@ export class TestRunnerService {
private readonly testRunRepository: TestRunRepository,
private readonly testMetricRepository: TestMetricRepository,
private readonly nodeTypes: NodeTypes,
private readonly errorReporter: ErrorReporter,
) {}

/**
Expand Down Expand Up @@ -243,6 +245,7 @@ export class TestRunnerService {
const testCaseExecution = await this.runTestCase(
workflow,
executionData,
pastExecution.executionData.workflowData,
test.mockedNodes,
user.id,
);
Expand Down Expand Up @@ -280,8 +283,7 @@ export class TestRunnerService {
// In case of an unexpected error, increment the failed count and continue with the next test case
await this.testRunRepository.incrementFailed(testRun.id);

// Report error to Sentry
captureException(e);
this.errorReporter.error(e);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any chance the execution might fail due to user misconfiguring things? We already get a lot of spam into sentry, so we should avoid adding anymore that could potentially be user caused

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand – errors originating in the workflow under test or evaluation workflow will not throw an exception, and will have the original exception stored in data.resultData.error instead, so it will be handled without sending an event to Sentry in the code above.

}

Expand Down
Loading