The `unit-tests` workflow runs code coverage for pushes to `main` and pull requests, but wasn't limited to this repo.
This resulted in failing builds for contributors who have actions enabled on their fork. The reason for the failure is that they don't have access to the `CODECOV_TOKEN`.
This commit changes the conditions on when to run the tests normally and when to run with code coverage to take the organisation where the repo lives into account. That should prevent the failing builds in forks.
* Composer: raise the minimum supported PHPCSUtils version to 1.1.0 to benefit from new functionality.
This also automatically raises the minimum PHPCS version to `3.13.0` for PHP 8.2 DNF type support and tokenizer support for PHP 8.4 `final` properties and asymmetric visibility.
Includes syncing the PHPCSExtra version to a version which also has a minimum PHPCSUtils version of 1.1.0.
Ref:
* https://github.com/PHPCSStandards/PHPCSUtils/releases/tag/1.1.0
* https://github.com/PHPCSStandards/PHPCSExtra/releases/tag/1.4.0
* NamingConventions/ValidVariableName: allow for PHP 8.4 properties in interfaces
* Builds against PHP 8.4 are no longer allowed to fail.
* Add _allowed to fail_ build against PHP 8.5.
* Update the "tested against" badge in the README.
Ref: https://www.php.net/releases/8.4/en.php
As things were, when testing against "dev" versions, only PHPCS "dev" was used, while WPCS now has more dependencies, i.e. PHPCSUtils and PHPCSExtra.
This commit updates the workflows to take that into account better.
It also improves the readability of the workflows by using multi-line command in a few places.
Notes:
* This renames the `phpcs_version` matrix variable to `dependencies` in all workflows to make it clear this is not just about PHPCS itself, but about all dependencies.
* The CS check for the own codebase is always run against "dev" versions of all dependencies as an early detection system for upstream issues.
* The "Ruleset" checks are run against low/stable/dev.
* The fixer conflict check is run against dev only, as conflicts detected cannot be fixed anymore in already released versions.
* The "quick check" now only runs against low/stable.
* The "test" run run against both low/stable by default and now has some extra builds against "dev" versions.
- These extra builds could be set to "continue on error", but I've chosen not to do so as if any issues are detected, they should be fixed asap as they will impact WPCS sooner rather than later.
- Code coverage will now be run against low/high PHP with low/stable dependencies and no longer against dev.
* Also note that, while the "quick test"/"test" runs shouldn't need PHPCSExtra, as that is a dependency used only in the ruleset, not in our sniff code, I've still included PHPCSExtra in the upgrade/downgrade block for consistency.
These changes do mean that the "required build"/branch protection will need to be updated (again) before the PR can be merged.
I will do so once the PR has been approved.
PHP_CodeSniffer 3.8.0 now allows for running the tests, which are based on the PHPCS native test suite, with PHPUnit 8 and 9.
This commit updates the package to take advantage of that.
Includes:
* Widening the PHPUnit version requirements.
* Adding the PHPUnit 8+ cache file to `.gitignore`.
* Updating the PHPUnit configuration to make sure that deprecations will always show, even when on a high PHPUnit 9.6 version.
* Updating the info in `CONTRIBUTING`.
* Simplifications to the `quicktest` and `test` workflows.
Also, the code coverage "high" run can now be run against PHP 8.3.
* Running PHPStan against PHP `latest` (couldn't previously be done due to the old PHPUnit version).
* Minor tweak to the PHPStan config to make sure PHP 8.x specific issues don't get flagged for this codebase.
* Removing a no longer needed `--ignore-platform*` argument.
* Updating the "setUp" method in one test to call the parent "setUp" method via the new method name.
Ref:
* PHPCSStandards/PHP_CodeSniffer 59
Follow up after PR 2225 and other PRs which added more tests and raised code coverage.
The Codecov service is a way to monitor test vs code coverage of a project over time and allows for the code coverage % + delta to be reported in each PR.
This commit:
* Adds CodeCov configuration and ignores it for creating the package file.
* Note: by default CodeCov reports back with build statuses (in the table at the bottom of a PR) + posts a really extensive comment on a PR.
I personally find those comments + mail on each PR terribly annoying and unnecessary when the build statuses are set to required statuses anyway, so I've turned the posting of those comments off (via a setting in the configuration file).
* Another thing to note is the patch - threshold setting being at `3%` (= 3% deviation allowed). The reason for setting that threshold a little wider is that code removals for fully covered code would otherwise quickly fail builds.
* Adds a badge showing off code coverage to the README.
* Adjusts the GH Actions workflows.
* For the `test` workflow (PRs), the tests will selectively be run with code coverage (high/low PHP x high/low PHPCS).
* For the `quicktest` workflow (pushes), which only runs high/low PHP/PHPCS, the code coverage will only be run for (merges to) the `develop` branch.
This will allow us keep track of code coverage over time, while not slowing down the "quick test" for feature branch pushes.
Note: I've simplified the conditions a little for this workflow as we only run against two known PHP versions anyway.
* Makes a minor tweak to the code coverage logging config to only display a summary after a code coverage run.
Note: As this is a public repo, we shouldn't need a repo secret for the CodeCov token. The normal GHA token will be used (and should work fine).
Refs:
* https://docs.codecov.com/docs
* https://docs.codecov.com/docs/codecovyml-reference
Note: as this PR can only partially be tested without it being merged, minor touch-up may be needed after this initial setup, but :fingers_crossed: we should be good.
As things were, whenever the minimum PHPCS version would be changed, the branch protection settings for both the `master` and the `develop` branch would need to be updated and all "required builds" referencing the old PHPCS version would need to be removed, while new "required builds" would need to be added referencing the new minimum PHPCS version.
This was a fiddly process and time-consuming.
The change proposed in this commit takes advantage of the Composer `--prefer-lowest` setting to archive the same without a hard-coded PHPCS version in the build name, which means that once the branch protection settings have been updated for this PR, they shouldn't need updating anymore for future PHPCS version bumps.
Since Composer 2.2, we can be more specific about which platform requirements should be ignored.
This change ensures that only the "high" end of a PHP requirement will be ignored and no other platform requirements are ignored.
Ref: https://blog.packagist.com/composer-2-2/#-ignore-platform-req-improvements
This commit fixes two, closely related, issues:
1. The "is a name in snake case ?" determination was done in three slightly different ways between the `ValidFunctionName` sniff and the `ValidVariableName` sniff and the `SnakeCaseHelper::get_suggestion()` method, which meant that a name which would be regarded valid snakecase for the one, may not be regarded as valid snakecase for the other.
It could also result in the suggested name being the same as the existing name.
2. The "is a name in snake case ?" as well as the suggestion determination didn't take non-ASCII characters into account consistently.
While PHPCSUtils will at some point get better methods for handling these things, for now, we can still make a quick fix to improve this situation.
To that end, I've made the following changes:
* Instead of having each sniff determine whether or not something is valid snakecase itself, let the sniffs retrieve the suggested alternative name and compare the original name against that and only throw an error if the suggestion is not the same as the original name.
This also allows for removing some code related to potential leading underscores (which looks to be redundant anyway).
* Improved the creation of the suggestion to handle non-ASCII names better (in as far as possible).
Includes:
* The `Mbstring` extension has been added as a suggested extension to the `composer.json` file.
* Adding one extra test run to the CI workflow which runs the tests without the Mbstring extension to safeguard the fallback code.
Includes unit tests.
Fixes 1891
---
Note: as of PHP 8.2, `strtolower()` no longer acts on non-ASCII characters, which largely got rid of the issue already and made for some interesting debugging. Unfortunately, we cannot rely on everyone being on PHP 8.2...
PHP 8.2 has been released today 🎉 and the `setup-php` action has announced support for PHP 8.3, so adding PHP 8.3 to the matrix.
Note: PHPCS does not (yet) have full syntax support for PHP 8.2, but it does have runtime support (for the most part, see squizlabs/PHP_CodeSniffer 3629).
Builds against PHP 8.3 are still allowed to fail for now.
This step is for PHPCS 4.x, but we're not testing against PHPCS 4.x at this time and when we do enable it again, this step may well no longer be needed anymore anyway, so let's remove it for now and re-evaluate what is needed when we re-enable testing against PHPCS 4.x.
All steps in the workflows have descriptive names, so re-iterating the steps in a "docblock" above each job is redundant and error-prone maintenance-wise.
Caches used in GH Actions do not get updated, they can only be replaced by a different cache with a different cache key.
Now the predefined Composer install action this repo is using already creates a pretty comprehensive cache key:
> `ramsey/composer-install` will auto-generate a cache key which is composed of
the following elements:
> * The OS image name, like `ubuntu-latest`.
> * The exact PHP version, like `8.1.11`.
> * The options passed via `composer-options`.
> * The dependency version setting as per `dependency-versions`.
> * The working directory as per `working-directory`.
> * A hash of the `composer.json` and/or `composer.lock` files.
This means that aside from other factors, the cache will always be busted when changes are made to the (committed) `composer.json` or the `composer.lock` file (if the latter exists in the repo).
For packages running on recent versions of PHP, it also means that the cache will automatically be busted once a month when a new PHP version comes out.
### The problem
For runs on older PHP versions which don't receive updates anymore, the cache will not be busted via new PHP version releases, so effectively, the cache will only be busted when a change is made to the `composer.json`/`composer.lock` file - which may not happen that frequently on low-traffic repos.
But... packages _in use_ on those older PHP versions - especially dependencies of declared dependencies - may still release new versions and those new versions will not exist in the cache and will need to be downloaded each time the action is run and over time the cache gets less and less relevant as more and more packages will need to be downloaded for each run.
### The solution
To combat this issue, a new `custom-cache-suffix` option has been added to the Composer install action in version 2.2.0.
This new option allows for providing some extra information to add to the cache key, which allows for busting the cache based on your own additional criteria.
This commit implements the use of this `custom-cache-suffix` option for all relevant workflows in this repo.
Refs:
* https://github.com/ramsey/composer-install/#custom-cache-suffix
* https://github.com/ramsey/composer-install/releases/tag/2.2.0
Follow up on 2058
The minimum PHPCS version for PHPCSUtils 1.0.0 will be 3.7.1, so the minimum PHPCS version for WPCS should be in line with that.
The difference between 3.7.0 and 3.7.1 is very small. They were released just days apart with 3.7.1 fixing a pertinent bug in the retokenization of reserved keywords, which was updated in PHPCS 3.7.0.
Includes updating the GH Actions matrixes for this change.
As discussed in issue 2048 and PR 2049, to add support for PHP 8.1 features to WPCS and prevent false positives related to PHP 8.1 features, this commit ups the minimum supported PHPCS version to PHPCS 3.7.0.
Includes updating the GH Actions matrixes for this change.
Fixes 2048
While the minimum PHPCS version has already been upped to PHPCS 3.5.0 previously for the WPCS 3.0.0 branch, I'm proposing to up it yet further to the latest PHPCS version available.
PHP 8.0 and 8.1 introduced a lot of new syntaxes and, while a lot of this can be supported for older PHPCS versions via PHPCSUtils, this will always impact performance and precision, so it is strongly advisable to use the latest PHPCS version as the new minimum if we want to support the newly available PHP syntaxes.
For now, I'm upping the minimum version to PHPCS 3.6.2, which is the latest PHPCS release. This will unblock PRs to add support for PHP 8.0 features to WPCS.
Once PHPCS 3.7.0 has been released, the minimum PHPCS version should be upped again to unblock adding support for PHP 8.1 features to WPCS.
Includes updating the GH Actions matrixes for this change.
Also see #2048
A number of predefined actions have had major release, which warrant an update the workflow(s).
These updates don't actually contain any changed functionality, they are mostly just a change of the Node version used by the action itself (from Node 14 to Node 16).
Refs:
* https://github.com/actions/checkout/releases
This commit makes the necessary adjustments to the test matrix to account for that.
Note: the protected branch settings should be adjusted after this change to include the two new PHP 8.1 builds.
Turns out the default setting for `error_reporting` used by the SetupPHP action is `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT` and `display_errors` is set to `Off`.
For the purposes of CI, I'd recommend running with `E_ALL` and `display_errors=On` to ensure **all** PHP notices are shown.
In this script, error_reporting was already enabled, but the error display was not yet fixed. Sorted now.
Letting PHPUnit read the config file causes an error on PHP 8.1 in combination with PHPUnit 7.
For now, special case the test run on PHP 8.1 and pass the only necessary config from the configuration file on via CLI arguments.
As it looks like PHPCS 4.x is still quite a while away (2022 at the earliest), let's stop testing against PHPCS 4.x for the time being.
This should allow build failure reporting to be more accurate, as currently every PR has a failure on PHPCS 4.x due to a bug in some of the new code in 4.x (fix for this was pulled six months ago and still not merged).
The build against PHPCS 4.x should be re-enabled closer to the PHPCS 4.x release.
Note: PHPCSUtils will continue to test against PHPCS 4.x and will update the provided utilities ahead of time, so with a bit of luck, by the time the build against PHPCS 4.x is re-enabled, the build should largely pass thanks to the compatibility layers in PHPCSUtils.
* Move the "sniffs" stage over to a GitHub Action workflow.
* Update Sniff workflow.
* Load `libxml2-utils`.
* Correct extension syntax.
* Fix typo.
* Manually install xmllint.
* First pass at a Ruleset stage workflow.
* Pass the matrix defined PHP version.
* Correct the ruleset name.
* Set error reporting through the `setup-php` action.
* Correct typo.
* First pass at quick tests.
* Correct casing of environment variable.
* Update the inline documentation for the sniff workflow.
* Cleanup the ruleset workflow.
* Add full test suite file and other changes.
- Update each file with full inline documentation.
- Update each file to use consistent naming and approaches across workflow files.
- Remove travis.yml file.
* Remove TravisCI configuration file from the `.gitattributes` file.
* Update .github/workflows/unit-tests.yml
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
* Update .github/workflows/unit-tests.yml
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
* `--no-suggest` is now the default behavior.
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
* Clarify the step name for setting the PHPCS version.
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
* `--no-suggest` is now the default behavior.
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
* Prefix each job name and remove the check for `lint`.
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
* Remove the check for the `lint` key in the matrix.
* Use the `latest` alias to test on PHP 8.0 instead of 7.4.
* Explicitly specify `coverage` as `none` for the `setup-php` action.
* Ignore documentation changes for the quick test workflow.
* Fix syntax error in job name.
* Prefix each ruleset job name.
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
* Don't run the quick tests on `master`, and skip documentation only fixes.
* Use `error_reporting = E_ALL & ~E_DEPRECATED` for both runs in the ruleset workflow.
* Revert to using 7.4 to avoid failures.
* Released versions of PHPCS should not be expected to pass on PHP nightly.
* Don't run the full test suite on `develop`.
The full suite will have just run in the PR context before being merged.
* `$(pwd)` is not necessary here.
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
* Ignore markdown files for the sniff workflow.
* Consolidate PHP installation steps.
* Combine the sniffs and ruleset checks into one workflow.
* Remove `--no-suggest`, which was deprecated in Composer 2.
* List Composer install steps in a more logical order.
* Only run `phpcbf` once.
This updates the `phpcbf` command to interpret a `1` exit code as success, as only fixable errors were discovered. This prevents having to run the command twice.
* Remove duplicate PHP install step.
* Simplify managing Composer dependencies and caches.
* Cleanup some inline docs.
* GH Actions: move `ruleset check` job down
* GH Actions: allow for manually triggering a workflow
Triggering a workflow for a branch manually is not supported by default in GH Actions, but has to be explicitly allowed.
Ref: https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/
* GH Actions: remove the ignores for markdown files
... as it would prevent PRs with only markdown changes from being merged due to required statuses.
* GH actions: move environment variable to job
The environment variable is only used in one job, so make it only available to *that* job.
* GH actions: add "setup ini config" step to Unit Test job
This was already implemented based on feedback for the QuickTest workflow, but not for the UnitTest workflow.
* GH Actions: report PHP lint violations in the PR
The cs2pr tool will allow to display the results from an action run in checkstyle format in-line in the PR code view, which should improve usability of the workflow results.
This means that for the `lint` action an additional parameter `--checkstyle` is needed, which does not give suitable output for when the Composer script is run locally.
The command with the extra parameter could be placed directly in the CI workflow, but this runs the risk of changes made to the local command not being made to the CI version and visa versa.
For that reason, an additional script has been added to the Composer `scripts` section.
Ref: https://github.com/staabm/annotate-pull-request-from-checkstyle
* GH Actions: report PHP code style violations in the PR
The cs2pr tool will allow to display the results from an action run in checkstyle format in-line in the PR code view, which should improve usability of the workflow results.
Ref: https://github.com/staabm/annotate-pull-request-from-checkstyle
* GH Actions: report XML lint violations in the PR
The `korelstar/xmllint-problem-matcher` tool will allow to display the results from an xmllint action directly in the PR.
Ref: https://github.com/staabm/annotate-pull-request-from-checkstyle
* GH Actions: use an action to handle the caching for Composer
This was already implemented based on feedback for the QuickTest workflow, but not for the other workflows/jobs.
* GH Actions: split long line with multiple commands
* GH Actions: fix up various step names
* `Install PHPCS` -> `Set PHPCS version`
In that particular step, we're not actually _installing_ PHPCS (`--no-update`), just updating the version being required.
* `Check that all sniffs have unit tests` => `Check sniff feature completeness`
The script run in this step checks feature completeness for PHPCS sniffs. The fact that the documentation check is currently disabled is irrelevant (and runs the risk of when it _does_ get enabled for the step name to become incorrect).
* `Check the ... ruleset` => `Test the ... ruleset`
This is a _test_ where we are testing that no unexpected errors/warnings are being thrown.
Also fixes a typo.
* GH Actions: minor inline documentation tweaks
Remove some documentation which isn't relevant (anymore) or which was in the wrong place.
Add some documentation where necessary.
* GH Actions: shorter name for the basic QA workflow
The name of the workflow is used in the badges displayed in the Readme. The name as was would make for an awkwardly long badge name.
* GH Actions: rename `phpcs_branch` to `phpcs_version`
PHPCS doesn't have named branches for the various releases. It has tags and the `master` branch, so `phpcs_version` is more descriptive.
* GH action: miscellaneous tweaks
* Readme: replace the CI status badges
* GH Actions: try slightly different command syntax for CS check
... as the `ignore_warnings_on_exit` does not seem to get respected when this is split into two commands.
* GH Actions: undo "show CS violations in PR"
... as the cs2pr combined with the `--runtime-set ignore_warnings_on_exit 1` appears to be buggy.
To be revisited later.
* Remove quotes around action name.
Co-authored-by: Denis Žoljom <dingo-d@users.noreply.github.com>
* Remove extra space.
Co-authored-by: Denis Žoljom <dingo-d@users.noreply.github.com>
* Remove quotes around action name.
Co-authored-by: Denis Žoljom <dingo-d@users.noreply.github.com>
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
Co-authored-by: Denis Žoljom <dingo-d@users.noreply.github.com>
