From a3d30d6fe42d1f5abdfc40d9d3f294451c6c911c Mon Sep 17 00:00:00 2001 From: clee2000 <44682903+clee2000@users.noreply.github.com> Date: Thu, 14 Nov 2024 11:00:48 -0800 Subject: [PATCH] [ez][drci] Make merge blocking sevs be based on label instead of string (#5913) Corresponds to https://github.com/pytorch/pytorch/pull/140636 --- .../clickhouse_queries/issue_query/query.sql | 3 +- torchci/lib/drciUtils.ts | 10 +-- torchci/lib/types.ts | 1 + torchci/test/disableFlakyBot.test.ts | 18 ++++- torchci/test/drci.test.ts | 76 ++----------------- torchci/test/jobUtils.test.ts | 4 + 6 files changed, 29 insertions(+), 83 deletions(-) diff --git a/torchci/clickhouse_queries/issue_query/query.sql b/torchci/clickhouse_queries/issue_query/query.sql index cc0ee8537b..a2392418eb 100644 --- a/torchci/clickhouse_queries/issue_query/query.sql +++ b/torchci/clickhouse_queries/issue_query/query.sql @@ -6,8 +6,9 @@ SELECT issue.body, issue.updated_at, issue.author_association, + arrayMap(x -> x.'name', issue.labels) as labels FROM - issues AS issue final + default.issues AS issue final array join issue.labels AS label WHERE label.name = {label: String} diff --git a/torchci/lib/drciUtils.ts b/torchci/lib/drciUtils.ts index 5a9c178147..89447efac4 100644 --- a/torchci/lib/drciUtils.ts +++ b/torchci/lib/drciUtils.ts @@ -131,15 +131,7 @@ export async function getDrciComment( } export function isMergeBlockingSev(issue: IssueData): boolean { - let body = issue.body; - let previousBody; - do { - // Remove all matching pairs of comments - previousBody = body; - body = body.replace(//gm, ""); - } while (body !== previousBody); - - return body.includes("merge blocking"); + return issue.labels.includes("merge blocking"); } export function getActiveSEVs(issues: IssueData[]): [IssueData[], IssueData[]] { diff --git a/torchci/lib/types.ts b/torchci/lib/types.ts index b76c908075..8067149442 100644 --- a/torchci/lib/types.ts +++ b/torchci/lib/types.ts @@ -106,6 +106,7 @@ export interface IssueData { body: string; updated_at: string; author_association: string; + labels: string[]; } export interface HudParams { diff --git a/torchci/test/disableFlakyBot.test.ts b/torchci/test/disableFlakyBot.test.ts index 2977c2efa2..5cce98d839 100644 --- a/torchci/test/disableFlakyBot.test.ts +++ b/torchci/test/disableFlakyBot.test.ts @@ -166,6 +166,7 @@ describe("Disable Flaky Test Bot Across Jobs", () => { body: "random", updated_at: dayjs().toString(), author_association: "MEMBER", + labels: [], }, ]; @@ -188,6 +189,7 @@ describe("Disable Flaky Test Bot Across Jobs", () => { body: "random", updated_at: dayjs().toString(), author_association: "MEMBER", + labels: [], }, ]; @@ -223,6 +225,7 @@ describe("Disable Flaky Test Bot Across Jobs", () => { body: "random", updated_at: dayjs().toString(), author_association: "MEMBER", + labels: [], }, ]; @@ -245,6 +248,7 @@ describe("Disable Flaky Test Bot Across Jobs", () => { body: "random", updated_at: dayjs().toString(), author_association: "MEMBER", + labels: [], }, ]; @@ -329,7 +333,7 @@ describe("Disable Flaky Test Bot Integration Tests", () => { }) .reply(200, {}); - const issues = [ + const issues: IssueData[] = [ { number: 1, title: "DISABLED test_a (__main__.suite_a)", @@ -338,6 +342,7 @@ describe("Disable Flaky Test Bot Integration Tests", () => { body: "random", updated_at: dayjs().toString(), author_association: "MEMBER", + labels: [], }, ]; @@ -363,7 +368,7 @@ describe("Disable Flaky Test Bot Integration Tests", () => { }) .reply(200, {}); - const issues = [ + const issues: IssueData[] = [ { number: 1, title: "DISABLED test_a (__main__.suite_a)", @@ -372,6 +377,7 @@ describe("Disable Flaky Test Bot Integration Tests", () => { body: "random", updated_at: dayjs().toString(), author_association: "MEMBER", + labels: [], }, ]; @@ -405,7 +411,7 @@ describe("Disable Flaky Test Bot Integration Tests", () => { }) .reply(200, {}); - const issues = [ + const issues: IssueData[] = [ { number: 1, title: "DISABLED test_a (__main__.suite_a)", @@ -419,6 +425,7 @@ describe("Disable Flaky Test Bot Integration Tests", () => { ) .toString(), author_association: "MEMBER", + labels: [], }, ]; @@ -445,7 +452,7 @@ describe("Disable Flaky Test Bot Integration Tests", () => { }) .reply(200, {}); - const issues = [ + const issues: IssueData[] = [ { number: 1, title: "DISABLED test_a (__main__.suite_a)", @@ -459,6 +466,7 @@ describe("Disable Flaky Test Bot Integration Tests", () => { ) .toString(), author_association: "MEMBER", + labels: [], }, ]; @@ -931,6 +939,7 @@ describe("Disable Flaky Test Bot Unit Tests", () => { body: "", updated_at: "", author_association: "MEMBER", + labels: [], }; const closedSmall: IssueData = { ...openSmall, number: 2, state: "closed" }; const openBig: IssueData = { ...openSmall, number: 3 }; @@ -990,6 +999,7 @@ describe("Disable Flaky Test Bot Unit Tests", () => { body: "", updated_at: "", author_association: "MEMBER", + labels: [], }; beforeEach(() => {}); diff --git a/torchci/test/drci.test.ts b/torchci/test/drci.test.ts index 4a623a68ab..e3f391f0cb 100644 --- a/torchci/test/drci.test.ts +++ b/torchci/test/drci.test.ts @@ -9,7 +9,6 @@ import { formDrciSevBody, getActiveSEVs, HUD_URL, - isMergeBlockingSev, OH_URL, } from "lib/drciUtils"; import * as fetchPR from "lib/fetchPR"; @@ -266,6 +265,7 @@ const sev: IssueData = { body: "random stuff", updated_at: dayjs().toString(), author_association: "MEMBER", + labels: [], }; const mergeBlockingSev: IssueData = { @@ -273,9 +273,10 @@ const mergeBlockingSev: IssueData = { title: "Linux CUDA builds are failing due to missing deps", html_url: "https://github.com/pytorch/pytorch/issues/74967", state: "open", - body: "**merge blocking**", + body: "", updated_at: dayjs().toString(), author_association: "MEMBER", + labels: ["merge blocking"], }; const notMergeBlockingSev: IssueData = { @@ -283,9 +284,10 @@ const notMergeBlockingSev: IssueData = { title: "Linux CUDA builds are failing due to missing deps", html_url: "https://github.com/pytorch/pytorch/issues/74967", state: "open", - body: " ", + body: "not merge blocking", updated_at: dayjs().toString(), author_association: "MEMBER", + labels: [], }; const closedSev: IssueData = { @@ -296,6 +298,7 @@ const closedSev: IssueData = { body: "random stuff", updated_at: dayjs().toString(), author_association: "MEMBER", + labels: [], }; function constructResultsCommentHelper({ @@ -492,72 +495,6 @@ describe("Update Dr. CI Bot Unit Tests", () => { expect(formDrciSevBody(getActiveSEVs([closedSev])) === "").toBeTruthy(); }); - function createIssueData(body: string): IssueData { - return { - number: 1, - title: "Test Issue", - html_url: "https://example.com", - state: "open", - body: body, - updated_at: "2023-10-10T10:00:00Z", - author_association: "MEMBER", - }; - } - - test("should correctly identify merge blocking issues", () => { - expect(isMergeBlockingSev(createIssueData("**merge blocking**"))).toBe( - true - ); - expect( - isMergeBlockingSev(createIssueData("")) - ).toBe(false); - expect( - isMergeBlockingSev(createIssueData("This is a merge blocking issue.")) - ).toBe(true); - expect( - isMergeBlockingSev( - createIssueData( - "Some text before **merge blocking** and some text after." - ) - ) - ).toBe(true); - expect( - isMergeBlockingSev( - createIssueData( - "Some text before and some text after." - ) - ) - ).toBe(false); - expect( - isMergeBlockingSev(createIssueData("Line 1\n**merge blocking**\nLine 3")) - ).toBe(true); - expect( - isMergeBlockingSev( - createIssueData("Line 1\n\nLine 3") - ) - ).toBe(false); - expect( - isMergeBlockingSev( - createIssueData("Line 1\n\nLine 4") - ) - ).toBe(false); - expect( - isMergeBlockingSev( - createIssueData("Line 1\n\nLine 4") - ) - ).toBe(false); - expect( - isMergeBlockingSev( - createIssueData(" -->") - ) - ).toBe(false); - expect( - isMergeBlockingSev( - createIssueData(" **merge blocking** ") - ) - ).toBe(true); - }); - test("test form dr ci comment with sevs", async () => { const originalWorkflows = [successfulA, pendingA, failedA]; const workflowsByPR = await updateDrciBot.reorganizeWorkflows( @@ -667,6 +604,7 @@ describe("Update Dr. CI Bot Unit Tests", () => { body: "", updated_at: "2024-04-24 00:44:19", author_association: "CONTRIBUTOR", + labels: [], }, ]; const pr_1001 = workflowsByPR.get(1001)!; diff --git a/torchci/test/jobUtils.test.ts b/torchci/test/jobUtils.test.ts index c18beae022..612a1e74da 100644 --- a/torchci/test/jobUtils.test.ts +++ b/torchci/test/jobUtils.test.ts @@ -574,6 +574,7 @@ describe("Test various job utils", () => { updated_at: "", author_association: "", html_url: "", + labels: [], }; expect(isDisabledTest([])).toEqual(false); @@ -642,6 +643,7 @@ describe("Test various job utils", () => { updated_at: "", author_association: "", html_url: "", + labels: [], }; expect(isDisabledTestMentionedInPR([], prInfo)).toEqual(false); @@ -793,6 +795,7 @@ describe("Test various job utils", () => { updated_at: "", author_association: "", html_url: "", + labels: [], }; // At least one of the issue is still open @@ -888,6 +891,7 @@ describe("Test various job utils", () => { updated_at: "2024-05-06T00:30:00Z", author_association: "", html_url: "", + labels: [], }; // Invalid input should return nothing