From 9a4b9743b36d1519ac5b364dc0e97d90e6351a59 Mon Sep 17 00:00:00 2001 From: dolfinus Date: Sun, 20 Feb 2022 14:52:40 +0300 Subject: [PATCH 1/7] Do not approve the same PR twice --- src/approve.test.ts | 326 +++++++++++++++++++++++++++++++++++++++++++- src/approve.ts | 42 +++++- 2 files changed, 364 insertions(+), 4 deletions(-) diff --git a/src/approve.test.ts b/src/approve.test.ts index e20e115..a7485d8 100644 --- a/src/approve.test.ts +++ b/src/approve.test.ts @@ -12,6 +12,16 @@ beforeEach(() => { }); test("when a review is successfully created", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, []); + nock("https://api.github.com") .post("/repos/hmarr/test/pulls/101/reviews") .reply(200, { id: 1 }); @@ -24,6 +34,238 @@ test("when a review is successfully created", async () => { }); test("when a review is successfully created using pull-request-number", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, []); + + nock("https://api.github.com") + .post("/repos/hmarr/test/pulls/101/reviews") + .reply(200, { id: 1 }); + + await approve("gh-tok", new Context(), 101); + + expect(core.info).toHaveBeenCalledWith( + expect.stringContaining("Approved pull request #101") + ); +}); + +test("when a review has already been approved by current user", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, [ + { + user: { login: "hmarr" }, + commit_id: "24c5451bbf1fb09caa3ac8024df4788aff4d4974", + state: "APPROVED", + }, + ]); + + await approve("gh-tok", ghContext()); + + expect(core.info).toHaveBeenCalledWith( + expect.stringContaining( + "Current user already approved pull request #101, nothing to do" + ) + ); +}); + +test("when a review is pending", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, [ + { + user: { login: "hmarr" }, + commit_id: "24c5451bbf1fb09caa3ac8024df4788aff4d4974", + state: "PENDING", + }, + ]); + + nock("https://api.github.com") + .post("/repos/hmarr/test/pulls/101/reviews") + .reply(200, { id: 1 }); + + await approve("gh-tok", new Context(), 101); + + expect(core.info).toHaveBeenCalledWith( + expect.stringContaining("Approved pull request #101") + ); +}); + +test("when a review is dismissed", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, [ + { + user: { login: "hmarr" }, + commit_id: "24c5451bbf1fb09caa3ac8024df4788aff4d4974", + state: "DISMISSED", + }, + ]); + + nock("https://api.github.com") + .post("/repos/hmarr/test/pulls/101/reviews") + .reply(200, { id: 1 }); + + await approve("gh-tok", new Context(), 101); + + expect(core.info).toHaveBeenCalledWith( + expect.stringContaining("Approved pull request #101") + ); +}); + +test("when a review is not approved", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, [ + { + user: { login: "hmarr" }, + commit_id: "24c5451bbf1fb09caa3ac8024df4788aff4d4974", + state: "CHANGES_REQUESTED", + }, + ]); + + nock("https://api.github.com") + .post("/repos/hmarr/test/pulls/101/reviews") + .reply(200, { id: 1 }); + + await approve("gh-tok", new Context(), 101); + + expect(core.info).toHaveBeenCalledWith( + expect.stringContaining("Approved pull request #101") + ); +}); + +test("when a review is commented", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, [ + { + user: { login: "hmarr" }, + commit_id: "24c5451bbf1fb09caa3ac8024df4788aff4d4974", + state: "COMMENTED", + }, + ]); + + nock("https://api.github.com") + .post("/repos/hmarr/test/pulls/101/reviews") + .reply(200, { id: 1 }); + + await approve("gh-tok", new Context(), 101); + + expect(core.info).toHaveBeenCalledWith( + expect.stringContaining("Approved pull request #101") + ); +}); + +test("when an old commit has already been approved", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, [ + { + user: { login: "hmarr" }, + commit_id: "6a9ec7556f0a7fa5b49527a1eea4878b8a22d2e0", + state: "APPROVED", + }, + ]); + + nock("https://api.github.com") + .post("/repos/hmarr/test/pulls/101/reviews") + .reply(200, { id: 1 }); + + await approve("gh-tok", ghContext()); + + expect(core.info).toHaveBeenCalledWith( + expect.stringContaining("Approved pull request #101") + ); +}); + +test("when a review has already been approved by another user", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, [ + { + user: { login: "some" }, + commit_id: "24c5451bbf1fb09caa3ac8024df4788aff4d4974", + state: "APPROVED", + }, + ]); + + nock("https://api.github.com") + .post("/repos/hmarr/test/pulls/101/reviews") + .reply(200, { id: 1 }); + + await approve("gh-tok", new Context(), 101); + + expect(core.info).toHaveBeenCalledWith( + expect.stringContaining("Approved pull request #101") + ); +}); + +test("when a review has already been approved by unknown user", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, [ + { + user: null, + commit_id: "24c5451bbf1fb09caa3ac8024df4788aff4d4974", + state: "APPROVED", + }, + ]); + nock("https://api.github.com") .post("/repos/hmarr/test/pulls/101/reviews") .reply(200, { id: 1 }); @@ -45,7 +287,7 @@ test("without a pull request", async () => { test("when the token is invalid", async () => { nock("https://api.github.com") - .post("/repos/hmarr/test/pulls/101/reviews") + .get("/user") .reply(401, { message: "Bad credentials" }); await approve("gh-tok", ghContext()); @@ -56,6 +298,16 @@ test("when the token is invalid", async () => { }); test("when the token doesn't have write permissions", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, []); + nock("https://api.github.com") .post("/repos/hmarr/test/pulls/101/reviews") .reply(403, { message: "Resource not accessible by integration" }); @@ -68,6 +320,16 @@ test("when the token doesn't have write permissions", async () => { }); test("when a user tries to approve their own pull request", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, []); + nock("https://api.github.com") .post("/repos/hmarr/test/pulls/101/reviews") .reply(422, { message: "Unprocessable Entity" }); @@ -79,7 +341,67 @@ test("when a user tries to approve their own pull request", async () => { ); }); -test("when the token doesn't have access to the repository", async () => { +test("when pull request does not exist", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(404, { message: "Not Found" }); + + await approve("gh-tok", ghContext()); + + expect(core.setFailed).toHaveBeenCalledWith( + expect.stringContaining("doesn't have access") + ); +}); + +test("when the token doesn't have read access to the repository", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(404, { message: "Not Found" }); + + await approve("gh-tok", ghContext()); + + expect(core.setFailed).toHaveBeenCalledWith( + expect.stringContaining("doesn't have access") + ); +}); + +test("when the token is read-only", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, []); + + nock("https://api.github.com") + .post("/repos/hmarr/test/pulls/101/reviews") + .reply(403, { message: "Not Authorized" }); + + await approve("gh-tok", ghContext()); + + expect(core.setFailed).toHaveBeenCalledWith( + expect.stringContaining("are read-only") + ); +}); + +test("when the token doesn't have write access to the repository", async () => { + nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101") + .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); + + nock("https://api.github.com") + .get("/repos/hmarr/test/pulls/101/reviews") + .reply(200, []); + nock("https://api.github.com") .post("/repos/hmarr/test/pulls/101/reviews") .reply(404, { message: "Not Found" }); diff --git a/src/approve.ts b/src/approve.ts index bf94340..9e597b8 100644 --- a/src/approve.ts +++ b/src/approve.ts @@ -2,6 +2,7 @@ import * as core from "@actions/core"; import * as github from "@actions/github"; import { RequestError } from "@octokit/request-error"; import { Context } from "@actions/github/lib/context"; +import { userInfo } from "os"; export async function approve( token: string, @@ -22,8 +23,46 @@ export async function approve( const client = github.getOctokit(token); - core.info(`Creating approving review for pull request #${prNumber}`); try { + core.info(`Getting current user info`); + const { data: user } = await client.users.getAuthenticated(); + core.info(`Current user is ${user.login}`); + + core.info(`Getting pull request #${prNumber} info`); + const pull_request = await client.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + const commit = pull_request.data.head.sha; + + core.info(`Commit SHA is ${commit}`); + + core.info( + `Getting reviews for pull request #${prNumber} and commit ${commit}` + ); + const reviews = await client.pulls.listReviews({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + + for (const review of reviews.data) { + if ( + review.user?.login == user.login && + review.commit_id == commit && + review.state == "APPROVED" + ) { + core.info( + `Current user already approved pull request #${prNumber}, nothing to do` + ); + return; + } + } + + core.info( + `Pull request #${prNumber} has not been approved yet, creating approving review` + ); await client.pulls.createReview({ owner: context.repo.owner, repo: context.repo.repo, @@ -67,7 +106,6 @@ export async function approve( } return; } - core.setFailed(error.message); return; } From 1048f6c1876ea30d125ae2eef99ae8da3b95866e Mon Sep 17 00:00:00 2001 From: Harry Marr Date: Wed, 30 Mar 2022 15:52:18 +0100 Subject: [PATCH 2/7] Disable network in tests --- src/approve.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/approve.test.ts b/src/approve.test.ts index a7485d8..76667e2 100644 --- a/src/approve.test.ts +++ b/src/approve.test.ts @@ -7,10 +7,15 @@ beforeEach(() => { jest.restoreAllMocks(); jest.spyOn(core, "setFailed").mockImplementation(jest.fn()); jest.spyOn(core, "info").mockImplementation(jest.fn()); + nock.disableNetConnect(); process.env["GITHUB_REPOSITORY"] = "hmarr/test"; }); +afterEach(() => { + nock.enableNetConnect(); +}); + test("when a review is successfully created", async () => { nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); From 36f645ff88e71f8cd82aef172df8deab2f6cb55a Mon Sep 17 00:00:00 2001 From: Harry Marr Date: Wed, 30 Mar 2022 15:52:33 +0100 Subject: [PATCH 3/7] Clean up nock mocks --- src/approve.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/approve.test.ts b/src/approve.test.ts index 76667e2..12de75e 100644 --- a/src/approve.test.ts +++ b/src/approve.test.ts @@ -13,6 +13,7 @@ beforeEach(() => { }); afterEach(() => { + nock.cleanAll(); nock.enableNetConnect(); }); From 39b9fd5b5d9ce277c5832febd7b8ebbc31a5d968 Mon Sep 17 00:00:00 2001 From: Harry Marr Date: Wed, 30 Mar 2022 15:58:48 +0100 Subject: [PATCH 4/7] Change PR number in test --- src/approve.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/approve.test.ts b/src/approve.test.ts index 12de75e..b629fd8 100644 --- a/src/approve.test.ts +++ b/src/approve.test.ts @@ -43,21 +43,21 @@ test("when a review is successfully created using pull-request-number", async () nock("https://api.github.com").get("/user").reply(200, { login: "hmarr" }); nock("https://api.github.com") - .get("/repos/hmarr/test/pulls/101") + .get("/repos/hmarr/test/pulls/102") .reply(200, { head: { sha: "24c5451bbf1fb09caa3ac8024df4788aff4d4974" } }); nock("https://api.github.com") - .get("/repos/hmarr/test/pulls/101/reviews") + .get("/repos/hmarr/test/pulls/102/reviews") .reply(200, []); nock("https://api.github.com") - .post("/repos/hmarr/test/pulls/101/reviews") + .post("/repos/hmarr/test/pulls/102/reviews") .reply(200, { id: 1 }); - await approve("gh-tok", new Context(), 101); + await approve("gh-tok", new Context(), 102); expect(core.info).toHaveBeenCalledWith( - expect.stringContaining("Approved pull request #101") + expect.stringContaining("Approved pull request #102") ); }); From c003acbfd07ad2abdf6ac9c21c80ef3af6f8ea7d Mon Sep 17 00:00:00 2001 From: Harry Marr Date: Wed, 30 Mar 2022 15:59:22 +0100 Subject: [PATCH 5/7] Remove redundant import --- src/approve.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/approve.ts b/src/approve.ts index 9e597b8..9c2eb38 100644 --- a/src/approve.ts +++ b/src/approve.ts @@ -2,7 +2,6 @@ import * as core from "@actions/core"; import * as github from "@actions/github"; import { RequestError } from "@octokit/request-error"; import { Context } from "@actions/github/lib/context"; -import { userInfo } from "os"; export async function approve( token: string, From f24dcb53d083298ac4b7bd3d399260df3cc85874 Mon Sep 17 00:00:00 2001 From: Harry Marr Date: Sat, 2 Apr 2022 13:55:43 +0100 Subject: [PATCH 6/7] Clean env in tests for Actions CI builds When the tests are run in GitHub Actions, the GITHUB_* environment variables were present, and causing tests to fail. Now we (mostly) clear the environment variables before each test. --- src/approve.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/approve.test.ts b/src/approve.test.ts index b629fd8..cc797e7 100644 --- a/src/approve.test.ts +++ b/src/approve.test.ts @@ -3,18 +3,21 @@ import { Context } from "@actions/github/lib/context"; import nock from "nock"; import { approve } from "./approve"; +const originalEnv = process.env; + beforeEach(() => { jest.restoreAllMocks(); jest.spyOn(core, "setFailed").mockImplementation(jest.fn()); jest.spyOn(core, "info").mockImplementation(jest.fn()); nock.disableNetConnect(); - process.env["GITHUB_REPOSITORY"] = "hmarr/test"; + process.env = { GITHUB_REPOSITORY: "hmarr/test" }; }); afterEach(() => { nock.cleanAll(); nock.enableNetConnect(); + process.env = originalEnv; }); test("when a review is successfully created", async () => { From f9c1af5845de6297db586118708aacda6f253285 Mon Sep 17 00:00:00 2001 From: Harry Marr Date: Sat, 2 Apr 2022 13:58:13 +0100 Subject: [PATCH 7/7] Rebuild action --- dist/index.js | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/dist/index.js b/dist/index.js index 01ae5f8..4b833b9 100644 --- a/dist/index.js +++ b/dist/index.js @@ -5867,7 +5867,7 @@ const core = __importStar(__nccwpck_require__(186)); const github = __importStar(__nccwpck_require__(438)); const request_error_1 = __nccwpck_require__(537); function approve(token, context, prNumber) { - var _a; + var _a, _b; return __awaiter(this, void 0, void 0, function* () { if (!prNumber) { prNumber = (_a = context.payload.pull_request) === null || _a === void 0 ? void 0 : _a.number; @@ -5878,8 +5878,33 @@ function approve(token, context, prNumber) { return; } const client = github.getOctokit(token); - core.info(`Creating approving review for pull request #${prNumber}`); try { + core.info(`Getting current user info`); + const { data: user } = yield client.users.getAuthenticated(); + core.info(`Current user is ${user.login}`); + core.info(`Getting pull request #${prNumber} info`); + const pull_request = yield client.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + const commit = pull_request.data.head.sha; + core.info(`Commit SHA is ${commit}`); + core.info(`Getting reviews for pull request #${prNumber} and commit ${commit}`); + const reviews = yield client.pulls.listReviews({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + for (const review of reviews.data) { + if (((_b = review.user) === null || _b === void 0 ? void 0 : _b.login) == user.login && + review.commit_id == commit && + review.state == "APPROVED") { + core.info(`Current user already approved pull request #${prNumber}, nothing to do`); + return; + } + } + core.info(`Pull request #${prNumber} has not been approved yet, creating approving review`); yield client.pulls.createReview({ owner: context.repo.owner, repo: context.repo.repo,