diff --git a/README.md b/README.md index 9e007cd..236e5f5 100644 --- a/README.md +++ b/README.md @@ -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 @@ -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) diff --git a/src/config.ts b/src/config.ts new file mode 100644 index 0000000..7cf1f7a --- /dev/null +++ b/src/config.ts @@ -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, +}; + +/** + * Configuration file path in repositories + */ +export const OpsBotConfigPath = ".github/ops-bot.yaml"; diff --git a/src/plugins/AutoMerger/auto_merger.ts b/src/plugins/AutoMerger/auto_merger.ts index fd08558..8d7072d 100644 --- a/src/plugins/AutoMerger/auto_merger.ts +++ b/src/plugins/AutoMerger/auto_merger.ts @@ -7,6 +7,7 @@ import { UsersGetByUsernameResponseData, } from "../../types"; import strip from "strip-comments"; +import { exitIfFeatureIsDisabled } from "../../shared"; const MERGE_COMMENT = "@gpucibot merge"; @@ -18,6 +19,7 @@ export class AutoMerger { async maybeMergePR(): Promise { 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 diff --git a/src/plugins/BranchChecker/pull_request.ts b/src/plugins/BranchChecker/pull_request.ts index 0d5e9b4..c15a3da 100644 --- a/src/plugins/BranchChecker/pull_request.ts +++ b/src/plugins/BranchChecker/pull_request.ts @@ -1,3 +1,4 @@ +import { exitIfFeatureIsDisabled } from "../../shared"; import { PRContext } from "../../types"; import { checkPR } from "./check_pr"; @@ -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); } } diff --git a/src/plugins/BranchChecker/repository.ts b/src/plugins/BranchChecker/repository.ts index bd364d6..defb8ce 100644 --- a/src/plugins/BranchChecker/repository.ts +++ b/src/plugins/BranchChecker/repository.ts @@ -1,3 +1,4 @@ +import { exitIfFeatureIsDisabled } from "../../shared"; import { RepositoryContext } from "../../types"; import { checkPR } from "./check_pr"; @@ -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, { diff --git a/src/plugins/LabelChecker/label_checker.ts b/src/plugins/LabelChecker/label_checker.ts index 7ac3757..0652c3d 100644 --- a/src/plugins/LabelChecker/label_checker.ts +++ b/src/plugins/LabelChecker/label_checker.ts @@ -1,4 +1,8 @@ -import { createSetCommitStatus, isReleasePR } from "../../shared"; +import { + createSetCommitStatus, + exitIfFeatureIsDisabled, + isReleasePR, +} from "../../shared"; import { PRContext } from "../../types"; export class LabelChecker { @@ -11,6 +15,8 @@ export class LabelChecker { async checkLabels(): Promise { const context = this.context; + await exitIfFeatureIsDisabled(context, "label_checker"); + const setCommitStatus = createSetCommitStatus(context.octokit, { context: "Label Checker", owner: context.payload.repository.owner.login, @@ -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; @@ -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" ); } diff --git a/src/plugins/ReleaseDrafter/release_drafter.ts b/src/plugins/ReleaseDrafter/release_drafter.ts index c561103..eaee002 100644 --- a/src/plugins/ReleaseDrafter/release_drafter.ts +++ b/src/plugins/ReleaseDrafter/release_drafter.ts @@ -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; @@ -32,6 +36,7 @@ export class ReleaseDrafter { async draftRelease(): Promise { const { context, branchName, repo } = this; + await exitIfFeatureIsDisabled(context, "release_drafter"); const { created, deleted } = context.payload; // Don't run on branch created/delete pushes diff --git a/src/shared.ts b/src/shared.ts index 47d2cf1..d02fe42 100644 --- a/src/shared.ts +++ b/src/shared.ts @@ -1,3 +1,5 @@ +import { Context } from "probot"; +import { DefaultOpsBotConfig, OpsBotConfig, OpsBotConfigPath } from "./config"; import { CommitStatus, ProbotOctokit, PullsGetResponseData } from "./types"; /** @@ -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 => { + 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); +}; diff --git a/test/auto_merger.test.ts b/test/auto_merger.test.ts index bc82cfc..4b96007 100644 --- a/test/auto_merger.test.ts +++ b/test/auto_merger.test.ts @@ -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, @@ -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(() => { @@ -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); diff --git a/test/branch_checker.test.ts b/test/branch_checker.test.ts index 2317688..4300779 100644 --- a/test/branch_checker.test.ts +++ b/test/branch_checker.test.ts @@ -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; @@ -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 () => { @@ -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", @@ -115,7 +135,7 @@ describe("Label Checker", () => { "Base branch is not under active development" ); }); -}); + }); describe("Repository Event", () => { beforeEach(() => { diff --git a/test/exit_if_feature_disabled.test.ts b/test/exit_if_feature_disabled.test.ts new file mode 100644 index 0000000..d3cb82a --- /dev/null +++ b/test/exit_if_feature_disabled.test.ts @@ -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); + }); +}); diff --git a/test/fixtures/contexts/base.ts b/test/fixtures/contexts/base.ts index dd875c6..848ad85 100644 --- a/test/fixtures/contexts/base.ts +++ b/test/fixtures/contexts/base.ts @@ -1,4 +1,6 @@ import { + mockConfigGet, + mockContextRepo, mockCreateCommitStatus, mockCreateRelease, mockGetByUsername, @@ -21,6 +23,7 @@ export const makeContext = (payload, name: EmitterWebhookEventName) => { return { name, payload, + repo: mockContextRepo, octokit: { issues: { listComments: mockListComments, @@ -43,10 +46,13 @@ export const makeContext = (payload, name: EmitterWebhookEventName) => { users: { getByUsername: mockGetByUsername, }, - orgs : { - checkMembershipForUser: mockOrgMembership + orgs: { + checkMembershipForUser: mockOrgMembership, }, paginate: mockPaginate, + config: { + get: mockConfigGet, + }, }, }; }; diff --git a/test/fixtures/responses/context_repo.json b/test/fixtures/responses/context_repo.json new file mode 100644 index 0000000..1f4ddf3 --- /dev/null +++ b/test/fixtures/responses/context_repo.json @@ -0,0 +1,4 @@ +{ + "owner": "rapidsai", + "repo": "cudf" +} diff --git a/test/fixtures/responses/get_config.ts b/test/fixtures/responses/get_config.ts new file mode 100644 index 0000000..439d5af --- /dev/null +++ b/test/fixtures/responses/get_config.ts @@ -0,0 +1,7 @@ +import { OpsBotConfig } from "../../../src/config"; + +export const makeConfigReponse = >( + opsBotConfig: E +): { config: E } => { + return { config: opsBotConfig }; +}; diff --git a/test/label_checker.test.ts b/test/label_checker.test.ts index adf6f47..c940dfb 100644 --- a/test/label_checker.test.ts +++ b/test/label_checker.test.ts @@ -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(); @@ -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/" diff --git a/test/mocks.ts b/test/mocks.ts index 59dd7fb..3fd7dc0 100644 --- a/test/mocks.ts +++ b/test/mocks.ts @@ -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(); diff --git a/test/release_drafter.test.ts b/test/release_drafter.test.ts index 44eee1b..da34b23 100644 --- a/test/release_drafter.test.ts +++ b/test/release_drafter.test.ts @@ -8,7 +8,12 @@ import { mockCreateRelease, mockPaginate, mockListPulls, + mockContextRepo, + mockConfigGet, + mockExit, } from "./mocks"; +import { default as repoResp } from "./fixtures/responses/context_repo.json"; +import { makeConfigReponse } from "./fixtures/responses/get_config"; describe("Release Drafter", () => { beforeEach(() => { @@ -19,6 +24,18 @@ describe("Release Drafter", () => { mockListPulls.mockReset(); }); + beforeAll(() => { + mockContextRepo.mockReturnValue(repoResp); + mockExit.mockReset(); + mockConfigGet.mockResolvedValue( + makeConfigReponse({ release_drafter: true }) + ); + }); + + afterAll(() => { + expect(mockExit).toBeCalledTimes(0); + }); + test("doesn't run on non-versioned branches", async () => { await new ReleaseDrafter(context.nonVersionedBranch).draftRelease(); expect(mockPaginate).not.toHaveBeenCalled();