From f6490990de84ed59f2a465166a49e23e8578208a Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 9 Oct 2024 13:06:43 +0200 Subject: [PATCH 1/3] tests: Implement tests for ExecutionStep model --- .../__snapshots__/execution-step.test.js.snap | 54 +++++++ packages/backend/src/models/execution-step.js | 34 ++-- .../backend/src/models/execution-step.test.js | 153 ++++++++++++++++++ 3 files changed, 229 insertions(+), 12 deletions(-) create mode 100644 packages/backend/src/models/__snapshots__/execution-step.test.js.snap create mode 100644 packages/backend/src/models/execution-step.test.js diff --git a/packages/backend/src/models/__snapshots__/execution-step.test.js.snap b/packages/backend/src/models/__snapshots__/execution-step.test.js.snap new file mode 100644 index 00000000..c075661b --- /dev/null +++ b/packages/backend/src/models/__snapshots__/execution-step.test.js.snap @@ -0,0 +1,54 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`ExecutionStep model > jsonSchema should have correct validations 1`] = ` +{ + "properties": { + "createdAt": { + "type": "string", + }, + "dataIn": { + "type": [ + "object", + "null", + ], + }, + "dataOut": { + "type": [ + "object", + "null", + ], + }, + "deletedAt": { + "type": "string", + }, + "errorDetails": { + "type": [ + "object", + "null", + ], + }, + "executionId": { + "format": "uuid", + "type": "string", + }, + "id": { + "format": "uuid", + "type": "string", + }, + "status": { + "enum": [ + "success", + "failure", + ], + "type": "string", + }, + "stepId": { + "type": "string", + }, + "updatedAt": { + "type": "string", + }, + }, + "type": "object", +} +`; diff --git a/packages/backend/src/models/execution-step.js b/packages/backend/src/models/execution-step.js index 267bbfb9..7d8de60c 100644 --- a/packages/backend/src/models/execution-step.js +++ b/packages/backend/src/models/execution-step.js @@ -47,21 +47,31 @@ class ExecutionStep extends Base { return this.status === 'failure'; } + async isNormalSucceededRun() { + const execution = await this.$relatedQuery('execution'); + return !execution.testRun && !this.isFailed; + } + + async updateUsageData() { + const execution = await this.$relatedQuery('execution'); + + const flow = await execution.$relatedQuery('flow'); + const user = await flow.$relatedQuery('user'); + const usageData = await user.$relatedQuery('currentUsageData'); + + await usageData.increaseConsumedTaskCountByOne(); + } + + async increaseUsageCount() { + if (appConfig.isCloud && this.isNormalSucceededRun()) { + await this.updateUsageData(); + } + } + async $afterInsert(queryContext) { await super.$afterInsert(queryContext); Telemetry.executionStepCreated(this); - - if (appConfig.isCloud) { - const execution = await this.$relatedQuery('execution'); - - if (!execution.testRun && !this.isFailed) { - const flow = await execution.$relatedQuery('flow'); - const user = await flow.$relatedQuery('user'); - const usageData = await user.$relatedQuery('currentUsageData'); - - await usageData.increaseConsumedTaskCountByOne(); - } - } + await this.increaseUsageCount(); } } diff --git a/packages/backend/src/models/execution-step.test.js b/packages/backend/src/models/execution-step.test.js new file mode 100644 index 00000000..2865bd91 --- /dev/null +++ b/packages/backend/src/models/execution-step.test.js @@ -0,0 +1,153 @@ +import { vi, describe, it, expect } from 'vitest'; +import Execution from './execution'; +import ExecutionStep from './execution-step'; +import Step from './step'; +import Base from './base'; +import UsageData from './usage-data.ee'; +import Telemetry from '../helpers/telemetry'; +import appConfig from '../config/app'; +import { createExecution } from '../../test/factories/execution'; +import { createExecutionStep } from '../../test/factories/execution-step'; + +describe('ExecutionStep model', () => { + it('tableName should return correct name', () => { + expect(ExecutionStep.tableName).toBe('execution_steps'); + }); + + it('jsonSchema should have correct validations', () => { + expect(ExecutionStep.jsonSchema).toMatchSnapshot(); + }); + + it('relationMappings should return correct associations', () => { + const relationMappings = ExecutionStep.relationMappings(); + + const expectedRelations = { + execution: { + relation: Base.BelongsToOneRelation, + modelClass: Execution, + join: { + from: 'execution_steps.execution_id', + to: 'executions.id', + }, + }, + step: { + relation: Base.BelongsToOneRelation, + modelClass: Step, + join: { + from: 'execution_steps.step_id', + to: 'steps.id', + }, + }, + }; + + expect(relationMappings).toStrictEqual(expectedRelations); + }); + + describe('isFailed', () => { + it('should return true if status is failure', async () => { + const executionStep = await createExecutionStep({ + status: 'failure', + }); + + expect(executionStep.isFailed).toBe(true); + }); + + it('should return false if status is not failure', async () => { + const executionStep = await createExecutionStep({ + status: 'success', + }); + + expect(executionStep.isFailed).toBe(false); + }); + }); + + describe('isNormalSuccededRun', () => { + it('should return false if it has a test run execution', async () => { + const execution = await createExecution({ + testRun: true, + }); + + const executionStep = await createExecutionStep({ + executionId: execution.id, + }); + + expect(await executionStep.isNormalSucceededRun()).toBe(false); + }); + + it('should return false if it has a failure status', async () => { + const executionStep = await createExecutionStep({ + status: 'failure', + }); + + expect(await executionStep.isNormalSucceededRun()).toBe(false); + }); + + it('should return true if it has a succeeded normal execution', async () => { + const executionStep = await createExecutionStep({ + status: 'success', + }); + + expect(await executionStep.isNormalSucceededRun()).toBe(true); + }); + }); + + describe('updateUsageData', () => { + it('should call usageData.increaseConsumedTaskCountByOne', async () => { + const executionStep = await createExecutionStep(); + + const increaseConsumedTaskCountByOneSpy = vi.spyOn( + UsageData.prototype, + 'increaseConsumedTaskCountByOne' + ); + + await executionStep.updateUsageData(); + + expect(increaseConsumedTaskCountByOneSpy).toHaveBeenCalledOnce(); + }); + }); + + describe('increaseUsageCount', () => { + it('should call updateUsageData for cloud and normal succeeded run', async () => { + vi.spyOn(appConfig, 'isCloud', 'get').mockReturnValue(true); + vi.spyOn(ExecutionStep.prototype, 'isNormalSucceededRun').mockReturnValue( + true + ); + + const executionStep = await createExecutionStep(); + + const updateUsageDataSpy = vi.spyOn( + ExecutionStep.prototype, + 'updateUsageData' + ); + + await executionStep.increaseUsageCount(); + + expect(updateUsageDataSpy).toHaveBeenCalledOnce(); + }); + }); + + describe('$afterInsert', () => { + it('should call Telemetry.executionStepCreated', async () => { + const telemetryExecutionStepCreatedSpy = vi + .spyOn(Telemetry, 'executionStepCreated') + .mockImplementation(() => {}); + + const executionStep = await createExecutionStep(); + + expect(telemetryExecutionStepCreatedSpy).toHaveBeenCalledWith( + executionStep + ); + }); + + it('should call increaseUsageCount', async () => { + const increaseUsageCountSpy = vi.spyOn( + ExecutionStep.prototype, + 'increaseUsageCount' + ); + + await createExecutionStep(); + + expect(increaseUsageCountSpy).toHaveBeenCalledOnce(); + }); + }); +}); From 4ce9976dbc64d5943c1f2e6e54a135845cbb4200 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 9 Oct 2024 15:41:27 +0200 Subject: [PATCH 2/3] refactor: Use isSucceededNonTestRun naming for execution steps --- packages/backend/src/models/execution-step.js | 4 ++-- .../backend/src/models/execution-step.test.js | 19 ++++++++++--------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/backend/src/models/execution-step.js b/packages/backend/src/models/execution-step.js index 7d8de60c..b17343bb 100644 --- a/packages/backend/src/models/execution-step.js +++ b/packages/backend/src/models/execution-step.js @@ -47,7 +47,7 @@ class ExecutionStep extends Base { return this.status === 'failure'; } - async isNormalSucceededRun() { + async isSucceededNonTestRun() { const execution = await this.$relatedQuery('execution'); return !execution.testRun && !this.isFailed; } @@ -63,7 +63,7 @@ class ExecutionStep extends Base { } async increaseUsageCount() { - if (appConfig.isCloud && this.isNormalSucceededRun()) { + if (appConfig.isCloud && this.isSucceededNonTestRun()) { await this.updateUsageData(); } } diff --git a/packages/backend/src/models/execution-step.test.js b/packages/backend/src/models/execution-step.test.js index 2865bd91..4ca9301e 100644 --- a/packages/backend/src/models/execution-step.test.js +++ b/packages/backend/src/models/execution-step.test.js @@ -61,7 +61,7 @@ describe('ExecutionStep model', () => { }); }); - describe('isNormalSuccededRun', () => { + describe('isSucceededNonTestRun', () => { it('should return false if it has a test run execution', async () => { const execution = await createExecution({ testRun: true, @@ -71,7 +71,7 @@ describe('ExecutionStep model', () => { executionId: execution.id, }); - expect(await executionStep.isNormalSucceededRun()).toBe(false); + expect(await executionStep.isSucceededNonTestRun()).toBe(false); }); it('should return false if it has a failure status', async () => { @@ -79,15 +79,15 @@ describe('ExecutionStep model', () => { status: 'failure', }); - expect(await executionStep.isNormalSucceededRun()).toBe(false); + expect(await executionStep.isSucceededNonTestRun()).toBe(false); }); - it('should return true if it has a succeeded normal execution', async () => { + it('should return true if it has a succeeded non test run', async () => { const executionStep = await createExecutionStep({ status: 'success', }); - expect(await executionStep.isNormalSucceededRun()).toBe(true); + expect(await executionStep.isSucceededNonTestRun()).toBe(true); }); }); @@ -107,11 +107,12 @@ describe('ExecutionStep model', () => { }); describe('increaseUsageCount', () => { - it('should call updateUsageData for cloud and normal succeeded run', async () => { + it('should call updateUsageData for cloud and succeeded non test run', async () => { vi.spyOn(appConfig, 'isCloud', 'get').mockReturnValue(true); - vi.spyOn(ExecutionStep.prototype, 'isNormalSucceededRun').mockReturnValue( - true - ); + vi.spyOn( + ExecutionStep.prototype, + 'isSucceededNonTestRun' + ).mockReturnValue(true); const executionStep = await createExecutionStep(); From 163d6a7a28a1419895e6f34a6234c322c5d595b7 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 9 Oct 2024 15:42:39 +0200 Subject: [PATCH 3/3] refactor: Do not use persisted instance for execution step is failed --- packages/backend/src/models/execution-step.test.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/models/execution-step.test.js b/packages/backend/src/models/execution-step.test.js index 4ca9301e..3148d6dd 100644 --- a/packages/backend/src/models/execution-step.test.js +++ b/packages/backend/src/models/execution-step.test.js @@ -45,17 +45,15 @@ describe('ExecutionStep model', () => { describe('isFailed', () => { it('should return true if status is failure', async () => { - const executionStep = await createExecutionStep({ - status: 'failure', - }); + const executionStep = new ExecutionStep(); + executionStep.status = 'failure'; expect(executionStep.isFailed).toBe(true); }); it('should return false if status is not failure', async () => { - const executionStep = await createExecutionStep({ - status: 'success', - }); + const executionStep = new ExecutionStep(); + executionStep.status = 'success'; expect(executionStep.isFailed).toBe(false); });