[GitHub] [pulsar] lhotari commented on a change in pull request #10376: [Build] Fix skipping tests when there are only documentation changes

[GitBox <gi...@apache.org>]

lhotari commented on a change in pull request #10376:
URL: https://github.com/apache/pulsar/pull/10376#discussion_r621841708



##########
File path: .github/workflows/ci-build-macos.yaml
##########
@@ -45,7 +43,27 @@ jobs:
       - name: Tune Runner VM
         uses: ./.github/actions/tune-runner-vm
 
+      - name: Detect changed files
+        id:   changes
+        uses: apache/pulsar-test-infra/paths-filter@master
+        with:
+          filters: |
+            # pattern syntax: https://github.com/micromatch/picomatch
+            all:
+              - '**'
+            docs:
+              - 'site2/**'
+              - 'deployment/**'
+              - '.asf.yaml'
+              - '*.md'
+              - '**/*.md'
+
+      - name: Check changed files
+        id: check_changes
+        run: echo "::set-output name=docs_only::${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.docs_count) && fromJSON(steps.changes.outputs.docs_count) > 0 }}"

Review comment:
       this seems complex, but it's necessary.
   
   picomatch translates a pattern to a regex under the covers. combining including and excluding can lead to odd results.
   
   It's possible to test picomatch patterns using npm on the command line, for example:
   ```
   # check picomatch version from https://github.com/apache/pulsar-test-infra/blob/master/paths-filter/package-lock.json
   npm install picomatch@2.2.1  
   function test_picomatch_pattern() { node -e 'const pattern=process.argv[1]; const matcher=require("picomatch")(pattern, {"dot": true}); console.log(`pattern: ${pattern}`); process.argv.splice(2).forEach(arg => console.log(`${arg} -> ${matcher(arg)}`));' "$@"  }
   
   test_picomatch_pattern '**/*.md' README.md some/path/README.md
   ```
   
   This example shows a problem with inversion (`!`) in the pattern:
   ```
   ❯ test_picomatch_pattern '**/*.md' README.md some/path/README.md
   pattern: **/*.md
   README.md -> true
   some/path/README.md -> true
   ❯ test_picomatch_pattern '!(**/*.md)' README.md some/path/README.md
   pattern: !(**/*.md)
   README.md -> true
   some/path/README.md -> false
   ```
   
   That is the reason why the logic for checking documentation only changes is done based on the comparison on match counts,  
   `fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.docs_count) && fromJSON(steps.changes.outputs.docs_count) > 0` .
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org