Skip to content

Commit 6123872

Browse files
authored
Buld/TestRun handle multithread delete (#277)
1 parent 0e96415 commit 6123872

File tree

7 files changed

+85
-19
lines changed

7 files changed

+85
-19
lines changed

src/_data_/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export const TEST_PROJECT: Project = {
66
id: '1',
77
name: 'Test Project',
88
buildsCounter: 2,
9-
maxBuildAllowed: 100,
9+
maxBuildAllowed: 1,
1010
maxBranchLifetime: 30,
1111
mainBranchName: 'master',
1212
createdAt: new Date(),

src/builds/builds.service.ts

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -69,22 +69,28 @@ export class BuildsService {
6969
},
7070
});
7171

72+
if (!build) {
73+
this.logger.warn(`Build not found ${id}`);
74+
return;
75+
}
76+
7277
await Promise.all(build.testRuns.map((testRun) => this.testRunsService.delete(testRun.id)));
7378

74-
const promise = this.prismaService.build
75-
.delete({
76-
where: { id },
77-
})
78-
.then((build) => {
79-
this.logger.log('Deleted build:' + JSON.stringify(build.id));
80-
this.eventsGateway.buildDeleted(
81-
new BuildDto({
82-
...build,
83-
})
84-
);
85-
return build;
86-
});
87-
return promise;
79+
try {
80+
await this.prismaService.build.delete({ where: { id } });
81+
} catch (e) {
82+
if (e instanceof Prisma.PrismaClientKnownRequestError) {
83+
// workaround https://github.com/Visual-Regression-Tracker/Visual-Regression-Tracker/issues/435
84+
if (e.code === 'P2025') {
85+
this.logger.warn(`Build already deleted ${id}`);
86+
return;
87+
}
88+
}
89+
}
90+
91+
this.logger.log(`Build deleted ${id}`);
92+
this.eventsGateway.buildDeleted(new BuildDto({ ...build }));
93+
return build;
8894
}
8995

9096
async deleteOldBuilds(projectId: string, keepBuilds: number) {

src/projects/projects.service.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ export class ProjectsService {
4646
autoApproveFeature: projectDto.autoApproveFeature,
4747
imageComparison: projectDto.imageComparison,
4848
imageComparisonConfig: projectDto.imageComparisonConfig,
49+
maxBuildAllowed: projectDto.maxBuildAllowed,
50+
maxBranchLifetime: projectDto.maxBranchLifetime,
4951
},
5052
});
5153
}

src/test-runs/test-runs.service.spec.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,26 @@ describe('TestRunsService', () => {
425425
expect(eventTestRunDeletedMock).toHaveBeenCalledWith(testRun);
426426
});
427427

428+
it('delete not found', async () => {
429+
const id = 'some id';
430+
const findOneMock = jest.fn().mockResolvedValueOnce(undefined);
431+
const deleteImageMock = jest.fn();
432+
const testRunDeleteMock = jest.fn();
433+
const eventTestRunDeletedMock = jest.fn();
434+
service = await initService({
435+
deleteImageMock,
436+
testRunDeleteMock,
437+
eventTestRunDeletedMock,
438+
});
439+
service.findOne = findOneMock;
440+
441+
const result = await service.delete(id);
442+
443+
expect(deleteImageMock).not.toHaveBeenCalled();
444+
expect(testRunDeleteMock).not.toHaveBeenCalled();
445+
expect(result).toBeUndefined();
446+
});
447+
428448
it('updateIgnoreAreas', async () => {
429449
const testRun = {
430450
id: 'testRunId',

src/test-runs/test-runs.service.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { CreateTestRequestDto } from './dto/create-test-request.dto';
44
import { IgnoreAreaDto } from './dto/ignore-area.dto';
55
import { StaticService } from '../shared/static/static.service';
66
import { PrismaService } from '../prisma/prisma.service';
7-
import { Baseline, TestRun, TestStatus, TestVariation } from '@prisma/client';
7+
import { Baseline, Prisma, TestRun, TestStatus, TestVariation } from '@prisma/client';
88
import { DiffResult } from './diffResult';
99
import { EventsGateway } from '../shared/events/events.gateway';
1010
import { TestRunResultDto } from '../test-runs/dto/testRunResult.dto';
@@ -244,16 +244,32 @@ export class TestRunsService {
244244
}
245245

246246
async delete(id: string): Promise<TestRun> {
247+
this.logger.debug(`Going to remove TestRun ${id}`);
247248
const testRun = await this.findOne(id);
248249

250+
if (!testRun) {
251+
this.logger.warn(`TestRun not found ${id}`);
252+
return;
253+
}
254+
249255
await Promise.all([
250256
this.staticService.deleteImage(testRun.diffName),
251257
this.staticService.deleteImage(testRun.imageName),
252-
this.prismaService.testRun.delete({
253-
where: { id },
254-
}),
255258
]);
256259

260+
try {
261+
await this.prismaService.testRun.delete({ where: { id } });
262+
} catch (e) {
263+
if (e instanceof Prisma.PrismaClientKnownRequestError) {
264+
// workaround https://github.com/Visual-Regression-Tracker/Visual-Regression-Tracker/issues/435
265+
if (e.code === 'P2025') {
266+
this.logger.warn(`TestRun already deleted ${id}`);
267+
return;
268+
}
269+
}
270+
}
271+
272+
this.logger.log(`TestRun deleted ${id}`);
257273
this.eventsGateway.testRunDeleted(testRun);
258274
return testRun;
259275
}

test/builds.e2e-spec.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,22 @@ describe('Builds (e2e)', () => {
157157
branchName: 'branchName',
158158
project: project.name,
159159
};
160+
await haveTestRunCreated(
161+
buildsService,
162+
testRunsService,
163+
project.id,
164+
project.mainBranchName,
165+
'./test/image.png',
166+
false
167+
);
168+
await haveTestRunCreated(
169+
buildsService,
170+
testRunsService,
171+
project.id,
172+
project.mainBranchName,
173+
'./test/image.png',
174+
false
175+
);
160176

161177
const builds = await Promise.all([
162178
buildsController.create(createBuildDto),

test/projects.e2e-spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@ describe('Projects (e2e)', () => {
6868
.expect(201)
6969
.expect((res) => {
7070
expect(res.body.name).toBe(project.name);
71+
expect(res.body.mainBranchName).toBe(project.mainBranchName);
72+
expect(res.body.autoApproveFeature).toBe(project.autoApproveFeature);
73+
expect(res.body.imageComparison).toBe(project.imageComparison);
74+
expect(res.body.imageComparisonConfig).toBe(project.imageComparisonConfig);
75+
expect(res.body.maxBuildAllowed).toBe(project.maxBuildAllowed);
76+
expect(res.body.maxBranchLifetime).toBe(project.maxBranchLifetime);
7177
});
7278
});
7379

0 commit comments

Comments
 (0)