From dde0b32e173af101388d3dc0cead147140f50be4 Mon Sep 17 00:00:00 2001 From: Edward Yang Date: Fri, 8 May 2026 11:19:24 -0400 Subject: [PATCH] Update [ghstack-poisoned] --- torchci/pages/api/drci/drci.ts | 32 ++++++++++++------ torchci/test/drci.test.ts | 59 +++++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 10 deletions(-) diff --git a/torchci/pages/api/drci/drci.ts b/torchci/pages/api/drci/drci.ts index e633ad597a..f433dba0c6 100644 --- a/torchci/pages/api/drci/drci.ts +++ b/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); diff --git a/torchci/test/drci.test.ts b/torchci/test/drci.test.ts index 311f504066..99485cd09b 100644 --- a/torchci/test/drci.test.ts +++ b/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([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(() =>