From 0da14ec4f475435b9d7d0ba06e49f05a5daac226 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Mon, 10 Feb 2025 23:01:50 -0800 Subject: [PATCH] [ARMAutoSignOff] Trigger on issue_comment:edited when body contains "next steps to merge" (#32554) --- .github/src/context.js | 73 ++++++----------- .github/test/context.test.js | 81 +++++-------------- .github/test/mocks.js | 5 +- .../workflows/arm-auto-signoff-preview.yaml | 30 +++++-- 4 files changed, 73 insertions(+), 116 deletions(-) diff --git a/.github/src/context.js b/.github/src/context.js index 1b30b4e1b27b..e994cd4c475d 100644 --- a/.github/src/context.js +++ b/.github/src/context.js @@ -29,72 +29,45 @@ export async function extractInputs(github, context, core) { issue_number: payload.pull_request.number, run_id: NaN, }; - } else if (context.eventName === "workflow_dispatch") { - const payload = - /** @type {import("@octokit/webhooks-types").WorkflowDispatchEvent} */ ( - context.payload - ); - - inputs = { - owner: payload.repository.owner.login, - repo: payload.repository.name, - head_sha: "", - issue_number: NaN, - run_id: NaN, - }; } else if ( - context.eventName === "check_suite" && - context.payload.action === "completed" + context.eventName === "issue_comment" && + context.payload.action === "edited" ) { const payload = - /** @type {import("@octokit/webhooks-types").CheckSuiteCompletedEvent} */ ( + /** @type {import("@octokit/webhooks-types").IssueCommentEditedEvent} */ ( context.payload ); const owner = payload.repository.owner.login; const repo = payload.repository.name; - const head_sha = payload.check_suite.head_sha; - - let issue_number; - - const pull_requests = payload.check_suite.pull_requests; - if (pull_requests && pull_requests.length > 0) { - // For non-fork PRs, we should be able to extract the PR number from the payload, which avoids an - // unnecessary API call. The listPullRequestsAssociatedWithCommit() API also seems to return - // empty for non-fork PRs. - issue_number = pull_requests[0].number; - } else { - // For fork PRs, we must call an API in the base repository to get the PR number - core.info( - `listPullRequestsAssociatedWithCommit(${owner}, ${repo}, ${head_sha})`, - ); - const { data: pullRequests } = - await github.rest.repos.listPullRequestsAssociatedWithCommit({ - owner: owner, - repo: repo, - commit_sha: head_sha, - }); + const issue_number = payload.issue.number; - if (pullRequests.length === 1) { - issue_number = pullRequests[0].number; - } else { - throw new Error( - `Unexpected number of pull requests associated with commit '${head_sha}'. Expected: '1'. Actual '${pullRequests.length}'.`, - ); - } - } + const { data: pr } = await github.rest.pulls.get({ + owner: owner, + repo: repo, + pull_number: issue_number, + }); - const inputs = { + inputs = { owner: owner, repo: repo, - head_sha: head_sha, + head_sha: pr.head.sha, issue_number: issue_number, run_id: NaN, }; + } else if (context.eventName === "workflow_dispatch") { + const payload = + /** @type {import("@octokit/webhooks-types").WorkflowDispatchEvent} */ ( + context.payload + ); - core.info(`inputs: ${JSON.stringify(inputs)}`); - - return inputs; + inputs = { + owner: payload.repository.owner.login, + repo: payload.repository.name, + head_sha: "", + issue_number: NaN, + run_id: NaN, + }; } else if ( context.eventName === "workflow_run" && context.payload.action === "completed" diff --git a/.github/test/context.test.js b/.github/test/context.test.js index 9dcd9740a8fa..c37990fb7933 100644 --- a/.github/test/context.test.js +++ b/.github/test/context.test.js @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import { extractInputs } from "../src/context.js"; -import { createMockCore } from "./mocks.js"; +import { createMockCore, createMockGithub } from "./mocks.js"; describe("extractInputs", () => { it("unsupported_event", async () => { @@ -46,50 +46,30 @@ describe("extractInputs", () => { }); }); - it("workflow_dispatch", async () => { - const context = { - eventName: "workflow_dispatch", - payload: { - repository: { - name: "TestRepoName", - owner: { - login: "TestRepoOwnerLogin", - }, - }, - }, - }; - - await expect( - extractInputs(null, context, createMockCore()), - ).resolves.toEqual({ - owner: "TestRepoOwnerLogin", - repo: "TestRepoName", - head_sha: "", - issue_number: NaN, - run_id: NaN, + it("issue_comment:edited", async () => { + const github = createMockGithub(); + github.rest.pulls.get.mockResolvedValue({ + data: { head: { sha: "abc123" } }, }); - }); - it("check_suite:completed (same repo)", async () => { const context = { - eventName: "check_suite", + eventName: "issue_comment", payload: { - action: "completed", - check_suite: { - head_sha: "abc123", - pull_requests: [{ number: 123 }], - }, + action: "edited", repository: { name: "TestRepoName", owner: { login: "TestRepoOwnerLogin", }, }, + issue: { + number: 123, + }, }, }; await expect( - extractInputs(null, context, createMockCore()), + extractInputs(github, context, createMockCore()), ).resolves.toEqual({ owner: "TestRepoOwnerLogin", repo: "TestRepoName", @@ -97,17 +77,18 @@ describe("extractInputs", () => { issue_number: 123, run_id: NaN, }); + + expect(github.rest.pulls.get).toHaveBeenCalledWith({ + owner: "TestRepoOwnerLogin", + repo: "TestRepoName", + pull_number: 123, + }); }); - it("check_suite:completed (fork repo)", async () => { + it("workflow_dispatch", async () => { const context = { - eventName: "check_suite", + eventName: "workflow_dispatch", payload: { - action: "completed", - check_suite: { - head_sha: "abc123", - pull_requests: null, - }, repository: { name: "TestRepoName", owner: { @@ -117,33 +98,15 @@ describe("extractInputs", () => { }, }; - const github = { - rest: { - repos: { - listPullRequestsAssociatedWithCommit: vi.fn().mockResolvedValue({ - data: [{ number: 123 }], - }), - }, - }, - }; - await expect( - extractInputs(github, context, createMockCore()), + extractInputs(null, context, createMockCore()), ).resolves.toEqual({ owner: "TestRepoOwnerLogin", repo: "TestRepoName", - head_sha: "abc123", - issue_number: 123, + head_sha: "", + issue_number: NaN, run_id: NaN, }); - - expect( - github.rest.repos.listPullRequestsAssociatedWithCommit, - ).toHaveBeenCalledWith({ - owner: "TestRepoOwnerLogin", - repo: "TestRepoName", - commit_sha: "abc123", - }); }); it("workflow_run:unsupported_action", async () => { diff --git a/.github/test/mocks.js b/.github/test/mocks.js index 529fbe6c7cec..399f869ca91e 100644 --- a/.github/test/mocks.js +++ b/.github/test/mocks.js @@ -14,9 +14,12 @@ export function createMockGithub() { }, issues: { addLabels: vi.fn(), - listLabelsOnIssue: vi.fn().mockRejectedValue({ data: [] }), + listLabelsOnIssue: vi.fn().mockResolvedValue({ data: [] }), removeLabel: vi.fn(), }, + pulls: { + get: vi.fn(), + }, }, }; } diff --git a/.github/workflows/arm-auto-signoff-preview.yaml b/.github/workflows/arm-auto-signoff-preview.yaml index 26e73bdfb18b..5b27f88b34fe 100644 --- a/.github/workflows/arm-auto-signoff-preview.yaml +++ b/.github/workflows/arm-auto-signoff-preview.yaml @@ -1,14 +1,12 @@ name: ARM Auto Signoff (Preview) on: - # TODO: Re-enable once we figure out how to find the PR number from a check_suite:completed event from a fork PR - # check_suite: - # Depends on check_run status, so must re-evaluate whenever a check_suite is completed. - # Could instead trigger on check_run events, but I think this is unnecessarily noisy. - # types: [completed] + issue_comment: + types: + - edited pull_request: types: - # Depends on labels, so must re-evaluate whenever a label is manually added or removed. + # Depends on labels, so must re-evaluate whenever a relevant label is manually added or removed. - labeled - unlabeled # Depends on changed files and check_run status, so must re-evaluate whenever either could change @@ -42,6 +40,26 @@ jobs: arm-auto-signoff-preview: name: ARM Auto Signoff (Preview) + # pull_request:labeled - filter to only the input and output labels + # issue_comment:edited - filter to only PR comments containing "next steps to merge", + # a signal that "Swagger LintDiff" status may have changed + if: | + github.event_name == 'workflow_dispatch' || + (github.event_name == 'pull_request' && + ((github.event.action == 'opened' || + github.event.action == 'reopened' || + github.event.action == 'synchronize') || + ((github.event.action == 'labeled' || + github.event.action == 'unlabeled') && + (github.event.label.name == 'ARMReview' || + github.event.label.name == 'NotReadyForARMReview' || + github.event.label.name == 'SuppressionReviewRequired' || + github.event.label.name == 'Suppression-Approved' || + github.event.label.name == 'ARMAutoSignOffPreview')))) || + (github.event_name == 'issue_comment' && + github.event.issue.pull_request && + contains(github.event.comment.body, 'next steps to merge')) + runs-on: ubuntu-24.04 steps: