Cleanup logging and duplicate test cases

Signed-off-by: Kyle Harding <[email protected]>

__tests__/approval.test.js

@@ -17,8 +17,7 @@ describe('ApprovalProcess', () => {
       getPullRequestMergeRef: jest.fn(),
       getRefSha: jest.fn(),
       getAuthenticatedUser: jest.fn(),
-      getPullRequestRepository: jest.fn(),
-      findIssueComment: jest.fn(),
+      throwOnContextMismatch: jest.fn(),
       createIssueComment: jest.fn(),
       deleteStaleIssueComments: jest.fn(),
       getWorkflowRunUrl: jest.fn()
@@ -56,7 +55,7 @@ describe('ApprovalProcess', () => {
       mockGitHubClient.getAuthenticatedUser.mockResolvedValue({
         id: 'test-user-id'
       })
-      mockGitHubClient.getPullRequestRepository.mockReturnValue(null)
+      mockGitHubClient.throwOnContextMismatch.mockReturnValue(null)
       mockGitHubClient.createIssueComment.mockResolvedValue({
         id: 'test-comment-id',
         html_url: 'http://test-url.com'
@@ -159,7 +158,6 @@ describe('ApprovalProcess', () => {
       await waitPromise
 
       expect(mockReactionManager.getEligibleReactions).toHaveBeenCalledTimes(3)
-      expect(core.debug).toHaveBeenCalledWith('Waiting for reactions...')
       expect(core.setOutput).toHaveBeenCalledWith('approved-by', 'approver')
     }, 2000) // Set test timeout to 2 seconds
   })

__tests__/client.test.js

@@ -77,21 +77,18 @@ describe('GitHubClient', () => {
     )
   })
 
-  test('getPullRequestRepository returns the correct repository', () => {
-    expect(gitHubClient.getPullRequestRepository()).toEqual({
-      owner: 'testOwner',
-      repo: 'testRepo'
-    })
-  })
-
-  test('getPullRequestRepository throws an error when the context repo does not match the payload base repo', () => {
+  test('throwOnContextMismatch throws an error when the context repo does not match the payload base repo', () => {
     gitHubClient.context.payload.pull_request.base.repo.owner.login =
       'otherOwner'
-    expect(() => gitHubClient.getPullRequestRepository()).toThrow(
+    expect(() => gitHubClient.throwOnContextMismatch()).toThrow(
       'Context repo does not match payload pull request base repo!'
     )
   })
 
+  test('throwOnContextMismatch does not throw an error when the context repo matches the payload base repo', () => {
+    expect(() => gitHubClient.throwOnContextMismatch()).not.toThrow()
+  })
+
   test('getPullRequestAuthors returns the correct author IDs', async () => {
     const mockCommits = [
       {
@@ -122,69 +119,6 @@ describe('GitHubClient', () => {
   })
 
   test('createIssueComment creates a new comment', async () => {
-    const mockComment = { id: 1, html_url: 'http://test.com/comment' }
-    mockOctokit.rest.issues.createComment.mockResolvedValue({
-      data: mockComment
-    })
-
-    const comment = await gitHubClient.createIssueComment('Test comment body')
-    expect(comment).toEqual(mockComment)
-    expect(mockOctokit.rest.issues.createComment).toHaveBeenCalledWith({
-      owner: 'testOwner',
-      repo: 'testRepo',
-      body: 'Test comment body',
-      issue_number: 1
-    })
-    expect(core.info).toHaveBeenCalledWith(
-      `Created new issue comment: ${mockComment.url}`
-    )
-  })
-
-  test('findIssueComment finds an existing comment', async () => {
-    const mockComments = [
-      {
-        id: 1,
-        body: 'Test PR comment',
-        created_at: '2023-01-01',
-        updated_at: '2023-01-01',
-        user: { id: 'testUserId' }
-      },
-      {
-        id: 2,
-        body: 'Another PR comment',
-        created_at: '2023-01-02',
-        updated_at: '2023-01-03',
-        user: { id: 'testUserId' }
-      }
-    ]
-    mockOctokit.rest.issues.listComments.mockResolvedValue({
-      data: mockComments
-    })
-
-    const comment = await gitHubClient.findIssueComment(
-      'testUserId',
-      'Test PR comment'
-    )
-    expect(comment).toEqual(mockComments[0])
-    expect(mockOctokit.rest.issues.listComments).toHaveBeenCalledWith({
-      owner: 'testOwner',
-      repo: 'testRepo',
-      issue_number: 1
-    })
-  })
-
-  test('findIssueComment returns null when no matching comment is found', async () => {
-    mockOctokit.rest.issues.listComments.mockResolvedValue({ data: [] })
-
-    const comment = await gitHubClient.findIssueComment(
-      'testUserId',
-      'Non-existent PR comment'
-    )
-    expect(comment).toBeNull()
-    expect(core.info).toHaveBeenCalledWith('No matching issue comment found.')
-  })
-
-  test('createIssueComment creates a new PR comment', async () => {
     const mockComment = { id: 1, url: 'http://test.com/pr-comment' }
     mockOctokit.rest.issues.createComment.mockResolvedValue({
       data: mockComment
@@ -201,7 +135,7 @@ describe('GitHubClient', () => {
       body: 'Test PR comment body'
     })
     expect(core.info).toHaveBeenCalledWith(
-      `Created new issue comment: ${mockComment.url}`
+      `Created new issue comment with ID: ${mockComment.id}`
     )
   })
 

src/approval.js

@@ -10,8 +10,8 @@ class ApprovalProcess {
   async run() {
     const tokenUser = await this.gitHubClient.getAuthenticatedUser()
 
-    // used for validation purposes only
-    this.gitHubClient.getPullRequestRepository()
+    // FIXME: remove this once we manually test reviewer permissions with PRs from forks
+    this.gitHubClient.throwOnContextMismatch()
 
     const runUrl = await this.gitHubClient.getWorkflowRunUrl()
 
@@ -54,7 +54,7 @@ class ApprovalProcess {
 
   // Wait for approval by checking reactions on a comment
   async waitForApproval(commentId, interval = 30) {
-    core.info('Waiting for reactions on comment ID:', commentId)
+    core.info(`Waiting for reactions at ${interval}-second intervals...`)
     for (;;) {
       const reactions = await this.reactionManager.getEligibleReactions(
         commentId,
@@ -82,7 +82,6 @@ class ApprovalProcess {
         return
       }
 
-      core.debug('Waiting for reactions...')
       await new Promise(resolve => setTimeout(resolve, interval * 1000))
     }
   }