diff --git a/cypress/e2e/34-template-credentials-setup.cy.ts b/cypress/e2e/34-template-credentials-setup.cy.ts index 386c83eb0a..8d372acac0 100644 --- a/cypress/e2e/34-template-credentials-setup.cy.ts +++ b/cypress/e2e/34-template-credentials-setup.cy.ts @@ -3,6 +3,7 @@ import * as formStep from '../composables/setup-template-form-step'; import { getSetupWorkflowCredentialsButton } from '../composables/setup-workflow-credentials-button'; import TestTemplate1 from '../fixtures/Test_Template_1.json'; import TestTemplate2 from '../fixtures/Test_Template_2.json'; +import { clearNotifications } from '../pages/notifications'; import { clickUseWorkflowButtonByTitle, visitTemplateCollectionPage, @@ -111,16 +112,19 @@ describe('Template credentials setup', () => { templateCredentialsSetupPage.fillInDummyCredentialsForAppWithConfirm('X (Formerly Twitter)'); templateCredentialsSetupPage.fillInDummyCredentialsForApp('Telegram'); + clearNotifications(); + templateCredentialsSetupPage.finishCredentialSetup(); workflowPage.getters.canvasNodes().should('have.length', 3); + cy.grantBrowserPermissions('clipboardReadWrite', 'clipboardSanitizedWrite'); + // Focus the canvas so the copy to clipboard works workflowPage.getters.canvasNodes().eq(0).realClick(); workflowPage.actions.hitSelectAll(); workflowPage.actions.hitCopy(); - cy.grantBrowserPermissions('clipboardReadWrite', 'clipboardSanitizedWrite'); // Check workflow JSON by copying it to clipboard cy.readClipboard().then((workflowJSON) => { const workflow = JSON.parse(workflowJSON); @@ -154,6 +158,8 @@ describe('Template credentials setup', () => { templateCredentialsSetupPage.fillInDummyCredentialsForApp('Email (IMAP)'); templateCredentialsSetupPage.fillInDummyCredentialsForApp('Nextcloud'); + clearNotifications(); + templateCredentialsSetupPage.finishCredentialSetup(); workflowPage.getters.canvasNodes().should('have.length', 3); @@ -176,6 +182,8 @@ describe('Template credentials setup', () => { templateCredentialsSetupPage.visitTemplateCredentialSetupPage(testTemplate.id); templateCredentialsSetupPage.fillInDummyCredentialsForApp('Shopify'); + clearNotifications(); + templateCredentialsSetupPage.finishCredentialSetup(); getSetupWorkflowCredentialsButton().should('be.visible'); @@ -192,6 +200,8 @@ describe('Template credentials setup', () => { templateCredentialsSetupPage.fillInDummyCredentialsForAppWithConfirm('X (Formerly Twitter)'); templateCredentialsSetupPage.fillInDummyCredentialsForApp('Telegram'); + clearNotifications(); + setupCredsModal.closeModalFromContinueButton(); setupCredsModal.getWorkflowCredentialsModal().should('not.exist'); diff --git a/cypress/fixtures/Test_Subworkflow-Inputs.json b/cypress/fixtures/Test_Subworkflow-Inputs.json index aeb4d601fd..ee2b34513e 100644 --- a/cypress/fixtures/Test_Subworkflow-Inputs.json +++ b/cypress/fixtures/Test_Subworkflow-Inputs.json @@ -19,7 +19,6 @@ "value": {}, "matchingColumns": [], "schema": [], - "ignoreTypeMismatchErrors": false, "attemptToConvertTypes": false, "convertFieldsToString": true }, diff --git a/cypress/pages/modals/credentials-modal.ts b/cypress/pages/modals/credentials-modal.ts index b8907386a0..ac8e966bc0 100644 --- a/cypress/pages/modals/credentials-modal.ts +++ b/cypress/pages/modals/credentials-modal.ts @@ -61,6 +61,7 @@ export class CredentialsModal extends BasePage { this.getters .credentialInputs() .find('input[type=text], input[type=password]') + .filter(':not([readonly])') .each(($el) => { cy.wrap($el).type('test'); }); diff --git a/cypress/pages/notifications.ts b/cypress/pages/notifications.ts index 162c536007..af2a51e369 100644 --- a/cypress/pages/notifications.ts +++ b/cypress/pages/notifications.ts @@ -13,5 +13,10 @@ export const infoToast = () => cy.get('.el-notification:has(.el-notification--in * Actions */ export const clearNotifications = () => { - successToast().find('.el-notification__closeBtn').click({ multiple: true }); + const buttons = successToast().find('.el-notification__closeBtn'); + buttons.then(($buttons) => { + if ($buttons.length) { + buttons.click({ multiple: true }); + } + }); }; diff --git a/packages/@n8n/nodes-langchain/nodes/llms/LmChatGoogleVertex/LmChatGoogleVertex.node.ts b/packages/@n8n/nodes-langchain/nodes/llms/LmChatGoogleVertex/LmChatGoogleVertex.node.ts index 5ca6091378..ab3320c2ba 100644 --- a/packages/@n8n/nodes-langchain/nodes/llms/LmChatGoogleVertex/LmChatGoogleVertex.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/llms/LmChatGoogleVertex/LmChatGoogleVertex.node.ts @@ -131,6 +131,7 @@ export class LmChatGoogleVertex implements INodeType { const credentials = await this.getCredentials('googleApi'); const privateKey = formatPrivateKey(credentials.privateKey as string); const email = (credentials.email as string).trim(); + const region = credentials.region as string; const modelName = this.getNodeParameter('modelName', itemIndex) as string; @@ -165,6 +166,7 @@ export class LmChatGoogleVertex implements INodeType { private_key: privateKey, }, }, + location: region, model: modelName, topK: options.topK, topP: options.topP, diff --git a/packages/cli/src/databases/entities/test-run.ee.ts b/packages/cli/src/databases/entities/test-run.ee.ts index 17f7f91a97..79c7bc9f07 100644 --- a/packages/cli/src/databases/entities/test-run.ee.ts +++ b/packages/cli/src/databases/entities/test-run.ee.ts @@ -35,4 +35,23 @@ export class TestRun extends WithTimestampsAndStringId { @Column(jsonColumnType, { nullable: true }) metrics: AggregatedTestRunMetrics; + + /** + * Total number of the test cases, matching the filter condition of the test definition (specified annotationTag) + */ + @Column('integer', { nullable: true }) + totalCases: number; + + /** + * Number of test cases that passed (evaluation workflow was executed successfully) + */ + @Column('integer', { nullable: true }) + passedCases: number; + + /** + * Number of failed test cases + * (any unexpected exception happened during the execution or evaluation workflow ended with an error) + */ + @Column('integer', { nullable: true }) + failedCases: number; } diff --git a/packages/cli/src/databases/migrations/common/1736172058779-AddStatsColumnsToTestRun.ts b/packages/cli/src/databases/migrations/common/1736172058779-AddStatsColumnsToTestRun.ts new file mode 100644 index 0000000000..8d6a5cdc20 --- /dev/null +++ b/packages/cli/src/databases/migrations/common/1736172058779-AddStatsColumnsToTestRun.ts @@ -0,0 +1,31 @@ +import type { MigrationContext, ReversibleMigration } from '@/databases/types'; + +const columns = ['totalCases', 'passedCases', 'failedCases'] as const; + +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. + // 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 + )`); + } + } + + async down({ escape, runQuery }: MigrationContext) { + const tableName = escape.tableName('test_run'); + const columnNames = columns.map((name) => escape.columnName(name)); + + for (const name of columnNames) { + await runQuery(`ALTER TABLE ${tableName} DROP COLUMN ${name}`); + } + } +} diff --git a/packages/cli/src/databases/migrations/mysqldb/index.ts b/packages/cli/src/databases/migrations/mysqldb/index.ts index 89df273472..b76409c0c1 100644 --- a/packages/cli/src/databases/migrations/mysqldb/index.ts +++ b/packages/cli/src/databases/migrations/mysqldb/index.ts @@ -76,6 +76,7 @@ import { CreateTestMetricTable1732271325258 } from '../common/1732271325258-Crea import { CreateTestRun1732549866705 } from '../common/1732549866705-CreateTestRunTable'; import { AddMockedNodesColumnToTestDefinition1733133775640 } from '../common/1733133775640-AddMockedNodesColumnToTestDefinition'; import { AddManagedColumnToCredentialsTable1734479635324 } from '../common/1734479635324-AddManagedColumnToCredentialsTable'; +import { AddStatsColumnsToTestRun1736172058779 } from '../common/1736172058779-AddStatsColumnsToTestRun'; export const mysqlMigrations: Migration[] = [ InitialMigration1588157391238, @@ -154,4 +155,5 @@ export const mysqlMigrations: Migration[] = [ AddMockedNodesColumnToTestDefinition1733133775640, AddManagedColumnToCredentialsTable1734479635324, AddProjectIcons1729607673469, + AddStatsColumnsToTestRun1736172058779, ]; diff --git a/packages/cli/src/databases/migrations/postgresdb/index.ts b/packages/cli/src/databases/migrations/postgresdb/index.ts index d5d72282f4..7cf90bde5b 100644 --- a/packages/cli/src/databases/migrations/postgresdb/index.ts +++ b/packages/cli/src/databases/migrations/postgresdb/index.ts @@ -76,6 +76,7 @@ import { CreateTestMetricTable1732271325258 } from '../common/1732271325258-Crea import { CreateTestRun1732549866705 } from '../common/1732549866705-CreateTestRunTable'; import { AddMockedNodesColumnToTestDefinition1733133775640 } from '../common/1733133775640-AddMockedNodesColumnToTestDefinition'; import { AddManagedColumnToCredentialsTable1734479635324 } from '../common/1734479635324-AddManagedColumnToCredentialsTable'; +import { AddStatsColumnsToTestRun1736172058779 } from '../common/1736172058779-AddStatsColumnsToTestRun'; export const postgresMigrations: Migration[] = [ InitialMigration1587669153312, @@ -154,4 +155,5 @@ export const postgresMigrations: Migration[] = [ AddMockedNodesColumnToTestDefinition1733133775640, AddManagedColumnToCredentialsTable1734479635324, AddProjectIcons1729607673469, + AddStatsColumnsToTestRun1736172058779, ]; diff --git a/packages/cli/src/databases/migrations/sqlite/index.ts b/packages/cli/src/databases/migrations/sqlite/index.ts index 7fec59baf2..363a6e47c3 100644 --- a/packages/cli/src/databases/migrations/sqlite/index.ts +++ b/packages/cli/src/databases/migrations/sqlite/index.ts @@ -73,6 +73,7 @@ import { CreateTestMetricTable1732271325258 } from '../common/1732271325258-Crea import { CreateTestRun1732549866705 } from '../common/1732549866705-CreateTestRunTable'; import { AddMockedNodesColumnToTestDefinition1733133775640 } from '../common/1733133775640-AddMockedNodesColumnToTestDefinition'; import { AddManagedColumnToCredentialsTable1734479635324 } from '../common/1734479635324-AddManagedColumnToCredentialsTable'; +import { AddStatsColumnsToTestRun1736172058779 } from '../common/1736172058779-AddStatsColumnsToTestRun'; const sqliteMigrations: Migration[] = [ InitialMigration1588102412422, @@ -148,6 +149,7 @@ const sqliteMigrations: Migration[] = [ AddMockedNodesColumnToTestDefinition1733133775640, AddManagedColumnToCredentialsTable1734479635324, AddProjectIcons1729607673469, + AddStatsColumnsToTestRun1736172058779, ]; 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 index a5658196ce..728f0bc464 100644 --- a/packages/cli/src/databases/repositories/test-run.repository.ee.ts +++ b/packages/cli/src/databases/repositories/test-run.repository.ee.ts @@ -21,8 +21,14 @@ export class TestRunRepository extends Repository { return await this.save(testRun); } - async markAsRunning(id: string) { - return await this.update(id, { status: 'running', runAt: new Date() }); + async markAsRunning(id: string, totalCases: number) { + return await this.update(id, { + status: 'running', + runAt: new Date(), + totalCases, + passedCases: 0, + failedCases: 0, + }); } async markAsCompleted(id: string, metrics: AggregatedTestRunMetrics) { @@ -33,6 +39,14 @@ export class TestRunRepository extends Repository { return await this.update(id, { status: 'cancelled' }); } + async incrementPassed(id: string) { + return await this.increment({ id }, 'passedCases', 1); + } + + async incrementFailed(id: string) { + return await this.increment({ id }, 'failedCases', 1); + } + async getMany(testDefinitionId: string, options: ListQuery.Options) { const findManyOptions: FindManyOptions = { where: { testDefinition: { id: testDefinitionId } }, diff --git a/packages/cli/src/evaluation.ee/test-runner/__tests__/test-runner.service.ee.test.ts b/packages/cli/src/evaluation.ee/test-runner/__tests__/test-runner.service.ee.test.ts index a3b0c84978..672222f7df 100644 --- a/packages/cli/src/evaluation.ee/test-runner/__tests__/test-runner.service.ee.test.ts +++ b/packages/cli/src/evaluation.ee/test-runner/__tests__/test-runner.service.ee.test.ts @@ -2,7 +2,8 @@ import type { SelectQueryBuilder } from '@n8n/typeorm'; import { stringify } from 'flatted'; import { readFileSync } from 'fs'; import { mock, mockDeep } from 'jest-mock-extended'; -import type { GenericValue, IRun } from 'n8n-workflow'; +import type { ErrorReporter } from 'n8n-core'; +import type { ExecutionError, GenericValue, IRun } from 'n8n-workflow'; import path from 'path'; import type { ActiveExecutions } from '@/active-executions'; @@ -90,6 +91,16 @@ function mockExecutionData() { }); } +function mockErrorExecutionData() { + return mock({ + data: { + resultData: { + error: mock(), + }, + }, + }); +} + function mockEvaluationExecutionData(metrics: Record) { return mock({ data: { @@ -110,6 +121,9 @@ function mockEvaluationExecutionData(metrics: Record) { }, ], }, + // error is an optional prop, but jest-mock-extended will mock it by default, + // which affects the code logic. So, we need to explicitly set it to undefined. + error: undefined, }, }, }); @@ -158,6 +172,8 @@ describe('TestRunnerService', () => { afterEach(() => { jest.resetAllMocks(); + testRunRepository.incrementFailed.mockClear(); + testRunRepository.incrementPassed.mockClear(); }); test('should create an instance of TestRunnerService', async () => { @@ -169,6 +185,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, + mock(), ); expect(testRunnerService).toBeInstanceOf(TestRunnerService); @@ -183,6 +200,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, + mock(), ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -220,6 +238,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, + mock(), ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -300,12 +319,185 @@ 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.markAsRunning).toHaveBeenCalledWith('test-run-id', expect.any(Number)); expect(testRunRepository.markAsCompleted).toHaveBeenCalledTimes(1); expect(testRunRepository.markAsCompleted).toHaveBeenCalledWith('test-run-id', { metric1: 0.75, metric2: 0, }); + + expect(testRunRepository.incrementPassed).toHaveBeenCalledTimes(2); + expect(testRunRepository.incrementFailed).not.toHaveBeenCalled(); + }); + + test('should properly count passed and failed executions', async () => { + const testRunnerService = new TestRunnerService( + workflowRepository, + workflowRunner, + executionRepository, + activeExecutions, + testRunRepository, + testMetricRepository, + mockNodeTypes, + mock(), + ); + + workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ + id: 'workflow-under-test-id', + ...wfUnderTestJson, + }); + + workflowRepository.findById.calledWith('evaluation-workflow-id').mockResolvedValueOnce({ + id: 'evaluation-workflow-id', + ...wfEvaluationJson, + }); + + workflowRunner.run.mockResolvedValueOnce('some-execution-id'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-2'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-3'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-4'); + + // Mock executions of workflow under test + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id') + .mockResolvedValue(mockExecutionData()); + + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-3') + .mockResolvedValue(mockExecutionData()); + + // Mock executions of evaluation workflow + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-2') + .mockResolvedValue(mockEvaluationExecutionData({ metric1: 1, metric2: 0 })); + + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-4') + .mockRejectedValue(new Error('Some error')); + + await testRunnerService.runTest( + mock(), + mock({ + workflowId: 'workflow-under-test-id', + evaluationWorkflowId: 'evaluation-workflow-id', + mockedNodes: [], + }), + ); + + expect(testRunRepository.incrementPassed).toHaveBeenCalledTimes(1); + expect(testRunRepository.incrementFailed).toHaveBeenCalledTimes(1); + }); + + test('should properly count failed test executions', async () => { + const testRunnerService = new TestRunnerService( + workflowRepository, + workflowRunner, + executionRepository, + activeExecutions, + testRunRepository, + testMetricRepository, + mockNodeTypes, + mock(), + ); + + workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ + id: 'workflow-under-test-id', + ...wfUnderTestJson, + }); + + workflowRepository.findById.calledWith('evaluation-workflow-id').mockResolvedValueOnce({ + id: 'evaluation-workflow-id', + ...wfEvaluationJson, + }); + + workflowRunner.run.mockResolvedValueOnce('some-execution-id'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-2'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-3'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-4'); + + // Mock executions of workflow under test + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id') + .mockResolvedValue(mockExecutionData()); + + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-3') + .mockResolvedValue(mockErrorExecutionData()); + + // Mock executions of evaluation workflow + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-2') + .mockResolvedValue(mockEvaluationExecutionData({ metric1: 1, metric2: 0 })); + + await testRunnerService.runTest( + mock(), + mock({ + workflowId: 'workflow-under-test-id', + evaluationWorkflowId: 'evaluation-workflow-id', + mockedNodes: [], + }), + ); + + expect(testRunRepository.incrementPassed).toHaveBeenCalledTimes(1); + expect(testRunRepository.incrementFailed).toHaveBeenCalledTimes(1); + }); + + test('should properly count failed evaluations', async () => { + const testRunnerService = new TestRunnerService( + workflowRepository, + workflowRunner, + executionRepository, + activeExecutions, + testRunRepository, + testMetricRepository, + mockNodeTypes, + mock(), + ); + + workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ + id: 'workflow-under-test-id', + ...wfUnderTestJson, + }); + + workflowRepository.findById.calledWith('evaluation-workflow-id').mockResolvedValueOnce({ + id: 'evaluation-workflow-id', + ...wfEvaluationJson, + }); + + workflowRunner.run.mockResolvedValueOnce('some-execution-id'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-2'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-3'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-4'); + + // Mock executions of workflow under test + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id') + .mockResolvedValue(mockExecutionData()); + + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-3') + .mockResolvedValue(mockExecutionData()); + + // Mock executions of evaluation workflow + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-2') + .mockResolvedValue(mockEvaluationExecutionData({ metric1: 1, metric2: 0 })); + + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-4') + .mockResolvedValue(mockErrorExecutionData()); + + await testRunnerService.runTest( + mock(), + mock({ + workflowId: 'workflow-under-test-id', + evaluationWorkflowId: 'evaluation-workflow-id', + mockedNodes: [], + }), + ); + + expect(testRunRepository.incrementPassed).toHaveBeenCalledTimes(1); + expect(testRunRepository.incrementFailed).toHaveBeenCalledTimes(1); }); test('should specify correct start nodes when running workflow under test', async () => { @@ -317,6 +509,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, + mock(), ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -390,6 +583,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, + mock(), ); const startNodesData = (testRunnerService as any).getStartNodesData( @@ -414,6 +608,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, + mock(), ); const startNodesData = (testRunnerService as any).getStartNodesData( @@ -443,6 +638,7 @@ describe('TestRunnerService', () => { testRunRepository, testMetricRepository, mockNodeTypes, + mock(), ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ 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 ab66b4e776..8be9916e5b 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 @@ -1,5 +1,6 @@ import { Service } from '@n8n/di'; import { parse } from 'flatted'; +import { ErrorReporter } from 'n8n-core'; import { ExecutionCancelledError, NodeConnectionType, Workflow } from 'n8n-workflow'; import type { IDataObject, @@ -48,6 +49,7 @@ export class TestRunnerService { private readonly testRunRepository: TestRunRepository, private readonly testMetricRepository: TestMetricRepository, private readonly nodeTypes: NodeTypes, + private readonly errorReporter: ErrorReporter, ) {} /** @@ -260,16 +262,17 @@ export class TestRunnerService { const testMetricNames = await this.getTestMetricNames(test.id); // 2. Run over all the test cases - await this.testRunRepository.markAsRunning(testRun.id); + await this.testRunRepository.markAsRunning(testRun.id, pastExecutions.length); // Object to collect the results of the evaluation workflow executions const metrics = new EvaluationMetrics(testMetricNames); - for (const { id: pastExecutionId } of pastExecutions) { - if (abortSignal.aborted) { - break; - } + for (const { id: pastExecutionId } of pastExecutions) { + if (abortSignal.aborted) { + break; + } + try { // Fetch past execution with data const pastExecution = await this.executionRepository.findOne({ where: { id: pastExecutionId }, @@ -290,8 +293,9 @@ export class TestRunnerService { ); // In case of a permission check issue, the test case execution will be undefined. - // Skip them and continue with the next test case + // Skip them, increment the failed count and continue with the next test case if (!testCaseExecution) { + await this.testRunRepository.incrementFailed(testRun.id); continue; } @@ -313,7 +317,18 @@ export class TestRunnerService { // Extract the output of the last node executed in the evaluation workflow metrics.addResults(this.extractEvaluationResult(evalExecution)); + if (evalExecution.data.resultData.error) { + await this.testRunRepository.incrementFailed(testRun.id); + } else { + await this.testRunRepository.incrementPassed(testRun.id); + } + } 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); + + this.errorReporter.error(e); } + } // Mark the test run as completed or cancelled if (abortSignal.aborted) { diff --git a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts index 9b4d8aecd2..c41ea773dd 100644 --- a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts @@ -5,7 +5,9 @@ import type { INode, INodesGraphResult } from 'n8n-workflow'; import { NodeApiError, TelemetryHelpers, type IRun, type IWorkflowBase } from 'n8n-workflow'; import { N8N_VERSION } from '@/constants'; +import type { CredentialsEntity } from '@/databases/entities/credentials-entity'; import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; +import type { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import type { ProjectRelationRepository } from '@/databases/repositories/project-relation.repository'; import type { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository'; import type { WorkflowRepository } from '@/databases/repositories/workflow.repository'; @@ -52,6 +54,7 @@ describe('TelemetryEventRelay', () => { const nodeTypes = mock(); const sharedWorkflowRepository = mock(); const projectRelationRepository = mock(); + const credentialsRepository = mock(); const eventService = new EventService(); let telemetryEventRelay: TelemetryEventRelay; @@ -67,6 +70,7 @@ describe('TelemetryEventRelay', () => { nodeTypes, sharedWorkflowRepository, projectRelationRepository, + credentialsRepository, ); await telemetryEventRelay.init(); @@ -90,6 +94,7 @@ describe('TelemetryEventRelay', () => { nodeTypes, sharedWorkflowRepository, projectRelationRepository, + credentialsRepository, ); // @ts-expect-error Private method const setupListenersSpy = jest.spyOn(telemetryEventRelay, 'setupListeners'); @@ -112,6 +117,7 @@ describe('TelemetryEventRelay', () => { nodeTypes, sharedWorkflowRepository, projectRelationRepository, + credentialsRepository, ); // @ts-expect-error Private method const setupListenersSpy = jest.spyOn(telemetryEventRelay, 'setupListeners'); @@ -1197,6 +1203,9 @@ describe('TelemetryEventRelay', () => { it('should call telemetry.track when manual node execution finished', async () => { sharedWorkflowRepository.findSharingRole.mockResolvedValue('workflow:editor'); + credentialsRepository.findOneBy.mockResolvedValue( + mock({ type: 'openAiApi', isManaged: false }), + ); const runData = { status: 'error', @@ -1276,6 +1285,8 @@ describe('TelemetryEventRelay', () => { error_node_id: '1', node_id: '1', node_type: 'n8n-nodes-base.jira', + is_managed: false, + credential_type: null, node_graph_string: JSON.stringify(nodeGraph.nodeGraph), }), ); @@ -1498,5 +1509,118 @@ describe('TelemetryEventRelay', () => { }), ); }); + + it('should call telemetry.track when manual node execution finished with is_managed and credential_type properties', async () => { + sharedWorkflowRepository.findSharingRole.mockResolvedValue('workflow:editor'); + credentialsRepository.findOneBy.mockResolvedValue( + mock({ type: 'openAiApi', isManaged: true }), + ); + + const runData = { + status: 'error', + mode: 'manual', + data: { + executionData: { + nodeExecutionStack: [{ node: { credentials: { openAiApi: { id: 'nhu-l8E4hX' } } } }], + }, + startData: { + destinationNode: 'OpenAI', + runNodeFilter: ['OpenAI'], + }, + resultData: { + runData: {}, + lastNodeExecuted: 'OpenAI', + error: new NodeApiError( + { + id: '1', + typeVersion: 1, + name: 'Jira', + type: 'n8n-nodes-base.jira', + parameters: {}, + position: [100, 200], + }, + { + message: 'Error message', + description: 'Incorrect API key provided', + httpCode: '401', + stack: '', + }, + { + message: 'Error message', + description: 'Error description', + level: 'warning', + functionality: 'regular', + }, + ), + }, + }, + } as unknown as IRun; + + const nodeGraph: INodesGraphResult = { + nodeGraph: { node_types: [], node_connections: [], webhookNodeNames: [] }, + nameIndices: { + Jira: '1', + OpenAI: '1', + }, + } as unknown as INodesGraphResult; + + jest.spyOn(TelemetryHelpers, 'generateNodesGraph').mockImplementation(() => nodeGraph); + + jest + .spyOn(TelemetryHelpers, 'getNodeTypeForName') + .mockImplementation( + () => ({ type: 'n8n-nodes-base.jira', version: 1, name: 'Jira' }) as unknown as INode, + ); + + const event: RelayEventMap['workflow-post-execute'] = { + workflow: mockWorkflowBase, + executionId: 'execution123', + userId: 'user123', + runData, + }; + + eventService.emit('workflow-post-execute', event); + + await flushPromises(); + + expect(credentialsRepository.findOneBy).toHaveBeenCalledWith({ + id: 'nhu-l8E4hX', + }); + + expect(telemetry.track).toHaveBeenCalledWith( + 'Manual node exec finished', + expect.objectContaining({ + webhook_domain: null, + user_id: 'user123', + workflow_id: 'workflow123', + status: 'error', + executionStatus: 'error', + sharing_role: 'sharee', + error_message: 'Error message', + error_node_type: 'n8n-nodes-base.jira', + error_node_id: '1', + node_id: '1', + node_type: 'n8n-nodes-base.jira', + + is_managed: true, + credential_type: 'openAiApi', + node_graph_string: JSON.stringify(nodeGraph.nodeGraph), + }), + ); + + expect(telemetry.trackWorkflowExecution).toHaveBeenCalledWith( + expect.objectContaining({ + workflow_id: 'workflow123', + success: false, + is_manual: true, + execution_mode: 'manual', + version_cli: N8N_VERSION, + error_message: 'Error message', + error_node_type: 'n8n-nodes-base.jira', + node_graph_string: JSON.stringify(nodeGraph.nodeGraph), + error_node_id: '1', + }), + ); + }); }); }); diff --git a/packages/cli/src/events/relays/telemetry.event-relay.ts b/packages/cli/src/events/relays/telemetry.event-relay.ts index 221449bbab..d2bc61c733 100644 --- a/packages/cli/src/events/relays/telemetry.event-relay.ts +++ b/packages/cli/src/events/relays/telemetry.event-relay.ts @@ -9,6 +9,7 @@ import { get as pslGet } from 'psl'; import config from '@/config'; import { N8N_VERSION } from '@/constants'; +import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import { ProjectRelationRepository } from '@/databases/repositories/project-relation.repository'; import { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; @@ -34,6 +35,7 @@ export class TelemetryEventRelay extends EventRelay { private readonly nodeTypes: NodeTypes, private readonly sharedWorkflowRepository: SharedWorkflowRepository, private readonly projectRelationRepository: ProjectRelationRepository, + private readonly credentialsRepository: CredentialsRepository, ) { super(eventService); } @@ -693,6 +695,8 @@ export class TelemetryEventRelay extends EventRelay { error_node_id: telemetryProperties.error_node_id as string, webhook_domain: null, sharing_role: userRole, + credential_type: null, + is_managed: false, }; if (!manualExecEventProperties.node_graph_string) { @@ -703,7 +707,18 @@ export class TelemetryEventRelay extends EventRelay { } if (runData.data.startData?.destinationNode) { - const telemetryPayload = { + const credentialsData = TelemetryHelpers.extractLastExecutedNodeCredentialData(runData); + if (credentialsData) { + manualExecEventProperties.credential_type = credentialsData.credentialType; + const credential = await this.credentialsRepository.findOneBy({ + id: credentialsData.credentialId, + }); + if (credential) { + manualExecEventProperties.is_managed = credential.isManaged; + } + } + + const telemetryPayload: ITelemetryTrackProperties = { ...manualExecEventProperties, node_type: TelemetryHelpers.getNodeTypeForName( workflow, diff --git a/packages/cli/test/integration/evaluation/test-runs.api.test.ts b/packages/cli/test/integration/evaluation/test-runs.api.test.ts index 7b12e55f11..9690d1b50e 100644 --- a/packages/cli/test/integration/evaluation/test-runs.api.test.ts +++ b/packages/cli/test/integration/evaluation/test-runs.api.test.ts @@ -97,7 +97,7 @@ describe('GET /evaluation/test-definitions/:testDefinitionId/runs', () => { const testRunRepository = Container.get(TestRunRepository); const testRun1 = await testRunRepository.createTestRun(testDefinition.id); // Mark as running just to make a slight delay between the runs - await testRunRepository.markAsRunning(testRun1.id); + await testRunRepository.markAsRunning(testRun1.id, 10); const testRun2 = await testRunRepository.createTestRun(testDefinition.id); // Fetch the first page diff --git a/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts b/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts index adac8c3a78..d058c50a52 100644 --- a/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts +++ b/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts @@ -61,11 +61,7 @@ const validateResourceMapperValue = ( }); if (!validationResult.valid) { - if (!resourceMapperField.ignoreTypeMismatchErrors) { - return { ...validationResult, fieldName: key }; - } else { - paramValues[key] = resolvedValue; - } + return { ...validationResult, fieldName: key }; } else { // If it's valid, set the casted value paramValues[key] = validationResult.newValue; diff --git a/packages/editor-ui/src/components/FreeAiCreditsCallout.test.ts b/packages/editor-ui/src/components/FreeAiCreditsCallout.test.ts index 667b3e3efd..dd459f5110 100644 --- a/packages/editor-ui/src/components/FreeAiCreditsCallout.test.ts +++ b/packages/editor-ui/src/components/FreeAiCreditsCallout.test.ts @@ -11,11 +11,16 @@ import { useRootStore } from '@/stores/root.store'; import { useToast } from '@/composables/useToast'; import { renderComponent } from '@/__tests__/render'; import { mockedStore } from '@/__tests__/utils'; +import { useTelemetry } from '@/composables/useTelemetry'; vi.mock('@/composables/useToast', () => ({ useToast: vi.fn(), })); +vi.mock('@/composables/useTelemetry', () => ({ + useTelemetry: vi.fn(), +})); + vi.mock('@/stores/settings.store', () => ({ useSettingsStore: vi.fn(), })); @@ -55,7 +60,17 @@ const assertUserCanClaimCredits = () => { }; const assertUserClaimedCredits = () => { - expect(screen.getByText('Claimed 100 free OpenAI API credits')).toBeInTheDocument(); + expect( + screen.getByText( + 'Claimed 100 free OpenAI API credits! Please note these free credits are only for the following models:', + ), + ).toBeInTheDocument(); + + expect( + screen.getByText( + 'gpt-4o-mini, text-embedding-3-small, dall-e-3, tts-1, whisper-1, and text-moderation-latest', + ), + ).toBeInTheDocument(); }; describe('FreeAiCreditsCallout', () => { @@ -86,7 +101,7 @@ describe('FreeAiCreditsCallout', () => { }); (usePostHog as any).mockReturnValue({ - isFeatureEnabled: vi.fn().mockReturnValue(true), + getVariant: vi.fn().mockReturnValue('variant'), }); (useProjectsStore as any).mockReturnValue({ @@ -100,6 +115,10 @@ describe('FreeAiCreditsCallout', () => { (useToast as any).mockReturnValue({ showError: vi.fn(), }); + + (useTelemetry as any).mockReturnValue({ + track: vi.fn(), + }); }); it('should shows the claim callout when the user can claim credits', () => { @@ -120,6 +139,7 @@ describe('FreeAiCreditsCallout', () => { await fireEvent.click(claimButton); expect(credentialsStore.claimFreeAiCredits).toHaveBeenCalledWith('test-project-id'); + expect(useTelemetry().track).toHaveBeenCalledWith('User claimed OpenAI credits'); assertUserClaimedCredits(); }); @@ -150,7 +170,7 @@ describe('FreeAiCreditsCallout', () => { it('should not be able to claim credits if user it is not in experiment', async () => { (usePostHog as any).mockReturnValue({ - isFeatureEnabled: vi.fn().mockReturnValue(false), + getVariant: vi.fn().mockReturnValue('control'), }); renderComponent(FreeAiCreditsCallout); diff --git a/packages/editor-ui/src/components/FreeAiCreditsCallout.vue b/packages/editor-ui/src/components/FreeAiCreditsCallout.vue index 29f99a0e48..922df4cf89 100644 --- a/packages/editor-ui/src/components/FreeAiCreditsCallout.vue +++ b/packages/editor-ui/src/components/FreeAiCreditsCallout.vue @@ -1,5 +1,6 @@