Extend filter syntax with optional specification of file status: add, modified, deleted (dorny#22)

* Add support for specification of change type (add,modified,delete)

* Use NULL as separator in git-diff command output

* Improve PR test workflow

* Fix the workflow file

Author: dorny

.editorconfig

@@ -0,0 +1,9 @@
+root = true
+
+[*]
+charset = utf-8
+end_of_line = lf
+insert_final_newline = true
+trim_trailing_whitespace = true
+indent_style = space
+indent_size = 2

.gitattributes

@@ -0,0 +1 @@
+* text=auto eol=lf

.github/workflows/pull-request-verification.yml

@@ -70,3 +70,29 @@ jobs:
     - name: filter-test
       if: steps.filter.outputs.any != 'true' || steps.filter.outputs.error == 'true'
       run: exit 1
+
+  test-change-type:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - run: touch add.txt && rm README.md && echo "TEST" > LICENSE && git add -A
+    - uses: ./
+      id: filter
+      with:
+        token: ''
+        filters: |
+          add:
+            - added: "add.txt"
+          rm:
+            - deleted: "README.md"
+          modified:
+            - modified: "LICENSE"
+          any:
+            - added|deleted|modified: "*"
+    - name: filter-test
+      if: |
+        steps.filter.outputs.add != 'true'
+        || steps.filter.outputs.rm != 'true'
+        || steps.filter.outputs.modified != 'true'
+        || steps.filter.outputs.any != 'true'
+      run: exit 1

README.md

@@ -20,8 +20,10 @@ Supported workflows:
 ## Usage
 
 Filter rules are defined using YAML format.
-Each filter rule is a list of [glob expressions](https://github.com/isaacs/minimatch).
-Corresponding output variable will be created to indicate if there's a changed file matching any of the rule glob expressions.
+Each filter has a name and set of rules.
+Rule is a [glob expressions](https://github.com/isaacs/minimatch).
+Optionally you specify if the file should be added, modified or deleted to be matched.
+For each filter there will be corresponding output variable to indicate if there's a changed file matching any of the rules.
 Output variables can be later used in the `if` clause to conditionally run specific steps.
 
 ### Inputs
@@ -30,7 +32,7 @@ Output variables can be later used in the `if` clause to conditionally run speci
 - **`base`**: Git reference (e.g. branch name) against which the changes will be detected. Defaults to repository default branch (e.g. master).
               If it references same branch it was pushed to, changes are detected against the most recent commit before the push.
               This option is ignored if action is triggered by *pull_request* event.
-- **`filters`**: Path to the configuration file or directly embedded string in YAML format. Filter configuration is a dictionary, where keys specifies rule names and values are lists of file path patterns.
+- **`filters`**: Path to the configuration file or directly embedded string in YAML format.
 
 ### Outputs
 - For each rule it sets output variable named by the rule to text:
@@ -41,6 +43,7 @@ Output variables can be later used in the `if` clause to conditionally run speci
 - minimatch [dot](https://www.npmjs.com/package/minimatch#dot) option is set to true - therefore
   globbing will match also paths where file or folder name starts with a dot.
 - You can use YAML anchors to reuse path expression(s) inside another rule. See example in the tests.
+- It's recommended to put quote your path expressions with `'` or `"`. Otherwise you will get an error if it starts with `*`.
 - If changes are detected against the previous commit and there is none (i.e. first push of a new branch), all filter rules will report changed files.
 - You can use `base: ${{ github.ref }}` to configure change detection against previous commit for every branch you create.
 

__tests__/filter.test.ts

@@ -1,19 +1,12 @@
 import Filter from '../src/filter'
+import {File, ChangeStatus} from '../src/file'
 
 describe('yaml filter parsing tests', () => {
   test('throws if yaml is not a dictionary', () => {
     const yaml = 'not a dictionary'
     const t = () => new Filter(yaml)
     expect(t).toThrow(/^Invalid filter.*/)
   })
-  test('throws on invalid yaml', () => {
-    const yaml = `
-    src:
-      src/**/*.js
-    `
-    const t = () => new Filter(yaml)
-    expect(t).toThrow(/^Invalid filter.*/)
-  })
   test('throws if pattern is not a string', () => {
     const yaml = `
     src:
@@ -27,13 +20,21 @@ describe('yaml filter parsing tests', () => {
 })
 
 describe('matching tests', () => {
+  test('matches single inline rule', () => {
+    const yaml = `
+    src: "src/**/*.js"
+    `
+    let filter = new Filter(yaml)
+    const match = filter.match(modified(['src/app/module/file.js']))
+    expect(match.src).toBeTruthy()
+  })
   test('matches single rule in single group', () => {
     const yaml = `
     src:
       - src/**/*.js
     `
     const filter = new Filter(yaml)
-    const match = filter.match(['src/app/module/file.js'])
+    const match = filter.match(modified(['src/app/module/file.js']))
     expect(match.src).toBeTruthy()
   })
 
@@ -43,7 +44,7 @@ describe('matching tests', () => {
       - src/**/*.js
     `
     const filter = new Filter(yaml)
-    const match = filter.match(['not_src/other_file.js'])
+    const match = filter.match(modified(['not_src/other_file.js']))
     expect(match.src).toBeFalsy()
   })
 
@@ -55,7 +56,7 @@ describe('matching tests', () => {
       - test/**/*.js
     `
     const filter = new Filter(yaml)
-    const match = filter.match(['test/test.js'])
+    const match = filter.match(modified(['test/test.js']))
     expect(match.src).toBeFalsy()
     expect(match.test).toBeTruthy()
   })
@@ -67,7 +68,7 @@ describe('matching tests', () => {
       - test/**/*.js
     `
     const filter = new Filter(yaml)
-    const match = filter.match(['test/test.js'])
+    const match = filter.match(modified(['test/test.js']))
     expect(match.src).toBeTruthy()
   })
 
@@ -77,7 +78,7 @@ describe('matching tests', () => {
       - "**/*"
     `
     const filter = new Filter(yaml)
-    const match = filter.match(['test/test.js'])
+    const match = filter.match(modified(['test/test.js']))
     expect(match.any).toBeTruthy()
   })
 
@@ -87,7 +88,7 @@ describe('matching tests', () => {
       - "**/*.js"
     `
     const filter = new Filter(yaml)
-    const match = filter.match(['.test/.test.js'])
+    const match = filter.match(modified(['.test/.test.js']))
     expect(match.dot).toBeTruthy()
   })
 
@@ -101,7 +102,44 @@ describe('matching tests', () => {
       - src/**/*
     `
     let filter = new Filter(yaml)
-    const match = filter.match(['config/settings.yml'])
+    const match = filter.match(modified(['config/settings.yml']))
     expect(match.src).toBeTruthy()
   })
 })
+
+describe('matching specific change status', () => {
+  test('does not match modified file as added', () => {
+    const yaml = `
+    add:
+      - added: "**/*"
+    `
+    let filter = new Filter(yaml)
+    const match = filter.match(modified(['file.js']))
+    expect(match.add).toBeFalsy()
+  })
+
+  test('match added file as added', () => {
+    const yaml = `
+    add:
+      - added: "**/*"
+    `
+    let filter = new Filter(yaml)
+    const match = filter.match([{status: ChangeStatus.Added, filename: 'file.js'}])
+    expect(match.add).toBeTruthy()
+  })
+  test('matches when multiple statuses are configured', () => {
+    const yaml = `
+    addOrModify:
+      - added|modified: "**/*"
+    `
+    let filter = new Filter(yaml)
+    const match = filter.match([{status: ChangeStatus.Modified, filename: 'file.js'}])
+    expect(match.addOrModify).toBeTruthy()
+  })
+})
+
+function modified(paths: string[]): File[] {
+  return paths.map(filename => {
+    return {filename, status: ChangeStatus.Modified}
+  })
+}

__tests__/git.test.ts

@@ -1,4 +1,27 @@
 import * as git from '../src/git'
+import {ExecOptions} from '@actions/exec'
+import {ChangeStatus} from '../src/file'
+
+describe('parsing of the git diff-index command', () => {
+  test('getChangedFiles returns files with correct change status', async () => {
+    const files = await git.getChangedFiles(git.FETCH_HEAD, (cmd, args, opts) => {
+      const stdout = opts?.listeners?.stdout
+      if (stdout) {
+        stdout(Buffer.from('A\u0000LICENSE\u0000'))
+        stdout(Buffer.from('M\u0000src/index.ts\u0000'))
+        stdout(Buffer.from('D\u0000src/main.ts\u0000'))
+      }
+      return Promise.resolve(0)
+    })
+    expect(files.length).toBe(3)
+    expect(files[0].filename).toBe('LICENSE')
+    expect(files[0].status).toBe(ChangeStatus.Added)
+    expect(files[1].filename).toBe('src/index.ts')
+    expect(files[1].status).toBe(ChangeStatus.Modified)
+    expect(files[2].filename).toBe('src/main.ts')
+    expect(files[2].status).toBe(ChangeStatus.Deleted)
+  })
+})
 
 describe('git utility function tests (those not invoking git)', () => {
   test('Detects if ref references a tag', () => {