From 7254ee152922e1f64d4cf5851650b4d7dff09739 Mon Sep 17 00:00:00 2001 From: clee2000 <44682903+clee2000@users.noreply.github.com> Date: Fri, 6 Sep 2024 15:44:26 -0700 Subject: [PATCH] [drci] Fix check labels being marked as flaky (#5629) Move check for if something is in the `EXCLUDED_FROM_FLAKINESS` to be earlier and more explicit since some of the checks for flakiness weren't checking if it was on the list first Some lower/upper case stuff Tested on https://togithub.com/pytorch/pytorch/pull/135391#issuecomment-2334801691. If it's no longer there, it's because it got overwritten, but here's a screenshot image and the output after calling the API (see that it is in the failed list) ``` csl@csl-mbp ~/zzzzzzzz/test-infra/torchci [csl/fix_drci_check_labels_flaky] $ (forpytorch) curl "http://localhost:3000/api/drci/drci?prNumber=135391" --data 'repo=pytorch' {"135391":{"FAILED":[{"workflowId":10745156151,"workflowUniqueId":38959716,"id":29803617987,"runnerName":"linux.20_04.4x_2f4e0a1ae568","authorEmail":"zou3519@gmail.com","name":"Check Labels / Check labels","jobName":"Check labels","conclusion":"failure","completed_at":"2024-09-06T21:00:09Z","html_url":"https://github.com/pytorch/pytorch/actions/runs/10745156151/job/29803617987","head_branch":"gh/zou3519/1065/head","pr_number":135391,"head_sha":"3e458267c6cd378e9b8485daaa2be9fa1d369293","head_sha_timestamp":"2024-09-06T13:59:24-07:00","failure_captures":["# This PR needs a `release notes:` label"],"failure_lines":["# This PR needs a `release notes:` label"],"failure_context":["+ python3 .github/scripts/check_labels.py --exit-non-zero 135391"],"time":"2024-09-06T21:00:12.817762Z"}],"FLAKY":[],"BROKEN_TRUNK":[],"UNSTABLE":[]}}% ``` --- torchci/lib/drciUtils.ts | 5 +++-- torchci/pages/api/drci/drci.ts | 10 +++++++--- torchci/test/drci.test.ts | 15 ++++++++++----- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/torchci/lib/drciUtils.ts b/torchci/lib/drciUtils.ts index f6add1df75..be7c9e8a0e 100644 --- a/torchci/lib/drciUtils.ts +++ b/torchci/lib/drciUtils.ts @@ -40,7 +40,7 @@ export const EXCLUDED_FROM_FLAKINESS = [ "pr-sanity-checks", // TODO (huydhn): Figure out a way to do flaky check accurately for build jobs "/ build", - "Check labels", + "check labels", ]; // If the base commit is too old, don't query for similar failures because // it increases the risk of getting misclassification. This guardrail can @@ -404,7 +404,8 @@ export function isExcludedFromFlakiness(job: RecentWorkflowsData): boolean { _.find( EXCLUDED_FROM_FLAKINESS, (exclude: string) => - job.name !== undefined && job.name.toLowerCase().includes(exclude) + job.name !== undefined && + job.name.toLowerCase().includes(exclude.toLowerCase()) ) !== undefined ); } diff --git a/torchci/pages/api/drci/drci.ts b/torchci/pages/api/drci/drci.ts index 255ca9d970..ab090e87f6 100644 --- a/torchci/pages/api/drci/drci.ts +++ b/torchci/pages/api/drci/drci.ts @@ -824,6 +824,7 @@ export async function getWorkflowJobsStatuses( const flakyJobs: RecentWorkflowsData[] = []; const brokenTrunkJobs: RecentWorkflowsData[] = []; const unstableJobs: RecentWorkflowsData[] = []; + const failedJobs: RecentWorkflowsData[] = []; // This map holds the list of the base failures for broken trunk jobs or the similar // failures for flaky jobs @@ -867,6 +868,11 @@ export async function getWorkflowJobsStatuses( continue; } + if (isExcludedFromFlakiness(job)) { + failedJobs.push(job); + continue; + } + const flakyRule = isFlaky(job, flakyRules); if (flakyRule !== undefined) { flakyJobs.push(job); @@ -883,7 +889,7 @@ export async function getWorkflowJobsStatuses( continue; } - if ((await isLogClassifierFailed(job)) && !isExcludedFromFlakiness(job)) { + if (await isLogClassifierFailed(job)) { flakyJobs.push(job); relatedInfo.set( job.id, @@ -972,8 +978,6 @@ export async function getWorkflowJobsStatuses( } } - const failedJobs: RecentWorkflowsData[] = []; - // Verify that the failed job is unique and there is no similar flaky, broken trunk, // or unstable jobs in the same pull request. If there are some, these failures are // also considered unrelated diff --git a/torchci/test/drci.test.ts b/torchci/test/drci.test.ts index b0ea45fd82..955cadc316 100644 --- a/torchci/test/drci.test.ts +++ b/torchci/test/drci.test.ts @@ -597,7 +597,12 @@ describe("Update Dr. CI Bot Unit Tests", () => { }); test(" test flaky rule regex", async () => { - const originalWorkflows = [failedA, failedG, failedH, failedI]; + const originalWorkflows = [ + failedA, // failure + failedG, // failure, matches rule, but is build -> failure + failedH, // failure, matches rule -> flaky + failedI, // failure, matches rule -> flaky + ]; const workflowsByPR = await updateDrciBot.reorganizeWorkflows( "pytorch", "pytorch", @@ -625,9 +630,9 @@ describe("Update Dr. CI Bot Unit Tests", () => { ], new Map() ); - expect(failedJobs.length).toBe(1); + expect(failedJobs.length).toBe(2); expect(brokenTrunkJobs.length).toBe(0); - expect(flakyJobs.length).toBe(3); + expect(flakyJobs.length).toBe(2); expect(unstableJobs.length).toBe(0); }); @@ -787,7 +792,7 @@ describe("Update Dr. CI Bot Unit Tests", () => { }) ); - const originalWorkflows = [failedA, failedB]; + const originalWorkflows = [failedB]; const workflowsByPR = await updateDrciBot.reorganizeWorkflows( "pytorch", "pytorch", @@ -798,7 +803,7 @@ describe("Update Dr. CI Bot Unit Tests", () => { await updateDrciBot.getWorkflowJobsStatuses(pr_1001, [], new Map()); expect(failedJobs.length).toBe(0); expect(brokenTrunkJobs.length).toBe(0); - expect(flakyJobs.length).toBe(2); + expect(flakyJobs.length).toBe(1); expect(unstableJobs.length).toBe(0); });