Merge pull request #1492 from argos-ci/cache-conclusion-stats

feat: cache conclusion and stats in database

Procfile

@@ -2,4 +2,4 @@ release: ./scripts/release.sh
 web: ./scripts/heroku-node-settings.sh apps/backend/dist/processes/proc/web.js
 buildAndSynchronize: ./scripts/heroku-node-settings.sh apps/backend/dist/processes/proc/build-and-synchronize.js
 screenshotDiff: ./scripts/heroku-node-settings.sh apps/backend/dist/processes/proc/screenshot-diff.js
-crawlAndCapture: ./scripts/heroku-node-settings.sh apps/backend/dist/processes/proc/crawl-and-capture.js
+concludeBuilds: ./scripts/heroku-node-settings.sh apps/backend/dist/processes/proc/conclude-builds.js

apps/backend/db/migrations/20241231154644_build-status.js

@@ -0,0 +1,19 @@
+/**
+ * @param {import('knex').Knex} knex
+ */
+export const up = async (knex) => {
+  await knex.schema.alterTable("builds", async (table) => {
+    table.enum("conclusion", ["no-changes", "changes-detected"]);
+    table.jsonb("stats");
+  });
+};
+
+/**
+ * @param {import('knex').Knex} knex
+ */
+export const down = async (knex) => {
+  await knex.schema.alterTable("builds", async (table) => {
+    table.dropColumn("conclusion");
+    table.dropColumn("stats");
+  });
+};

apps/backend/db/structure.sql

@@ -226,7 +226,10 @@ CREATE TABLE public.builds (
     metadata jsonb,
     "baseBranchResolvedFrom" text,
     "parentCommits" jsonb,
+    conclusion text,
+    stats jsonb,
     CONSTRAINT "builds_baseBranchResolvedFrom_check" CHECK (("baseBranchResolvedFrom" = ANY (ARRAY['user'::text, 'pull-request'::text, 'project'::text]))),
+    CONSTRAINT builds_conclusion_check CHECK ((conclusion = ANY (ARRAY['no-changes'::text, 'changes-detected'::text]))),
     CONSTRAINT builds_mode_check CHECK ((mode = ANY (ARRAY['ci'::text, 'monitoring'::text]))),
     CONSTRAINT builds_type_check CHECK ((type = ANY (ARRAY['reference'::text, 'check'::text, 'orphan'::text])))
 );
@@ -2919,4 +2922,5 @@ INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('2024120
 INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241201100628_remove-github-access-token.js', 1, NOW());
 INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241201210158_google-account.js', 1, NOW());
 INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241201222408_gitlab-last-loggedat.js', 1, NOW());
-INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241215091232_project-default-role.js', 1, NOW());
+INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241215091232_project-default-role.js', 1, NOW());
+INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241231154644_build-status.js', 1, NOW());

apps/backend/src/api/handlers/finalizeBuilds.e2e.test.ts

@@ -71,7 +71,7 @@ describe("finalizeBuilds", () => {
               {
                 id: build.id,
                 number: 1,
-                status: "stable",
+                status: "no-changes",
                 url: "http://localhost:3000/awesome-team/awesome-project/builds/1",
                 notification: {
                   description: "Everything's good!",
@@ -122,7 +122,7 @@ describe("finalizeBuilds", () => {
               {
                 id: build.id,
                 number: 1,
-                status: "stable",
+                status: "no-changes",
                 url: "http://localhost:3000/awesome-team/awesome-project/builds/1",
                 notification: {
                   description: "Everything's good!",

apps/backend/src/api/schema/primitives/build.ts

@@ -46,13 +46,13 @@ function getBuildNotificationTypeFromBuildStatus(
       return "diff-accepted";
     case "rejected":
       return "diff-rejected";
-    case "diffDetected":
+    case "changes-detected":
       return "diff-detected";
     case "pending":
       return "queued";
     case "progress":
       return "progress";
-    case "stable":
+    case "no-changes":
       return "no-diff-detected";
     default:
       return null;

apps/backend/src/build-notification/comment.ts

@@ -66,7 +66,8 @@ export const getCommentBody = async (props: {
         invariant(status, "unknown build status");
 
         const label = getBuildLabel(build.type, status);
-        const review = status === "diffDetected" ? ` ([Review](${url}))` : "";
+        const review =
+          status === "changes-detected" ? ` ([Review](${url}))` : "";
         const name = getBuildName({
           build,
           project: hasMultipleProjects ? build.project : null,

apps/backend/src/build/bin/queue-unconcluded-builds.ts

@@ -0,0 +1,41 @@
+#!/usr/bin/env node
+import { callbackify } from "node:util";
+
+import { Build } from "@/database/models/index.js";
+import logger from "@/logger/index.js";
+
+import { concludeBuildsJob } from "../conclude-job";
+
+const main = callbackify(async () => {
+  const batch = 500;
+  const totalCount = await Build.query()
+    .where("jobStatus", "complete")
+    .whereNull("conclusion")
+    .resultSize();
+
+  let total = 0;
+  for (let offset = 0; offset < totalCount; offset += batch) {
+    const nodes = await Build.query()
+      .where("jobStatus", "complete")
+      .whereNull("conclusion")
+      .limit(batch)
+      .offset(offset)
+      .orderBy("id", "desc");
+
+    const ids = nodes.map((node) => node.id);
+    const percentage = Math.round((total / totalCount) * 100);
+    logger.info(
+      `Processing ${total}/${totalCount} (${percentage}%) - Pushing ${ids.length} builds in queue`,
+    );
+    await concludeBuildsJob.push(...ids);
+    total += nodes.length;
+  }
+
+  logger.info(`${total} builds pushed in queue (100% complete)`);
+});
+
+main((err) => {
+  if (err) {
+    throw err;
+  }
+});

apps/backend/src/build/conclude-job.ts

@@ -0,0 +1,16 @@
+import { createJob } from "@/job-core/index.js";
+import logger from "@/logger/index.js";
+
+import { concludeBuild } from "./concludeBuild.js";
+
+export const concludeBuildsJob = createJob("conclude-builds", {
+  complete: () => {},
+  error: (value, error) => {
+    console.error("Error while processing build", value, error);
+  },
+  perform: async (buildId: string) => {
+    logger.info(`Concluding build ${buildId}`);
+    await concludeBuild({ buildId });
+    logger.info(`Build ${buildId} concluded`);
+  },
+});

apps/backend/src/build/concludeBuild.ts

@@ -0,0 +1,50 @@
+import { assertNever } from "@argos/util/assertNever";
+import { invariant } from "@argos/util/invariant";
+
+import { job as buildNotificationJob } from "@/build-notification/job.js";
+import { transaction } from "@/database/index.js";
+import { Build, BuildConclusion, BuildNotification } from "@/database/models";
+
+/**
+ * Concludes the build by updating the conclusion and the stats.
+ * Called when all diffs are processed.
+ */
+export async function concludeBuild(input: { buildId: string }) {
+  const { buildId } = input;
+  const statuses = await Build.getScreenshotDiffsStatuses([buildId]);
+  const [[conclusion], [stats]] = await Promise.all([
+    Build.getConclusions([buildId], statuses),
+    Build.getStats([buildId]),
+  ]);
+  invariant(stats !== undefined, "No stats found");
+  invariant(conclusion !== undefined, "No conclusion found");
+  // If the build is not yet concluded, we don't want to update it.
+  if (conclusion === null) {
+    return;
+  }
+  const [, buildNotification] = await transaction(async (trx) => {
+    return Promise.all([
+      Build.query(trx).findById(buildId).patch({
+        conclusion,
+        stats,
+      }),
+      BuildNotification.query(trx).insert({
+        buildId,
+        type: getNotificationType(conclusion),
+        jobStatus: "pending",
+      }),
+    ]);
+  });
+  await buildNotificationJob.push(buildNotification.id);
+}
+
+function getNotificationType(conclusion: BuildConclusion) {
+  switch (conclusion) {
+    case "changes-detected":
+      return "diff-detected";
+    case "no-changes":
+      return "no-diff-detected";
+    default:
+      return assertNever(conclusion);
+  }
+}

apps/backend/src/build/createBuild.ts

@@ -168,10 +168,7 @@ export async function createBuild(params: {
     },
   );
 
-  await pushBuildNotification({
-    buildId: build.id,
-    type: "queued",
-  });
+  await pushBuildNotification({ buildId: build.id, type: "queued" });
 
   return build;
 }

apps/backend/src/build/createBuildDiffs.ts

@@ -91,6 +91,8 @@ export async function createBuildDiffs(build: Build) {
     "no compare screenshot bucket found for build",
   );
 
+  invariant(compareScreenshotBucket.complete, "compare bucket is not complete");
+
   const compareScreenshots = compareScreenshotBucket.screenshots;
   invariant(compareScreenshots, "no compare screenshots found for build");
 

apps/backend/src/build/job.e2e.test.ts

@@ -31,7 +31,7 @@ describe("build", () => {
         baseScreenshotBucketId: null,
         compareScreenshotBucketId: compareBucket.id,
         projectId: project.id,
-        jobStatus: "pending",
+        jobStatus: "progress",
       });
       const file = await factory.File.create({
         type: "screenshot",
@@ -43,6 +43,7 @@ describe("build", () => {
       });
       await factory.ScreenshotBucket.create({
         projectId: project.id,
+        complete: true,
       });
     });
 

apps/backend/src/build/job.ts

@@ -8,19 +8,18 @@ import { createModelJob, UnretryableError } from "@/job-core/index.js";
 import { job as screenshotDiffJob } from "@/screenshot-diff/index.js";
 import { updateStripeUsage } from "@/stripe/index.js";
 
+import { concludeBuild } from "./concludeBuild.js";
 import { createBuildDiffs } from "./createBuildDiffs.js";
 
 /**
  * Pushes the diffs to the screenshot-diff job queue.
  * If there is no diff to proceed, it pushes a notification.
  */
-async function pushDiffs(buildId: string, screenshotDiffs: ScreenshotDiff[]) {
-  if (screenshotDiffs.length === 0) {
-    await pushBuildNotification({ buildId, type: "no-diff-detected" });
-    return;
-  }
-
-  const toProcessedDiffIds = screenshotDiffs
+async function pushDiffs(input: {
+  build: Build;
+  screenshotDiffs: ScreenshotDiff[];
+}) {
+  const toProcessedDiffIds = input.screenshotDiffs
     .filter(({ jobStatus }) => jobStatus !== "complete")
     .map(({ id }) => id);
 
@@ -29,18 +28,7 @@ async function pushDiffs(buildId: string, screenshotDiffs: ScreenshotDiff[]) {
     return;
   }
 
-  await ScreenshotDiff.fetchGraph(screenshotDiffs, "compareScreenshot");
-  const statuses = await Promise.all(
-    screenshotDiffs.map((screenshotDiff) => screenshotDiff.$getDiffStatus()),
-  );
-
-  const type = statuses.some(
-    (status) => status === "added" || status === "removed",
-  )
-    ? ("diff-detected" as const)
-    : ("no-diff-detected" as const);
-
-  await pushBuildNotification({ buildId, type });
+  await concludeBuild({ buildId: input.build.id });
 }
 
 /**
@@ -109,8 +97,8 @@ export async function performBuild(build: Build) {
       .withGraphFetched("[gitlabProject, account]")
       .throwIfNotFound(),
     pushBuildNotification({ buildId: build.id, type: "progress" }),
-    createBuildDiffs(build).then(async (diffs) => {
-      pushDiffs(build.id, diffs);
+    createBuildDiffs(build).then(async (screenshotDiffs) => {
+      await pushDiffs({ build, screenshotDiffs });
     }),
   ]);
 

apps/backend/src/build/label.ts

@@ -11,7 +11,7 @@ function getBuildStatusLabel(status: BuildAggregatedStatus): string {
       return "👍 Changes approved";
     case "aborted":
       return "🙅 Build aborted";
-    case "diffDetected":
+    case "changes-detected":
       return "⚠️ Changes detected";
     case "error":
       return "❌ An error happened";
@@ -23,7 +23,7 @@ function getBuildStatusLabel(status: BuildAggregatedStatus): string {
       return "🚜 Diffing screenshots";
     case "rejected":
       return "👎 Changes rejected";
-    case "stable":
+    case "no-changes":
       return "✅ No changes detected";
     default:
       assertNever(status);

apps/backend/src/database/models/Build.e2e.test.ts

@@ -233,35 +233,35 @@ describe("models/Build", () => {
       expect(conclusions).toEqual([null, null, null, null]);
     });
 
-    it("should return 'stable' when empty", async () => {
+    it("should return 'no-changes' when empty", async () => {
       const build = await factory.Build.create({
         jobStatus: "complete",
       });
       const statuses = await Build.getStatuses([build]);
       const conclusions = await Build.getConclusions([build.id], statuses);
-      expect(conclusions).toEqual(["stable"]);
+      expect(conclusions).toEqual(["no-changes"]);
     });
 
-    it("should return 'stable' when no diff detected", async () => {
+    it("should return 'no-changes' when no diff detected", async () => {
       const build = await factory.Build.create();
       await factory.ScreenshotDiff.createMany(2, [
         { buildId: build.id },
         { buildId: build.id },
       ]);
       const statuses = await Build.getStatuses([build]);
       const conclusions = await Build.getConclusions([build.id], statuses);
-      expect(conclusions).toEqual(["stable"]);
+      expect(conclusions).toEqual(["no-changes"]);
     });
 
-    it("should return 'diff-detected' when diff are detected", async () => {
+    it("should return 'changes-detected' when diff are detected", async () => {
       const build = await factory.Build.create();
       await factory.ScreenshotDiff.createMany(2, [
         { buildId: build.id },
         { buildId: build.id, score: 0.8 },
       ]);
       const statuses = await Build.getStatuses([build]);
       const conclusions = await Build.getConclusions([build.id], statuses);
-      expect(conclusions).toEqual(["diffDetected"]);
+      expect(conclusions).toEqual(["changes-detected"]);
     });
   });