[drci] Mark unstable pending jobs as unstable (#6283)

Jobs only get categorized after they fail, but if a job is marked as
unstable, it will always end up in the unstable group. This PR makes it
so that unstable pending jobs get put in the unstable group. I think
this should help with trymerge waiting for unstable jobs to finish,
since we sometimes mark jobs as unstable if the queue time is very long

Pending unstable jobs still count towards the pending count

This was tested on a random PR on pytorch by curling my local
development

Pending jobs will show up in Dr. CI, which might be confusing as people
usually expect only failing jobs to show up, so it also adds a hour
glass icon to the pending unstable jobs, which looks like
<img width="844" alt="image"
src="https://github.com/user-attachments/assets/52a6df6b-5aef-4550-ac04-5d6d9b5322aa"
/>

Author: clee2000

torchci/pages/api/drci/drci.ts

@@ -544,7 +544,8 @@ function constructResultsJobsSections(
   const hudPrUrl = `${hudBaseUrl}/pr/${owner}/${repo}/${prNumber}`;
   const jobsSorted = jobs.sort((a, b) => a.name.localeCompare(b.name));
   for (const job of jobsSorted) {
-    output += `* [${job.name}](${hudPrUrl}#${job.id}) ([gh](${job.html_url}))`;
+    const isPendingIcon = isPending(job) ? ":hourglass_flowing_sand: " : "";
+    output += `* ${isPendingIcon}[${job.name}](${hudPrUrl}#${job.id}) ([gh](${job.html_url}))`;
 
     const relatedJob = relatedJobs.get(job.id);
     // Show the related trunk failure for broken trunk or the similar failure for flaky
@@ -622,8 +623,12 @@ export function constructResultsComment(
   prNumber: number
 ): string {
   let output = `\n`;
-  const unrelatedFailureCount =
-    flakyJobs.length + brokenTrunkJobs.length + unstableJobs.length;
+  // Filter out unstable pending jobs
+  const unrelatedFailureCount = _(flakyJobs)
+    .concat(brokenTrunkJobs)
+    .concat(unstableJobs)
+    .filter((job) => !isPending(job))
+    .value().length;
   const newFailedJobs: RecentWorkflowsData[] = failedJobs.filter(
     (job) =>
       job.conclusion !== "cancelled" &&
@@ -634,11 +639,7 @@ export function constructResultsComment(
       job.conclusion === "cancelled" ||
       job.failure_captures.includes(CANCELLED_STEP_ERROR)
   );
-  const failing =
-    failedJobs.length +
-    flakyJobs.length +
-    brokenTrunkJobs.length +
-    unstableJobs.length;
+  const failing = failedJobs.length + unrelatedFailureCount;
   const headerPrefix = `## `;
   const pendingIcon = `:hourglass_flowing_sand:`;
   const successIcon = `:white_check_mark:`;
@@ -662,8 +663,7 @@ export function constructResultsComment(
   const hasSignificantFailures = newFailedJobs.length > 0;
   const hasCancelledFailures = cancelledJobs.length > 0;
   const hasPending = pending > 0;
-  const hasUnrelatedFailures =
-    flakyJobs.length + brokenTrunkJobs.length + unstableJobs.length;
+  const hasUnrelatedFailures = unrelatedFailureCount > 0;
 
   let icon = "";
   if (hasSignificantFailures || hasCancelledFailures) {
@@ -795,14 +795,11 @@ export function constructResultsComment(
     repo,
     prNumber,
     "UNSTABLE",
-    `The following ${pluralize(
-      "job",
-      unstableJobs.length
-    )} failed but ${pluralize(
-      "was",
+    `The following ${pluralize("job", unstableJobs.length)} ${pluralize(
+      "is",
       unstableJobs.length,
-      "were"
-    )} likely due to flakiness present on trunk and has been marked as unstable`,
+      "are"
+    )} marked as unstable, possibly due to flakiness on trunk`,
     unstableJobs,
     "",
     true,
@@ -855,6 +852,10 @@ function getTrunkFailure(
     .find((baseJob) => isSameFailure(baseJob, job));
 }
 
+function isPending(job: RecentWorkflowsData): boolean {
+  return job.conclusion === "" && isTime0(job.completed_at);
+}
+
 export async function getWorkflowJobsStatuses(
   prInfo: PRandJobs,
   flakyRules: FlakyRule[],
@@ -889,8 +890,15 @@ export async function getWorkflowJobsStatuses(
   const relatedInfo: Map<number, string> = new Map();
 
   for (const job of prInfo.jobs) {
-    if (job.conclusion === "" && isTime0(job.completed_at)) {
+    if (isPending(job)) {
       pending++;
+      if (isUnstableJob(job as any, unstableIssues)) {
+        unstableJobs.push(job);
+        relatedIssues.set(
+          job.id,
+          getOpenUnstableIssues(job.name, unstableIssues)
+        );
+      }
     } else if (job.conclusion === "failure" || job.conclusion === "cancelled") {
       const suppressedLabels = await getSuppressedLabels(job, labels);
       if (prInfo.repo === "pytorch" && suppressedLabels.length !== 0) {

torchci/test/drci.test.ts

@@ -636,14 +636,41 @@ describe("Update Dr. CI Bot Unit Tests", () => {
       "The following job has failed",
       "The following job failed but was likely due to flakiness present on trunk",
       "The following job failed but was present on the merge base",
-      "The following job failed but was likely due to flakiness present on trunk and has been marked as unstable",
+      "The following job is marked as unstable",
       failedA.name,
       failedB.name,
       failedC.name,
       unstableA.name,
     ];
     expect(
-      expectToContain.every((s) => failureInfoComment.includes(s!))
+      expectToContain.every((s) => failureInfoComment.includes(s))
+    ).toBeTruthy();
+  });
+
+  test("test pending unstable job", async () => {
+    // Test that a pending unstable job gets included in the comment as
+    // unstable, but doesn't count as failed in the overall count
+    const failureInfoComment = constructResultsCommentHelper({
+      pending: 1,
+      failedJobs: [failedA],
+      flakyJobs: [failedB],
+      brokenTrunkJobs: [failedC],
+      unstableJobs: [{ ...unstableA, conclusion: "", completed_at: TIME_0 }],
+      merge_base: "random base sha",
+    });
+    const expectToContain = [
+      "1 New Failure, 1 Pending, 2 Unrelated Failures",
+      "The following job has failed",
+      "The following job failed but was likely due to flakiness present on trunk",
+      "The following job failed but was present on the merge base",
+      "The following job is marked as unstable",
+      failedA.name,
+      failedB.name,
+      failedC.name,
+      unstableA.name,
+    ];
+    expect(
+      expectToContain.every((s) => failureInfoComment.includes(s))
     ).toBeTruthy();
   });