Allow closing pull request instead of exiting with error (#1065)

* fix naming * add option to close PR * fix tests and build * fix PR issues and tests * revert githubservice renaming * Add negative test * Improve documentation * Remove duplicate input --------- Co-authored-by: Xavier Alvarez <[email protected]>
xalvarez · Dec 29, 2024 · 267c166 · 267c166
1 parent 9490285
commit 267c166
Show file tree

Hide file tree

Showing 10 changed files with 112 additions and 16 deletions.
diff --git a/README.md b/README.md
@@ -31,6 +31,9 @@ The action has the following inputs:
   pattern should be allowed in the pull request. If set to `true`, the action will not fail even if
   a new file that matches the pattern is added in the pull request. If not provided or set to
   `false`, the action will fail if a new file that matches the pattern is added.
+- `closePR`: (**Optional**) A boolean value that determines whether the pull request should be closed
+  if a pattern match is found. If set to `true`, the action will close the pull request instead of failing.
+  If not provided or set to `false`, the action will fail if a pattern match is found.
 
 > [!IMPORTANT]
 > This Action supports pull_request and pull_request_target events only.

diff --git a/__tests__/github-service.test.ts b/__tests__/github-service.test.ts
@@ -9,7 +9,8 @@ jest.mock('@actions/github', () => ({
     paginate: jest.fn().mockResolvedValue([{filename: 'exampleFile1.md'}, {filename: 'exampleFile2.md'}]),
     rest: {
       pulls: {
-        listFiles: jest.fn()
+        listFiles: jest.fn(),
+        update: jest.fn()
       }
     }
   })
@@ -48,4 +49,19 @@ describe('github-service', () => {
     )
     expect(changedFiles).toEqual(expectedFiles)
   })
+
+  it('Should close pull request', async () => {
+    const owner = 'exampleOwner'
+    const repo = 'exampleRepo'
+    const pullRequestNumber = 1
+
+    await gitHubService.closePullRequest(owner, repo, pullRequestNumber)
+
+    expect(octokit.getOctokit(GITHUB_TOKEN).rest.pulls.update).toHaveBeenCalledWith({
+      owner: owner,
+      repo: repo,
+      pull_number: pullRequestNumber,
+      state: 'closed'
+    })
+  })
 })
diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts
@@ -38,13 +38,14 @@ describe('main', () => {
           return trustedAuthors
         case 'pattern':
           return pattern
+        case 'githubToken':
+          return 'exampleToken'
         default:
           return ''
       }
     })
 
     isTrustedAuthorSpy = jest.spyOn(authorChecker, 'isTrustedAuthor').mockResolvedValue(false)
-
     context.eventName = 'pull_request'
     context.payload = {
       pull_request: {
@@ -65,7 +66,16 @@ describe('main', () => {
     await run()
 
     expect(getChangedFilesSpy).toHaveBeenCalledWith('exampleOwner', 'exampleOwner/exampleRepo', 1)
-    expect(checkChangedFilesAgainstPatternSpy).toHaveBeenCalledWith(changedFiles, pattern, false)
+    expect(checkChangedFilesAgainstPatternSpy).toHaveBeenCalledWith(
+      changedFiles,
+      pattern,
+      expect.any(GitHubService),
+      'exampleOwner/exampleRepo',
+      'exampleOwner',
+      1,
+      false,
+      false
+    )
     expect(core.setFailed).not.toHaveBeenCalled()
   })
 

diff --git a/__tests__/pattern-matcher.test.ts b/__tests__/pattern-matcher.test.ts
@@ -1,10 +1,20 @@
 import * as core from '@actions/core'
-import {IFile} from '../src/github-service'
+import GitHubService, {IFile} from '../src/github-service'
 import {checkChangedFilesAgainstPattern} from '../src/pattern-matcher'
 
+const GITHUB_TOKEN = 'exampleGitHubToken'
+
 jest.mock('@actions/core')
+jest.mock('../src/github-service')
 
 describe('pattern-matcher', () => {
+  let gitHubService: GitHubService
+  let closePullRequestSpy: jest.SpyInstance
+
+  beforeEach(() => {
+    gitHubService = new GitHubService(GITHUB_TOKEN)
+    closePullRequestSpy = jest.spyOn(GitHubService.prototype, 'closePullRequest').mockResolvedValue()
+  })
   afterEach(() => {
     jest.restoreAllMocks()
   })
@@ -13,40 +23,53 @@ describe('pattern-matcher', () => {
     const files: IFile[] = givenFiles()
     const pattern = '.*.js'
 
-    await checkChangedFilesAgainstPattern(files, pattern)
+    await checkChangedFilesAgainstPattern(files, pattern, gitHubService, 'exampleRepo', 'exampleOwner', 1, false)
 
     expect(core.setFailed).toHaveBeenCalledWith(`There is at least one file matching the pattern ${pattern}`)
     expect(core.debug).not.toHaveBeenCalled()
+    expect(closePullRequestSpy).not.toHaveBeenCalled()
   })
 
   it('Should not reject non matching pattern', async () => {
     const files: IFile[] = givenFiles()
     const pattern = '.*.ts'
 
-    await checkChangedFilesAgainstPattern(files, pattern)
+    await checkChangedFilesAgainstPattern(files, pattern, gitHubService, 'exampleRepo', 'exampleOwner', 1, false)
 
     expect(core.setFailed).not.toHaveBeenCalled()
     expect(core.debug).toHaveBeenCalledWith(`There isn't any file matching the pattern ${pattern}`)
+    expect(closePullRequestSpy).not.toHaveBeenCalled()
   })
 
   it('Should not reject empty commit', async () => {
     const files: IFile[] = []
     const pattern = '.*'
 
-    await checkChangedFilesAgainstPattern(files, pattern)
+    await checkChangedFilesAgainstPattern(files, pattern, gitHubService, 'exampleRepo', 'exampleOwner', 1, false)
 
     expect(core.setFailed).not.toHaveBeenCalled()
     expect(core.debug).toHaveBeenCalledWith(`This commit doesn't contain any files`)
+    expect(closePullRequestSpy).not.toHaveBeenCalled()
   })
 
   it('Should not reject matching added file when allowNewFiles is true', async () => {
     const files: IFile[] = givenFiles()
     const pattern = '.*.js'
 
-    await checkChangedFilesAgainstPattern(files, pattern, true)
+    await checkChangedFilesAgainstPattern(files, pattern, gitHubService, 'exampleRepo', 'exampleOwner', 1, false, true)
 
     expect(core.setFailed).not.toHaveBeenCalled()
     expect(core.debug).toHaveBeenCalledWith(`There isn't any file matching the pattern ${pattern}`)
+    expect(closePullRequestSpy).not.toHaveBeenCalled()
+  })
+  it('Should close PR', async () => {
+    const files: IFile[] = givenFiles()
+    const pattern = '.*.js'
+
+    await checkChangedFilesAgainstPattern(files, pattern, gitHubService, 'exampleRepo', 'exampleOwner', 1, true)
+
+    expect(core.setFailed).not.toHaveBeenCalled()
+    expect(closePullRequestSpy).toHaveBeenCalledWith('exampleOwner', 'exampleRepo', 1)
   })
 })
 

diff --git a/action.yml b/action.yml
@@ -17,6 +17,9 @@ inputs:
   allowNewFiles:
     required: false
     description: 'Allow matching files to be included in pull requests'
+  closePR:
+    required: false
+    description: 'If set to true, the action will close the pull request if a pattern match is found. By default, the action will fail if a pattern match is found.'
 runs:
   using: 'node20'
   main: 'dist/index.js'
diff --git a/dist/index.js b/dist/index.js
diff --git a/dist/index.js.map b/dist/index.js.map
diff --git a/src/github-service.ts b/src/github-service.ts
@@ -29,4 +29,13 @@ export default class GitHubService {
 
     return files
   }
+
+  async closePullRequest(owner: string, repo: string, pullRequestNumber: number): Promise<void> {
+    await this.octokit.rest.pulls.update({
+      owner: owner,
+      repo: repo,
+      pull_number: pullRequestNumber,
+      state: 'closed'
+    })
+  }
 }
diff --git a/src/main.ts b/src/main.ts
@@ -29,7 +29,17 @@ export async function run(): Promise<void> {
         )
         const pattern: string = core.getInput('pattern', {required: true})
         const allowNewFiles: boolean = 'true' === core.getInput('allowNewFiles')
-        await checkChangedFilesAgainstPattern(files, pattern, allowNewFiles)
+        const closePR: boolean = core.getInput('closePR') === 'true'
+        await checkChangedFilesAgainstPattern(
+          files,
+          pattern,
+          gitHubService,
+          context.repo.repo,
+          context.repo.owner,
+          pullRequestNumber,
+          closePR,
+          allowNewFiles
+        )
       } else {
         core.setFailed('Pull request number is missing in github event payload')
       }

diff --git a/src/pattern-matcher.ts b/src/pattern-matcher.ts
@@ -1,9 +1,14 @@
 import * as core from '@actions/core'
-import {IFile} from './github-service'
+import GitHubService, {IFile} from './github-service'
 
 export async function checkChangedFilesAgainstPattern(
   files: IFile[],
   pattern: string,
+  githubService: GitHubService,
+  repo: string,
+  owner: string,
+  pullRequestNumber: number,
+  closePR: boolean,
   allowNewFiles = false
 ): Promise<void> {
   if (files.length > 0) {
@@ -15,9 +20,12 @@ export async function checkChangedFilesAgainstPattern(
       }
       return isPatternMatched
     })
-
     if (shouldPreventFileChange) {
-      core.setFailed(`There is at least one file matching the pattern ${pattern}`)
+      if (closePR) {
+        await githubService.closePullRequest(owner, repo, pullRequestNumber)
+      } else {
+        core.setFailed(`There is at least one file matching the pattern ${pattern}`)
+      }
     } else {
       core.debug(`There isn't any file matching the pattern ${pattern}`)
     }