From 308b2fd250369ac423ef99b3d71bbd77dadb0a4e Mon Sep 17 00:00:00 2001 From: marocchino Date: Fri, 13 Mar 2026 20:35:57 +0900 Subject: [PATCH] Don't create a comment with hide: true (#1661) --- __tests__/main.test.ts | 54 +++++++++++++++++++++++++++++++++++++++--- src/main.ts | 22 ++++++++++------- 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts index 02d5ba4..8ddc0af 100644 --- a/__tests__/main.test.ts +++ b/__tests__/main.test.ts @@ -119,6 +119,24 @@ describe("run", () => { ) }) + test("fails when deleteOldComment and onlyCreateComment are both true", async () => { + mockConfig.deleteOldComment = true + mockConfig.onlyCreateComment = true + const {core} = await runMain() + expect(core.setFailed).toHaveBeenCalledWith( + "delete and only_create cannot be both set to true", + ) + }) + + test("fails when deleteOldComment and hideOldComment are both true", async () => { + mockConfig.deleteOldComment = true + mockConfig.hideOldComment = true + const {core} = await runMain() + expect(core.setFailed).toHaveBeenCalledWith( + "delete and hide cannot be both set to true", + ) + }) + test("fails when onlyCreateComment and onlyUpdateComment are both true", async () => { mockConfig.onlyCreateComment = true mockConfig.onlyUpdateComment = true @@ -137,7 +155,7 @@ describe("run", () => { ) }) - test("deletes previous comment when deleteOldComment is true and previous exists", async () => { + test("deletes previous comment when deleteOldComment is true and previous comment exists", async () => { mockConfig.deleteOldComment = true const previous = {id: "existing-id", body: "old body"} const {comment, core} = await runMain(({comment}) => { @@ -146,6 +164,7 @@ describe("run", () => { expect(core.setOutput).toHaveBeenCalledWith("previous_comment_id", "existing-id") expect(comment.deleteComment).toHaveBeenCalledWith(expect.anything(), "existing-id") expect(comment.createComment).not.toHaveBeenCalled() + expect(comment.updateComment).not.toHaveBeenCalled() }) test("skips delete when deleteOldComment is true but no previous comment exists", async () => { @@ -153,12 +172,33 @@ describe("run", () => { const {comment, core} = await runMain() expect(core.setOutput).toHaveBeenCalledWith("previous_comment_id", undefined) expect(comment.deleteComment).not.toHaveBeenCalled() + expect(comment.createComment).not.toHaveBeenCalled() + expect(comment.updateComment).not.toHaveBeenCalled() }) - test("skips creating comment when onlyUpdateComment is true and no previous exists", async () => { + test("Updates previous comment when onlyUpdateComment is true and previous comment exists", async () => { + mockConfig.onlyUpdateComment = true + const previous = {id: "existing-id", body: "old body"} + const {comment, core} = await runMain(({comment}) => { + vi.mocked(comment.findPreviousComment).mockResolvedValue(previous as any) + vi.mocked(comment.getBodyOf).mockReturnValue("previous body content") + }) + expect(comment.updateComment).toHaveBeenCalledWith( + expect.anything(), + "existing-id", + "test body", + "", + "previous body content", + ) + expect(comment.createComment).not.toHaveBeenCalled() + expect(core.setOutput).toHaveBeenCalledWith("previous_comment_id", "existing-id") + }) + + test("skips creating comment when onlyUpdateComment is true and no previous comment exists", async () => { mockConfig.onlyUpdateComment = true const {comment} = await runMain() expect(comment.createComment).not.toHaveBeenCalled() + expect(comment.updateComment).not.toHaveBeenCalled() }) test("creates comment when no previous comment exists", async () => { @@ -173,7 +213,7 @@ describe("run", () => { expect(core.setOutput).toHaveBeenCalledWith("created_comment_id", 456) }) - test("skips update when onlyCreateComment is true and previous exists", async () => { + test("skips update when onlyCreateComment is true and previous comment exists", async () => { mockConfig.onlyCreateComment = true const previous = {id: "existing-id", body: "old body"} const {comment} = await runMain(({comment}) => { @@ -197,6 +237,14 @@ describe("run", () => { expect(comment.updateComment).not.toHaveBeenCalled() }) + test("skips when hideOldComment is true and no previous comment exists", async () => { + mockConfig.hideOldComment = true + const {comment} = await runMain() + expect(comment.minimizeComment).not.toHaveBeenCalled() + expect(comment.createComment).not.toHaveBeenCalled() + expect(comment.updateComment).not.toHaveBeenCalled() + }) + test("skips update when skipUnchanged is true and body is unchanged", async () => { mockConfig.skipUnchanged = true const previous = {id: "existing-id", body: "old body"} diff --git a/src/main.ts b/src/main.ts index 95adb30..790bda7 100644 --- a/src/main.ts +++ b/src/main.ts @@ -50,6 +50,14 @@ async function run(): Promise { throw new Error("delete and recreate cannot be both set to true") } + if (deleteOldComment && onlyCreateComment) { + throw new Error("delete and only_create cannot be both set to true") + } + + if (deleteOldComment && hideOldComment) { + throw new Error("delete and hide cannot be both set to true") + } + if (onlyCreateComment && onlyUpdateComment) { throw new Error("only_create and only_update cannot be both set to true") } @@ -63,15 +71,8 @@ async function run(): Promise { core.setOutput("previous_comment_id", previous?.id) - if (deleteOldComment) { - if (previous) { - await deleteComment(octokit, previous.id) - } - return - } - if (!previous) { - if (onlyUpdateComment) { + if (onlyUpdateComment || hideOldComment || deleteOldComment) { return } const created = await createComment(octokit, repo, pullRequestNumber, body, header) @@ -90,6 +91,11 @@ async function run(): Promise { return } + if (deleteOldComment) { + await deleteComment(octokit, previous.id) + return + } + if (skipUnchanged && commentsEqual(body, previous.body || "", header)) { // don't recreate or update if the message is unchanged return