From c39dcf3041acfa29a3927c709d384b47a9feb0c7 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Sat, 30 Jan 2021 14:24:36 +0200 Subject: [PATCH 1/2] Add auto approve based on history https://github.com/Visual-Regression-Tracker/Visual-Regression-Tracker/issues/190 --- .env | 5 +- .../README.md | 37 ++++++ .../schema.prisma | 123 ++++++++++++++++++ .../steps.json | 12 ++ prisma/migrations/migrate.lock | 3 +- prisma/schema.prisma | 1 + src/test-runs/test-runs.service.spec.ts | 16 +++ src/test-runs/test-runs.service.ts | 95 +++++++++++--- .../test-variations.service.ts | 9 +- src/utils/index.ts | 12 ++ 10 files changed, 290 insertions(+), 23 deletions(-) create mode 100644 prisma/migrations/20210130115922-test-run-auto-approve-status-added/README.md create mode 100644 prisma/migrations/20210130115922-test-run-auto-approve-status-added/schema.prisma create mode 100644 prisma/migrations/20210130115922-test-run-auto-approve-status-added/steps.json create mode 100644 src/utils/index.ts diff --git a/.env b/.env index b1cc569f..bdab8951 100644 --- a/.env +++ b/.env @@ -14,4 +14,7 @@ POSTGRES_DB=vrt_db_dev # optional #HTTPS_KEY_PATH='./secrets/ssl.key' -#HTTPS_CERT_PATH='./secrets/ssl.cert' \ No newline at end of file +#HTTPS_CERT_PATH='./secrets/ssl.cert' + +# features +AUTO_APPROVE_BASED_ON_HISTORY=true diff --git a/prisma/migrations/20210130115922-test-run-auto-approve-status-added/README.md b/prisma/migrations/20210130115922-test-run-auto-approve-status-added/README.md new file mode 100644 index 00000000..a1063681 --- /dev/null +++ b/prisma/migrations/20210130115922-test-run-auto-approve-status-added/README.md @@ -0,0 +1,37 @@ +# Migration `20210130115922-test-run-auto-approve-status-added` + +This migration has been generated by Pavlo Strunkin at 1/30/2021, 1:59:22 PM. +You can check out the [state of the schema](./schema.prisma) after the migration. + +## Database Steps + +```sql +ALTER TYPE "TestStatus" ADD VALUE 'autoApproved' +``` + +## Changes + +```diff +diff --git schema.prisma schema.prisma +migration 20210118201534-build--project-id---ci-build-id-constraint..20210130115922-test-run-auto-approve-status-added +--- datamodel.dml ++++ datamodel.dml +@@ -3,9 +3,9 @@ + } + datasource db { + provider = "postgresql" +- url = "***" ++ url = "***" + } + model Build { + id String @id @default(uuid()) +@@ -118,5 +118,6 @@ + new + ok + unresolved + approved ++ autoApproved + } +``` + + diff --git a/prisma/migrations/20210130115922-test-run-auto-approve-status-added/schema.prisma b/prisma/migrations/20210130115922-test-run-auto-approve-status-added/schema.prisma new file mode 100644 index 00000000..f34e30cd --- /dev/null +++ b/prisma/migrations/20210130115922-test-run-auto-approve-status-added/schema.prisma @@ -0,0 +1,123 @@ +generator client { + provider = "prisma-client-js" +} + +datasource db { + provider = "postgresql" + url = "***" +} + +model Build { + id String @id @default(uuid()) + ciBuildId String? + number Int? + branchName String? + status String? + testRuns TestRun[] + projectId String + project Project @relation(fields: [projectId], references: [id]) + updatedAt DateTime @updatedAt + createdAt DateTime @default(now()) + user User? @relation(fields: [userId], references: [id]) + userId String? + isRunning Boolean? + + @@unique([projectId, ciBuildId]) +} + +model Project { + id String @id @default(uuid()) + name String + mainBranchName String @default("master") + builds Build[] + buildsCounter Int @default(0) + testVariations TestVariation[] + updatedAt DateTime @updatedAt + createdAt DateTime @default(now()) + + @@unique([name]) +} + +model TestRun { + id String @id @default(uuid()) + imageName String + diffName String? + diffPercent Float? + diffTollerancePercent Float @default(0) + pixelMisMatchCount Int? + status TestStatus + buildId String + build Build @relation(fields: [buildId], references: [id]) + testVariationId String + testVariation TestVariation @relation(fields: [testVariationId], references: [id]) + merge Boolean @default(false) + updatedAt DateTime @updatedAt + createdAt DateTime @default(now()) + // Test variation data + name String @default("") + browser String? + device String? + os String? + viewport String? + baselineName String? + comment String? + baseline Baseline? + branchName String @default("master") + baselineBranchName String? + ignoreAreas String @default("[]") + tempIgnoreAreas String @default("[]") +} + +model TestVariation { + id String @id @default(uuid()) + name String + branchName String @default("master") + browser String? + device String? + os String? + viewport String? + baselineName String? + ignoreAreas String @default("[]") + projectId String + project Project @relation(fields: [projectId], references: [id]) + testRuns TestRun[] + baselines Baseline[] + comment String? + updatedAt DateTime @updatedAt + createdAt DateTime @default(now()) + + @@unique([projectId, name, browser, device, os, viewport, branchName]) +} + +model Baseline { + id String @id @default(uuid()) + baselineName String + testVariationId String + testVariation TestVariation @relation(fields: [testVariationId], references: [id]) + testRunId String? + testRun TestRun? @relation(fields: [testRunId], references: [id]) + updatedAt DateTime @updatedAt + createdAt DateTime @default(now()) +} + +model User { + id String @id @default(uuid()) + email String @unique + password String + firstName String? + lastName String? + apiKey String @unique + isActive Boolean @default(true) + builds Build[] + updatedAt DateTime @updatedAt + createdAt DateTime @default(now()) +} + +enum TestStatus { + failed + new + ok + unresolved + approved + autoApproved +} diff --git a/prisma/migrations/20210130115922-test-run-auto-approve-status-added/steps.json b/prisma/migrations/20210130115922-test-run-auto-approve-status-added/steps.json new file mode 100644 index 00000000..3b608c05 --- /dev/null +++ b/prisma/migrations/20210130115922-test-run-auto-approve-status-added/steps.json @@ -0,0 +1,12 @@ +{ + "version": "0.3.14-fixed", + "steps": [ + { + "tag": "UpdateEnum", + "enum": "TestStatus", + "createdValues": [ + "autoApproved" + ] + } + ] +} \ No newline at end of file diff --git a/prisma/migrations/migrate.lock b/prisma/migrations/migrate.lock index 7fb1fb04..df95abc5 100644 --- a/prisma/migrations/migrate.lock +++ b/prisma/migrations/migrate.lock @@ -12,4 +12,5 @@ 20201007145002-builds-counter 20201115155739-ci-build-id-added 20201201211711-test-run--temp-ignore-areas-added -20210118201534-build--project-id---ci-build-id-constraint \ No newline at end of file +20210118201534-build--project-id---ci-build-id-constraint +20210130115922-test-run-auto-approve-status-added \ No newline at end of file diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 95573aa8..48a21c89 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -119,4 +119,5 @@ enum TestStatus { ok unresolved approved + autoApproved } diff --git a/src/test-runs/test-runs.service.spec.ts b/src/test-runs/test-runs.service.spec.ts index 1791ec23..7aec62b8 100644 --- a/src/test-runs/test-runs.service.spec.ts +++ b/src/test-runs/test-runs.service.spec.ts @@ -514,6 +514,8 @@ describe('TestRunsService', () => { service.getDiff = getDiffMock; const saveDiffResultMock = jest.fn(); service.saveDiffResult = saveDiffResultMock.mockResolvedValueOnce(testRunWithResult); + const tryAutoApproveBasedOnHistory = jest.fn(); + service['tryAutoApproveBasedOnHistory'] = tryAutoApproveBasedOnHistory.mockResolvedValueOnce(testRunWithResult); const result = await service.create(testVariation, createTestRequestDto); @@ -564,6 +566,20 @@ describe('TestRunsService', () => { }, ]); expect(saveDiffResultMock).toHaveBeenCalledWith(testRun.id, diffResult); + expect(tryAutoApproveBasedOnHistory).toHaveBeenCalledWith(testVariation, testRunWithResult, image, [ + { + x: 3, + y: 4, + width: 500, + height: 600, + }, + { + x: 1, + y: 2, + width: 100, + height: 200, + }, + ]); expect(eventTestRunCreatedMock).toHaveBeenCalledWith(testRunWithResult); expect(result).toBe(testRunWithResult); }); diff --git a/src/test-runs/test-runs.service.ts b/src/test-runs/test-runs.service.ts index 7379040a..e3853874 100644 --- a/src/test-runs/test-runs.service.ts +++ b/src/test-runs/test-runs.service.ts @@ -1,4 +1,4 @@ -import { forwardRef, Inject, Injectable } from '@nestjs/common'; +import { forwardRef, Inject, Injectable, Logger } from '@nestjs/common'; import { PNG } from 'pngjs'; import Pixelmatch from 'pixelmatch'; import { CreateTestRequestDto } from './dto/create-test-request.dto'; @@ -14,9 +14,12 @@ import { TestVariationsService } from '../test-variations/test-variations.servic import { convertBaselineDataToQuery } from '../shared/dto/baseline-data.dto'; import { TestRunDto } from './dto/testRun.dto'; import { PaginatedTestRunDto } from './dto/testRun-paginated.dto'; +import { getTestVariationUniqueData } from '../utils'; @Injectable() export class TestRunsService { + private readonly logger: Logger = new Logger(TestRunsService.name); + constructor( @Inject(forwardRef(() => TestVariationsService)) private testVariationService: TestVariationsService, @@ -81,7 +84,8 @@ export class TestRunsService { return new TestRunResultDto(testRun, testVariation); } - async approve(id: string, merge: boolean): Promise { + async approve(id: string, merge: boolean, autoApprove?: boolean): Promise { + const status = autoApprove ? TestStatus.autoApproved : TestStatus.approved; const testRun = await this.findOne(id); // save new baseline @@ -92,7 +96,7 @@ export class TestRunsService { testRunUpdated = await this.prismaService.testRun.update({ where: { id }, data: { - status: TestStatus.approved, + status, testVariation: { update: { baselineName, @@ -115,11 +119,7 @@ export class TestRunsService { data: { project: { connect: { id: testRun.testVariation.projectId } }, baselineName, - name: testRun.name, - browser: testRun.browser, - device: testRun.device, - os: testRun.os, - viewport: testRun.viewport, + ...getTestVariationUniqueData(testRun), ignoreAreas: testRun.ignoreAreas, comment: testRun.comment, branchName: testRun.branchName, @@ -141,7 +141,7 @@ export class TestRunsService { testRunUpdated = await this.prismaService.testRun.update({ where: { id }, data: { - status: TestStatus.approved, + status, testVariation: { connect: { id: newTestVariation.id }, }, @@ -209,11 +209,7 @@ export class TestRunsService { id: createTestRequestDto.buildId, }, }, - name: testVariation.name, - browser: testVariation.browser, - device: testVariation.device, - os: testVariation.os, - viewport: testVariation.viewport, + ...getTestVariationUniqueData(testVariation), baselineName: testVariation.baselineName, baselineBranchName: testVariation.branchName, ignoreAreas: testVariation.ignoreAreas, @@ -235,7 +231,10 @@ export class TestRunsService { } const diffResult = this.getDiff(baseline, image, testRun.diffTollerancePercent, ignoreAreas); - const testRunWithResult = await this.saveDiffResult(testRun.id, diffResult); + let testRunWithResult = await this.saveDiffResult(testRun.id, diffResult); + + testRunWithResult = await this.tryAutoApproveBasedOnHistory(testVariation, testRunWithResult, image, ignoreAreas); + this.eventsGateway.testRunCreated(testRunWithResult); return testRunWithResult; } @@ -334,4 +333,70 @@ export class TestRunsService { }); return image.data; } + + private async tryAutoApproveBasedOnHistory( + testVariation: TestVariation, + testRun: TestRun, + image: PNG, + ignoreAreas: IgnoreAreaDto[] + ): Promise { + if (process.env.AUTO_APPROVE_BASED_ON_HISTORY && testRun.status !== TestStatus.ok) { + this.logger.log(`Try auto approve testRun: ${testRun.id}`); + + const alreadyApprovedTestRuns: TestRun[] = await this.prismaService.testRun.findMany({ + where: { + ...getTestVariationUniqueData(testVariation), + baselineName: testVariation.baselineName, + status: TestStatus.approved, + }, + }); + + let autoApproved = false; + for (const approvedTestRun of alreadyApprovedTestRuns) { + this.logger.log( + `Found already approved baseline for testRun: ${testRun.id} + testVariation: ${approvedTestRun.testVariationId} + branch: ${approvedTestRun.branchName} + testRun: ${approvedTestRun.id} + build: ${approvedTestRun.buildId}` + ); + + const approvedTestVariation = await this.prismaService.testVariation.findUnique({ + where: { + id: approvedTestRun.testVariationId, + }, + }); + + const approvedBaseline = this.staticService.getImage(approvedTestVariation.baselineName); + const diffResult = this.getDiff(approvedBaseline, image, testRun.diffTollerancePercent, ignoreAreas); + + if (diffResult.status === TestStatus.ok) { + autoApproved = true; + const baseline = await this.prismaService.baseline.findFirst({ + where: { + testVariationId: approvedTestVariation.id, + baselineName: approvedTestVariation.baselineName, + }, + include: { + testRun: true, + }, + }); + this.logger.log( + `Found reason to auto approve testRun: ${testRun.id} + testVariation: ${baseline.testVariationId} + baseline: ${baseline.id} + branch: ${approvedTestVariation.branchName} + testRun: ${baseline.testRunId} + build: ${baseline.testRun.buildId}` + ); + } + } + + if (autoApproved) { + return this.approve(testRun.id, false, true); + } + this.logger.log(`Cannot auto approve testRun: ${testRun.id}`); + } + return testRun; + } } diff --git a/src/test-variations/test-variations.service.ts b/src/test-variations/test-variations.service.ts index 1a94ef8e..6abc1611 100644 --- a/src/test-variations/test-variations.service.ts +++ b/src/test-variations/test-variations.service.ts @@ -1,7 +1,7 @@ import { Injectable, Inject, forwardRef } from '@nestjs/common'; import { IgnoreAreaDto } from '../test-runs/dto/ignore-area.dto'; import { PrismaService } from '../prisma/prisma.service'; -import { TestVariation, Baseline, Project} from '@prisma/client'; +import { TestVariation, Baseline, Project } from '@prisma/client'; import { StaticService } from '../shared/static/static.service'; import { CommentDto } from '../shared/dto/comment.dto'; import { BaselineDataDto, convertBaselineDataToQuery } from '../shared/dto/baseline-data.dto'; @@ -10,6 +10,7 @@ import { TestRunsService } from '../test-runs/test-runs.service'; import { PNG } from 'pngjs'; import { CreateTestRequestDto } from 'src/test-runs/dto/create-test-request.dto'; import { BuildDto } from 'src/builds/dto/build.dto'; +import { getTestVariationUniqueData } from '../utils'; @Injectable() export class TestVariationsService { @@ -46,12 +47,8 @@ export class TestVariationsService { this.prismaService.testVariation.findMany({ where: { projectId, - name: baselineData.name, - os: baselineData.os, - device: baselineData.device, - browser: baselineData.browser, - viewport: baselineData.viewport, branchName: project.mainBranchName, + ...getTestVariationUniqueData(baselineData), }, }), // search current branch variation diff --git a/src/utils/index.ts b/src/utils/index.ts new file mode 100644 index 00000000..f5c7c1f2 --- /dev/null +++ b/src/utils/index.ts @@ -0,0 +1,12 @@ +import { TestRun, TestVariation } from '@prisma/client'; +import { BaselineDataDto } from 'src/shared/dto/baseline-data.dto'; + +export const getTestVariationUniqueData = (object: TestRun | TestVariation | BaselineDataDto) => { + return { + name: object.name, + os: object.os, + device: object.device, + browser: object.browser, + viewport: object.viewport, + }; +}; From 07ead43fd9d3c664685f6a58f00a700d6abeeece Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Sat, 30 Jan 2021 14:27:49 +0200 Subject: [PATCH 2/2] Update index.ts --- src/utils/index.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/utils/index.ts b/src/utils/index.ts index f5c7c1f2..32d73667 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,7 +1,15 @@ import { TestRun, TestVariation } from '@prisma/client'; import { BaselineDataDto } from 'src/shared/dto/baseline-data.dto'; -export const getTestVariationUniqueData = (object: TestRun | TestVariation | BaselineDataDto) => { +export const getTestVariationUniqueData = ( + object: TestRun | TestVariation | BaselineDataDto +): { + name: string; + os: string; + device: string; + browser: string; + viewport: string; +} => { return { name: object.name, os: object.os,