diff --git a/packages/cli/src/databases/entities/test-run.ee.ts b/packages/cli/src/databases/entities/test-run.ee.ts index 39d8e16ddd..79c7bc9f07 100644 --- a/packages/cli/src/databases/entities/test-run.ee.ts +++ b/packages/cli/src/databases/entities/test-run.ee.ts @@ -7,7 +7,7 @@ import { } from '@/databases/entities/abstract-entity'; import { TestDefinition } from '@/databases/entities/test-definition.ee'; -type TestRunStatus = 'new' | 'running' | 'completed' | 'error'; +type TestRunStatus = 'new' | 'running' | 'completed' | 'error' | 'cancelled'; export type AggregatedTestRunMetrics = Record; 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 037844734f..728f0bc464 100644 --- a/packages/cli/src/databases/repositories/test-run.repository.ee.ts +++ b/packages/cli/src/databases/repositories/test-run.repository.ee.ts @@ -35,6 +35,10 @@ 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 incrementPassed(id: string) { return await this.increment({ id }, 'passedCases', 1); } diff --git a/packages/cli/src/errors/response-errors/not-implemented.error.ts b/packages/cli/src/errors/response-errors/not-implemented.error.ts new file mode 100644 index 0000000000..f6c66391ff --- /dev/null +++ b/packages/cli/src/errors/response-errors/not-implemented.error.ts @@ -0,0 +1,7 @@ +import { ResponseError } from './abstract/response.error'; + +export class NotImplementedError extends ResponseError { + constructor(message: string, hint: string | undefined = undefined) { + super(message, 501, 501, hint); + } +} diff --git a/packages/cli/src/evaluation.ee/test-definitions.types.ee.ts b/packages/cli/src/evaluation.ee/test-definitions.types.ee.ts index b7441a2763..9b0507f248 100644 --- a/packages/cli/src/evaluation.ee/test-definitions.types.ee.ts +++ b/packages/cli/src/evaluation.ee/test-definitions.types.ee.ts @@ -92,4 +92,6 @@ export declare namespace TestRunsRequest { type GetOne = AuthenticatedRequest; type Delete = AuthenticatedRequest; + + type Cancel = AuthenticatedRequest; } 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 5d8fe3fe10..026b5d2eb8 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 @@ -132,6 +132,12 @@ function mockEvaluationExecutionData(metrics: Record) { const errorReporter = mock(); const logger = mockLogger(); +async function mockLongExecutionPromise(data: IRun, delay: number): Promise { + return await new Promise((resolve) => { + setTimeout(() => resolve(data), delay); + }); +} + describe('TestRunnerService', () => { const executionRepository = mock(); const workflowRepository = mock(); @@ -168,11 +174,7 @@ describe('TestRunnerService', () => { }); afterEach(() => { - activeExecutions.getPostExecutePromise.mockClear(); - workflowRunner.run.mockClear(); - testRunRepository.createTestRun.mockClear(); - testRunRepository.markAsRunning.mockClear(); - testRunRepository.markAsCompleted.mockClear(); + jest.resetAllMocks(); testRunRepository.incrementFailed.mockClear(); testRunRepository.incrementPassed.mockClear(); }); @@ -633,4 +635,87 @@ describe('TestRunnerService', () => { }), }); }); + + describe('Test Run cancellation', () => { + beforeAll(() => { + jest.useFakeTimers(); + }); + + test('should cancel test run', async () => { + const testRunnerService = new TestRunnerService( + logger, + 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 long execution of workflow under test + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id') + .mockReturnValue(mockLongExecutionPromise(mockExecutionData(), 1000)); + + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-3') + .mockReturnValue(mockLongExecutionPromise(mockExecutionData(), 1000)); + + // Mock executions of evaluation workflow + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-2') + .mockReturnValue( + mockLongExecutionPromise(mockEvaluationExecutionData({ metric1: 1, metric2: 0 }), 1000), + ); + + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-4') + .mockReturnValue( + mockLongExecutionPromise(mockEvaluationExecutionData({ metric1: 0.5 }), 1000), + ); + + // Do not await here to test canceling + void testRunnerService.runTest( + mock(), + mock({ + workflowId: 'workflow-under-test-id', + evaluationWorkflowId: 'evaluation-workflow-id', + mockedNodes: [{ id: '72256d90-3a67-4e29-b032-47df4e5768af' }], + }), + ); + + // Simulate the moment when first test case is running (wf under test execution) + await jest.advanceTimersByTimeAsync(100); + expect(workflowRunner.run).toHaveBeenCalledTimes(1); + + const abortController = (testRunnerService as any).abortControllers.get('test-run-id'); + expect(abortController).toBeDefined(); + + await testRunnerService.cancelTestRun('test-run-id'); + + expect(abortController.signal.aborted).toBe(true); + expect(activeExecutions.stopExecution).toBeCalledWith('some-execution-id'); + }); + + afterAll(() => { + jest.useRealTimers(); + }); + }); }); 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 732c803814..a594e15c05 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,7 +1,7 @@ import { Service } from '@n8n/di'; import { parse } from 'flatted'; import { ErrorReporter, Logger } from 'n8n-core'; -import { NodeConnectionType, Workflow } from 'n8n-workflow'; +import { ExecutionCancelledError, NodeConnectionType, Workflow } from 'n8n-workflow'; import type { IDataObject, IRun, @@ -15,6 +15,7 @@ import assert from 'node:assert'; import { ActiveExecutions } from '@/active-executions'; import type { ExecutionEntity } from '@/databases/entities/execution-entity'; import type { MockedNodeItem, 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 { WorkflowEntity } from '@/databases/entities/workflow-entity'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; @@ -38,6 +39,8 @@ import { createPinData, getPastExecutionTriggerNode } from './utils.ee'; */ @Service() export class TestRunnerService { + private abortControllers: Map = new Map(); + constructor( private readonly logger: Logger, private readonly workflowRepository: WorkflowRepository, @@ -101,7 +104,13 @@ export class TestRunnerService { pastExecutionWorkflowData: IWorkflowBase, mockedNodes: MockedNodeItem[], userId: string, + abortSignal: AbortSignal, ): Promise { + // Do not run if the test run is cancelled + if (abortSignal.aborted) { + return; + } + // Create pin data from the past execution data const pinData = createPinData( workflow, @@ -125,6 +134,11 @@ export class TestRunnerService { const executionId = await this.workflowRunner.run(data); assert(executionId); + // Listen to the abort signal to stop the execution in case test run is cancelled + abortSignal.addEventListener('abort', () => { + this.activeExecutions.stopExecution(executionId); + }); + // Wait for the execution to finish const executePromise = this.activeExecutions.getPostExecutePromise(executionId); @@ -138,8 +152,14 @@ export class TestRunnerService { evaluationWorkflow: WorkflowEntity, expectedData: IRunData, actualData: IRunData, + abortSignal: AbortSignal, testRunId?: string, ) { + // Do not run if the test run is cancelled + if (abortSignal.aborted) { + return; + } + // Prepare the evaluation wf input data. // Provide both the expected data and the actual data const evaluationInputData = { @@ -164,6 +184,11 @@ export class TestRunnerService { const executionId = await this.workflowRunner.run(data); assert(executionId); + // Listen to the abort signal to stop the execution in case test run is cancelled + abortSignal.addEventListener('abort', () => { + this.activeExecutions.stopExecution(executionId); + }); + // Wait for the execution to finish const executePromise = this.activeExecutions.getPostExecutePromise(executionId); @@ -217,99 +242,156 @@ export class TestRunnerService { const testRun = await this.testRunRepository.createTestRun(test.id); assert(testRun, 'Unable to create a test run'); - // 1. Make test cases from previous executions + // 0.1 Initialize AbortController + const abortController = new AbortController(); + this.abortControllers.set(testRun.id, abortController); - // Select executions with the annotation tag and workflow ID of the test. - // Fetch only ids to reduce the data transfer. - const pastExecutions: ReadonlyArray> = - await this.executionRepository - .createQueryBuilder('execution') - .select('execution.id') - .leftJoin('execution.annotation', 'annotation') - .leftJoin('annotation.tags', 'annotationTag') - .where('annotationTag.id = :tagId', { tagId: test.annotationTagId }) - .andWhere('execution.workflowId = :workflowId', { workflowId: test.workflowId }) - .getMany(); + const abortSignal = abortController.signal; + try { + // 1. Make test cases from previous executions - this.logger.debug('Found past executions', { count: pastExecutions.length }); + // Select executions with the annotation tag and workflow ID of the test. + // Fetch only ids to reduce the data transfer. + const pastExecutions: ReadonlyArray> = + await this.executionRepository + .createQueryBuilder('execution') + .select('execution.id') + .leftJoin('execution.annotation', 'annotation') + .leftJoin('annotation.tags', 'annotationTag') + .where('annotationTag.id = :tagId', { tagId: test.annotationTagId }) + .andWhere('execution.workflowId = :workflowId', { workflowId: test.workflowId }) + .getMany(); - // Get the metrics to collect from the evaluation workflow - const testMetricNames = await this.getTestMetricNames(test.id); + this.logger.debug('Found past executions', { count: pastExecutions.length }); - // 2. Run over all the test cases + // Get the metrics to collect from the evaluation workflow + const testMetricNames = await this.getTestMetricNames(test.id); - await this.testRunRepository.markAsRunning(testRun.id, pastExecutions.length); + // 2. Run over all the test cases + await this.testRunRepository.markAsRunning(testRun.id, pastExecutions.length); - // Object to collect the results of the evaluation workflow executions - const metrics = new EvaluationMetrics(testMetricNames); + // Object to collect the results of the evaluation workflow executions + const metrics = new EvaluationMetrics(testMetricNames); - for (const { id: pastExecutionId } of pastExecutions) { - this.logger.debug('Running test case', { pastExecutionId }); - - try { - // Fetch past execution with data - const pastExecution = await this.executionRepository.findOne({ - where: { id: pastExecutionId }, - relations: ['executionData', 'metadata'], - }); - assert(pastExecution, 'Execution not found'); - - const executionData = parse(pastExecution.executionData.data) as IRunExecutionData; - - // Run the test case and wait for it to finish - const testCaseExecution = await this.runTestCase( - workflow, - executionData, - pastExecution.executionData.workflowData, - test.mockedNodes, - user.id, - ); - - this.logger.debug('Test case execution finished', { pastExecutionId }); - - // In case of a permission check issue, the test case execution will be undefined. - // Skip them, increment the failed count and continue with the next test case - if (!testCaseExecution) { - await this.testRunRepository.incrementFailed(testRun.id); - continue; + for (const { id: pastExecutionId } of pastExecutions) { + if (abortSignal.aborted) { + this.logger.debug('Test run was cancelled', { + testId: test.id, + stoppedOn: pastExecutionId, + }); + break; } - // Collect the results of the test case execution - const testCaseRunData = testCaseExecution.data.resultData.runData; + this.logger.debug('Running test case', { pastExecutionId }); - // Get the original runData from the test case execution data - const originalRunData = executionData.resultData.runData; + try { + // Fetch past execution with data + const pastExecution = await this.executionRepository.findOne({ + where: { id: pastExecutionId }, + relations: ['executionData', 'metadata'], + }); + assert(pastExecution, 'Execution not found'); - // Run the evaluation workflow with the original and new run data - const evalExecution = await this.runTestCaseEvaluation( - evaluationWorkflow, - originalRunData, - testCaseRunData, - testRun.id, - ); - assert(evalExecution); + const executionData = parse(pastExecution.executionData.data) as IRunExecutionData; - this.logger.debug('Evaluation execution finished', { pastExecutionId }); + // Run the test case and wait for it to finish + const testCaseExecution = await this.runTestCase( + workflow, + executionData, + pastExecution.executionData.workflowData, + test.mockedNodes, + user.id, + abortSignal, + ); - metrics.addResults(this.extractEvaluationResult(evalExecution)); + this.logger.debug('Test case execution finished', { pastExecutionId }); - if (evalExecution.data.resultData.error) { + // In case of a permission check issue, the test case execution will be undefined. + // Skip them, increment the failed count and continue with the next test case + if (!testCaseExecution) { + await this.testRunRepository.incrementFailed(testRun.id); + continue; + } + + // Collect the results of the test case execution + const testCaseRunData = testCaseExecution.data.resultData.runData; + + // Get the original runData from the test case execution data + const originalRunData = executionData.resultData.runData; + + // Run the evaluation workflow with the original and new run data + const evalExecution = await this.runTestCaseEvaluation( + evaluationWorkflow, + originalRunData, + testCaseRunData, + abortSignal, + testRun.id, + ); + assert(evalExecution); + + this.logger.debug('Evaluation execution finished', { pastExecutionId }); + + // 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); - } 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); + this.errorReporter.error(e); + } } + + // Mark the test run as completed or cancelled + if (abortSignal.aborted) { + await this.testRunRepository.markAsCancelled(testRun.id); + } else { + const aggregatedMetrics = metrics.getAggregatedMetrics(); + await this.testRunRepository.markAsCompleted(testRun.id, aggregatedMetrics); + + this.logger.debug('Test run finished', { testId: test.id }); + } + } catch (e) { + if (e instanceof ExecutionCancelledError) { + this.logger.debug('Evaluation execution was cancelled. Cancelling test run', { + testRunId: testRun.id, + stoppedOn: e.extra?.executionId, + }); + + await this.testRunRepository.markAsCancelled(testRun.id); + } else { + throw e; + } + } finally { + // Clean up abort controller + this.abortControllers.delete(testRun.id); } + } - const aggregatedMetrics = metrics.getAggregatedMetrics(); + /** + * Checks if the test run in a cancellable state. + */ + canBeCancelled(testRun: TestRun) { + return testRun.status !== 'running' && testRun.status !== 'new'; + } - await this.testRunRepository.markAsCompleted(testRun.id, aggregatedMetrics); - - this.logger.debug('Test run finished', { testId: test.id }); + /** + * Cancels the test run with the given ID. + * TODO: Implement the cancellation of the test run in a multi-main scenario + */ + async cancelTestRun(testRunId: string) { + const abortController = this.abortControllers.get(testRunId); + if (abortController) { + abortController.abort(); + this.abortControllers.delete(testRunId); + } else { + // If there is no abort controller - just mark the test run as cancelled + await this.testRunRepository.markAsCancelled(testRunId); + } } } diff --git a/packages/cli/src/evaluation.ee/test-runs.controller.ee.ts b/packages/cli/src/evaluation.ee/test-runs.controller.ee.ts index aae71376e4..7e95cb4dce 100644 --- a/packages/cli/src/evaluation.ee/test-runs.controller.ee.ts +++ b/packages/cli/src/evaluation.ee/test-runs.controller.ee.ts @@ -1,7 +1,13 @@ +import express from 'express'; +import { InstanceSettings } from 'n8n-core'; + import { TestRunRepository } from '@/databases/repositories/test-run.repository.ee'; -import { Delete, Get, RestController } from '@/decorators'; +import { Delete, Get, Post, RestController } from '@/decorators'; +import { ConflictError } from '@/errors/response-errors/conflict.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; +import { NotImplementedError } from '@/errors/response-errors/not-implemented.error'; import { TestRunsRequest } from '@/evaluation.ee/test-definitions.types.ee'; +import { TestRunnerService } from '@/evaluation.ee/test-runner/test-runner.service.ee'; import { listQueryMiddleware } from '@/middlewares'; import { getSharedWorkflowIds } from '@/public-api/v1/handlers/workflows/workflows.service'; @@ -12,9 +18,12 @@ export class TestRunsController { constructor( private readonly testDefinitionService: TestDefinitionService, private readonly testRunRepository: TestRunRepository, + private readonly testRunnerService: TestRunnerService, + private readonly instanceSettings: InstanceSettings, ) {} - /** This method is used in multiple places in the controller to get the test definition + /** + * This method is used in multiple places in the controller to get the test definition * (or just check that it exists and the user has access to it). */ private async getTestDefinition( @@ -34,6 +43,23 @@ export class TestRunsController { return testDefinition; } + /** + * Get the test run (or just check that it exists and the user has access to it) + */ + private async getTestRun( + req: TestRunsRequest.GetOne | TestRunsRequest.Delete | TestRunsRequest.Cancel, + ) { + const { id: testRunId, testDefinitionId } = req.params; + + const testRun = await this.testRunRepository.findOne({ + where: { id: testRunId, testDefinition: { id: testDefinitionId } }, + }); + + if (!testRun) throw new NotFoundError('Test run not found'); + + return testRun; + } + @Get('/:testDefinitionId/runs', { middlewares: listQueryMiddleware }) async getMany(req: TestRunsRequest.GetMany) { const { testDefinitionId } = req.params; @@ -45,33 +71,43 @@ export class TestRunsController { @Get('/:testDefinitionId/runs/:id') async getOne(req: TestRunsRequest.GetOne) { - const { id: testRunId, testDefinitionId } = req.params; - await this.getTestDefinition(req); - const testRun = await this.testRunRepository.findOne({ - where: { id: testRunId, testDefinition: { id: testDefinitionId } }, - }); - - if (!testRun) throw new NotFoundError('Test run not found'); - - return testRun; + return await this.getTestRun(req); } @Delete('/:testDefinitionId/runs/:id') async delete(req: TestRunsRequest.Delete) { - const { id: testRunId, testDefinitionId } = req.params; + const { id: testRunId } = req.params; + // Check test definition and test run exist await this.getTestDefinition(req); - - const testRun = await this.testRunRepository.findOne({ - where: { id: testRunId, testDefinition: { id: testDefinitionId } }, - }); - - if (!testRun) throw new NotFoundError('Test run not found'); + await this.getTestRun(req); await this.testRunRepository.delete({ id: testRunId }); return { success: true }; } + + @Post('/:testDefinitionId/runs/:id/cancel') + async cancel(req: TestRunsRequest.Cancel, res: express.Response) { + if (this.instanceSettings.isMultiMain) { + throw new NotImplementedError('Cancelling test runs is not yet supported in multi-main mode'); + } + + const { id: testRunId } = req.params; + + // Check test definition and test run exist + await this.getTestDefinition(req); + const testRun = await this.getTestRun(req); + + if (this.testRunnerService.canBeCancelled(testRun)) { + const message = `The test run "${testRunId}" cannot be cancelled`; + throw new ConflictError(message); + } + + await this.testRunnerService.cancelTestRun(testRunId); + + res.status(202).json({ success: true }); + } } 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 3fcc321cc9..9690d1b50e 100644 --- a/packages/cli/test/integration/evaluation/test-runs.api.test.ts +++ b/packages/cli/test/integration/evaluation/test-runs.api.test.ts @@ -1,4 +1,5 @@ import { Container } from '@n8n/di'; +import { mockInstance } from 'n8n-core/test/utils'; import type { TestDefinition } from '@/databases/entities/test-definition.ee'; import type { User } from '@/databases/entities/user'; @@ -6,6 +7,7 @@ import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; import { ProjectRepository } from '@/databases/repositories/project.repository'; import { TestDefinitionRepository } from '@/databases/repositories/test-definition.repository.ee'; import { TestRunRepository } from '@/databases/repositories/test-run.repository.ee'; +import { TestRunnerService } from '@/evaluation.ee/test-runner/test-runner.service.ee'; import { createUserShell } from '@test-integration/db/users'; import { createWorkflow } from '@test-integration/db/workflows'; import * as testDb from '@test-integration/test-db'; @@ -19,9 +21,11 @@ let testDefinition: TestDefinition; let otherTestDefinition: TestDefinition; let ownerShell: User; +const testRunner = mockInstance(TestRunnerService); + const testServer = utils.setupTestServer({ endpointGroups: ['workflows', 'evaluation'], - enabledFeatures: ['feat:sharing'], + enabledFeatures: ['feat:sharing', 'feat:multipleMainInstances'], }); beforeAll(async () => { @@ -57,13 +61,13 @@ describe('GET /evaluation/test-definitions/:testDefinitionId/runs', () => { expect(resp.body.data).toEqual([]); }); - test('should retrieve 404 if test definition does not exist', async () => { + test('should return 404 if test definition does not exist', async () => { const resp = await authOwnerAgent.get('/evaluation/test-definitions/123/runs'); expect(resp.statusCode).toBe(404); }); - test('should retrieve 404 if user does not have access to test definition', async () => { + test('should return 404 if user does not have access to test definition', async () => { const resp = await authOwnerAgent.get( `/evaluation/test-definitions/${otherTestDefinition.id}/runs`, ); @@ -151,7 +155,7 @@ describe('GET /evaluation/test-definitions/:testDefinitionId/runs/:id', () => { ); }); - test('should retrieve 404 if test run does not exist', async () => { + test('should return 404 if test run does not exist', async () => { const resp = await authOwnerAgent.get( `/evaluation/test-definitions/${testDefinition.id}/runs/123`, ); @@ -159,7 +163,7 @@ describe('GET /evaluation/test-definitions/:testDefinitionId/runs/:id', () => { expect(resp.statusCode).toBe(404); }); - test('should retrieve 404 if user does not have access to test definition', async () => { + test('should return 404 if user does not have access to test definition', async () => { const testRunRepository = Container.get(TestRunRepository); const testRun = await testRunRepository.createTestRun(otherTestDefinition.id); @@ -218,7 +222,7 @@ describe('DELETE /evaluation/test-definitions/:testDefinitionId/runs/:id', () => expect(testRunAfterDelete).toBeNull(); }); - test('should retrieve 404 if test run does not exist', async () => { + test('should return 404 if test run does not exist', async () => { const resp = await authOwnerAgent.delete( `/evaluation/test-definitions/${testDefinition.id}/runs/123`, ); @@ -226,7 +230,7 @@ describe('DELETE /evaluation/test-definitions/:testDefinitionId/runs/:id', () => expect(resp.statusCode).toBe(404); }); - test('should retrieve 404 if user does not have access to test definition', async () => { + test('should return 404 if user does not have access to test definition', async () => { const testRunRepository = Container.get(TestRunRepository); const testRun = await testRunRepository.createTestRun(otherTestDefinition.id); @@ -237,3 +241,46 @@ describe('DELETE /evaluation/test-definitions/:testDefinitionId/runs/:id', () => expect(resp.statusCode).toBe(404); }); }); + +describe('POST /evaluation/test-definitions/:testDefinitionId/runs/:id/cancel', () => { + test('should cancel test run', async () => { + const testRunRepository = Container.get(TestRunRepository); + const testRun = await testRunRepository.createTestRun(testDefinition.id); + + jest.spyOn(testRunRepository, 'markAsCancelled'); + + const resp = await authOwnerAgent.post( + `/evaluation/test-definitions/${testDefinition.id}/runs/${testRun.id}/cancel`, + ); + + expect(resp.statusCode).toBe(202); + expect(resp.body).toEqual({ success: true }); + + expect(testRunner.cancelTestRun).toHaveBeenCalledWith(testRun.id); + }); + + test('should return 404 if test run does not exist', async () => { + const resp = await authOwnerAgent.post( + `/evaluation/test-definitions/${testDefinition.id}/runs/123/cancel`, + ); + + expect(resp.statusCode).toBe(404); + }); + + test('should return 404 if test definition does not exist', async () => { + const resp = await authOwnerAgent.post('/evaluation/test-definitions/123/runs/123/cancel'); + + expect(resp.statusCode).toBe(404); + }); + + test('should return 404 if user does not have access to test definition', async () => { + const testRunRepository = Container.get(TestRunRepository); + const testRun = await testRunRepository.createTestRun(otherTestDefinition.id); + + const resp = await authOwnerAgent.post( + `/evaluation/test-definitions/${otherTestDefinition.id}/runs/${testRun.id}/cancel`, + ); + + expect(resp.statusCode).toBe(404); + }); +}); diff --git a/packages/editor-ui/src/api/testDefinition.ee.ts b/packages/editor-ui/src/api/testDefinition.ee.ts index 5f4ce66ef9..1cc39e3bd5 100644 --- a/packages/editor-ui/src/api/testDefinition.ee.ts +++ b/packages/editor-ui/src/api/testDefinition.ee.ts @@ -43,7 +43,7 @@ export interface UpdateTestResponse { export interface TestRunRecord { id: string; testDefinitionId: string; - status: 'new' | 'running' | 'completed' | 'error'; + status: 'new' | 'running' | 'completed' | 'error' | 'cancelled'; metrics?: Record; createdAt: string; updatedAt: string; @@ -221,6 +221,21 @@ export const startTestRun = async (context: IRestApiContext, testDefinitionId: s return response as { success: boolean }; }; +export const cancelTestRun = async ( + context: IRestApiContext, + testDefinitionId: string, + testRunId: string, +) => { + const response = await request({ + method: 'POST', + baseURL: context.baseUrl, + endpoint: `${endpoint}/${testDefinitionId}/runs/${testRunId}/cancel`, + headers: { 'push-ref': context.pushRef }, + }); + // CLI is returning the response without wrapping it in `data` key + return response as { success: boolean }; +}; + // Delete a test run export const deleteTestRun = async (context: IRestApiContext, params: DeleteTestRunParams) => { return await makeRestApiRequest<{ success: boolean }>( diff --git a/packages/editor-ui/src/components/TestDefinition/ListDefinition/TestItem.vue b/packages/editor-ui/src/components/TestDefinition/ListDefinition/TestItem.vue index a4e5a4d4ac..4325c8698d 100644 --- a/packages/editor-ui/src/components/TestDefinition/ListDefinition/TestItem.vue +++ b/packages/editor-ui/src/components/TestDefinition/ListDefinition/TestItem.vue @@ -3,6 +3,7 @@ import type { TestListItem } from '@/components/TestDefinition/types'; import TimeAgo from '@/components/TimeAgo.vue'; import { useI18n } from '@/composables/useI18n'; import n8nIconButton from 'n8n-design-system/components/N8nIconButton'; +import { computed } from 'vue'; export interface TestItemProps { test: TestListItem; @@ -16,6 +17,7 @@ const emit = defineEmits<{ 'view-details': [testId: string]; 'edit-test': [testId: string]; 'delete-test': [testId: string]; + 'cancel-test-run': [testId: string, testRunId: string | null]; }>(); const actions = [ @@ -24,6 +26,14 @@ const actions = [ id: 'run', event: () => emit('run-test', props.test.id), tooltip: locale.baseText('testDefinition.runTest'), + show: () => props.test.execution.status !== 'running', + }, + { + icon: 'stop', + id: 'cancel', + event: () => emit('cancel-test-run', props.test.id, props.test.execution.id), + tooltip: locale.baseText('testDefinition.cancelTestRun'), + show: () => props.test.execution.status === 'running', }, { icon: 'list', @@ -44,6 +54,8 @@ const actions = [ tooltip: locale.baseText('testDefinition.deleteTest'), }, ]; + +const visibleActions = computed(() => actions.filter((action) => action.show?.() ?? true));