diff --git a/src/config.ts b/src/config.ts index f1d7ecd..b671bf5 100644 --- a/src/config.ts +++ b/src/config.ts @@ -24,6 +24,7 @@ export type OpsBotConfigFeatureNames = { label_checker: boolean; release_drafter: boolean; recently_updated: boolean; + forward_merger: boolean; }; /** @@ -49,6 +50,7 @@ export const DefaultOpsBotConfig: OpsBotConfig = { release_drafter: false, recently_updated: false, recently_updated_threshold: 5, + forward_merger: false, }; /** diff --git a/src/index.ts b/src/index.ts index 264ea4c..5318da1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -20,6 +20,7 @@ import { initBranchChecker } from "./plugins/BranchChecker"; import { initLabelChecker } from "./plugins/LabelChecker"; import { initRecentlyUpdated } from "./plugins/RecentlyUpdated"; import { initReleaseDrafter } from "./plugins/ReleaseDrafter"; +import { initForwardMerger } from "./plugins/ForwardMerger"; export = (app: Probot) => { initBranchChecker(app); @@ -27,4 +28,5 @@ export = (app: Probot) => { initReleaseDrafter(app); initAutoMerger(app); initRecentlyUpdated(app); + initForwardMerger(app); }; diff --git a/src/plugins/ForwardMerger/forward_merger.ts b/src/plugins/ForwardMerger/forward_merger.ts new file mode 100644 index 0000000..fe39545 --- /dev/null +++ b/src/plugins/ForwardMerger/forward_merger.ts @@ -0,0 +1,125 @@ +/* + * Copyright (c) 2024, NVIDIA CORPORATION. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { OpsBotPlugin } from "../../plugin"; +import { PayloadRepository } from "../../types"; +import { isVersionedBranch, getVersionFromBranch } from "../../shared"; +import { basename } from "path"; +import { Context } from "probot"; + +export class ForwardMerger extends OpsBotPlugin { + context: Context; + currentBranch: string; + repo: PayloadRepository; + + constructor( + context: Context, + private payload: Context<"push">["payload"] + ) { + super("forward_merger", context); + this.context = context; + this.currentBranch = basename(this.payload.ref); + this.repo = payload.repository; + } + + async mergeForward(): Promise { + if (await this.pluginIsDisabled()) return; + + if (!isVersionedBranch(this.currentBranch)) { + this.logger.info("Will not merge forward on non-versioned branch"); + return; + } + + const branches = await this.getBranches(); + const sortedBranches = this.sortBranches(branches); + const nextBranch = this.getNextBranch(sortedBranches); + + if (!nextBranch) return; + + const { data: pr } = await this.context.octokit.pulls.create({ + owner: this.repo.owner.login, + repo: this.repo.name, + title: "Forward-merge " + this.currentBranch + " into " + nextBranch, + head: this.currentBranch, + base: nextBranch, + maintainer_can_modify: false, + body: `Forward-merge triggered by push to ${this.currentBranch} that creates a PR to keep ${nextBranch} up-to-date. If this PR is unable to be immediately merged due to conflicts, it will remain open for the team to manually merge. See [forward-merger docs](https://docs.rapids.ai/maintainers/forward-merger/) for more info.`, + }); + + try { + this.logger.info("Merging PR"); + await this.context.octokit.pulls.merge({ + owner: this.repo.owner.login, + repo: this.repo.name, + pull_number: pr.number, + sha: pr.head.sha, + }); + } catch (error) { + await this.issueComment( + pr.number, + "**FAILURE** - Unable to forward-merge due to an error, **manual** merge is necessary. Do not use the `Resolve conflicts` option in this PR, follow these instructions https://docs.rapids.ai/maintainers/forward-merger/ \n **IMPORTANT**: When merging this PR, do not use the [auto-merger](https://docs.rapids.ai/resources/auto-merger/) (i.e. the `/merge` comment). Instead, an admin must manually merge by changing the merging strategy to `Create a Merge Commit`. Otherwise, history will be lost and the branches become incompatible." + ); + return; + } + + await this.issueComment( + pr.number, + "**SUCCESS** - forward-merge complete." + ); + } + + async getBranches(): Promise { + const branches = await this.context.octokit.paginate( + this.context.octokit.repos.listBranches, + { + owner: this.repo.owner.login, + repo: this.repo.name, + } + ); + return branches + .filter((branch) => isVersionedBranch(branch.name)) + .map((branch) => branch.name); + } + + sortBranches(branches: string[]): string[] { + return branches.sort((a, b) => { + const [yearA, monthA] = getVersionFromBranch(a).split(".").map(Number); + const [yearB, monthB] = getVersionFromBranch(b).split(".").map(Number); + if (yearA !== yearB) { + return yearA - yearB; + } else { + return monthA - monthB; + } + }); + } + + getNextBranch(sortedBranches: string[]): string | undefined { + const currentBranchIndex = sortedBranches.findIndex( + (branch) => branch === this.currentBranch + ); + const nextBranch = sortedBranches[currentBranchIndex + 1]; + return nextBranch; + } + + async issueComment(id, comment): Promise { + await this.context.octokit.issues.createComment({ + owner: this.repo.owner.login, + repo: this.repo.name, + issue_number: id, + body: comment, + }); + } +} diff --git a/src/plugins/ForwardMerger/index.ts b/src/plugins/ForwardMerger/index.ts new file mode 100644 index 0000000..947cc30 --- /dev/null +++ b/src/plugins/ForwardMerger/index.ts @@ -0,0 +1,24 @@ +/* +* Copyright (c) 2024, NVIDIA CORPORATION. +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +import { Probot } from "probot"; +import { ForwardMerger } from "./forward_merger"; + +export const initForwardMerger = (app: Probot) => { + app.on(["push"], async (context) => { + await new ForwardMerger(context, context.payload).mergeForward(); + }); +}; diff --git a/test/fixtures/contexts/base.ts b/test/fixtures/contexts/base.ts index 9be95f3..172aedf 100644 --- a/test/fixtures/contexts/base.ts +++ b/test/fixtures/contexts/base.ts @@ -35,6 +35,8 @@ import { mockPaginate, mockPullsGet, mockUpdateRelease, + mockCreatePR, + mockListBranches, } from "../../mocks"; import type { EmitterWebhookEventName } from "@octokit/webhooks/dist-types/types"; @@ -57,6 +59,7 @@ export const makeContext = (payload, name: EmitterWebhookEventName) => { list: mockListPulls, listReviews: mockListReviews, merge: mockMerge, + create: mockCreatePR, }, repos: { createCommitStatus: mockCreateCommitStatus, @@ -66,6 +69,7 @@ export const makeContext = (payload, name: EmitterWebhookEventName) => { getReleaseByTag: mockGetReleaseByTag, updateRelease: mockUpdateRelease, compareCommitsWithBasehead: mockCompareCommitsWithBasehead, + listBranches: mockListBranches, }, users: { getByUsername: mockGetByUsername, diff --git a/test/forward_merger.test.ts b/test/forward_merger.test.ts new file mode 100644 index 0000000..3d37cb3 --- /dev/null +++ b/test/forward_merger.test.ts @@ -0,0 +1,297 @@ +/* + * Copyright (c) 2024, NVIDIA CORPORATION. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ForwardMerger } from "../src/plugins/ForwardMerger/forward_merger"; +import { makePushContext } from "./fixtures/contexts/push"; +import { + mockConfigGet, + mockContextRepo, + mockCreateComment, + mockCreatePR, + mockListBranches, + mockMerge, + mockPaginate, +} from "./mocks"; +import { default as repoResp } from "./fixtures/responses/context_repo.json"; +import { makeConfigReponse } from "./fixtures/responses/get_config"; + +describe("Forward Merger", () => { + beforeEach(() => { + mockCreatePR.mockReset(); + mockMerge.mockReset(); + mockListBranches.mockReset(); + mockPaginate.mockReset(); + mockCreateComment.mockReset(); + }); + + beforeAll(() => { + mockContextRepo.mockReset(); + mockContextRepo.mockReturnValue(repoResp); + mockConfigGet.mockResolvedValue( + makeConfigReponse({ + forward_merger: true, + }) + ); + }); + + test("mergeForward should not run on push to non-versioned branch", async () => { + const context = makePushContext({ + ref: "refs/heads/unversioned", + }); + const forwardMerger = new ForwardMerger(context, context.payload); + const mockGetBranches = jest.fn().mockName("getBranches"); + forwardMerger.getBranches = mockGetBranches; + await forwardMerger.mergeForward(); + + expect(mockGetBranches).not.toBeCalled(); + }); + + test("mergeForward should not run when plugin is disabled", async () => { + const context = makePushContext({ + ref: "refs/heads/branch-21.12", + }); + const forwardMerger = new ForwardMerger(context, context.payload); + const mockGetBranches = jest.fn().mockName("getBranches"); + forwardMerger.getBranches = mockGetBranches; + const mockPluginIsDisabled = jest + .fn() + .mockName("pluginIsDisabled") + .mockResolvedValue(true); + forwardMerger.pluginIsDisabled = mockPluginIsDisabled; + await forwardMerger.mergeForward(); + + expect(mockPluginIsDisabled).toBeCalled(); + expect(mockGetBranches).not.toBeCalled(); + }); + + test("mergeForward should not open PR on invalid next branch", async () => { + const context = makePushContext({ + ref: "refs/heads/branch-22.02", + }); + const forwardMerger = new ForwardMerger(context, context.payload); + forwardMerger.getBranches = jest + .fn() + .mockName("getBranches") + .mockResolvedValue(null); + forwardMerger.sortBranches = jest + .fn() + .mockName("sortBranches") + .mockReturnValue(null); + const nextBranch = undefined; + const mockGetNextBranch = jest + .fn() + .mockName("getNextBranch") + .mockReturnValue(nextBranch); + forwardMerger.getNextBranch = mockGetNextBranch; + + await forwardMerger.mergeForward(); + + expect(mockCreatePR).not.toBeCalled(); + }); + + test("should comment success on PR if merge is successful", async () => { + const context = makePushContext({ + ref: "refs/heads/branch-21.12", + }); + const forwardMerger = new ForwardMerger(context, context.payload); + forwardMerger.getBranches = jest + .fn() + .mockName("getBranches") + .mockResolvedValue(null); + forwardMerger.sortBranches = jest + .fn() + .mockName("sortBranches") + .mockReturnValue(null); + const nextBranch = { + name: "branch-21.10", + }; + forwardMerger.getNextBranch = jest + .fn() + .mockName("getNextBranch") + .mockResolvedValue(nextBranch); + const pr = { data: { number: 1, head: { sha: 123456 } } }; + mockCreatePR.mockResolvedValue(pr); + + mockMerge.mockResolvedValue(true); + const mockIssueComment = jest + .fn() + .mockName("issueComment") + .mockResolvedValue(null); + forwardMerger.issueComment = mockIssueComment; + + await forwardMerger.mergeForward(); + + expect(mockIssueComment).toBeCalledWith( + pr.data.number, + "**SUCCESS** - forward-merge complete." + ); + }); + + test("should comment failure on PR if merge is unsuccessful", async () => { + const context = makePushContext({ + ref: "refs/heads/branch-21.12", + }); + const forwardMerger = new ForwardMerger(context, context.payload); + forwardMerger.getBranches = jest + .fn() + .mockName("getBranches") + .mockResolvedValue(null); + forwardMerger.sortBranches = jest + .fn() + .mockName("sortBranches") + .mockResolvedValue(null); + const nextBranch = { + name: "branch-21.10", + }; + forwardMerger.getNextBranch = jest + .fn() + .mockName("getNextBranch") + .mockReturnValue(nextBranch); + const pr = { data: { number: 1, head: { sha: 123456 } } }; + mockCreatePR.mockResolvedValue(pr); + mockMerge.mockRejectedValueOnce(new Error("error")); + const mockIssueComment = jest + .fn() + .mockName("issueComment") + .mockResolvedValue(null); + forwardMerger.issueComment = mockIssueComment; + + await forwardMerger.mergeForward(); + + expect(mockIssueComment).toBeCalledWith( + pr.data.number, + "**FAILURE** - Unable to forward-merge due to an error, **manual** merge is necessary. Do not use the `Resolve conflicts` option in this PR, follow these instructions https://docs.rapids.ai/maintainers/forward-merger/ \n **IMPORTANT**: When merging this PR, do not use the [auto-merger](https://docs.rapids.ai/resources/auto-merger/) (i.e. the `/merge` comment). Instead, an admin must manually merge by changing the merging strategy to `Create a Merge Commit`. Otherwise, history will be lost and the branches become incompatible." + ); + }); + + test("mergeForward should obtain the correct next branch from a given list of unsorted branches", async () => { + const context = makePushContext({ + ref: "refs/heads/branch-22.02", + }); + const forwardMerger = new ForwardMerger(context, context.payload); + const branches = [ + "branch-22.04", + "branch-21.12", + "branch-21.10", + "branch-22.02", + ]; + const mockGetBranches = jest + .fn() + .mockName("getBranches") + .mockResolvedValue(branches); + mockCreatePR.mockResolvedValue({ data: {} }); + forwardMerger.getBranches = mockGetBranches; + await forwardMerger.mergeForward(); + + expect(mockCreatePR.mock.calls[0][0].base).toBe("branch-22.04"); + }); + + test("getBranches should return versioned branches", async () => { + const context = makePushContext({ + ref: "refs/heads/branch-21.12", + }); + const forwardMerger = new ForwardMerger(context, context.payload); + const branches = [ + { + name: "branch-21.12", + }, + { + name: "non-versioned", + }, + { + name: "branch-21.10", + }, + ]; + mockPaginate.mockResolvedValue(branches); + mockListBranches.mockResolvedValue("something"); + + const result = await forwardMerger.getBranches(); + + expect(result.length).toEqual(2); + expect(result[0]).toEqual("branch-21.12"); + expect(result[1]).toEqual("branch-21.10"); + expect(mockPaginate.mock.calls[0][0]).toBe(mockListBranches); + expect(mockPaginate.mock.calls[0][1]).toMatchObject({ + owner: context.payload.repository.owner.login, + repo: context.payload.repository.name, + }); + }); + + test("sortBranches should sort branches", async () => { + const context = makePushContext({ + ref: "refs/heads/branch-21.12", + }); + const forwardMerger = new ForwardMerger(context, context.payload); + const branches = [ + "branch-0.10", + "branch-21.12", + "branch-21.10", + "branch-19.10", + "branch-22.02", + "branch-0.9", + ]; + + const result = forwardMerger.sortBranches(branches); + + expect(result.length).toEqual(6); + expect(result[0]).toEqual("branch-0.9"); + expect(result[1]).toEqual("branch-0.10"); + expect(result[2]).toEqual("branch-19.10"); + expect(result[3]).toEqual("branch-21.10"); + expect(result[4]).toEqual("branch-21.12"); + expect(result[5]).toEqual("branch-22.02"); + }); + + test("getNextBranch should return next branch", async () => { + const context = makePushContext({ + ref: "refs/heads/branch-21.12", + }); + const forwardMerger = new ForwardMerger(context, context.payload); + const branches = ["branch-21.12", "branch-21.10", "branch-22.02"]; + const sortedBranches = forwardMerger.sortBranches(branches); + const result = await forwardMerger.getNextBranch(sortedBranches); + + expect(result).toEqual("branch-22.02"); + }); + + test("getNextBranch should return undefined if there is no next branch", async () => { + const context = makePushContext({ + ref: "refs/heads/branch-22.02", + }); + const forwardMerger = new ForwardMerger(context, context.payload); + const branches = ["branch-21.12", "branch-21.10", "branch-22.02"]; + const sortedBranches = forwardMerger.sortBranches(branches); + const result = await forwardMerger.getNextBranch(sortedBranches); + + expect(result).toBeUndefined(); + }); + + test("issueComment should create comment", async () => { + const context = makePushContext({ + ref: "refs/heads/branch-22.02", + }); + const forwardMerger = new ForwardMerger(context, context.payload); + mockCreateComment.mockResolvedValue(null); + await forwardMerger.issueComment(1, "comment"); + + expect(mockCreateComment.mock.calls[0][0]).toMatchObject({ + owner: context.payload.repository.owner.login, + repo: context.payload.repository.name, + issue_number: 1, + body: "comment", + }); + }); +}); diff --git a/test/mocks.ts b/test/mocks.ts index 7929541..2ff525b 100644 --- a/test/mocks.ts +++ b/test/mocks.ts @@ -49,3 +49,5 @@ export const mockSearchIssuesAndPullRequests = jest .fn() .mockName("mockSearchIssuesAndPullRequests"); export const mockUpdateRelease = jest.fn().mockName("mockUpdateRelease"); +export const mockCreatePR = jest.fn().mockName("mockCreatePR"); +export const mockListBranches = jest.fn().mockName("mockListBranches");