Skip to content

Commit

Permalink
Merge pull request #1389 from argos-ci/multi-branches
Browse files Browse the repository at this point in the history
feat: multi-branches support
  • Loading branch information
gregberge authored Oct 7, 2024
2 parents 00ddf4a + 41f9d76 commit 4e3724b
Show file tree
Hide file tree
Showing 58 changed files with 1,073 additions and 937 deletions.
31 changes: 31 additions & 0 deletions apps/backend/db/migrations/20241006153157_reference-branch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* @param {import('knex').Knex} knex
*/
export const up = async (knex) => {
await knex.schema.alterTable("projects", async (table) => {
table.renameColumn("baselineBranch", "defaultBaseBranch");
table.string("autoApprovedBranchGlob");
});

await knex.schema.alterTable("builds", async (table) => {
table.renameColumn("referenceBranch", "baseBranch");
table.renameColumn("referenceCommit", "baseCommit");
table.enum("baseBranchResolvedFrom", ["sdk", "pull-request", "project"]);
});
};

/**
* @param {import('knex').Knex} knex
*/
export const down = async (knex) => {
await knex.schema.alterTable("projects", async (table) => {
table.renameColumn("defaultBaseBranch", "baselineBranch");
table.dropColumn("autoApprovedBranchGlob");
});

await knex.schema.alterTable("builds", async (table) => {
table.renameColumn("baseBranch", "referenceBranch");
table.renameColumn("baseCommit", "referenceCommit");
table.dropColumn("baseBranchResolvedFrom");
});
};
2 changes: 1 addition & 1 deletion apps/backend/db/seeds/seeds.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export const seed = async (knex) => {
name: "awesome",
token: "awesome-xxx",
accountId: helloAccount.id,
baselineBranch: "main",
defaultBaseBranch: "main",
},
{
...timeStamps,
Expand Down
12 changes: 8 additions & 4 deletions apps/backend/db/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ CREATE TABLE public.builds (
"totalBatch" integer,
"prNumber" integer,
"projectId" bigint NOT NULL,
"referenceCommit" character varying(255),
"referenceBranch" character varying(255),
"baseCommit" character varying(255),
"baseBranch" character varying(255),
"githubPullRequestId" bigint,
"prHeadCommit" character varying(255),
mode text DEFAULT 'ci'::text NOT NULL,
Expand All @@ -224,6 +224,8 @@ CREATE TABLE public.builds (
"runAttempt" integer,
partial boolean DEFAULT false NOT NULL,
metadata jsonb,
"baseBranchResolvedFrom" text,
CONSTRAINT "builds_baseBranchResolvedFrom_check" CHECK (("baseBranchResolvedFrom" = ANY (ARRAY['sdk'::text, 'pull-request'::text, 'project'::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])))
);
Expand Down Expand Up @@ -887,12 +889,13 @@ CREATE TABLE public.projects (
name character varying(255) NOT NULL,
token character varying(255) NOT NULL,
private boolean,
"baselineBranch" character varying(255),
"defaultBaseBranch" character varying(255),
"accountId" bigint NOT NULL,
"githubRepositoryId" bigint,
"prCommentEnabled" boolean DEFAULT true NOT NULL,
"gitlabProjectId" bigint,
"summaryCheck" text DEFAULT 'auto'::text NOT NULL,
"autoApprovedBranchGlob" character varying(255),
CONSTRAINT "projects_summaryCheck_check" CHECK (("summaryCheck" = ANY (ARRAY['always'::text, 'auto'::text, 'never'::text])))
);

Expand Down Expand Up @@ -2832,4 +2835,5 @@ INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('2024061
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 ('20240901150444_build-metadata.js', 1, NOW());
INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20240901150444_build-metadata.js', 1, NOW());
INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241006153157_reference-branch.js', 1, NOW());
1 change: 1 addition & 0 deletions apps/backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"jsonwebtoken": "^9.0.2",
"knex": "^3.1.0",
"lodash-es": "^4.17.21",
"minimatch": "^10.0.1",
"mime": "^4.0.4",
"moment": "^2.30.1",
"morgan": "^1.10.0",
Expand Down
6 changes: 4 additions & 2 deletions apps/backend/src/api/handlers/createBuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ const RequestBodySchema = z
parallelNonce: z.string().nullable().optional(),
prNumber: z.number().int().min(1).nullable().optional(),
prHeadCommit: z.string().nullable().optional(),
// To avoid breaking change, we keep referenceCommit instead of baseCommit
referenceCommit: z.string().nullable().optional(),
// To avoid breaking change, we keep referenceBranch instead of baseBranch
referenceBranch: z.string().nullable().optional(),
mode: z.enum(["ci", "monitoring"]).nullable().optional(),
ciProvider: z.string().nullable().optional(),
Expand Down Expand Up @@ -256,8 +258,8 @@ async function createBuildFromRequest(ctx: BuildContext) {
: null,
prNumber: body.prNumber ?? null,
prHeadCommit: body.prHeadCommit ?? null,
referenceCommit: body.referenceCommit ?? null,
referenceBranch: body.referenceBranch ?? null,
baseCommit: body.referenceCommit ?? null,
baseBranch: body.referenceBranch ?? null,
runId: body.runId ?? null,
runAttempt: body.runAttempt ?? null,
ciProvider: body.ciProvider ?? null,
Expand Down
4 changes: 2 additions & 2 deletions apps/backend/src/api/handlers/getAuthProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const getAuthProject: CreateAPIHandler = ({ get }) => {
"[githubRepository.repoInstallations.installation,gitlabProject]",
);

const referenceBranch = await req.authProject.$getReferenceBranch();
const defaultBaseBranch = await req.authProject.$getDefaultBaseBranch();

const installation = req.authProject.githubRepository
? GithubRepository.pickBestInstallation(req.authProject.githubRepository)
Expand All @@ -45,7 +45,7 @@ export const getAuthProject: CreateAPIHandler = ({ get }) => {

res.send({
id: req.authProject.id,
defaultBaseBranch: referenceBranch,
defaultBaseBranch,
hasRemoteContentAccess,
});
});
Expand Down
8 changes: 4 additions & 4 deletions apps/backend/src/build-notification/aggregated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {

export async function getAggregatedNotification(
commit: string,
isReference: boolean,
isAutoApproved: boolean,
summaryCheckConfig: Project["summaryCheck"],
): Promise<NotificationPayload | null> {
if (summaryCheckConfig === "never") {
Expand Down Expand Up @@ -74,7 +74,7 @@ export async function getAggregatedNotification(

const states = getNotificationStates({
buildNotificationType: type,
isReference,
isAutoApproved,
});
const base = {
context: "argos/summary",
Expand All @@ -100,7 +100,7 @@ export async function getAggregatedNotification(
case "diff-detected":
return {
...base,
description: isReference ? "Used as new baseline" : "Diff detected",
description: isAutoApproved ? "Auto-approved" : "Diff detected",
};
case "diff-accepted":
return { ...base, description: "Diff accepted" };
Expand All @@ -109,7 +109,7 @@ export async function getAggregatedNotification(
case "no-diff-detected":
return {
...base,
description: isReference ? "Used as new baseline" : "No diff detected",
description: isAutoApproved ? "Auto-approved" : "No diff detected",
};
default:
assertNever(type);
Expand Down
32 changes: 16 additions & 16 deletions apps/backend/src/build-notification/notification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,16 @@ async function getStatusContext(build: Build): Promise<string> {

/**
* Get the notification status for each platform based on the build
* notification type and if it's a reference build.
* notification type and if it's an auto-approved build.
*/
export function getNotificationStates(input: {
buildNotificationType: BuildNotification["type"];
isReference: boolean;
isAutoApproved: boolean;
}): {
github: NotificationPayload["github"]["state"];
gitlab: NotificationPayload["gitlab"]["state"];
} {
const { buildNotificationType, isReference } = input;
const { buildNotificationType, isAutoApproved } = input;
switch (buildNotificationType) {
case "queued": {
return {
Expand All @@ -97,8 +97,8 @@ export function getNotificationStates(input: {
}
case "diff-detected": {
return {
github: isReference ? "success" : "failure",
gitlab: isReference ? "success" : "failed",
github: isAutoApproved ? "success" : "failure",
gitlab: isAutoApproved ? "success" : "failed",
};
}
case "diff-accepted": {
Expand All @@ -125,14 +125,14 @@ export function getNotificationStates(input: {

/**
* Get the notification description for each platform based on the build
* notification type and if it's a reference build.
* notification type and if it's a auto-approved build.
*/
async function getNotificationDescription(input: {
buildNotificationType: BuildNotification["type"];
buildId: string;
isReference: boolean;
isAutoApproved: boolean;
}): Promise<string> {
const { buildNotificationType, buildId, isReference } = input;
const { buildNotificationType, buildId, isAutoApproved } = input;
switch (buildNotificationType) {
case "queued":
return "Build is queued";
Expand All @@ -141,20 +141,20 @@ async function getNotificationDescription(input: {
case "no-diff-detected": {
const statsMessage = await getStatsMessage(buildId);
if (!statsMessage) {
if (isReference) {
return "Used as comparison baseline, no changes found";
if (isAutoApproved) {
return "Auto-approved, no changes found";
}
return "Everything's good!";
}
if (isReference) {
return `Used a comparison baseline, no changes found — ${statsMessage}`;
if (isAutoApproved) {
return `Auto-approved, no changes found — ${statsMessage}`;
}
return `${statsMessage} — no changes found`;
}
case "diff-detected": {
const statsMessage = await getStatsMessage(buildId);
if (isReference) {
return `${statsMessage}, automatically approved and used as comparison baseline`;
if (isAutoApproved) {
return `${statsMessage}, automatically approved`;
}
return `${statsMessage} — waiting for your decision`;
}
Expand Down Expand Up @@ -183,13 +183,13 @@ export async function getNotificationPayload(input: {
getNotificationDescription({
buildNotificationType: input.buildNotification.type,
buildId: input.build.id,
isReference: input.build.type === "reference",
isAutoApproved: input.build.type === "reference",
}),
getStatusContext(input.build),
]);
const states = getNotificationStates({
buildNotificationType: input.buildNotification.type,
isReference: input.build.type === "reference",
isAutoApproved: input.build.type === "reference",
});

return {
Expand Down
4 changes: 2 additions & 2 deletions apps/backend/src/build-notification/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ export const processBuildNotification = async (
buildNotification.build.prHeadCommit ??
buildNotification.build.compareScreenshotBucket.commit;

const isReference = buildNotification.build.type === "reference";
const isAutoApproved = buildNotification.build.type === "reference";
const summaryCheckConfig = buildNotification.build.project.summaryCheck;

const [buildUrl, projectUrl, notification, aggregatedNotification] =
Expand All @@ -250,7 +250,7 @@ export const processBuildNotification = async (
}),
getAggregatedNotification(
buildNotification.build.compareScreenshotBucket.commit,
isReference,
isAutoApproved,
summaryCheckConfig,
),
]);
Expand Down
9 changes: 5 additions & 4 deletions apps/backend/src/build/createBuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ export async function createBuild(params: {
parallel: { nonce: string } | null;
prNumber: number | null;
prHeadCommit: string | null;
referenceCommit: string | null;
referenceBranch: string | null;
baseCommit: string | null;
baseBranch: string | null;
mode: BuildMode | null;
ciProvider: string | null;
argosSdk: string | null;
Expand Down Expand Up @@ -149,8 +149,9 @@ export async function createBuild(params: {
prNumber: params.prNumber ?? null,
prHeadCommit: params.prHeadCommit ?? null,
githubPullRequestId: pullRequest?.id ? String(pullRequest?.id) : null,
referenceCommit: params.referenceCommit ?? null,
referenceBranch: params.referenceBranch ?? null,
baseCommit: params.baseCommit ?? null,
baseBranch: params.baseBranch ?? null,
baseBranchResolvedFrom: params.baseBranch ? "user" : null,
compareScreenshotBucketId: bucket.id,
mode,
ciProvider: params.ciProvider ?? null,
Expand Down
11 changes: 6 additions & 5 deletions apps/backend/src/build/createBuildDiffs.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,13 @@ describe("#createBuildDiffs", () => {
).toMatchObject(["pending"]);
});

describe("when compare branch equal reference branch", () => {
describe("when compare branch that matches auto-approved branch glob", () => {
beforeEach(async () => {
const referenceBranch = await project.$getReferenceBranch();
invariant(referenceBranch, "reference branch not found");
const autoApprovedBranchGlob =
await project.$getAutoApprovedBranchGlob();
invariant(autoApprovedBranchGlob);
await ScreenshotBucket.query().findById(compareBucket.id).patch({
branch: referenceBranch,
branch: autoApprovedBranchGlob,
});
});

Expand All @@ -259,7 +260,7 @@ describe("#createBuildDiffs", () => {
});
});

describe("with compare branch different than reference branch", () => {
describe("with compare branch different than auto-approved branch", () => {
it("should update build type to 'check'", async () => {
await createBuildDiffs(build);
const updatedBuild = await Build.query().findById(build.id);
Expand Down
27 changes: 14 additions & 13 deletions apps/backend/src/build/createBuildDiffs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,21 @@ async function getOrRetrieveBaseScreenshotBucket(input: {
return build.baseScreenshotBucket;
}

const baseScreenshotBucket = await strategy.getBaseScreenshotBucket(build);

if (baseScreenshotBucket) {
await Promise.all([
Build.query()
.findById(build.id)
.patch({ baseScreenshotBucketId: baseScreenshotBucket.id }),
baseScreenshotBucket.$fetchGraph("screenshots"),
]);

return baseScreenshotBucket;
}
const { baseBranch, baseBranchResolvedFrom, baseScreenshotBucket } =
await strategy.getBase(build);

await Promise.all([
Build.query()
.findById(build.id)
.patch({
baseBranch,
baseBranchResolvedFrom,
baseScreenshotBucketId: baseScreenshotBucket?.id ?? null,
}),
baseScreenshotBucket?.$fetchGraph("screenshots"),
]);

return null;
return baseScreenshotBucket;
}

function getJobStatus({
Expand Down
7 changes: 7 additions & 0 deletions apps/backend/src/build/job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { invariant } from "@argos/util/invariant";

import { pushBuildNotification } from "@/build-notification/index.js";
import { Build, Project, ScreenshotDiff } from "@/database/models/index.js";
import { job as githubPullRequestJob } from "@/github-pull-request/job.js";
import { formatGlProject, getGitlabClientFromAccount } from "@/gitlab/index.js";
import { createModelJob, UnretryableError } from "@/job-core/index.js";
import { job as screenshotDiffJob } from "@/screenshot-diff/index.js";
Expand Down Expand Up @@ -96,6 +97,12 @@ async function syncGitlabProject(project: Project) {
}

export async function performBuild(build: Build) {
// Ensure that the GitHub pull request has been processed
// to be sure we get the base branch name.
if (build.githubPullRequestId) {
await githubPullRequestJob.run(build.githubPullRequestId);
}

const [project] = await Promise.all([
Project.query()
.findById(build.projectId)
Expand Down
2 changes: 1 addition & 1 deletion apps/backend/src/build/label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function getBuildLabel(
case "orphan":
return "🔘 Orphan build";
case "reference":
return "✅ Reference build";
return "✅ Auto-approved build";
case "check": {
return getBuildStatusLabel(status);
}
Expand Down
Loading

0 comments on commit 4e3724b

Please sign in to comment.