Merge pull request #1153 from argos-ci/improve-visual-testing-algo

feat: improve visual testing algo

apps/backend/src/screenshot-diff/computeScreenshotDiff.ts

@@ -9,7 +9,7 @@ import { File, Screenshot, ScreenshotDiff } from "@/database/models/index.js";
 import { S3ImageFile } from "@/storage/index.js";
 import { getRedisLock } from "@/util/redis/index.js";
 
-import { ImageDiffResult, diffImages } from "./util/image-diff/index.js";
+import { diffImages } from "./util/image-diff/index.js";
 import { chunk } from "@/util/chunk.js";
 
 const hashFile = async (filepath: string): Promise<string> => {
@@ -68,28 +68,25 @@ async function ensureFileDimensions({
   });
 }
 
-async function lockAndUploadDiffFile({
-  key,
-  diffResult,
-  diffImage,
-}: {
+async function lockAndUploadDiffFile(args: {
   key: string;
-  diffResult: ImageDiffResult;
-  diffImage: S3ImageFile;
+  width: number;
+  height: number;
+  image: S3ImageFile;
 }) {
   const lock = await getRedisLock();
-  return lock.acquire(`diffUpload-${key}`, async () => {
+  return lock.acquire(`diffUpload-${args.key}`, async () => {
     // Check if the diff file has been uploaded by another process
-    const existingDiffFile = await File.query().findOne({ key });
+    const existingDiffFile = await File.query().findOne({ key: args.key });
     if (existingDiffFile) return existingDiffFile;
 
-    await diffImage.upload();
+    await args.image.upload();
 
     return File.query()
       .insert({
-        key,
-        width: diffResult.width,
-        height: diffResult.height,
+        key: args.key,
+        width: args.width,
+        height: args.height,
         type: "screenshotDiff",
       })
       .returning("*");
@@ -100,29 +97,31 @@ async function lockAndUploadDiffFile({
  * Processes the diff result. Returns the existing file if found.
  * If not, uploads the new diff using a lock to avoid concurrency issues.
  */
-async function getOrCreateDiffFile({
-  diffResult,
-  s3,
-  bucket,
-  key,
-}: {
-  diffResult: ImageDiffResult;
+async function getOrCreateDiffFile(args: {
+  filepath: string;
+  width: number;
+  height: number;
   s3: S3Client;
   bucket: string;
   key: string;
 }) {
-  const diffImage = new S3ImageFile({
-    s3,
-    bucket: bucket,
-    filepath: diffResult.filepath,
-    key,
+  const image = new S3ImageFile({
+    s3: args.s3,
+    bucket: args.bucket,
+    filepath: args.filepath,
+    key: args.key,
   });
-  const existingDiffFile = await File.query().findOne({ key });
-  const diffFile =
-    existingDiffFile ||
-    (await lockAndUploadDiffFile({ key, diffResult, diffImage }));
-  await diffImage.unlink();
-  return diffFile;
+  let file = await File.query().findOne({ key: args.key });
+  if (!file) {
+    file = await lockAndUploadDiffFile({
+      key: args.key,
+      width: args.width,
+      height: args.height,
+      image,
+    });
+  }
+  await image.unlink();
+  return file;
 }
 
 async function areAllDiffsCompleted(buildId: string): Promise<{
@@ -212,16 +211,17 @@ export const computeScreenshotDiff = async (
   let diffKey: string | null = null;
 
   if (baseImage && baseImage.key !== compareImage.key && !screenshotDiff.s3Id) {
-    const diffResult = await diffImages({ baseImage, compareImage });
-
-    if (diffResult.score === 0) {
+    const diffResult = await diffImages(baseImage, compareImage);
+    if (diffResult === null) {
       await ScreenshotDiff.query()
         .findById(screenshotDiff.id)
-        .patch({ score: diffResult.score });
+        .patch({ score: 0 });
     } else {
       diffKey = await hashFile(diffResult.filepath);
       const diffFile = await getOrCreateDiffFile({
-        diffResult,
+        filepath: diffResult.filepath,
+        width: diffResult.width,
+        height: diffResult.height,
         s3,
         bucket,
         key: diffKey,

apps/backend/src/screenshot-diff/util/image-diff/index.test.ts

@@ -1,149 +1,52 @@
 /* eslint-disable @typescript-eslint/no-unused-vars */
-import { join } from "node:path";
+import { resolve } from "node:path";
 import { fileURLToPath } from "node:url";
-import { describe, expect, it } from "vitest";
+import { describe, expect, test } from "vitest";
+import { copyFile, unlink } from "node:fs/promises";
 
 import { LocalImageFile } from "@/storage/index.js";
 
 import { diffImages } from "./index.js";
 
 const __dirname = fileURLToPath(new URL(".", import.meta.url));
 
-describe("#diffImages", () => {
-  it("simple", async () => {
-    const { filepath, ...result } = await diffImages({
-      baseImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/simple/compare.png"),
-      }),
-      compareImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/simple/base.png"),
-      }),
-    });
-
-    expect(result).toEqual({ score: 0, width: 1425, height: 1146 });
-  });
-
-  it("alphaBackground", async () => {
-    const { filepath, ...result } = await diffImages({
-      baseImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/alphaBackground/compare.png"),
-      }),
-      compareImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/alphaBackground/base.png"),
-      }),
-    });
-
-    expect(result).toEqual({ score: 0, width: 1400, height: 300 });
-  });
-
-  it("boxShadow", async () => {
-    const { filepath, ...result } = await diffImages({
-      baseImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/boxShadow/compare.png"),
-      }),
-      compareImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/boxShadow/base.png"),
-      }),
-    });
-
-    expect(result).toEqual({ score: 0, width: 250, height: 300 });
-  });
-
-  it("border", async () => {
-    const { filepath, ...result } = await diffImages({
-      baseImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/border/compare.png"),
-      }),
-      compareImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/border/base.png"),
-      }),
-    });
-
-    expect(result).toEqual({
-      score: 0,
-      width: 1000,
-      height: 786,
-    });
-  });
-
-  it("fontAliasing", async () => {
-    const { filepath, ...result } = await diffImages({
-      baseImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/fontAliasing/compare.png"),
-      }),
-      compareImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/fontAliasing/base.png"),
-      }),
-    });
-
-    expect(result).toEqual({
-      score: 0.00182697201018,
-      width: 250,
-      height: 786,
-    });
-  });
+const tests = [
+  ["alphaBackground", false],
+  ["big-images", true],
+  ["border", false],
+  ["boxShadow", false],
+  ["fontAliasing", false],
+  ["imageCompression1", false],
+  ["imageCompression2", false],
+  ["imageCompression3", false],
+  ["imageCompression4", false],
+  ["imageCompression4", false],
+  ["simple", true],
+  ["tableAlpha", true],
+  ["minorChange1", true],
+  ["minorChange2", true],
+];
 
-  it("imageCompression", async () => {
-    const { filepath, ...result } = await diffImages({
-      baseImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/imageCompression/compare.png"),
-      }),
-      compareImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/imageCompression/base.png"),
-      }),
-    });
-
-    expect(result).toEqual({ score: 0, width: 327, height: 665 });
-  });
-
-  it("imageCompression2", async () => {
-    const { filepath, ...result } = await diffImages({
-      baseImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/imageCompression2/compare.png"),
-      }),
-      compareImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/imageCompression2/base.png"),
-      }),
-    });
-
-    expect(result).toEqual({
-      height: 665,
-      score: 0,
-      width: 327,
-    });
-  });
+describe("#diffImages", () => {
+  test.each(tests)("diffImages %s", async (name, hasDiff) => {
+    const dir = resolve(__dirname, `__fixtures__/${name}`);
 
-  it("imageCompression3", async () => {
-    const { filepath, ...result } = await diffImages({
-      baseImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/imageCompression3/compare.png"),
+    const result = await diffImages(
+      new LocalImageFile({
+        filepath: resolve(dir, "compare.png"),
       }),
-      compareImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/imageCompression3/base.png"),
+      new LocalImageFile({
+        filepath: resolve(dir, "base.png"),
       }),
-    });
+    );
 
-    expect(result).toEqual({
-      score: 0.0000729166666667,
-      width: 1280,
-      height: 600,
-    });
-  });
+    const diffPath = resolve(dir, "diff_tmp.png");
+    await unlink(diffPath).catch(() => {});
 
-  it("big images", async () => {
-    const { filepath, ...result } = await diffImages({
-      baseImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/big-images/compare.png"),
-      }),
-      compareImage: new LocalImageFile({
-        filepath: join(__dirname, "__fixtures__/big-images/base.png"),
-      }),
-    });
+    if (result) {
+      await copyFile(result.filepath, diffPath);
+    }
 
-    expect(result).toEqual({
-      score: 0.846446632356,
-      width: 1000,
-      height: 4469,
-    });
+    expect(Boolean(result)).toBe(hasDiff);
   });
 });