[ARMAutoSignOff] Trigger on issue_comment:edited when body contains "…

…next steps to merge" (#32554)
Azure · Feb 11, 2025 · 0da14ec · 0da14ec
1 parent 982bec0
commit 0da14ec
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 116 deletions.
diff --git 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
@@ -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,68 +46,49 @@ 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",
       head_sha: "abc123",
       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
@@ -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
@@ -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: