From 6aa7589b1a225db7167ea276864d0549b013fed7 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 12:21:25 +0200 Subject: [PATCH 01/10] added silent mode input logic --- src/bot.ts | 29 +++++++++++++++++------------ src/index.ts | 4 +++- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/bot.ts b/src/bot.ts index 6334786..3dab869 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -24,7 +24,8 @@ export class Bot { private readonly pr: Issue, private readonly logger: ActionLogger, private readonly commentsApi: CommentsApi, - ) {} + private readonly silentMode: boolean = false + ) { } /** Verifies if the author is the author of the PR or a member of the org */ async canTriggerBot(): Promise { @@ -58,13 +59,13 @@ export class Bot { const org = this.commentsApi.pullData.owner; this.logger.warn( "User is not allowed to trigger the bot. " + - `He is not the author of the PR and does not *publicly* belong to the org: https://github.com/orgs/${org}/people`, + `He is not the author of the PR and does not *publicly* belong to the org: https://github.com/orgs/${org}/people`, ); await this.commentsApi.reactToComment(this.comment.id, "-1"); await this.commentsApi.comment( "## Auto-Merge-Bot\n" + - `User @${login} is not the author of the PR and does not [*publicly* belong to the org \`${org}\`](https://github.com/orgs/${org}/people).\n\n` + - "Only author or *public* org members can trigger the bot.", + `User @${login} is not the author of the PR and does not [*publicly* belong to the org \`${org}\`](https://github.com/orgs/${org}/people).\n\n` + + "Only author or *public* org members can trigger the bot.", ); return; } @@ -77,17 +78,21 @@ export class Bot { case undefined: await this.commentsApi.reactToComment(this.comment.id, "+1"); await merger.enableAutoMerge(); - await this.commentsApi.comment( - "Enabled `auto-merge` in Pull Request", - ); + if (!this.silentMode) { + await this.commentsApi.comment( + "Enabled `auto-merge` in Pull Request", + ); + } break; // `/merge cancel` case "cancel": await this.commentsApi.reactToComment(this.comment.id, "+1"); await merger.disableAutoMerge(); - await this.commentsApi.comment( - "Disabled `auto-merge` in Pull Request", - ); + if (!this.silentMode) { + await this.commentsApi.comment( + "Disabled `auto-merge` in Pull Request", + ); + } break; // `/merge help` case "help": @@ -98,8 +103,8 @@ export class Bot { await this.commentsApi.reactToComment(this.comment.id, "confused"); await this.commentsApi.comment( "## Auto-Merge-Bot\n" + - `Command \`${command}\` not recognized.\n\n` + - botCommands, + `Command \`${command}\` not recognized.\n\n` + + botCommands, ); } } diff --git a/src/index.ts b/src/index.ts index 912eebc..ef2d290 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,4 @@ -import { getInput, setFailed, setOutput } from "@actions/core"; +import { getInput, setFailed, setOutput, getBooleanInput } from "@actions/core"; import { context, getOctokit } from "@actions/github"; import { Context } from "@actions/github/lib/context"; import { graphql } from "@octokit/graphql/dist-types/types"; @@ -52,6 +52,8 @@ const getMergeMethod = (): PullRequestMergeMethod => { return method; }; +const silentMode = getBooleanInput("SILENT", { required: false }); + if (context.payload.comment) { const token = getInput("GITHUB_TOKEN", { required: true }); const comment = context.payload.comment as unknown as IssueComment; From 33bc18d6c48d7d6e47668cade405369f05904965 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 12:21:31 +0200 Subject: [PATCH 02/10] added silent mode action input --- action.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/action.yml b/action.yml index 35c3047..16f4821 100644 --- a/action.yml +++ b/action.yml @@ -12,6 +12,9 @@ inputs: required: false description: The merge method to use. Must be one of MERGE, SQUASH or REBASE. default: SQUASH + SILENT: + required: false + description: If true, the bot will not post a comment on the PR. outputs: repo: description: 'The name of the repo in owner/repo pattern' From 53bb0b422e25ef04942b42f77f6557436d512083 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 12:24:48 +0200 Subject: [PATCH 03/10] fixed wrong workflow action command --- .github/workflows/auto-merge-bot.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/auto-merge-bot.yml b/.github/workflows/auto-merge-bot.yml index adb259d..820cf4c 100644 --- a/.github/workflows/auto-merge-bot.yml +++ b/.github/workflows/auto-merge-bot.yml @@ -9,7 +9,7 @@ jobs: set-auto-merge: runs-on: ubuntu-latest # Important! This forces the job to run only on Pull Requests - if: ${{ github.event.issue.pull_request && startsWith(github.event.comment.body, '/bot') }} + if: ${{ github.event.issue.pull_request && startsWith(github.event.comment.body, '/merge') }} steps: - name: Set auto merge uses: paritytech/auto-merge-bot@main From c0b73a248a262819e5537a03652c491a737c30bf Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 12:27:26 +0200 Subject: [PATCH 04/10] fixed silent mode input not being used --- src/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index ef2d290..c6b710b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -54,6 +54,8 @@ const getMergeMethod = (): PullRequestMergeMethod => { const silentMode = getBooleanInput("SILENT", { required: false }); +logger.info(`Silent mode is ${silentMode ? "enabled" : "disabled. Bot will comment actions"}`); + if (context.payload.comment) { const token = getInput("GITHUB_TOKEN", { required: true }); const comment = context.payload.comment as unknown as IssueComment; @@ -66,7 +68,7 @@ if (context.payload.comment) { headers: { authorization: `token ${token}` }, }) as graphql; const merger = new Merger(issue.node_id, gql, logger, getMergeMethod()); - const bot = new Bot(comment, issue, logger, commentsApi); + const bot = new Bot(comment, issue, logger, commentsApi, silentMode); bot .run(merger) .then(() => logger.info("Finished!")) From f4a9ffa20ca90ca23564b4bd9c079eb04e84928e Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 12:31:09 +0200 Subject: [PATCH 05/10] updated readme --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 2083074..7383f01 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,8 @@ You can find all the inputs in [the action file](./action.yml), but let's walk t - **Optional**: Defaults to `SQUASH`. - Available types are `MERGE`, `REBASE` and `SQUASH`. - Make sure that the type of merge you selected is available in the repository merge options. +- `SILENT`: If the bot should be silent and not comment when enabling/disabling auto-merge. + - **Optional**: Defaults to `false`. ## Usage From 7eb846b171820404b2a1bf0323d86420488bfef0 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 12:32:20 +0200 Subject: [PATCH 06/10] fixed linter problems --- src/bot.ts | 14 +++++++------- src/index.ts | 8 ++++++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/bot.ts b/src/bot.ts index 3dab869..90e93be 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -24,8 +24,8 @@ export class Bot { private readonly pr: Issue, private readonly logger: ActionLogger, private readonly commentsApi: CommentsApi, - private readonly silentMode: boolean = false - ) { } + private readonly silentMode: boolean = false, + ) {} /** Verifies if the author is the author of the PR or a member of the org */ async canTriggerBot(): Promise { @@ -59,13 +59,13 @@ export class Bot { const org = this.commentsApi.pullData.owner; this.logger.warn( "User is not allowed to trigger the bot. " + - `He is not the author of the PR and does not *publicly* belong to the org: https://github.com/orgs/${org}/people`, + `He is not the author of the PR and does not *publicly* belong to the org: https://github.com/orgs/${org}/people`, ); await this.commentsApi.reactToComment(this.comment.id, "-1"); await this.commentsApi.comment( "## Auto-Merge-Bot\n" + - `User @${login} is not the author of the PR and does not [*publicly* belong to the org \`${org}\`](https://github.com/orgs/${org}/people).\n\n` + - "Only author or *public* org members can trigger the bot.", + `User @${login} is not the author of the PR and does not [*publicly* belong to the org \`${org}\`](https://github.com/orgs/${org}/people).\n\n` + + "Only author or *public* org members can trigger the bot.", ); return; } @@ -103,8 +103,8 @@ export class Bot { await this.commentsApi.reactToComment(this.comment.id, "confused"); await this.commentsApi.comment( "## Auto-Merge-Bot\n" + - `Command \`${command}\` not recognized.\n\n` + - botCommands, + `Command \`${command}\` not recognized.\n\n` + + botCommands, ); } } diff --git a/src/index.ts b/src/index.ts index c6b710b..74cc176 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,4 @@ -import { getInput, setFailed, setOutput, getBooleanInput } from "@actions/core"; +import { getBooleanInput, getInput, setFailed, setOutput } from "@actions/core"; import { context, getOctokit } from "@actions/github"; import { Context } from "@actions/github/lib/context"; import { graphql } from "@octokit/graphql/dist-types/types"; @@ -54,7 +54,11 @@ const getMergeMethod = (): PullRequestMergeMethod => { const silentMode = getBooleanInput("SILENT", { required: false }); -logger.info(`Silent mode is ${silentMode ? "enabled" : "disabled. Bot will comment actions"}`); +logger.info( + `Silent mode is ${ + silentMode ? "enabled" : "disabled. Bot will comment actions" + }`, +); if (context.payload.comment) { const token = getInput("GITHUB_TOKEN", { required: true }); From e8d6527a9eedacdda6c23c3fe134d1211144a2c6 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 12:37:55 +0200 Subject: [PATCH 07/10] fixed problem with yaml parsing --- src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 74cc176..40177c4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -52,7 +52,7 @@ const getMergeMethod = (): PullRequestMergeMethod => { return method; }; -const silentMode = getBooleanInput("SILENT", { required: false }); +const silentMode = getInput("SILENT", { required: false }) === "true"; logger.info( `Silent mode is ${ From 076636b83f251dc1bcce71d81bd34e461fce0a4a Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 14:42:05 +0200 Subject: [PATCH 08/10] moved silent mode logic to comment class --- src/bot.ts | 23 +++++++++++------------ src/github/comments.ts | 24 ++++++++++++++++++------ src/index.ts | 17 +++++++++++------ 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/src/bot.ts b/src/bot.ts index 90e93be..6cd0e57 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -24,7 +24,6 @@ export class Bot { private readonly pr: Issue, private readonly logger: ActionLogger, private readonly commentsApi: CommentsApi, - private readonly silentMode: boolean = false, ) {} /** Verifies if the author is the author of the PR or a member of the org */ @@ -78,25 +77,24 @@ export class Bot { case undefined: await this.commentsApi.reactToComment(this.comment.id, "+1"); await merger.enableAutoMerge(); - if (!this.silentMode) { - await this.commentsApi.comment( - "Enabled `auto-merge` in Pull Request", - ); - } + await this.commentsApi.comment( + "Enabled `auto-merge` in Pull Request", + ); break; // `/merge cancel` case "cancel": await this.commentsApi.reactToComment(this.comment.id, "+1"); await merger.disableAutoMerge(); - if (!this.silentMode) { - await this.commentsApi.comment( - "Disabled `auto-merge` in Pull Request", - ); - } + await this.commentsApi.comment( + "Disabled `auto-merge` in Pull Request", + ); break; // `/merge help` case "help": - await this.commentsApi.comment("## Auto-Merge-Bot\n" + botCommands); + await this.commentsApi.comment( + "## Auto-Merge-Bot\n" + botCommands, + true, + ); break; // `/merge anything else` default: { @@ -105,6 +103,7 @@ export class Bot { "## Auto-Merge-Bot\n" + `Command \`${command}\` not recognized.\n\n` + botCommands, + true, ); } } diff --git a/src/github/comments.ts b/src/github/comments.ts index d2a279a..311e179 100644 --- a/src/github/comments.ts +++ b/src/github/comments.ts @@ -6,14 +6,26 @@ export class CommentsApi { private readonly api: GitHubClient, private readonly logger: ActionLogger, public readonly pullData: { repo: string; owner: string; number: number }, + private readonly silentMode: boolean = false, ) {} - async comment(message: string): Promise { - await this.api.rest.issues.createComment({ - ...this.pullData, - body: message, - issue_number: this.pullData.number, - }); + /** + * Logs and writes a comments. Will only log if silentMode is true + * @param message Message to write in the comment + * @param overrideSilentMode If silent mode should be overriden + */ + async comment( + message: string, + overrideSilentMode: boolean = false, + ): Promise { + this.logger.info("Commenting: " + message); + if (!this.silentMode || overrideSilentMode) { + await this.api.rest.issues.createComment({ + ...this.pullData, + body: message, + issue_number: this.pullData.number, + }); + } } async reactToComment( diff --git a/src/index.ts b/src/index.ts index 40177c4..bfd9844 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,4 @@ -import { getBooleanInput, getInput, setFailed, setOutput } from "@actions/core"; +import { getInput, setFailed, setOutput } from "@actions/core"; import { context, getOctokit } from "@actions/github"; import { Context } from "@actions/github/lib/context"; import { graphql } from "@octokit/graphql/dist-types/types"; @@ -64,15 +64,20 @@ if (context.payload.comment) { const token = getInput("GITHUB_TOKEN", { required: true }); const comment = context.payload.comment as unknown as IssueComment; const issue = context.payload.issue as unknown as Issue; - const commentsApi = new CommentsApi(getOctokit(token), logger, { - ...repo, - number: issue.number, - }); + const commentsApi = new CommentsApi( + getOctokit(token), + logger, + { + ...repo, + number: issue.number, + }, + silentMode, + ); const gql = getOctokit(token).graphql.defaults({ headers: { authorization: `token ${token}` }, }) as graphql; const merger = new Merger(issue.node_id, gql, logger, getMergeMethod()); - const bot = new Bot(comment, issue, logger, commentsApi, silentMode); + const bot = new Bot(comment, issue, logger, commentsApi); bot .run(merger) .then(() => logger.info("Finished!")) From 9394fcc1bf761ddf80784ad150ff99f5fb763bbf Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 14:55:27 +0200 Subject: [PATCH 09/10] added tests for the comment class --- package.json | 1 + src/test/comments.test.ts | 55 +++++++++++++++++++++++++++++++++++++++ src/test/index.ts | 3 --- yarn.lock | 12 +++++++++ 4 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 src/test/comments.test.ts delete mode 100644 src/test/index.ts diff --git a/package.json b/package.json index 247da51..bd617cf 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "@types/jest": "^29.5.5", "@vercel/ncc": "^0.38.0", "jest": "^29.7.0", + "jest-mock-extended": "^3.0.5", "ts-jest": "^29.1.1", "typescript": "^5.2.2" } diff --git a/src/test/comments.test.ts b/src/test/comments.test.ts new file mode 100644 index 0000000..6919018 --- /dev/null +++ b/src/test/comments.test.ts @@ -0,0 +1,55 @@ +/* eslint-disable @typescript-eslint/unbound-method */ +import { DeepMockProxy, mock, mockDeep, MockProxy } from "jest-mock-extended"; + +import { CommentsApi } from "../github/comments"; +import { ActionLogger, GitHubClient } from "../github/types"; + +describe("Comments", () => { + let comments: CommentsApi; + let api: DeepMockProxy; + let logger: MockProxy; + const repo = { repo: "example", owner: "me", number: 123 }; + beforeEach(() => { + api = mockDeep(); + logger = mock(); + comments = new CommentsApi(api, logger, repo); + }); + + describe("Comment action", () => { + test("Should comment", async () => { + await comments.comment("Hello"); + expect(api.rest.issues.createComment).toHaveBeenCalledWith({ + body: "Hello", + ...repo, + issue_number: repo.number, + }); + }); + + test("Should log when commenting", async () => { + await comments.comment("Log this"); + expect(logger.info).toHaveBeenCalledWith("Commenting: Log this"); + }); + + test("Should not comment on silent mode", async () => { + comments = new CommentsApi(api, logger, repo, true); + await comments.comment("Example"); + expect(api.rest.issues.createComment).not.toHaveBeenCalled(); + }); + + test("Should log while in silent mode", async () => { + comments = new CommentsApi(api, logger, repo, true); + await comments.comment("Bye"); + expect(logger.info).toHaveBeenCalledWith("Commenting: Bye"); + }); + + test("Should override silent mode", async () => { + comments = new CommentsApi(api, logger, repo, true); + await comments.comment("Overrider", true); + expect(api.rest.issues.createComment).toHaveBeenCalledWith({ + body: "Overrider", + ...repo, + issue_number: repo.number, + }); + }); + }); +}); diff --git a/src/test/index.ts b/src/test/index.ts deleted file mode 100644 index 3a327af..0000000 --- a/src/test/index.ts +++ /dev/null @@ -1,3 +0,0 @@ -test("adds 1 + 2 to equal 3", () => { - expect(1 + 2).toBe(3); -}); diff --git a/yarn.lock b/yarn.lock index 7932878..0020cfc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2697,6 +2697,13 @@ jest-message-util@^29.7.0: slash "^3.0.0" stack-utils "^2.0.3" +jest-mock-extended@^3.0.5: + version "3.0.5" + resolved "https://registry.yarnpkg.com/jest-mock-extended/-/jest-mock-extended-3.0.5.tgz#ebf208e363f4f1db603b81fb005c4055b7c1c8b7" + integrity sha512-/eHdaNPUAXe7f65gHH5urc8SbRVWjYxBqmCgax2uqOBJy8UUcCBMN1upj1eZ8y/i+IqpyEm4Kq0VKss/GCCTdw== + dependencies: + ts-essentials "^7.0.3" + jest-mock@^29.7.0: version "29.7.0" resolved "https://registry.yarnpkg.com/jest-mock/-/jest-mock-29.7.0.tgz#4e836cf60e99c6fcfabe9f99d017f3fdd50a6347" @@ -3781,6 +3788,11 @@ tr46@~0.0.3: resolved "https://registry.yarnpkg.com/tr46/-/tr46-0.0.3.tgz#8184fd347dac9cdc185992f3a6622e14b9d9ab6a" integrity sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw== +ts-essentials@^7.0.3: + version "7.0.3" + resolved "https://registry.yarnpkg.com/ts-essentials/-/ts-essentials-7.0.3.tgz#686fd155a02133eedcc5362dc8b5056cde3e5a38" + integrity sha512-8+gr5+lqO3G84KdiTSMRLtuyJ+nTBVRKuCrK4lidMPdVeEp0uqC875uE5NMcaA7YYMN7XsNiFQuMvasF8HT/xQ== + ts-jest@^29.1.1: version "29.1.1" resolved "https://registry.yarnpkg.com/ts-jest/-/ts-jest-29.1.1.tgz#f58fe62c63caf7bfcc5cc6472082f79180f0815b" From b3a4d5f48a431951f20136763749451ffbf86bd5 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 15:01:06 +0200 Subject: [PATCH 10/10] added tests to userBelongsToOrg --- src/test/comments.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/test/comments.test.ts b/src/test/comments.test.ts index 6919018..9ac1e72 100644 --- a/src/test/comments.test.ts +++ b/src/test/comments.test.ts @@ -52,4 +52,22 @@ describe("Comments", () => { }); }); }); + + describe("userBelongsToOrg", () => { + test("Should return false when http call fails", async () => { + api.rest.orgs.checkPublicMembershipForUser.mockRejectedValue( + "This is an error", + ); + const userBelongs = await comments.userBelongsToOrg("example"); + expect(userBelongs).toBeFalsy(); + }); + + test("Should return false when http call returns 204", async () => { + api.rest.orgs.checkPublicMembershipForUser.mockResolvedValue({ + status: 204, + } as never); + const userBelongs = await comments.userBelongsToOrg("example"); + expect(userBelongs).toBeTruthy(); + }); + }); });