Skip to content

Commit

Permalink
feat: support build metadata
Browse files Browse the repository at this point in the history
In order to avoid using a build if some tests have failed.
  • Loading branch information
gregberge committed Sep 1, 2024
1 parent d6faaea commit d167ba5
Show file tree
Hide file tree
Showing 24 changed files with 548 additions and 57 deletions.
55 changes: 55 additions & 0 deletions apps/backend/db/migrations/20240901150444_build-metadata.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* @param {import('knex').Knex} knex
*/
export const up = async (knex) => {
await knex.schema.alterTable("builds", async (table) => {
table.jsonb("metadata");
});

await knex.schema.alterTable("build_shards", async (table) => {
table.jsonb("metadata");
});

await knex.schema.alterTable("screenshot_buckets", (table) => {
table.boolean("valid");
});

await knex.raw(
`UPDATE screenshot_buckets SET valid = true WHERE valid IS NULL`,
);

await knex.raw(`
ALTER TABLE screenshot_buckets ADD CONSTRAINT screenshot_buckets_valid_not_null_constraint CHECK (valid IS NOT NULL) NOT VALID;
`);

await knex.raw(`
ALTER TABLE screenshot_buckets VALIDATE CONSTRAINT screenshot_buckets_valid_not_null_constraint;
`);

await knex.raw(`
ALTER TABLE screenshot_buckets ALTER column valid SET NOT NULL;
`);

await knex.raw(`
ALTER TABLE screenshot_buckets DROP CONSTRAINT screenshot_buckets_valid_not_null_constraint;
`);
};

/**
* @param {import('knex').Knex} knex
*/
export const down = async (knex) => {
await knex.schema.alterTable("builds", async (table) => {
table.dropColumn("metadata");
});

await knex.schema.alterTable("build_shards", async (table) => {
table.dropColumn("metadata");
});

await knex.schema.alterTable("screenshot_buckets", async (table) => {
table.dropColumn("valid");
});
};

export const config = { transaction: false };
1 change: 1 addition & 0 deletions apps/backend/db/seeds/seeds.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export const seed = async (knex) => {
createdAt: "2016-12-08T22:59:55Z",
updatedAt: "2016-12-08T22:59:55Z",
complete: true,
valid: true,
screenshotCount: 0,
};

Expand Down
8 changes: 6 additions & 2 deletions apps/backend/db/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ CREATE TABLE public.build_shards (
"createdAt" timestamp with time zone NOT NULL,
"updatedAt" timestamp with time zone NOT NULL,
"buildId" bigint NOT NULL,
index integer
index integer,
metadata jsonb
);


Expand Down Expand Up @@ -222,6 +223,7 @@ CREATE TABLE public.builds (
"runId" character varying(255),
"runAttempt" integer,
partial boolean DEFAULT false NOT NULL,
metadata jsonb,
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])))
);
Expand Down Expand Up @@ -933,6 +935,7 @@ CREATE TABLE public.screenshot_buckets (
"projectId" bigint NOT NULL,
"screenshotCount" integer,
mode text DEFAULT 'ci'::text NOT NULL,
valid boolean NOT NULL,
CONSTRAINT chk_complete_true_screenshotcount_not_null CHECK (((complete = false) OR ("screenshotCount" IS NOT NULL))),
CONSTRAINT screenshot_buckets_mode_check CHECK ((mode = ANY (ARRAY['ci'::text, 'monitoring'::text])))
);
Expand Down Expand Up @@ -2828,4 +2831,5 @@ INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('2024061
INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20240616142430_build_shards_indices.js', 1, NOW());
INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20240630151704_screenshot-threshold.js', 1, NOW());
INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20240706121810_screenshot-base-name.js', 1, NOW());
INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20240822082247_github-light.js', 1, NOW());
INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20240822082247_github-light.js', 1, NOW());
INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20240901150444_build-metadata.js', 1, NOW());
31 changes: 22 additions & 9 deletions apps/backend/src/api/handlers/updateBuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@ import { invariant } from "@argos/util/invariant";
import express from "express";
import { ZodOpenApiOperationObject } from "zod-openapi";

import { finalizeBuild } from "@/build/finalizeBuild.js";
import {
checkIsBucketValidFromMetadata,
finalizeBuild,
} from "@/build/finalizeBuild.js";
import { job as buildJob } from "@/build/index.js";
import { raw, transaction } from "@/database/index.js";
import { Build, BuildShard, Project } from "@/database/models/index.js";
import { BuildMetadataSchema } from "@/database/schemas/BuildMetadata.js";
import { insertFilesAndScreenshots } from "@/database/services/screenshots.js";
import { getRedisLock } from "@/util/redis";
import { repoAuth } from "@/web/middlewares/repoAuth.js";
Expand All @@ -30,6 +34,7 @@ const RequestBodySchema = z
parallel: z.boolean().nullable().optional(),
parallelTotal: z.number().int().min(-1).nullable().optional(),
parallelIndex: z.number().int().min(1).nullable().optional(),
metadata: BuildMetadataSchema.optional(),
})
.strict();

Expand Down Expand Up @@ -154,6 +159,7 @@ async function handleUpdateParallel(ctx: Context) {
? BuildShard.query(trx).insert({
buildId: build.id,
index: body.parallelIndex,
metadata: body.metadata ?? null,
})
: null,
Build.query(trx)
Expand Down Expand Up @@ -198,15 +204,22 @@ async function handleUpdateParallel(ctx: Context) {
async function handleUpdateSingle(ctx: Context) {
const { body, build } = ctx;
await transaction(async (trx) => {
const screenshotCount = await insertFilesAndScreenshots({
screenshots: body.screenshots,
build,
trx,
const [screenshotCount] = await Promise.all([
insertFilesAndScreenshots({
screenshots: body.screenshots,
build,
trx,
}),
ctx.body.metadata
? build.$clone().$query(trx).patch({ metadata: ctx.body.metadata })
: null,
]);

await build.compareScreenshotBucket!.$query(trx).patchAndFetch({
complete: true,
valid: checkIsBucketValidFromMetadata(build.metadata),
screenshotCount,
});

await build
.compareScreenshotBucket!.$query(trx)
.patchAndFetch({ complete: true, screenshotCount });
});
await buildJob.push(build.id);
}
2 changes: 1 addition & 1 deletion apps/backend/src/api/schema/primitives/screenshot.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ScreenshotMetadataSchema } from "@/database/models/ScreenshotMetadata.js";
import { ScreenshotMetadataSchema } from "@/database/schemas/index.js";
import { SHA256_REGEX } from "@/web/constants.js";

import { z } from "../util/zod.js";
Expand Down
23 changes: 12 additions & 11 deletions apps/backend/src/build/createBuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,17 @@ export async function createBuild(params: {
project: Project;
commit: string;
branch: string;
buildName?: string | null;
parallel?: { nonce: string } | null;
prNumber?: number | null;
prHeadCommit?: string | null;
referenceCommit?: string | null;
referenceBranch?: string | null;
mode?: BuildMode | null;
ciProvider?: string | null;
argosSdk?: string | null;
runId?: string | null;
runAttempt?: number | null;
buildName: string | null;
parallel: { nonce: string } | null;
prNumber: number | null;
prHeadCommit: string | null;
referenceCommit: string | null;
referenceBranch: string | null;
mode: BuildMode | null;
ciProvider: string | null;
argosSdk: string | null;
runId: string | null;
runAttempt: number | null;
}) {
const account = await params.project.$relatedQuery("account");
invariant(account, "Account should be fetched");
Expand Down Expand Up @@ -135,6 +135,7 @@ export async function createBuild(params: {
branch: params.branch,
projectId: params.project.id,
complete: false,
valid: false,
mode,
});

Expand Down
118 changes: 111 additions & 7 deletions apps/backend/src/build/finalizeBuild.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,122 @@
import { TransactionOrKnex } from "objection";

import { Build, Screenshot } from "@/database/models/index.js";
import { Build, BuildShard, Screenshot } from "@/database/models/index.js";
import { BuildMetadata } from "@/database/schemas";

/**
* Check if the bucket is valid from the metadata.
*/
export function checkIsBucketValidFromMetadata(metadata: BuildMetadata | null) {
return !metadata?.testReport || metadata.testReport.status === "passed";
}

function add(a: number | undefined, b: number | undefined) {
if (!a) {
return b;
}
if (!b) {
return a;
}
return a + b;
}

/**
* Aggregate metadata from multiple shards.
*/
function aggregateMetadata(allMetatada: (BuildMetadata | null)[]) {
return allMetatada.reduce<BuildMetadata | null>((acc, item) => {
if (!item?.testReport) {
return acc;
}

if (!acc) {
return item;
}

const status = (() => {
if (acc.testReport?.status === "failed") {
return "failed";
}
if (item.testReport.status === "passed") {
return "passed";
}
return "failed";
})();

const startTime = (() => {
if (!acc.testReport?.stats?.startTime) {
return item.testReport?.stats?.startTime;
}
if (!item.testReport?.stats?.startTime) {
return acc.testReport.stats.startTime;
}
return item.testReport.stats.startTime < acc.testReport.stats.startTime
? item.testReport.stats.startTime
: acc.testReport.stats.startTime;
})();

return {
testReport: {
status,
stats: {
startTime,
duration: add(
acc.testReport?.stats?.duration,
item.testReport?.stats?.duration,
),
tests: add(
acc.testReport?.stats?.tests,
item.testReport?.stats?.tests,
),
expected: add(
acc.testReport?.stats?.expected,
item.testReport?.stats?.expected,
),
unexpected: add(
acc.testReport?.stats?.unexpected,
item.testReport?.stats?.unexpected,
),
},
},
};
}, null);
}

/**
* Finalize a build.
* - Count the number of screenshots in the compare bucket.
* - Check if the build is considered valid.
* - Update the compare bucket with the screenshot count and validity.
*/
export async function finalizeBuild(input: {
trx?: TransactionOrKnex;
build: Build;
screenshotCount?: number;
}) {
const screenshotCount = await Screenshot.query(input.trx)
.where("screenshotBucketId", input.build.compareScreenshotBucketId)
.resultSize();
await input.build
.$relatedQuery("compareScreenshotBucket", input.trx)
.patch({ complete: true, screenshotCount });
const { trx, build } = input;
const [screenshotCount, shards] = await Promise.all([
Screenshot.query(trx)
.where("screenshotBucketId", build.compareScreenshotBucketId)
.resultSize(),
BuildShard.query(trx).select("metadata").where("buildId", build.id),
]);

const valid =
shards.length > 0
? shards.every((shard) => checkIsBucketValidFromMetadata(shard.metadata))
: checkIsBucketValidFromMetadata(build.metadata);

const metadata =
shards.length > 0
? aggregateMetadata(shards.map((shard) => shard.metadata))
: build.metadata;

await Promise.all([
metadata !== build.metadata
? build.$clone().$query(trx).patch({ metadata })
: null,
build
.$relatedQuery("compareScreenshotBucket", trx)
.patch({ complete: true, screenshotCount, valid }),
]);
}
1 change: 1 addition & 0 deletions apps/backend/src/build/strategy/strategies/ci/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function queryBaseBucket(build: Build) {
projectId: build.projectId,
name: build.name,
complete: true,
valid: true,
mode: build.mode,
});
}
8 changes: 8 additions & 0 deletions apps/backend/src/database/models/Build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import { z } from "zod";

import config from "@/config/index.js";

import {
BuildMetadata,
BuildMetadataJsonSchema,
} from "../schemas/BuildMetadata.js";
import { Model } from "../util/model.js";
import {
jobModelSchema,
Expand Down Expand Up @@ -91,6 +95,9 @@ export class Build extends Model {
runId: { type: ["string", "null"] },
runAttempt: { type: ["integer", "null"] },
partial: { type: ["boolean", "null"] },
metadata: {
oneOf: [BuildMetadataJsonSchema, { type: "null" }],
},
},
});

Expand All @@ -115,6 +122,7 @@ export class Build extends Model {
runId!: string | null;
runAttempt!: number | null;
partial!: boolean | null;
metadata!: BuildMetadata | null;

static override get relationMappings(): RelationMappings {
return {
Expand Down
8 changes: 8 additions & 0 deletions apps/backend/src/database/models/BuildShard.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import type { RelationMappings } from "objection";

import {
BuildMetadata,
BuildMetadataJsonSchema,
} from "../schemas/BuildMetadata.js";
import { Model } from "../util/model.js";
import { mergeSchemas, timestampsSchema } from "../util/schemas.js";
import { Build } from "./Build.js";
Expand All @@ -13,11 +17,15 @@ export class BuildShard extends Model {
properties: {
buildId: { type: "string" },
index: { type: ["integer", "null"] },
metadata: {
oneOf: [BuildMetadataJsonSchema, { type: "null" }],
},
},
});

buildId!: string;
index!: number | null;
metadata!: BuildMetadata | null;

static override get relationMappings(): RelationMappings {
return {
Expand Down
Loading

0 comments on commit d167ba5

Please sign in to comment.