Fix bad committer user information (#179)

rapidsai · Mar 12, 2024 · 4413ab2 · 4413ab2
1 parent 929e4bc
commit 4413ab2
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 15 deletions.
diff --git a/src/plugins/AutoMerger/auto_merger.ts b/src/plugins/AutoMerger/auto_merger.ts
@@ -199,12 +199,18 @@ export class AutoMerger extends OpsBotPlugin {
       uniqueAuthors.push(commitAuthor);
     }
 
-    return Promise.all(
-      uniqueAuthors.map(
-        async (author) =>
-          (await octokit.users.getByUsername({ username: author })).data
+    return (
+      await Promise.all(
+        uniqueAuthors.map(async (author) => {
+          try {
+            return (await octokit.users.getByUsername({ username: author }))
+              .data;
+          } catch (error) {
+            return null;
+          }
+        })
       )
-    );
+    ).filter((x): x is UsersGetByUsernameResponseData => Boolean(x));
   }
 
   /**
@@ -232,15 +238,17 @@ export class AutoMerger extends OpsBotPlugin {
       uniqueApprovers.push(approvalAuthor);
     }
 
-    return Promise.all(
-      uniqueApprovers.map(
-        async (approver) =>
-          (
-            await octokit.users.getByUsername({
-              username: approver,
-            })
-          ).data
+    return (
+      await Promise.all(
+        uniqueApprovers.map(async (approver) => {
+          try {
+            return (await octokit.users.getByUsername({ username: approver }))
+              .data;
+          } catch (error) {
+            return null;
+          }
+        })
       )
-    );
+    ).filter((x): x is UsersGetByUsernameResponseData => Boolean(x));
   }
 }
diff --git a/test/auto_merger.test.ts b/test/auto_merger.test.ts
@@ -64,13 +64,16 @@ describe("Auto Merger", () => {
     mockGetUserPermissionLevel.mockResolvedValueOnce(user_permission);
     mockPaginate.mockResolvedValueOnce(list_commits); // listCommits in getAuthors
     mockGetByUsername.mockResolvedValueOnce(userNoName);
+    mockGetByUsername.mockRejectedValueOnce(null);
     mockPaginate.mockResolvedValueOnce(list_reviews); // listReviews in getApprovers
     mockGetByUsername.mockResolvedValueOnce(user);
+    mockGetByUsername.mockRejectedValueOnce(null);
 
     await new AutoMerger(statusContext.successStatus).maybeMergePR();
 
     expect(mockPullsGet).toBeCalledTimes(1);
     expect(mockPullsGet.mock.calls[0][0].pull_number).toBe(1234);
+    expect(mockGetByUsername).toBeCalledTimes(4);
 
     expect(mockMerge.mock.calls[0][0]).toMatchObject({
       owner: "rapidsai",
@@ -115,13 +118,16 @@ URL: https://github.com/rapidsai/cudf/pull/6775`,
     mockGetUserPermissionLevel.mockResolvedValueOnce(user_permission);
     mockPaginate.mockResolvedValueOnce(list_commits); // listCommits in getAuthors
     mockGetByUsername.mockResolvedValueOnce(userNoName);
+    mockGetByUsername.mockRejectedValueOnce(null);
     mockPaginate.mockResolvedValueOnce(list_reviews); // listReviews in getApprovers
     mockGetByUsername.mockResolvedValueOnce(user);
+    mockGetByUsername.mockRejectedValueOnce(null);
 
     await new AutoMerger(statusContext.successStatus).maybeMergePR();
 
     expect(mockPullsGet).toBeCalledTimes(1);
     expect(mockPullsGet.mock.calls[0][0].pull_number).toBe(1234);
+    expect(mockGetByUsername).toBeCalledTimes(4);
 
     expect(mockMerge.mock.calls[0][0]).toMatchObject({
       owner: "rapidsai",

diff --git a/test/fixtures/responses/list_commits.json b/test/fixtures/responses/list_commits.json
@@ -27,7 +27,7 @@
     {
       "sha": "0ddcf220b748a343f78941ecb30ddcfb92f76441",
       "author": {
-        "login": "VibhuJawa"
+        "login": "another_user"
       }
     },
     {

diff --git a/test/fixtures/responses/list_reviews.json b/test/fixtures/responses/list_reviews.json
@@ -16,5 +16,11 @@
       "login": "VibhuJawa"
     },
     "state": "APPROVED"
+  },
+  {
+    "user": {
+      "login": "another_user"
+    },
+    "state": "APPROVED"
   }
 ]