Skip to content

Commit

Permalink
[drci] Exclude lint from broken trunk (#5717)
Browse files Browse the repository at this point in the history
Fixes #5596

The current log classifier/line capture mechanism allows real failures
to be hidden by existing failures, resulting in multiple lint breakages.
Lint is a fast and stable job so we should force people to rebase
  • Loading branch information
clee2000 authored Sep 26, 2024
1 parent 33fcd21 commit 616ee5d
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 9 deletions.
17 changes: 13 additions & 4 deletions torchci/lib/drciUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const EXCLUDED_FROM_FLAKINESS = [
"/ build",
"check labels",
];
export const EXCLUDED_FROM_BROKEN_TRUNK = ["lint"];
// If the base commit is too old, don't query for similar failures because
// it increases the risk of getting misclassification. This guardrail can
// be relaxed once we achieve better accuracy from the log classifier. This
Expand Down Expand Up @@ -408,19 +409,27 @@ export async function isLogClassifierFailed(
return job.conclusion === "failure" && (!hasFailureLines || !hasLog);
}

export function isExcludedFromFlakiness(job: RecentWorkflowsData): boolean {
// Lintrunner job are generally stable and should be excluded from flakiness
// detection
function isExcluded(job: RecentWorkflowsData, excludedJobs: string[]): boolean {
return (
_.find(
EXCLUDED_FROM_FLAKINESS,
excludedJobs,
(exclude: string) =>
job.name !== "" &&
job.name.toLowerCase().includes(exclude.toLowerCase())
) !== undefined
);
}

export function isExcludedFromBrokenTrunk(job: RecentWorkflowsData): boolean {
// Lintrunner job are generally stable and should be excluded from broken trunk
// detection
return isExcluded(job, EXCLUDED_FROM_BROKEN_TRUNK);
}

export function isExcludedFromFlakiness(job: RecentWorkflowsData): boolean {
return isExcluded(job, EXCLUDED_FROM_FLAKINESS);
}

export async function fetchIssueLabels(
octokit: Octokit,
owner: string,
Expand Down
6 changes: 6 additions & 0 deletions torchci/pages/api/drci/drci.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
hasSimilarFailures,
hasSimilarFailuresInSamePR,
HUD_URL,
isExcludedFromBrokenTrunk,
isExcludedFromFlakiness,
isExcludedFromSimilarityPostProcessing,
isInfraFlakyJob,
Expand Down Expand Up @@ -859,6 +860,11 @@ export async function getWorkflowJobsStatuses(
continue;
}

if (isExcludedFromBrokenTrunk(job)) {
failedJobs.push(job);
continue;
}

const trunkFailure = getTrunkFailure(job, baseJobs);
if (trunkFailure !== undefined) {
brokenTrunkJobs.push(job);
Expand Down
11 changes: 6 additions & 5 deletions torchci/test/drci.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const pendingA = getDummyJob({
});

const failedA = getDummyJob({
name: "Lint",
name: "somethingA",
conclusion: "failure",
completed_at: "2022-07-13 19:34:03",
html_url: "a",
Expand All @@ -88,7 +88,7 @@ const failedA = getDummyJob({
});

const failedASuccessfulRetry = getDummyJob({
name: "Lint",
name: "somethingA",
conclusion: "success",
completed_at: "2022-07-14 19:34:03",
html_url: "a",
Expand All @@ -100,7 +100,7 @@ const failedASuccessfulRetry = getDummyJob({
});

const failedAFailedRetry = getDummyJob({
name: "Lint",
name: "somethingA",
conclusion: "failure",
completed_at: "2022-07-15 19:34:03",
html_url: "a",
Expand Down Expand Up @@ -384,10 +384,11 @@ describe("Update Dr. CI Bot Unit Tests", () => {

expect(failureInfo.includes("3 New Failures, 1 Pending")).toBeTruthy();
expect(failureInfo.includes(failedJobName)).toBeTruthy();
const expectedFailureOrder = `* [Lint](${HUD_URL}/pr/pytorch/pytorch/123#1) ([gh](a))
\`mind blown\`
const expectedFailureOrder = `
* [something](${HUD_URL}/pr/pytorch/pytorch/123#1) ([gh](a))
\`cde\`
* [somethingA](${HUD_URL}/pr/pytorch/pytorch/123#1) ([gh](a))
\`mind blown\`
* [z-docs / build-docs (cpp)](${HUD_URL}/pr/pytorch/pytorch/123#1) ([gh](a))
\`bababa\``;
expect(failureInfo.includes(expectedFailureOrder)).toBeTruthy();
Expand Down
11 changes: 11 additions & 0 deletions torchci/test/drciUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,17 @@ describe("Test various utils used by Dr.CI", () => {
expect(isExcludedFromFlakiness(notExcludedJob)).toEqual(false);
});

test("test isExcludedFromBrokenTrunk", () => {
const notExcludedJob: RecentWorkflowsData = getDummyJob({});
expect(drciUtils.isExcludedFromBrokenTrunk(notExcludedJob)).toEqual(false);

const excludedJob: RecentWorkflowsData = {
...notExcludedJob,
name: "LinT / quick-checks / linux-job",
};
expect(drciUtils.isExcludedFromBrokenTrunk(excludedJob)).toEqual(true);
});

test("test getSuppressedLabels", () => {
const job: RecentWorkflowsData = getDummyJob({
jobName: "not suppressed job",
Expand Down

0 comments on commit 616ee5d

Please sign in to comment.