From 612387243732b38e7f202daff9ca257800b485f0 Mon Sep 17 00:00:00 2001 From: Pavel Strunkin Date: Tue, 18 Jun 2024 11:51:41 +0300 Subject: [PATCH] Buld/TestRun handle multithread delete (#277) --- src/_data_/index.ts | 2 +- src/builds/builds.service.ts | 34 +++++++++++++++---------- src/projects/projects.service.ts | 2 ++ src/test-runs/test-runs.service.spec.ts | 20 +++++++++++++++ src/test-runs/test-runs.service.ts | 24 ++++++++++++++--- test/builds.e2e-spec.ts | 16 ++++++++++++ test/projects.e2e-spec.ts | 6 +++++ 7 files changed, 85 insertions(+), 19 deletions(-) diff --git a/src/_data_/index.ts b/src/_data_/index.ts index ae4fb32c..c0d5ac10 100644 --- a/src/_data_/index.ts +++ b/src/_data_/index.ts @@ -6,7 +6,7 @@ export const TEST_PROJECT: Project = { id: '1', name: 'Test Project', buildsCounter: 2, - maxBuildAllowed: 100, + maxBuildAllowed: 1, maxBranchLifetime: 30, mainBranchName: 'master', createdAt: new Date(), diff --git a/src/builds/builds.service.ts b/src/builds/builds.service.ts index bde61b84..90414c0c 100644 --- a/src/builds/builds.service.ts +++ b/src/builds/builds.service.ts @@ -69,22 +69,28 @@ export class BuildsService { }, }); + if (!build) { + this.logger.warn(`Build not found ${id}`); + return; + } + await Promise.all(build.testRuns.map((testRun) => this.testRunsService.delete(testRun.id))); - const promise = this.prismaService.build - .delete({ - where: { id }, - }) - .then((build) => { - this.logger.log('Deleted build:' + JSON.stringify(build.id)); - this.eventsGateway.buildDeleted( - new BuildDto({ - ...build, - }) - ); - return build; - }); - return promise; + try { + await this.prismaService.build.delete({ where: { id } }); + } catch (e) { + if (e instanceof Prisma.PrismaClientKnownRequestError) { + // workaround https://github.com/Visual-Regression-Tracker/Visual-Regression-Tracker/issues/435 + if (e.code === 'P2025') { + this.logger.warn(`Build already deleted ${id}`); + return; + } + } + } + + this.logger.log(`Build deleted ${id}`); + this.eventsGateway.buildDeleted(new BuildDto({ ...build })); + return build; } async deleteOldBuilds(projectId: string, keepBuilds: number) { diff --git a/src/projects/projects.service.ts b/src/projects/projects.service.ts index 539a54c3..c7e5ee17 100644 --- a/src/projects/projects.service.ts +++ b/src/projects/projects.service.ts @@ -46,6 +46,8 @@ export class ProjectsService { autoApproveFeature: projectDto.autoApproveFeature, imageComparison: projectDto.imageComparison, imageComparisonConfig: projectDto.imageComparisonConfig, + maxBuildAllowed: projectDto.maxBuildAllowed, + maxBranchLifetime: projectDto.maxBranchLifetime, }, }); } diff --git a/src/test-runs/test-runs.service.spec.ts b/src/test-runs/test-runs.service.spec.ts index f3b656e4..c6cc96c8 100644 --- a/src/test-runs/test-runs.service.spec.ts +++ b/src/test-runs/test-runs.service.spec.ts @@ -425,6 +425,26 @@ describe('TestRunsService', () => { expect(eventTestRunDeletedMock).toHaveBeenCalledWith(testRun); }); + it('delete not found', async () => { + const id = 'some id'; + const findOneMock = jest.fn().mockResolvedValueOnce(undefined); + const deleteImageMock = jest.fn(); + const testRunDeleteMock = jest.fn(); + const eventTestRunDeletedMock = jest.fn(); + service = await initService({ + deleteImageMock, + testRunDeleteMock, + eventTestRunDeletedMock, + }); + service.findOne = findOneMock; + + const result = await service.delete(id); + + expect(deleteImageMock).not.toHaveBeenCalled(); + expect(testRunDeleteMock).not.toHaveBeenCalled(); + expect(result).toBeUndefined(); + }); + it('updateIgnoreAreas', async () => { const testRun = { id: 'testRunId', diff --git a/src/test-runs/test-runs.service.ts b/src/test-runs/test-runs.service.ts index f18ab2b3..efebd903 100644 --- a/src/test-runs/test-runs.service.ts +++ b/src/test-runs/test-runs.service.ts @@ -4,7 +4,7 @@ import { CreateTestRequestDto } from './dto/create-test-request.dto'; import { IgnoreAreaDto } from './dto/ignore-area.dto'; import { StaticService } from '../shared/static/static.service'; import { PrismaService } from '../prisma/prisma.service'; -import { Baseline, TestRun, TestStatus, TestVariation } from '@prisma/client'; +import { Baseline, Prisma, TestRun, TestStatus, TestVariation } from '@prisma/client'; import { DiffResult } from './diffResult'; import { EventsGateway } from '../shared/events/events.gateway'; import { TestRunResultDto } from '../test-runs/dto/testRunResult.dto'; @@ -244,16 +244,32 @@ export class TestRunsService { } async delete(id: string): Promise { + this.logger.debug(`Going to remove TestRun ${id}`); const testRun = await this.findOne(id); + if (!testRun) { + this.logger.warn(`TestRun not found ${id}`); + return; + } + await Promise.all([ this.staticService.deleteImage(testRun.diffName), this.staticService.deleteImage(testRun.imageName), - this.prismaService.testRun.delete({ - where: { id }, - }), ]); + try { + await this.prismaService.testRun.delete({ where: { id } }); + } catch (e) { + if (e instanceof Prisma.PrismaClientKnownRequestError) { + // workaround https://github.com/Visual-Regression-Tracker/Visual-Regression-Tracker/issues/435 + if (e.code === 'P2025') { + this.logger.warn(`TestRun already deleted ${id}`); + return; + } + } + } + + this.logger.log(`TestRun deleted ${id}`); this.eventsGateway.testRunDeleted(testRun); return testRun; } diff --git a/test/builds.e2e-spec.ts b/test/builds.e2e-spec.ts index aa332e3a..eb3346d9 100644 --- a/test/builds.e2e-spec.ts +++ b/test/builds.e2e-spec.ts @@ -157,6 +157,22 @@ describe('Builds (e2e)', () => { branchName: 'branchName', project: project.name, }; + await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + project.mainBranchName, + './test/image.png', + false + ); + await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + project.mainBranchName, + './test/image.png', + false + ); const builds = await Promise.all([ buildsController.create(createBuildDto), diff --git a/test/projects.e2e-spec.ts b/test/projects.e2e-spec.ts index a771fbb2..16f297f5 100644 --- a/test/projects.e2e-spec.ts +++ b/test/projects.e2e-spec.ts @@ -68,6 +68,12 @@ describe('Projects (e2e)', () => { .expect(201) .expect((res) => { expect(res.body.name).toBe(project.name); + expect(res.body.mainBranchName).toBe(project.mainBranchName); + expect(res.body.autoApproveFeature).toBe(project.autoApproveFeature); + expect(res.body.imageComparison).toBe(project.imageComparison); + expect(res.body.imageComparisonConfig).toBe(project.imageComparisonConfig); + expect(res.body.maxBuildAllowed).toBe(project.maxBuildAllowed); + expect(res.body.maxBranchLifetime).toBe(project.maxBranchLifetime); }); });