[ez][drci] Make merge blocking sevs be based on label instead of stri…

…ng (#5913) Corresponds to pytorch/pytorch#140636
pytorch · Nov 14, 2024 · a3d30d6 · a3d30d6
1 parent 6c870c6
commit a3d30d6
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 83 deletions.
diff --git 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
@@ -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(/<!--[\s\S]*?-->/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
@@ -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
@@ -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
@@ -9,7 +9,6 @@ import {
   formDrciSevBody,
   getActiveSEVs,
   HUD_URL,
-  isMergeBlockingSev,
   OH_URL,
 } from "lib/drciUtils";
 import * as fetchPR from "lib/fetchPR";
@@ -266,26 +265,29 @@ const sev: IssueData = {
   body: "random stuff",
   updated_at: dayjs().toString(),
   author_association: "MEMBER",
+  labels: [],
 };
 
 const mergeBlockingSev: IssueData = {
   number: 74967,
   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 = {
   number: 74967,
   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: "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("<!-- **merge blocking** -->"))
-    ).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 <!-- **merge blocking** --> and some text after."
-        )
-      )
-    ).toBe(false);
-    expect(
-      isMergeBlockingSev(createIssueData("Line 1\n**merge blocking**\nLine 3"))
-    ).toBe(true);
-    expect(
-      isMergeBlockingSev(
-        createIssueData("Line 1\n<!-- **merge blocking** -->\nLine 3")
-      )
-    ).toBe(false);
-    expect(
-      isMergeBlockingSev(
-        createIssueData("Line 1\n<!--\n **merge blocking** -->\nLine 4")
-      )
-    ).toBe(false);
-    expect(
-      isMergeBlockingSev(
-        createIssueData("Line 1\n<!--\nLine 2 merge blocking** -->\nLine 4")
-      )
-    ).toBe(false);
-    expect(
-      isMergeBlockingSev(
-        createIssueData("<!-- **merge blocking** <!-- --> -->")
-      )
-    ).toBe(false);
-    expect(
-      isMergeBlockingSev(
-        createIssueData("<!-- test --> **merge blocking**  <!-- test2 -->")
-      )
-    ).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
@@ -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