Skip to content

Commit

Permalink
[drci] Fix check labels being marked as flaky (#5629)
Browse files Browse the repository at this point in the history
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
<img width="807" alt="image"
src="https://github.com/user-attachments/assets/f28265b8-507a-45ea-8ea7-b5832e15026b">

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":"[email protected]","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":[]}}%       
```
  • Loading branch information
clee2000 authored Sep 6, 2024
1 parent 215af5b commit 7254ee1
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 10 deletions.
5 changes: 3 additions & 2 deletions torchci/lib/drciUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
);
}
Expand Down
10 changes: 7 additions & 3 deletions torchci/pages/api/drci/drci.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down
15 changes: 10 additions & 5 deletions torchci/test/drci.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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);
});

Expand Down Expand Up @@ -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",
Expand All @@ -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);
});

Expand Down

0 comments on commit 7254ee1

Please sign in to comment.