#63170 Increase the specificity of workflow path filtering

[johnbillion]

The main aim here is to reduce the chance of unnecessary workflows running when changes are not made to src files. As an example, a change to a phpunit test file currently triggers the e2e tests, the performance tests, and the build process testing, all of which are unnecessary.

I would appreciate the proposed path changes being double checked in case I've missed something.

Ironically, changing these workflow files triggers the corresponding workflows in this PR.

Trac ticket: https://core.trac.wordpress.org/ticket/63170

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props johnbillion, peterwilsoncc, desrosj.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[peterwilsoncc]

Added a couple of notes inline.

[peterwilsoncc]

Misses files in the root directory, currently the PHP sample configs, webpack.

Which files in particular are you trying to ignore here?

🔢 This applies to other changes too, so I won't repeat myself.

[johnbillion]

For example, with this change the workflow wouldn't unnecessarily run when an e2e, performance, or QUnit test file is changed (which are all .js files).

[desrosj]

It looks like we do currently include QUnit test files in the JSHint scans.

Suggested change

	- 'src/**.php'
	- 'tests/**.php'
	- 'src/**.php'
	- 'tests/**.php'
	- 'tests/qunit/**/*.js

It looks like Gruntfile.js specifies the exclusion of tests/qunit/(vendor|editor)/** as well. But unless I'm missing something, those directories no longer exist, so there's no need to include these exclusions here.

It also looks like grunt jshint:tests uses the tests/qunit/.jshintrc file for configuration. I haven't looked into why there are different configurations, but for the time being, below should also be changed to include that file in case it's updated.

TIL that you can't add comments or suggested changes to unmodified lines in a PR. So instead here's what I suggest for below.

      # This file configures JSHint. Changes could affect the outcome.
      - '**.jshintrc'

[desrosj]

I also wish that there were an easy way to prevent a specific job from running within a workflow when certain path filters do not match (don't run PHPCS when only .js files are touched). However, it seems that can only be accomplished by having a job that detects this (without relying on a third-party action). ChatGPT came up with the following rough example.

  check-files:
    runs-on: ubuntu-latest
    outputs:
      changed: ${{ steps.filter.outputs.changed }}
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - id: filter
        run: |
          if git diff --name-only ${{ github.base_ref || github.event.before }} ${{ github.sha }} | grep -qE '\.php$'; then
            echo "changed=true" >> "$GITHUB_OUTPUT"
          else
            echo "changed=false" >> "$GITHUB_OUTPUT"
          fi

I don't think there's much benefit in this specific workflow because these jobs complete in under 30 seconds. But if there are any longer-running ones, it may be worthwhile to try.

[desrosj]

It looks like we do currently include QUnit test files in the JSHint scans.

Suggested change

	- 'src/**.php'
	- 'tests/**.php'
	- 'src/**.php'
	- 'tests/**.php'
	- 'tests/qunit/**/*.js

It looks like Gruntfile.js specifies the exclusion of tests/qunit/(vendor|editor)/** as well. But unless I'm missing something, those directories no longer exist, so there's no need to include these exclusions here.

It also looks like grunt jshint:tests uses the tests/qunit/.jshintrc file for configuration. I haven't looked into why there are different configurations, but for the time being, below should also be changed to include that file in case it's updated.

TIL that you can't add comments or suggested changes to unmodified lines in a PR. So instead here's what I suggest for below.

      # This file configures JSHint. Changes could affect the outcome.
      - '**.jshintrc'

[desrosj]

I also wish that there were an easy way to prevent a specific job from running within a workflow when certain path filters do not match (don't run PHPCS when only .js files are touched). However, it seems that can only be accomplished by having a job that detects this (without relying on a third-party action). ChatGPT came up with the following rough example.

  check-files:
    runs-on: ubuntu-latest
    outputs:
      changed: ${{ steps.filter.outputs.changed }}
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - id: filter
        run: |
          if git diff --name-only ${{ github.base_ref || github.event.before }} ${{ github.sha }} | grep -qE '\.php$'; then
            echo "changed=true" >> "$GITHUB_OUTPUT"
          else
            echo "changed=false" >> "$GITHUB_OUTPUT"
          fi

I don't think there's much benefit in this specific workflow because these jobs complete in under 30 seconds. But if there are any longer-running ones, it may be worthwhile to try.

[desrosj]

Including SECURITY.md makes sense because there are specific tests that confirm the contents are accurate. But there are a few other Markdown files in the src directory that could be excluded with !src/**/*.md.

[desrosj]

Ah, good catch!

[desrosj]

@johnbillion What do you think about removing the composer.* pattern here? The only dependency managed in Composer that is used in the build process is composer/ca-bundle. But the build process will never be affected by this without first manually updating the version (it's pinned to an exact version vs. using a semantic range) AND running grunt certificates:upgrade.

Otherwise, the only files related to certificates that would affect the build process are the .pem and .crt files that are in src/wp-includes/certificates. It may be worth adding **/*.pem and **/*.crt to this list of path filters for those scenarios.