Run unit tests only for plugins with changes #1838
+147
−37
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ on: | |
| - '**/package.json' | ||
| - 'package-lock.json' | ||
| - 'phpunit.xml.dist' | ||
| - 'tools/phpunit/bootstrap.php' | ||
| - 'composer.json' | ||
| - 'composer.lock' | ||
| pull_request: | ||
|
|
@@ -24,6 +25,7 @@ on: | |
| - '**/package.json' | ||
| - 'package-lock.json' | ||
| - 'phpunit.xml.dist' | ||
| - 'tools/phpunit/bootstrap.php' | ||
| - 'composer.json' | ||
| - 'composer.lock' | ||
| types: | ||
|
|
@@ -57,6 +59,55 @@ jobs: | |
| steps: | ||
| - uses: styfle/[email protected] | ||
| - uses: actions/checkout@v4 | ||
| - name: Get changed files | ||
| id: changed-files | ||
| uses: tj-actions/changed-files@v45 | ||
| with: | ||
| dir_names: true # Output unique changed directories. | ||
| dir_names_max_depth: 2 | ||
|
There was a problem hiding this comment. This limits the directory output to a maximum depth of 2. For example, |
||
| files_yaml: | | ||
| plugins: | ||
| - 'plugins/**' | ||
| config: | ||
| - '.github/workflows/php-test-plugins.yml' | ||
| - '.wp-env.json' | ||
| - '**/package.json' | ||
| - 'package-lock.json' | ||
| - 'phpunit.xml.dist' | ||
| - 'tools/phpunit/bootstrap.php' | ||
| - 'composer.json' | ||
| - 'composer.lock' | ||
| - name: Get changed plugins | ||
| id: changed-plugins | ||
| run: | | ||
| if [[ "${{ github.event_name }}" == "push" || "${{ steps.changed-files.outputs.config_any_changed }}" == "true" ]]; then | ||
| ALL_CHANGED_PLUGINS=($(ls plugins)) | ||
| echo "all_changed_plugins=${ALL_CHANGED_PLUGINS[*]}" >> $GITHUB_OUTPUT | ||
| exit 0 | ||
| fi | ||
|
|
||
| declare -a ALL_CHANGED_PLUGINS=() | ||
| for DIR in ${{ steps.changed-files.outputs.plugins_all_changed_files }}; do | ||
| PLUGIN_NAME=$(basename "$DIR") | ||
| ALL_CHANGED_PLUGINS+=("$PLUGIN_NAME") | ||
| done | ||
|
|
||
| # Define and add plugin dependencies (e.g., optimization-detective triggers others). | ||
| declare -A PLUGIN_DEPENDENCIES=( | ||
|
||
| ["optimization-detective"]="embed-optimizer image-prioritizer" | ||
| ) | ||
| for PLUGIN in "${ALL_CHANGED_PLUGINS[@]}"; do | ||
| if [[ -n "${PLUGIN_DEPENDENCIES[$PLUGIN]}" ]]; then | ||
| for DEP in ${PLUGIN_DEPENDENCIES[$PLUGIN]}; do | ||
| if [[ ! " ${ALL_CHANGED_PLUGINS[*]} " =~ " ${DEP} " ]]; then | ||
| ALL_CHANGED_PLUGINS+=("$DEP") | ||
| fi | ||
| done | ||
| fi | ||
| done | ||
|
|
||
| ALL_CHANGED_PLUGINS=($(echo "${ALL_CHANGED_PLUGINS[@]}" | tr ' ' '\n' | sort | tr '\n' ' ')) | ||
| echo "all_changed_plugins=${ALL_CHANGED_PLUGINS[*]}" >> $GITHUB_OUTPUT | ||
| - name: Setup Node.js (.nvmrc) | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
|
|
@@ -88,32 +139,24 @@ jobs: | |
| - name: Running single site unit tests | ||
| run: | | ||
| if [ "${{ matrix.coverage }}" == "true" ]; then | ||
| for PLUGIN in ${{ steps.changed-plugins.outputs.all_changed_plugins }}; do | ||
| npm run test-php:$PLUGIN -- -- -- --coverage-clover=./single-site-reports/coverage-$PLUGIN.xml | ||
| done | ||
| else | ||
| for PLUGIN in ${{ steps.changed-plugins.outputs.all_changed_plugins }}; do | ||
| npm run test-php:$PLUGIN | ||
| done | ||
| fi | ||
| - name: Running multisite unit tests | ||
| run: | | ||
| if [ "${{ matrix.coverage }}" == "true" ]; then | ||
| for PLUGIN in ${{ steps.changed-plugins.outputs.all_changed_plugins }}; do | ||
| npm run test-php-multisite:$PLUGIN -- -- -- --coverage-clover=./multisite-reports/coverage-multisite-$PLUGIN.xml | ||
| done | ||
| else | ||
| for PLUGIN in ${{ steps.changed-plugins.outputs.all_changed_plugins }}; do | ||
| npm run test-php-multisite:$PLUGIN | ||
| done | ||
| fi | ||
| - name: Upload single site coverage reports to Codecov | ||
| if: ${{ matrix.coverage == true }} | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This action was recently compromised. My recommendation would be to pin it to a specific SHA for better security.
Additionally, reviewing the workflow, I noticed there's quite a bit of scripting involved, which might not be the easiest to follow for all developers.
To improve clarity and flexibility, I'd suggest using a JavaScript or PHP script instead. This approach would make the code more accessible to a wider range of developers and allow for easier management of edge cases.
You can find a similar composite action here - https://github.com/ampproject/amp-wp/tree/develop/.github/actions/determine-changed-files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we pin it to a vulnerable version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it seems like based on the latest supply chain attack with qix today, pinning is a way to avoid being exploited. Perhaps Dependabot includes some checks for the dependencies it is proposing updates for which would address the concern of pinning to a vulnerable version? It doesn't seem this is the case right now, but this would sure be a great use of AI.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The best way to avoid these issues is to use your own solution rather than relying on third-party actions. It's generally a good idea to either use
@actions/*or create your own composite actions.Also, in core, it's very rare for maintainers to allow third-party actions, and when they do, they always pin to a specific SHA to mitigate potential security risks.
By default, pin to the SHA of a version that was published at least a week ago. This provides ample time to detect if the action has been compromised.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the advice. I've opened a PR to implement this: #2171