Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Config File Parsing #90

Merged
merged 7 commits into from
Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* Format of the .github/ops-bot.yaml file that can be
* set in each repository
*/
export type OpsBotConfig = {
auto_merger: boolean;
branch_checker: boolean;
label_checker: boolean;
release_drafter: boolean;
external_contributors: boolean;
};

/**
* Default configuration options if no config is present in repository
*/
export const DefaultOpsBotConfig: OpsBotConfig = {
auto_merger: false,
branch_checker: false,
label_checker: false,
release_drafter: false,
external_contributors: true,
jjacobelli marked this conversation as resolved.
Show resolved Hide resolved
};

/**
* Configuration file path in repositories
*/
export const OpsBotConfigPath = ".github/ops-bot.yaml";
2 changes: 2 additions & 0 deletions src/plugins/AutoMerger/auto_merger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
UsersGetByUsernameResponseData,
} from "../../types";
import strip from "strip-comments";
import { exitIfFeatureIsDisabled } from "../../shared";

const MERGE_COMMENT = "@gpucibot merge";

Expand All @@ -18,6 +19,7 @@ export class AutoMerger {

async maybeMergePR(): Promise<any> {
const context = this.context;
await exitIfFeatureIsDisabled(context, "auto_merger");
const { repository: repo } = context.payload;
let prNumbers: number[] = []; // will usually only contain 1 number, except in rare instances w/ status contexts

Expand Down
2 changes: 2 additions & 0 deletions src/plugins/BranchChecker/pull_request.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { exitIfFeatureIsDisabled } from "../../shared";
import { PRContext } from "../../types";
import { checkPR } from "./check_pr";

Expand All @@ -10,6 +11,7 @@ export class PRBranchChecker {

async checkPR() {
const { context } = this;
await exitIfFeatureIsDisabled(context, "branch_checker");
await checkPR(context.octokit, context.payload.pull_request);
}
}
2 changes: 2 additions & 0 deletions src/plugins/BranchChecker/repository.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { exitIfFeatureIsDisabled } from "../../shared";
import { RepositoryContext } from "../../types";
import { checkPR } from "./check_pr";

Expand All @@ -10,6 +11,7 @@ export class RepositoryBranchChecker {

async checkAllPRs() {
const { context } = this;
await exitIfFeatureIsDisabled(context, "branch_checker");
const repo = context.payload.repository;

const prs = await context.octokit.paginate(context.octokit.pulls.list, {
Expand Down
12 changes: 9 additions & 3 deletions src/plugins/LabelChecker/label_checker.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { createSetCommitStatus, isReleasePR } from "../../shared";
import {
createSetCommitStatus,
exitIfFeatureIsDisabled,
isReleasePR,
} from "../../shared";
import { PRContext } from "../../types";

export class LabelChecker {
Expand All @@ -11,6 +15,8 @@ export class LabelChecker {
async checkLabels(): Promise<any> {
const context = this.context;

await exitIfFeatureIsDisabled(context, "label_checker");

const setCommitStatus = createSetCommitStatus(context.octokit, {
context: "Label Checker",
owner: context.payload.repository.owner.login,
Expand Down Expand Up @@ -47,7 +53,7 @@ export class LabelChecker {

const categoryLabels = ["bug", "doc", "feature request", "improvement"];
const breakingLabels = ["breaking", "non-breaking"];
const doNotMergeLabelText = 'do not merge';
const doNotMergeLabelText = "do not merge";
const labelsOnPR = context.payload.pull_request.labels;

let categoryLabelCount = 0;
Expand All @@ -58,7 +64,7 @@ export class LabelChecker {

if (label.name.trim().toLowerCase().includes(doNotMergeLabelText)) {
return await setCommitStatus(
"Contains a \`DO NOT MERGE\` label",
"Contains a `DO NOT MERGE` label",
"failure"
);
}
Expand Down
7 changes: 6 additions & 1 deletion src/plugins/ReleaseDrafter/release_drafter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import { basename } from "path";
import { resolve } from "path";
import { readFileSync } from "fs";
import nunjucks from "nunjucks";
import { getVersionFromBranch, isVersionedBranch } from "../../shared";
import {
exitIfFeatureIsDisabled,
getVersionFromBranch,
isVersionedBranch,
} from "../../shared";

export class ReleaseDrafter {
context: PushContext;
Expand All @@ -32,6 +36,7 @@ export class ReleaseDrafter {

async draftRelease(): Promise<any> {
const { context, branchName, repo } = this;
await exitIfFeatureIsDisabled(context, "release_drafter");
const { created, deleted } = context.payload;

// Don't run on branch created/delete pushes
Expand Down
25 changes: 25 additions & 0 deletions src/shared.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Context } from "probot";
import { DefaultOpsBotConfig, OpsBotConfig, OpsBotConfigPath } from "./config";
import { CommitStatus, ProbotOctokit, PullsGetResponseData } from "./types";

/**
Expand Down Expand Up @@ -59,3 +61,26 @@ export const isReleasePR = (
pullRequest.title.toLowerCase().includes("[release]")
);
};

/**
*
* Exits the NodeJS process if a specified feature is not enabled.
* The configuration file is fetched from the repository's default branch.
*/
export const exitIfFeatureIsDisabled = async (
context: Context,
feature: keyof OpsBotConfig
): Promise<any> => {
const repoParams = context.repo();
const { config } = await context.octokit.config.get({
...repoParams,
path: OpsBotConfigPath,
defaults: DefaultOpsBotConfig,
});

console.log(`${repoParams.repo} config: `, JSON.stringify(config, null, 2));
if (config[feature]) return;

console.warn(`${feature} is not enabled on ${repoParams.repo}. Exiting...`);
process.exit(0);
};
15 changes: 15 additions & 0 deletions test/auto_merger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import { default as user_permission } from "./fixtures/responses/get_collaborato
import { default as commitPRs } from "./fixtures/responses/list_pull_requests_associated_with_commit.json";
import { user, userNoName } from "./fixtures/responses/get_by_username";
import {
mockConfigGet,
mockContextRepo,
mockExit,
mockGetByUsername,
mockGetUserPermissionLevel,
mockListComments,
Expand All @@ -19,6 +22,8 @@ import {
mockPaginate,
mockPullsGet,
} from "./mocks";
import { default as repoResp } from "./fixtures/responses/context_repo.json";
import { makeConfigReponse } from "./fixtures/responses/get_config";

describe("Auto Merger", () => {
beforeEach(() => {
Expand All @@ -32,6 +37,16 @@ describe("Auto Merger", () => {
mockPullsGet.mockReset();
});

beforeAll(() => {
mockContextRepo.mockReturnValue(repoResp);
mockExit.mockReset();
mockConfigGet.mockResolvedValue(makeConfigReponse({ auto_merger: true }));
});

afterAll(() => {
expect(mockExit).toBeCalledTimes(0);
});

test("status context", async () => {
mockListPullRequestsFromCommit.mockResolvedValueOnce(commitPRs);
mockPullsGet.mockResolvedValueOnce(pulls_get);
Expand Down
26 changes: 23 additions & 3 deletions test/branch_checker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,19 @@ import { PRBranchChecker } from "../src/plugins/BranchChecker/pull_request";
import { RepositoryBranchChecker } from "../src/plugins/BranchChecker/repository";
import { makePRContext } from "./fixtures/contexts/pull_request";
import { makeRepositoryContext } from "./fixtures/contexts/repository";
import { mockCreateCommitStatus, mockListPulls, mockPaginate } from "./mocks";
import {
mockConfigGet,
mockContextRepo,
mockCreateCommitStatus,
mockExit,
mockListPulls,
mockPaginate,
} from "./mocks";
import { branch_checker as listPullsResp } from "./fixtures/responses/list_pulls.json";
import { default as releasesJson } from "./fixtures/responses/releases.json";
import axios from "axios";
import { default as repoResp } from "./fixtures/responses/context_repo.json";
import { makeConfigReponse } from "./fixtures/responses/get_config";

jest.mock("axios");
const mockedAxios = axios as jest.Mocked<typeof axios>;
Expand All @@ -14,9 +23,20 @@ describe("Label Checker", () => {
describe("Pull Request Event", () => {
beforeEach(() => {
mockCreateCommitStatus.mockReset();
});

beforeAll(() => {
mockContextRepo.mockReturnValue(repoResp);
const resp = { data: releasesJson };
mockedAxios.get.mockResolvedValue(resp);
mockExit.mockReset();
mockConfigGet.mockResolvedValue(
makeConfigReponse({ branch_checker: true })
);
});

afterAll(() => {
expect(mockExit).toBeCalledTimes(0);
});

test("release PR", async () => {
Expand Down Expand Up @@ -87,7 +107,7 @@ describe("Label Checker", () => {
"Base branch is under active development"
);
});

test("next development branch", async () => {
const context = makePRContext({
baseRef: "branch-21.08",
Expand Down Expand Up @@ -115,7 +135,7 @@ describe("Label Checker", () => {
"Base branch is not under active development"
);
});
});
});

describe("Repository Event", () => {
beforeEach(() => {
Expand Down
25 changes: 25 additions & 0 deletions test/exit_if_feature_disabled.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { LabelChecker } from "../src/plugins/LabelChecker/label_checker";
import { makePRContext } from "./fixtures/contexts/pull_request";
import { mockConfigGet, mockContextRepo, mockExit } from "./mocks";
import { default as repoResp } from "./fixtures/responses/context_repo.json";
import { makeConfigReponse } from "./fixtures/responses/get_config";

const context = makePRContext({ labels: [] });
mockContextRepo.mockReturnValue(repoResp);

describe("Config Checker", () => {
beforeEach(() => {
mockExit.mockReset();
});

test.each([
{ enabled: true, mockExitCalls: 0 },
{ enabled: false, mockExitCalls: 1 },
])("label_checker: $enabled", async ({ enabled, mockExitCalls }) => {
mockConfigGet.mockResolvedValueOnce(
makeConfigReponse({ label_checker: enabled })
);
await new LabelChecker(context).checkLabels();
expect(mockExit).toBeCalledTimes(mockExitCalls);
});
});
10 changes: 8 additions & 2 deletions test/fixtures/contexts/base.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import {
mockConfigGet,
mockContextRepo,
mockCreateCommitStatus,
mockCreateRelease,
mockGetByUsername,
Expand All @@ -21,6 +23,7 @@ export const makeContext = (payload, name: EmitterWebhookEventName) => {
return {
name,
payload,
repo: mockContextRepo,
octokit: {
issues: {
listComments: mockListComments,
Expand All @@ -43,10 +46,13 @@ export const makeContext = (payload, name: EmitterWebhookEventName) => {
users: {
getByUsername: mockGetByUsername,
},
orgs : {
checkMembershipForUser: mockOrgMembership
orgs: {
checkMembershipForUser: mockOrgMembership,
},
paginate: mockPaginate,
config: {
get: mockConfigGet,
},
},
};
};
4 changes: 4 additions & 0 deletions test/fixtures/responses/context_repo.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"owner": "rapidsai",
"repo": "cudf"
}
7 changes: 7 additions & 0 deletions test/fixtures/responses/get_config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { OpsBotConfig } from "../../../src/config";

export const makeConfigReponse = <E extends Partial<OpsBotConfig>>(
opsBotConfig: E
): { config: E } => {
return { config: opsBotConfig };
};
21 changes: 19 additions & 2 deletions test/label_checker.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,29 @@
import { LabelChecker } from "../src/plugins/LabelChecker/label_checker";
import { makePRContext } from "./fixtures/contexts/pull_request";
import { mockCreateCommitStatus } from "./mocks";
import { makeConfigReponse } from "./fixtures/responses/get_config";
import {
mockConfigGet,
mockContextRepo,
mockCreateCommitStatus,
mockExit,
} from "./mocks";
import { default as repoResp } from "./fixtures/responses/context_repo.json";

describe("Label Checker", () => {
beforeEach(() => {
mockCreateCommitStatus.mockReset();
});

beforeAll(() => {
mockContextRepo.mockReturnValue(repoResp);
mockExit.mockReset();
mockConfigGet.mockResolvedValue(makeConfigReponse({ label_checker: true }));
});

afterAll(() => {
expect(mockExit).toBeCalledTimes(0);
});

test("no labels", async () => {
const context = makePRContext({ labels: [] });
await new LabelChecker(context).checkLabels();
Expand Down Expand Up @@ -127,7 +144,7 @@ describe("Label Checker", () => {
);
expect(mockCreateCommitStatus.mock.calls[1][0].state).toBe("failure");
expect(mockCreateCommitStatus.mock.calls[1][0].description).toBe(
"Contains a \`DO NOT MERGE\` label"
"Contains a `DO NOT MERGE` label"
);
expect(mockCreateCommitStatus.mock.calls[1][0].target_url).toBe(
"https://docs.rapids.ai/resources/label-checker/"
Expand Down
23 changes: 14 additions & 9 deletions test/mocks.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
export const mockConfigGet = jest.fn();
export const mockContextRepo = jest.fn();
export const mockCreateCommitStatus = jest.fn();
export const mockCreateRelease = jest.fn();
export const mockListCommits = jest.fn();
export const mockListPullRequestsFromCommit = jest.fn();
export const mockExit = jest
.spyOn(process, "exit")
.mockImplementation(() => null as never);
export const mockGetByUsername = jest.fn();
export const mockGetReleaseByTag = jest.fn();
export const mockUpdateRelease = jest.fn();
export const mockPullsGet = jest.fn();
export const mockPaginate = jest.fn();
export const mockListComments = jest.fn();
export const mockGetUserPermissionLevel = jest.fn();
export const mockGetByUsername = jest.fn();
export const mockListComments = jest.fn();
export const mockListCommits = jest.fn();
export const mockListPullRequestsFromCommit = jest.fn();
export const mockListPulls = jest.fn();
export const mockListReviews = jest.fn();
export const mockMerge = jest.fn();
export const mockListPulls = jest.fn();
export const mockUpdateRef = jest.fn();
export const mockOrgMembership = jest.fn();
export const mockPaginate = jest.fn();
export const mockPullsGet = jest.fn();
export const mockUpdateRef = jest.fn();
export const mockUpdateRelease = jest.fn();
Loading