Make DrCI require audit for reverted PRs

torchci/pages/api/drci/drci.ts

@@ -62,6 +62,9 @@ export interface UpdateCommentBody {
   org?: string;
 }
 
+export const PREVIOUSLY_REVERTED_PR_AUDIT_MESSAGE =
+  "this PR was reverted before, so DrCI cannot safely classify this failure; audit this job individually";
+
 // Attempt to set the maxDuration of this serveless function on Vercel https://vercel.com/docs/functions/configuring-functions/duration,
 // also according to https://vercel.com/docs/functions/runtimes#max-duration, the max duration
 // for an enterprise account is 900
@@ -256,7 +259,10 @@ export async function updateDrciComments(
         HUD_URL,
         owner,
         repo,
-        pr_info.pr_number
+        pr_info.pr_number,
+        mergeCommits.length > 0
+          ? `DrCI did not classify these jobs because this PR was reverted before. Audit each job individually before merging`
+          : undefined
       );
 
       const comment = formDrciComment(
@@ -681,7 +687,8 @@ export function constructResultsComment(
   hudBaseUrl: string,
   owner: string,
   repo: string,
-  prNumber: number
+  prNumber: number,
+  unknownJobsDescription?: string
 ): string {
   let output = `\n`;
   // Filter out unstable pending jobs
@@ -837,13 +844,14 @@ export function constructResultsComment(
         "FAILURE",
         unknownJobs.length
       ).toLocaleUpperCase()}`,
-      `DrCI could not classify the following ${pluralize(
-        "job",
-        unknownJobs.length
-      )} because the workflow did not run on the merge base. The ${pluralize(
-        "failure",
-        unknownJobs.length
-      )} may be pre-existing on trunk or introduced by this PR`,
+      unknownJobsDescription ||
+        `DrCI could not classify the following ${pluralize(
+          "job",
+          unknownJobs.length
+        )} because the workflow did not run on the merge base. The ${pluralize(
+          "failure",
+          unknownJobs.length
+        )} may be pre-existing on trunk or introduced by this PR`,
       unknownJobs,
       "",
       true,
@@ -1036,6 +1044,12 @@ export async function getWorkflowJobsStatuses(
         );
       }
     } else if (job.conclusion === "failure" || job.conclusion === "cancelled") {
+      if (mergeCommits.length > 0) {
+        unknownJobs.push(job);
+        relatedInfo.set(job.id, PREVIOUSLY_REVERTED_PR_AUDIT_MESSAGE);
+        continue;
+      }
+
       const suppressedLabels = await getSuppressedLabels(job, labels);
       if (prInfo.repo === "pytorch" && suppressedLabels.length !== 0) {
         flakyJobs.push(job);

torchci/test/drci.test.ts

@@ -274,6 +274,7 @@ function constructResultsCommentHelper({
   owner = "pytorch",
   repo = "pytorch",
   prNumber = 123,
+  unknownJobsDescription,
 }: {
   pending?: number;
   failedJobs?: RecentWorkflowsData[];
@@ -289,6 +290,7 @@ function constructResultsCommentHelper({
   owner?: string;
   repo?: string;
   prNumber?: number;
+  unknownJobsDescription?: string;
 }) {
   return updateDrciBot.constructResultsComment(
     pending,
@@ -307,7 +309,8 @@ function constructResultsCommentHelper({
     hudBaseUrl,
     owner,
     repo,
-    prNumber
+    prNumber,
+    unknownJobsDescription
   );
 }
 
@@ -923,6 +926,44 @@ describe("Update Dr. CI Bot Unit Tests", () => {
     expect(unstableJobs.length).toBe(0);
   });
 
+  test("jobs on previously reverted PRs are left Unclassified for manual audit", async () => {
+    const originalWorkflows = [failedA, failedC];
+    const workflowsByPR = await updateDrciBot.reorganizeWorkflows(
+      "pytorch",
+      "pytorch",
+      originalWorkflows
+    );
+    const pr_1001 = workflowsByPR.get(1001)!;
+    const {
+      failedJobs,
+      brokenTrunkJobs,
+      flakyJobs,
+      unstableJobs,
+      unknownJobs,
+      relatedInfo,
+    } = await updateDrciBot.getWorkflowJobsStatuses(
+      pr_1001,
+      [],
+      new Map(),
+      [],
+      [],
+      [],
+      ["previous-merge-commit"],
+      new Set<string>([failedA.name, failedC.name])
+    );
+
+    expect(unknownJobs.map((job) => job.name).sort()).toEqual(
+      [failedA.name, failedC.name].sort()
+    );
+    expect(failedJobs.length).toBe(0);
+    expect(brokenTrunkJobs.length).toBe(0);
+    expect(flakyJobs.length).toBe(0);
+    expect(unstableJobs.length).toBe(0);
+    expect(relatedInfo.get(failedA.id)).toBe(
+      updateDrciBot.PREVIOUSLY_REVERTED_PR_AUDIT_MESSAGE
+    );
+  });
+
   test("Unknown failures are rendered in their own section and excluded from New Failures count", async () => {
     const failureInfoComment = constructResultsCommentHelper({
       pending: 0,
@@ -940,6 +981,22 @@ describe("Update Dr. CI Bot Unit Tests", () => {
     expect(failureInfoComment.includes(failedC.name!)).toBeTruthy();
   });
 
+  test("previously reverted PR comments tell users to audit jobs individually", async () => {
+    const failureInfoComment = constructResultsCommentHelper({
+      pending: 0,
+      unknownJobs: [failedA],
+      unknownJobsDescription:
+        "DrCI did not classify these jobs because this PR was reverted before. Audit each job individually before merging",
+    });
+
+    expect(failureInfoComment.includes("1 Unclassified Failure")).toBeTruthy();
+    expect(
+      failureInfoComment.includes(
+        "DrCI did not classify these jobs because this PR was reverted before. Audit each job individually before merging"
+      )
+    ).toBeTruthy();
+  });
+
   test("test similar failures marked as flaky", async () => {
     const mock = jest.spyOn(drciUtils, "hasSimilarFailures");
     mock.mockImplementation(() =>