Skip to content

Commit

Permalink
GH Actions: tweak the way the PHPCS/WPCS versions are set
Browse files Browse the repository at this point in the history
Next iteration after PR 300.

YoastCS 3.0 will not just have PHPCS and WPCS as dependencies for the sniff tests, but also PHPCSUtils.

PR 300 already changed the logic of the workflow to take advantage of the Composer `--prefer-lowest` setting. This commit takes this yet another step further.

**Stable vs Dev**

As things were, the sniff tests were being run against the _lowest_ supported CS dependencies and the _development_ version of supported CS dependencies (with the exception of WPCS dev as WPCS 3.0.0 is not yet supported).

While testing against the _dev_ versions of CS dependencies is relevant, it is more important that the tests safeguard that the sniffs work correctly on user-installable combinations of the CS dependencies.

Now, it could be argued that every single combination of the different versions of each of these dependencies should be tested, but that would make the matrix _huge_ to little added benefit, especially as most of the time, the `lowest` and `stable` supported versions of CS dependencies will be the same for YoastCS (though not always, which still makes the distinction and running of tests against both `lowest` and `stable` relevant as it will allow for new releases of the CS dependencies automatically).

To that end, I'm changing the test matrices to test by default against the `lowest` and `stable` supported versions of dependencies with some select extra builds testing against the `dev` versions of supported CS dependencies.

PHP parse error linting will be run against all builds with the `stable` key now.

Note: `stable` is also the name used for the Composer sister-argument to `--prefer-lowest`: `--prefer-stable`, so should be clear-cut for anyone reading the workflow.

**Consequences**

Quicktest workflow:
* This will no longer contain exact versions of the CS dependencies in the test matrix, which should simplify workflow maintenance.
* The "Composer update" step for the `dev` dependencies has been adjusted. Instead of multiple commands, it now does all the updates in one combined command, which should make debugging of the workflow easier.
* One extra build has been added to test against `dev` dependencies.

Test workflow:
* The standard builds will now run against `lowest` and `stable` versions of dependencies and a number of extra builds have been added to test against `dev` versions of dependencies.
    I considered marking those as "allowed to fail" (`continue-on-error`), but decided against that as the sniffs should always be ready for the next minor/patch release of the CS dependencies.
* The commented out build against WPCS dev can now be removed as that will be handled via the "all CS dependencies" `dev` builds (once WPCS 3.0 is supported and re-enabled for update for the `dev` builds).
* The "Composer update" step for the `dev` dependencies has been adjusted. Instead of multiple commands, it now does all the updates in one combined command, which should make debugging of the workflow easier.
* The "Composer update" step for the `lowest` depencies has been adjusted to be more readable.
* Note that the direct Composer commands do not include `--update-with-dependencies` as our and our child dependencies overlap anyway.
* The code coverage builds will now run against `lowest`, and `dev`, which is basically the same as it was before.

Also note that the direct Composer command for updating the CS dependencies currently contain the `--ignore-platform-req=php+` argument.
This is needed for PHP 8.x due to the max supported PHPUnit version being PHPUnit 7.x.
This CLI argument can be removed once upstream PR squizlabs/PHP_CodeSniffer 3803 has been merged.
  • Loading branch information
jrfnl committed Nov 3, 2023
1 parent 53b3008 commit 6131d41
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 53 deletions.
45 changes: 23 additions & 22 deletions .github/workflows/quicktest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,38 +19,31 @@ concurrency:
jobs:
#### QUICK TEST STAGE ####
# This is a much quicker test which only runs the unit tests and linting against low/high
# supported PHP/PHPCS/WPCS combinations.
# of the supported PHP/CS dependencies combinations.
quicktest:
runs-on: ubuntu-latest

strategy:
matrix:
php_version: ['5.4', 'latest']
cs_dependencies: ['lowest', 'stable']

include:
- php_version: 'latest'
phpcs_version: 'dev-master'
wpcs_version: '2.3.0'
- php_version: 'latest'
phpcs_version: '3.7.2'
wpcs_version: '2.3.0'
- php_version: '5.4'
phpcs_version: 'dev-master'
wpcs_version: '2.3.0'
- php_version: '5.4'
phpcs_version: '3.7.2'
wpcs_version: '2.3.0'
cs_dependencies: 'dev'

name: "QTest${{ matrix.phpcs_version == 'dev-master' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - PHPCS ${{ matrix.phpcs_version }} - WPCS ${{ matrix.wpcs_version }}"
name: "QTest${{ matrix.cs_dependencies == 'stable' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - CS deps ${{ matrix.cs_dependencies }}"

steps:
- name: Checkout code
uses: actions/checkout@v3

# On stable PHPCS versions, allow for PHP deprecation notices.
# With stable PHPCS dependencies, allow for PHP deprecation notices.
# Unit tests don't need to fail on those for stable releases where those issues won't get fixed anymore.
- name: Setup ini config
id: set_ini
run: |
if [ "${{ matrix.phpcs_version }}" != "dev-master" ]; then
if [ "${{ matrix.cs_dependencies }}" != "dev" ]; then
echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED, display_errors=On' >> $GITHUB_OUTPUT
else
echo 'PHP_INI=error_reporting=-1, display_errors=On' >> $GITHUB_OUTPUT
Expand All @@ -63,12 +56,13 @@ jobs:
ini-values: ${{ steps.set_ini.outputs.PHP_INI }}
coverage: none

- name: 'Composer: adjust dependencies'
run: |
# Set the PHPCS version to test against.
composer require --no-update --no-scripts squizlabs/php_codesniffer:"${{ matrix.phpcs_version }}" --no-interaction
# Set the WPCS version to test against.
composer require --no-update --no-scripts wp-coding-standards/wpcs:"${{ matrix.wpcs_version }}" --no-interaction
- name: "Composer: set PHPCS dependencies for tests (dev)"
if: ${{ matrix.cs_dependencies == 'dev' }}
# Note: while WPCS 3.0.0 is not yet supported, WPCS will not be updated for the `dev` builds.
# Once YoastCS has been updated for WPCS 3.0.0, WPCS dev-develop should be added to this command.
run: >
composer require --no-update --no-scripts --no-interaction --ignore-platform-req=php+
squizlabs/php_codesniffer:"dev-master"
# Install dependencies and handle caching in one go.
# @link https://github.com/marketplace/actions/install-composer-dependencies
Expand All @@ -87,11 +81,18 @@ jobs:
composer-options: --ignore-platform-req=php+
custom-cache-suffix: $(date -u "+%Y-%m")

- name: "Composer: downgrade PHPCS dependencies for tests (lowest) (with ignore platform)"
if: ${{ matrix.cs_dependencies == 'lowest' }}
run: >
composer update --prefer-lowest --no-scripts --no-interaction --ignore-platform-req=php+
squizlabs/php_codesniffer
wp-coding-standards/wpcs
- name: Verify installed standards
run: vendor/bin/phpcs -i

- name: Lint against parse errors
if: matrix.phpcs_version == 'dev-master'
if: matrix.cs_dependencies == 'stable'
run: composer lint

- name: Run the unit tests - PHP 5.4 - 8.0
Expand Down
82 changes: 51 additions & 31 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,32 +40,42 @@ jobs:
#
# The matrix is set up so as not to duplicate the builds which are run for code coverage.
php_version: ['5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '8.0', '8.1', '8.2']
cs_dependencies: ['lowest', 'highest']
cs_dependencies: ['lowest', 'stable']

include:
# Experimental builds. These are allowed to fail.

# Make the matrix complete (when combined with the code coverage builds).
- php_version: '5.4'
cs_dependencies: 'stable'
- php_version: '7.4'
cs_dependencies: 'stable'

# Test against dev versions of all CS dependencies with select PHP versions for early detection of issues.
- php_version: '7.0'
cs_dependencies: 'dev'
- php_version: '8.0'
cs_dependencies: 'dev'
- php_version: '8.2'
cs_dependencies: 'dev'

# Experimental build(s). These are allowed to fail.
# PHP nightly
- php_version: '8.3'
cs_dependencies: 'highest'
# Test against WPCS unstable. Re-enable when WPCS is not in dev for the next major.
#- php_version: '8.0'
# cs_dependencies: 'highest'
cs_dependencies: 'dev'

name: "Test${{ matrix.cs_dependencies == 'highest' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - CS Deps ${{ matrix.cs_dependencies }}"
name: "Test${{ matrix.cs_dependencies == 'stable' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - CS Deps ${{ matrix.cs_dependencies }}"

continue-on-error: ${{ matrix.php == '8.3' }}

steps:
- name: Checkout code
uses: actions/checkout@v3

# On stable PHPCS versions, allow for PHP deprecation notices.
# With stable PHPCS dependencies, allow for PHP deprecation notices.
# Unit tests don't need to fail on those for stable releases where those issues won't get fixed anymore.
- name: Setup ini config
id: set_ini
run: |
if [[ "${{ matrix.cs_dependencies }}" != "highest" ]]; then
if [ "${{ matrix.cs_dependencies }}" != "dev" ]; then
echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED, display_errors=On' >> $GITHUB_OUTPUT
else
echo 'PHP_INI=error_reporting=-1, display_errors=On' >> $GITHUB_OUTPUT
Expand All @@ -79,11 +89,13 @@ jobs:
coverage: none
tools: cs2pr

- name: 'Composer: adjust CS dependencies (high)'
if: ${{ matrix.cs_dependencies == 'highest' }}
run: |
composer require --no-update --no-scripts squizlabs/php_codesniffer:"${{ env.PHPCS_HIGHEST }}" --no-interaction
composer require --no-update --no-scripts wp-coding-standards/wpcs:"${{ env.WPCS_HIGHEST }}" --no-interaction
- name: "Composer: set PHPCS dependencies for tests (dev)"
if: ${{ matrix.cs_dependencies == 'dev' }}
# Note: while WPCS 3.0.0 is not yet supported, WPCS will not be updated for the `dev` builds.
# Once YoastCS has been updated for WPCS 3.0.0, WPCS dev-develop should be added to this command.
run: >
composer require --no-update --no-scripts --no-interaction --ignore-platform-req=php+
squizlabs/php_codesniffer:"${{ env.PHPCS_HIGHEST }}"
# Install dependencies and handle caching in one go.
# @link https://github.com/marketplace/actions/install-composer-dependencies
Expand All @@ -102,9 +114,12 @@ jobs:
composer-options: --ignore-platform-req=php+
custom-cache-suffix: $(date -u "+%Y-%m")

- name: "Composer: set PHPCS version for tests (low)"
- name: "Composer: downgrade PHPCS dependencies for tests (lowest) (with ignore platform)"
if: ${{ matrix.cs_dependencies == 'lowest' }}
run: composer update squizlabs/php_codesniffer wp-coding-standards/wpcs --with-dependencies --prefer-lowest --ignore-platform-req=php+ --no-scripts --no-interaction
run: >
composer update --prefer-lowest --no-scripts --no-interaction --ignore-platform-req=php+
squizlabs/php_codesniffer
wp-coding-standards/wpcs
- name: Verify installed versions
run: composer info --no-dev
Expand All @@ -115,7 +130,7 @@ jobs:
# The results of the linting will be shown inline in the PR via the CS2PR tool.
# @link https://github.com/staabm/annotate-pull-request-from-checkstyle/
- name: Lint against parse errors
if: ${{ matrix.cs_dependencies == 'highest' }}
if: ${{ matrix.cs_dependencies == 'stable' }}
run: composer lint -- --checkstyle | cs2pr

- name: Run the unit tests - PHP 5.4 - 8.0
Expand All @@ -140,22 +155,22 @@ jobs:

strategy:
matrix:
# 7.4 should be updated to 8.2 when higher PHPUnit versions can be supported.
# 7.4 should be updated to 8.x when higher PHPUnit versions can be supported.
php_version: ['5.4', '7.4']
cs_dependencies: ['lowest', 'highest']
cs_dependencies: ['lowest', 'dev']

name: "Coverage${{ matrix.cs_dependencies == 'highest' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - CS Deps ${{ matrix.cs_dependencies }}"
name: "Coverage${{ matrix.cs_dependencies == 'stable' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - CS Deps ${{ matrix.cs_dependencies }}"

steps:
- name: Checkout code
uses: actions/checkout@v3

# On stable PHPCS versions, allow for PHP deprecation notices.
# With stable PHPCS dependencies, allow for PHP deprecation notices.
# Unit tests don't need to fail on those for stable releases where those issues won't get fixed anymore.
- name: Setup ini config
id: set_ini
run: |
if [[ "${{ matrix.cs_dependencies }}" != "highest" ]]; then
if [ "${{ matrix.cs_dependencies }}" != "dev" ]; then
echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED, display_errors=On' >> $GITHUB_OUTPUT
else
echo 'PHP_INI=error_reporting=-1, display_errors=On' >> $GITHUB_OUTPUT
Expand All @@ -169,11 +184,13 @@ jobs:
coverage: xdebug
tools: cs2pr

- name: 'Composer: adjust CS dependencies (high)'
if: ${{ matrix.cs_dependencies == 'highest' }}
run: |
composer require --no-update --no-scripts squizlabs/php_codesniffer:"${{ env.PHPCS_HIGHEST }}" --no-interaction
composer require --no-update --no-scripts wp-coding-standards/wpcs:"${{ env.WPCS_HIGHEST }}" --no-interaction
- name: "Composer: set PHPCS dependencies for tests (dev)"
if: ${{ matrix.cs_dependencies == 'dev' }}
# Note: while WPCS 3.0.0 is not yet supported, WPCS will not be updated for the `dev` builds.
# Once YoastCS has been updated for WPCS 3.0.0, WPCS dev-develop should be added to this command.
run: >
composer require --no-update --no-scripts --no-interaction
squizlabs/php_codesniffer:"${{ env.PHPCS_HIGHEST }}"
# Install dependencies and handle caching in one go.
# @link https://github.com/marketplace/actions/install-composer-dependencies
Expand All @@ -183,9 +200,12 @@ jobs:
# Bust the cache at least once a month - output format: YYYY-MM.
custom-cache-suffix: $(date -u "+%Y-%m")

- name: "Composer: set PHPCS version for tests (low)"
- name: "Composer: downgrade PHPCS dependencies for tests (lowest)"
if: ${{ matrix.cs_dependencies == 'lowest' }}
run: composer update squizlabs/php_codesniffer wp-coding-standards/wpcs --with-dependencies --prefer-lowest --ignore-platform-req=php+ --no-scripts --no-interaction
run: >
composer update --prefer-lowest --no-scripts --no-interaction
squizlabs/php_codesniffer
wp-coding-standards/wpcs
- name: Verify installed versions
run: composer info --no-dev
Expand All @@ -196,7 +216,7 @@ jobs:
# The results of the linting will be shown inline in the PR via the CS2PR tool.
# @link https://github.com/staabm/annotate-pull-request-from-checkstyle/
- name: Lint against parse errors
if: ${{ matrix.cs_dependencies == 'highest' }}
if: ${{ matrix.cs_dependencies == 'dev' }}
run: composer lint -- --checkstyle | cs2pr

- name: Run the unit tests with code coverage
Expand Down

0 comments on commit 6131d41

Please sign in to comment.