diff --git a/docker-compose.yml b/docker-compose.yml index 33b87eb4..08b9da81 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -51,7 +51,7 @@ services: # container_name: pgadmin4 # # https://hub.docker.com/r/dpage/pgadmin4 # # https://www.pgadmin.org/docs/pgadmin4/latest/release_notes_7_4.html - # image: dpage/pgadmin4:7.4 + # image: dpage/pgadmin4 # restart: always # environment: # PGADMIN_DEFAULT_EMAIL: admin@admin.com diff --git a/prisma/migrations/20250225101118_add_protected_branches/migration.sql b/prisma/migrations/20250225101118_add_protected_branches/migration.sql new file mode 100644 index 00000000..67f13033 --- /dev/null +++ b/prisma/migrations/20250225101118_add_protected_branches/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "Project" ADD COLUMN "protectedBranch" TEXT; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 014ea95a..e3fefac5 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -1,5 +1,5 @@ generator client { - provider = "prisma-client-js" + provider = "prisma-client-js" binaryTargets = ["native", "debian-openssl-3.0.x", "linux-arm64-openssl-3.0.x"] } @@ -41,6 +41,7 @@ model Project { builds Build[] TestRun TestRun[] testVariations TestVariation[] + protectedBranch String? } model TestRun { diff --git a/prisma/test/seed.spec.ts b/prisma/test/seed.spec.ts index e4a61035..08f496b6 100644 --- a/prisma/test/seed.spec.ts +++ b/prisma/test/seed.spec.ts @@ -16,6 +16,7 @@ const mockDefaultProject = { autoApproveFeature: true, imageComparison: 'pixelmatch', imageComparisonConfig: '{ "threshold": 0.1, "ignoreAntialiasing": true, "allowDiffDimensions": false }', + protectedBranch: null, } as const; const mockDefaultUser = { diff --git a/src/_data_/index.ts b/src/_data_/index.ts index 3c45ad77..4586285c 100644 --- a/src/_data_/index.ts +++ b/src/_data_/index.ts @@ -13,6 +13,7 @@ export const TEST_PROJECT: Project = { autoApproveFeature: true, imageComparisonConfig: '{ "threshold": 0.1, "ignoreAntialiasing": true, "allowDiffDimensions": false }', imageComparison: ImageComparison.pixelmatch, + protectedBranch: 'release-[0-9]+', }; export const TEST_BUILD: Build = { diff --git a/src/projects/dto/project.dto.ts b/src/projects/dto/project.dto.ts index da2c7288..28986415 100644 --- a/src/projects/dto/project.dto.ts +++ b/src/projects/dto/project.dto.ts @@ -1,5 +1,5 @@ import { ApiProperty } from '@nestjs/swagger'; -import { IsUUID, IsString, IsNumber, IsBoolean, IsEnum, IsJSON, IsDate } from 'class-validator'; +import { IsUUID, IsString, IsNumber, IsBoolean, IsEnum, IsJSON, IsDate, IsOptional } from 'class-validator'; import { ImageComparison, Project } from '@prisma/client'; import { Type } from 'class-transformer'; @@ -49,4 +49,9 @@ export class ProjectDto implements Project { @ApiProperty() @IsJSON() imageComparisonConfig: string; + + @ApiProperty() + @IsString() + @IsOptional() + protectedBranch: string; } diff --git a/src/projects/projects.service.ts b/src/projects/projects.service.ts index c7e5ee17..20acf194 100644 --- a/src/projects/projects.service.ts +++ b/src/projects/projects.service.ts @@ -48,6 +48,7 @@ export class ProjectsService { imageComparisonConfig: projectDto.imageComparisonConfig, maxBuildAllowed: projectDto.maxBuildAllowed, maxBranchLifetime: projectDto.maxBranchLifetime, + protectedBranch: projectDto.protectedBranch, }, }); } @@ -63,6 +64,7 @@ export class ProjectsService { maxBuildAllowed: projectDto.maxBuildAllowed, maxBranchLifetime: projectDto.maxBranchLifetime, imageComparisonConfig: projectDto.imageComparisonConfig, + protectedBranch: projectDto.protectedBranch, }, }); } diff --git a/src/shared/tasks/tasks.service.spec.ts b/src/shared/tasks/tasks.service.spec.ts index f65fc8ea..e44de4f6 100644 --- a/src/shared/tasks/tasks.service.spec.ts +++ b/src/shared/tasks/tasks.service.spec.ts @@ -6,7 +6,7 @@ import { TestVariationsService } from '../../test-variations/test-variations.ser const initService = async ({ projectFindManyMock = jest.fn(), - testVariationFindManyMock = jest.fn(), + findOldTestVariationsMock = jest.fn(), testVariationsDeleteMock = jest.fn(), }) => { const module: TestingModule = await Test.createTestingModule({ @@ -15,9 +15,6 @@ const initService = async ({ { provide: PrismaService, useValue: { - testVariation: { - findMany: testVariationFindManyMock, - }, project: { findMany: projectFindManyMock, }, @@ -27,6 +24,7 @@ const initService = async ({ provide: TestVariationsService, useValue: { delete: testVariationsDeleteMock, + findOldTestVariations: findOldTestVariationsMock, }, }, ], @@ -43,11 +41,11 @@ describe('cleanOldTestVariations', () => { const project = TEST_PROJECT; const testVariation = generateTestVariation(); const projectFindManyMock = jest.fn().mockResolvedValueOnce([project]); - const testVariationFindManyMock = jest.fn().mockResolvedValueOnce([testVariation]); + const findOldTestVariationsMock = jest.fn().mockResolvedValueOnce([testVariation]); const testVariationsDeleteMock = jest.fn(); service = await initService({ projectFindManyMock: projectFindManyMock, - testVariationFindManyMock: testVariationFindManyMock, + findOldTestVariationsMock: findOldTestVariationsMock, testVariationsDeleteMock: testVariationsDeleteMock, }); const dateNow = new Date('2022-10-23'); @@ -59,13 +57,7 @@ describe('cleanOldTestVariations', () => { await service.cleanOldTestVariations(); // .Assert - expect(testVariationFindManyMock).toHaveBeenCalledWith({ - where: { - updatedAt: { lte: dateRemoveAfter }, - branchName: { not: project.mainBranchName }, - projectId: project.id, - }, - }); + expect(findOldTestVariationsMock).toHaveBeenCalledWith(project, dateRemoveAfter); expect(testVariationsDeleteMock).toBeCalledWith(testVariation.id); }); }); diff --git a/src/shared/tasks/tasks.service.ts b/src/shared/tasks/tasks.service.ts index ae47622b..05536edb 100644 --- a/src/shared/tasks/tasks.service.ts +++ b/src/shared/tasks/tasks.service.ts @@ -21,13 +21,8 @@ export class TasksService { const dateRemoveAfter: Date = new Date(); dateRemoveAfter.setDate(dateRemoveAfter.getDate() - project.maxBranchLifetime); - const testVariations = await this.prismaService.testVariation.findMany({ - where: { - projectId: project.id, - updatedAt: { lte: dateRemoveAfter }, - branchName: { not: project.mainBranchName }, - }, - }); + const testVariations = await this.testVariationService.findOldTestVariations(project, dateRemoveAfter); + this.logger.debug( `Removing ${testVariations.length} TestVariations for ${project.name} later than ${dateRemoveAfter}` ); diff --git a/src/test-runs/dto/create-test-request.dto.ts b/src/test-runs/dto/create-test-request.dto.ts index 7f67e783..abcdf00a 100644 --- a/src/test-runs/dto/create-test-request.dto.ts +++ b/src/test-runs/dto/create-test-request.dto.ts @@ -34,4 +34,9 @@ export class CreateTestRequestDto extends BaselineDataDto { @IsOptional() @IsString() comment?: string; + + @ApiPropertyOptional() + @IsOptional() + @IsString() + baselineBranchName?: string; } diff --git a/src/test-runs/test-runs.service.ts b/src/test-runs/test-runs.service.ts index 40e7a254..998809fb 100644 --- a/src/test-runs/test-runs.service.ts +++ b/src/test-runs/test-runs.service.ts @@ -85,8 +85,16 @@ export class TestRunsService { // try auto approve if (project.autoApproveFeature) { - testRunWithResult = await this.tryAutoApproveByPastBaselines({ testVariation, testRun: testRunWithResult }); - testRunWithResult = await this.tryAutoApproveByNewBaselines({ testVariation, testRun: testRunWithResult }); + testRunWithResult = await this.tryAutoApproveByPastBaselines({ + testVariation, + testRun: testRunWithResult, + baselineBranchName: createTestRequestDto.baselineBranchName, + }); + testRunWithResult = await this.tryAutoApproveByNewBaselines({ + testVariation, + testRun: testRunWithResult, + baselineBranchName: createTestRequestDto.baselineBranchName, + }); } return new TestRunResultDto(testRunWithResult, testVariation); } @@ -348,7 +356,11 @@ export class TestRunsService { * @param testVariation * @param testRun */ - private async tryAutoApproveByNewBaselines({ testVariation, testRun }: AutoApproveProps): Promise { + private async tryAutoApproveByNewBaselines({ + testVariation, + testRun, + baselineBranchName, + }: AutoApproveProps): Promise { if (testRun.status === TestStatus.ok) { return testRun; } @@ -358,6 +370,7 @@ export class TestRunsService { where: { ...getTestVariationUniqueData(testVariation), baselineName: testVariation.baselineName, + baselineBranchName, status: TestStatus.approved, testVariation: { projectId: testVariation.projectId, @@ -407,4 +420,5 @@ export class TestRunsService { interface AutoApproveProps { testVariation: TestVariation; testRun: TestRun; + baselineBranchName?: string; } diff --git a/src/test-variations/test-variations.service.spec.ts b/src/test-variations/test-variations.service.spec.ts index 28d4a8e0..785c6b36 100644 --- a/src/test-variations/test-variations.service.spec.ts +++ b/src/test-variations/test-variations.service.spec.ts @@ -306,6 +306,126 @@ describe('TestVariationsService', () => { }); expect(result).toBe(variationMainMock); }); + + it('can find by baselineBranchName', async () => { + const createRequest: CreateTestRequestDto = { + buildId: 'buildId', + projectId: projectMock.id, + name: 'Test name', + os: 'OS', + browser: 'browser', + viewport: 'viewport', + device: 'device', + customTags: '', + branchName: 'develop', + baselineBranchName: 'main', + }; + + const variationMock: TestVariation = { + id: '123', + projectId: projectMock.id, + name: 'Test name', + baselineName: 'main', + os: 'OS', + browser: 'browser', + viewport: 'viewport', + device: 'device', + customTags: '', + ignoreAreas: '[]', + comment: 'some comment', + branchName: 'develop', + createdAt: new Date(), + updatedAt: new Date(), + }; + const projectFindUniqueMock = jest.fn().mockReturnValueOnce(projectMock); + service = await initModule({ projectFindUniqueMock }); + service.findUnique = jest.fn().mockResolvedValueOnce(variationMock).mockResolvedValueOnce(undefined); + + const result = await service.find(createRequest); + + expect(projectFindUniqueMock).toHaveBeenCalledWith({ where: { id: createRequest.projectId } }); + expect(service.findUnique).toHaveBeenNthCalledWith(1, { + name: createRequest.name, + projectId: createRequest.projectId, + os: createRequest.os, + browser: createRequest.browser, + viewport: createRequest.viewport, + device: createRequest.device, + customTags: createRequest.customTags, + branchName: createRequest.baselineBranchName, + }); + expect(service.findUnique).toHaveBeenNthCalledWith(2, { + name: createRequest.name, + projectId: createRequest.projectId, + os: createRequest.os, + browser: createRequest.browser, + viewport: createRequest.viewport, + device: createRequest.device, + customTags: createRequest.customTags, + branchName: createRequest.branchName, + }); + expect(result).toBe(variationMock); + }); + + it("can find by current branch if baselineBranchName doesn't exist", async () => { + const createRequest: CreateTestRequestDto = { + buildId: 'buildId', + projectId: projectMock.id, + name: 'Test name', + os: 'OS', + browser: 'browser', + viewport: 'viewport', + device: 'device', + customTags: '', + branchName: 'main', + baselineBranchName: 'release-1', + }; + + const variationMock: TestVariation = { + id: '123', + projectId: projectMock.id, + name: 'Test name', + baselineName: 'baselineName', + os: 'OS', + browser: 'browser', + viewport: 'viewport', + device: 'device', + customTags: '', + ignoreAreas: '[]', + comment: 'some comment', + branchName: 'develop', + createdAt: new Date(), + updatedAt: new Date(), + }; + const projectFindUniqueMock = jest.fn().mockReturnValueOnce(projectMock); + service = await initModule({ projectFindUniqueMock }); + service.findUnique = jest.fn().mockResolvedValueOnce(undefined).mockResolvedValueOnce(variationMock); + + const result = await service.find(createRequest); + + expect(projectFindUniqueMock).toHaveBeenCalledWith({ where: { id: createRequest.projectId } }); + expect(service.findUnique).toHaveBeenNthCalledWith(1, { + name: createRequest.name, + projectId: createRequest.projectId, + os: createRequest.os, + browser: createRequest.browser, + viewport: createRequest.viewport, + device: createRequest.device, + customTags: createRequest.customTags, + branchName: createRequest.baselineBranchName, + }); + expect(service.findUnique).toHaveBeenNthCalledWith(2, { + name: createRequest.name, + projectId: createRequest.projectId, + os: createRequest.os, + browser: createRequest.browser, + viewport: createRequest.viewport, + device: createRequest.device, + customTags: createRequest.customTags, + branchName: createRequest.branchName, + }); + expect(result).toBe(variationMock); + }); }); it('create', async () => { diff --git a/src/test-variations/test-variations.service.ts b/src/test-variations/test-variations.service.ts index 3767588d..c16c686a 100644 --- a/src/test-variations/test-variations.service.ts +++ b/src/test-variations/test-variations.service.ts @@ -1,6 +1,6 @@ import { Injectable, Inject, forwardRef, Logger } from '@nestjs/common'; import { PrismaService } from '../prisma/prisma.service'; -import { TestVariation, Baseline, Build, TestRun, User } from '@prisma/client'; +import { TestVariation, Baseline, Build, TestRun, User, type Project } from '@prisma/client'; import { StaticService } from '../static/static.service'; import { BuildsService } from '../builds/builds.service'; import { TestRunsService } from '../test-runs/test-runs.service'; @@ -79,21 +79,19 @@ export class TestVariationsService { * @param baselineData * @returns */ - async find( - createTestRequestDto: BaselineDataDto & { projectId: string; sourceBranch?: string } - ): Promise { + async find(createTestRequestDto: Omit): Promise { const project = await this.prismaService.project.findUnique({ where: { id: createTestRequestDto.projectId } }); - const mainBranch = createTestRequestDto.sourceBranch ?? project.mainBranchName; + const baselineBranch = createTestRequestDto.baselineBranchName ?? project.mainBranchName; const [mainBranchTestVariation, currentBranchTestVariation] = await Promise.all([ - // search main branch variation + // search baseline branch variation this.findUnique({ projectId: createTestRequestDto.projectId, - branchName: mainBranch, + branchName: baselineBranch, ...getTestVariationUniqueData(createTestRequestDto), }), // search current branch variation - createTestRequestDto.branchName !== mainBranch && + createTestRequestDto.branchName !== baselineBranch && this.findUnique({ projectId: createTestRequestDto.projectId, branchName: createTestRequestDto.branchName, @@ -101,14 +99,14 @@ export class TestVariationsService { }), ]); - if (!!currentBranchTestVariation) { + if (currentBranchTestVariation) { if (mainBranchTestVariation && mainBranchTestVariation.updatedAt > currentBranchTestVariation.updatedAt) { return mainBranchTestVariation; } return currentBranchTestVariation; } - if (!!mainBranchTestVariation) { + if (mainBranchTestVariation) { return mainBranchTestVariation; } } @@ -248,4 +246,14 @@ export class TestVariationsService { where: { id: baseline.id }, }); } + + async findOldTestVariations(project: Project, dateRemoveAfter: Date) { + return await this.prismaService.$queryRaw` + SELECT * from public."TestVariation" + WHERE "projectId" = ${project.id} + AND "updatedAt" <= ${dateRemoveAfter} + AND "branchName" NOT LIKE ${project.mainBranchName} + AND "branchName" NOT SIMILAR TO ${project.protectedBranch} + `; + } } diff --git a/test/preconditions.ts b/test/preconditions.ts index 8edaff9d..f7333424 100644 --- a/test/preconditions.ts +++ b/test/preconditions.ts @@ -51,7 +51,8 @@ export const haveTestRunCreated = async ( projectId: string, branchName: string, imagePath: string, - merge?: boolean + merge?: boolean, + baselineBranchName?: string ): Promise<{ testRun: TestRunResultDto; build: Build }> => { const build = await buildsService.findOrCreate({ projectId: projectId, branchName }); const testRun = await testRunsService.postTestRun({ @@ -61,6 +62,7 @@ export const haveTestRunCreated = async ( buildId: build.id, name: 'Image name', merge, + baselineBranchName, }, imageBuffer: readFileSync(imagePath), }); diff --git a/test/projects.e2e-spec.ts b/test/projects.e2e-spec.ts index c18b0e9c..9433cb32 100644 --- a/test/projects.e2e-spec.ts +++ b/test/projects.e2e-spec.ts @@ -20,6 +20,7 @@ const project: ProjectDto = { maxBuildAllowed: 0, maxBranchLifetime: 0, imageComparisonConfig: '{}', + protectedBranch: null, }; const projectServiceMock = { diff --git a/test/test-runs.e2e-spec.ts b/test/test-runs.e2e-spec.ts index d973cb12..791c7a50 100644 --- a/test/test-runs.e2e-spec.ts +++ b/test/test-runs.e2e-spec.ts @@ -197,6 +197,105 @@ describe('TestRuns (e2e)', () => { expect(testRun.status).toBe(TestStatus.autoApproved); }); + + it('Works with baselineBranchName', async () => { + const baselineBranchName = 'release-1'; + + // Add baseline to main branch + const { testRun: mainBranchTestRun } = await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + project.mainBranchName, + image_v1 + ); + await testRunsService.approve(mainBranchTestRun.id); + + // Add different baseline to release branch + const { testRun: releaseBranchTestRun } = await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + baselineBranchName, + image_v2, + false, + baselineBranchName + ); + await testRunsService.approve(releaseBranchTestRun.id); + + // Should find diff in main branch + const { testRun: testRunAgainstMainBranch } = await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + 'feature-to-main', + image_v2, + false, + project.mainBranchName // baselineBranchName have to be defined for auto-approve filtering + ); + expect(testRunAgainstMainBranch.status).toBe(TestStatus.unresolved); + + // Should be OK in in release branch + const { testRun: testRunAgainstReleaseBranch1 } = await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + 'feature-to-release', + image_v2, + false, + baselineBranchName + ); + expect(testRunAgainstReleaseBranch1.status).toBe(TestStatus.ok); + + // Should NOT auto-approve based on runs other than the ones against baselineBranchName + const { testRun: testRunAgainstReleaseBranch2 } = await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + 'feature-to-release', + image_v1, + false, + baselineBranchName + ); + expect(testRunAgainstReleaseBranch2.status).toBe(TestStatus.unresolved); + + // Should find diff in release branch + const { testRun: testRunAgainstReleaseBranch3 } = await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + 'feature-to-release', + image_v3, + false, + baselineBranchName + ); + expect(testRunAgainstReleaseBranch3.status).toBe(TestStatus.unresolved); + await testRunsService.approve(testRunAgainstReleaseBranch3.id); + + // Should auto-approve runs approved in release branch + const { testRun: testRunAgainstReleaseBranch4 } = await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + baselineBranchName, + image_v3, + false, + baselineBranchName + ); + expect(testRunAgainstReleaseBranch4.status).toBe(TestStatus.autoApproved); + + // Should NOT auto-approve runs in main branch, which were approved in release branch + const { testRun: testRunAgainstMainBranch2 } = await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + project.mainBranchName, + image_v3, + false, + project.mainBranchName + ); + expect(testRunAgainstMainBranch2.status).toBe(TestStatus.unresolved); + }); }); describe('POST /multipart', () => { diff --git a/test/test-variations.e2e-spec.ts b/test/test-variations.e2e-spec.ts index d343a339..64e86bc1 100644 --- a/test/test-variations.e2e-spec.ts +++ b/test/test-variations.e2e-spec.ts @@ -67,4 +67,52 @@ describe('TestVariations (e2e)', () => { expect((await testRunsService.findOne(testRun.id)).testVariationId).toBeNull(); }); }); + + describe('find old test variations', () => { + it('filters out test runs matching releaseBranch and mainBranchName', async () => { + const baselineBranchName = 'release-1'; + const image_v1 = './test/image.png'; + const image_v2 = './test/image_edited.png'; + + // Add variation to main branch + const { testRun: mainBranchTestRun } = await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + project.mainBranchName, + image_v1 + ); + await testRunsService.approve(mainBranchTestRun.id); + + // Add variation to release branch + const { testRun: releaseBranchTestRun } = await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + baselineBranchName, + image_v1, + false, + baselineBranchName + ); + await testRunsService.approve(releaseBranchTestRun.id); + + // Add variation to feature branch + const { testRun: featureBranchTestRun } = await haveTestRunCreated( + buildsService, + testRunsService, + project.id, + 'feature', + image_v2, + false, + project.mainBranchName + ); + const featureBranchTestRunApproved = await testRunsService.approve(featureBranchTestRun.id); + + const result = await testVariationsService.findOldTestVariations(project, new Date()); + expect(result).toHaveLength(1); + expect(result).toEqual( + expect.arrayContaining([expect.objectContaining({ id: featureBranchTestRunApproved.testVariationId })]) + ); + }); + }); });