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 all 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
8 changes: 6 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ This repo contains a [Probot](https://github.com/probot/probot) application. The

## Plugins

The plugins are listed in the [src/plugins](./src/plugins) folder directory.
The plugins are listed in the [src/plugins](./src/plugins) folder.

- **Label Checker** - Sets a status on PRs that passes if one (and only one) of the following labels from each list have been applied:
- `bug`, `doc`, `feature request`, or `improvement`
- `breaking` or `non-breaking`
- **Release Drafter** - Opens up a draft release on GitHub anytime a PR is merged to a versioned branch (i.e. `branch-0.17`, `branch-0.18`, etc.). The draft body includes a categorized changelog consisting of the PRs that have been merged on that branch.
- **Auto Merger** - Automatically merges PRs that include the `@gpucibot merge` comment and meet the merge criteria outlined in [kb/42](https://github.com/rapidsai/kb/issues/42).
- **Auto Merger** - Automatically merges PRs that include the `@gpucibot merge` comment and meet the merge criteria outlined in [https://docs.rapids.ai/resources/auto-merger/](https://docs.rapids.ai/resources/auto-merger/).
- **Branch Checker** - Set a status on PRs that checks whether they are targeting either the repo's _default branch_ or _default branch + 1_

## Deployment
Expand All @@ -29,3 +29,7 @@ npm run test
# Deploy
npm run deploy
```

## Contributing

Any new functionality should be introduced as a new plugin in the [src/plugins](./src/plugins) directory. New plugins should make use of the shared `exitIfFeatureIsDisabled` function so that repositories can disable the feature if they desire. New plugins should also have an entry added in [config.ts](./src/config.ts)
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
Loading