-
Notifications
You must be signed in to change notification settings - Fork 3
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
43 changed files
with
1,227 additions
and
335 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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 |
---|---|---|
@@ -0,0 +1,116 @@ | ||
# Contributing to YoastCS | ||
|
||
|
||
## Acceptance testing | ||
|
||
There are various ways to acceptance test a sniff change: | ||
1. Based on the tests included with the change. | ||
2. Based on a code sample with "good" and "bad" code. | ||
3. Based on an existing plugin code base. | ||
|
||
Note: the above list is ordered by difficulty. Testing via method 1 will generally be easiest, method 3 most complicated. | ||
|
||
In the fold-outs below are step by step instructions on how to acceptance test via each of these methods. | ||
|
||
--- | ||
<details> | ||
<summary>Step by step: acceptance test based on the integration tests included with the change</summary> | ||
|
||
1. Check out a local copy of this repo using your favourite git tool. | ||
2. Check out the branch containing the change. | ||
3. Run `composer install`. | ||
4. Revert the changes to the sniff file (do not commit!), but leave the changes to the test file(s) in place. | ||
5. Run the tests using `composer test`. The tests should fail in the expected places (see the commit message of the change for what to expect and the line numbers to expect errors and warnings on in the `*Test.php` file for the sniff). | ||
6. Reset the sniff file to its committed state. | ||
7. Run the tests again using `composer test`. The tests should now pass. | ||
|
||
</details> | ||
|
||
<details> | ||
<summary>Step by step: acceptance test based on a code sample with "good" and "bad" code</summary> | ||
|
||
1. Check out a local copy of this repo using your favourite git tool. | ||
2. Check out the `develop` branch. | ||
3. Run `composer install`. | ||
4. Create a simple PHP file with some code which should be flagged and some code which shouldn't be flagged. | ||
5. Save this file in a `temp` subdirectory in your local YoastCS copy and make sure to add this `temp` directory to your `.git/info/exclude` file. | ||
Do not commmit this file! | ||
6. Run `vendor/bin/phpcs -ps ./temp/yourfilename.php --standard=Yoast --report=full,source`. | ||
This should fail in the expected places (see the commit message of the change for what to expect), i.e. things not getting flagged which should get flagged or things getting flagged, which shouldn't get flagged. | ||
Pro-tip: if the test is just for a single sniff, you can limit the output to just that sniff by adding `--sniffs=Yoast.Category.SniffName` (replace `Category` and `SniffName` with the applicable names). | ||
7. Check out the branch containing the change. | ||
8. Run the command from [6] again. The sniff should now flag things correctly. | ||
|
||
If the sniff change includes changes to/adding of auto-fixers, the fixing should also be tested. | ||
|
||
9. Check out the `develop` branch again. | ||
10. Run `vendor/bin/phpcbf -ps ./temp/yourfilename.php --standard=Yoast --suffix=.fixed`. | ||
This will create a copy of your test file named `yourfilename.php.fixed`. | ||
Pro-tip: you can use the `--sniffs=Yoast.Category.SniffName` addition to the command for this step too. | ||
11. Examine the fixes made and expect them to be incorrect. | ||
12. Check out the branch containing the change. | ||
13. Repeat step 10 and 11. The sniff should now fix things correctly. | ||
|
||
If you like, you can now delete the `temp` directory and your test file(s), but leaving them in place for the next round of testing shouldn't do any harm. | ||
|
||
</details> | ||
|
||
<details> | ||
<summary>Step by step: acceptance test based on a existing plugin code base</summary> | ||
|
||
1. Create a simple PHP file with some code which should be flagged and some code which shouldn't be flagged (or adjust an existing file). | ||
2. Save this file somewhere in the plugin. | ||
Do not commmit this file (or the changes made to an existing file)! | ||
3. Run `vendor/bin/phpcs --report=full,source`. | ||
This should fail in the expected places (see the commit message of the change for what to expect), i.e. things not getting flagged which should get flagged or things getting flagged, which shouldn't get flagged. | ||
Pro-tip: if the test is just for a single sniff, you can limit the output to just that sniff by adding `--sniffs=Yoast.Category.SniffName` (replace `Category` and `SniffName` with the applicable names). | ||
4. Run `composer update yoast/yoastcs:"dev-featurebranchname"` from the root of the plugin in which you are testing the change. | ||
I.e. a branch in YoastCS named `feature/sniffname-change` should be referenced as `dev-feature/sniffname-change`. | ||
Again: do not commit this change to the `composer.json` and `composer.lock` files! | ||
5. Run the command from [3] again. The sniff should now flag things correctly. | ||
|
||
If the sniff change includes changes to/adding of auto-fixers, the fixing should also be tested. | ||
|
||
6. Reset the changes to the `composer.json` and `composer.lock` file, but do **not** reset the changes to your test file. | ||
7. Run `composer install`. | ||
8. Run `vendor/bin/phpcbf --suffix=.fixed`. | ||
This will create a copy of your test file with `fixed` at the end. So a file originally named `src/admin/classname.php` will now have a second copy named `src/admin/classname.php.fixed`. | ||
Pro-tip: you can use the `--sniffs=Yoast.Category.SniffName` addition to the command for this step too. | ||
9. Examine the fixes made and expect them to be incorrect. | ||
10. Run `composer update yoast/yoastcs:"dev-featurebranchname"` again from the root of the plugin in which you are testing the change. | ||
Again: do not commit this change to the `composer.json` and `composer.lock` files! | ||
11. Repeat step 8 and 9. The sniff should now fix things correctly. | ||
|
||
Clean up: make sure to reset all the file changes made during testing! | ||
|
||
</details> | ||
|
||
--- | ||
|
||
### Gotcha's when testing sniff changes | ||
|
||
#### Public properties | ||
|
||
Some sniffs will behave differently based on the value of the sniff's `public` properties. | ||
These properties can be set [from a custom ruleset](https://github.com/squizlabs/PHP_CodeSniffer/wiki/Annotated-Ruleset) or in a test situation, in-line using `// phpcs:set Yoast.Category.SniffName propertyName propertyValue`. | ||
|
||
If the results you are getting when testing are different from what you expected, first thing to do is to check whether the sniff has `public` properties, what those properties are set to (in your custom ruleset or in a test file in-line) and whether that setting is interferring with the results. | ||
|
||
#### Severity | ||
|
||
The default severity level in PHPCS to show notices is `5`. | ||
|
||
Occassionaly (not very often), certain notices will be given a lower severity to prevent "noise" in the CI, while still allowing for something to be (manually) checked. | ||
|
||
If the results you are getting when testing are different from what you expected, check if the `$severity` is set for the error message you are missing (fifth parameter in a call to `addError()` or `addWarning()`) and if so, add `--severity=1` to the command line command you are running to get those messages to show up. | ||
|
||
|
||
### For regular sniff testers | ||
|
||
If you regularly test sniffs and/or want to contribute to YoastCS with sniff additions or sniff changes, all the setup needed for the above testing methods can get tedious. | ||
|
||
In that case, I'd recommend adding the `vendor/squizlabs/php_codesniffer/bin` directory within `YoastCS` to your system path. This should make the `phpcs` and `phpcbf` commands available anywhere on your system. You can then use those commands anywhere and whichever branch you have checked-out in your YoastCS clone will be used to run the requested scans. | ||
|
||
In effect, this means that if you run a scan using `phpcs` within a plugin directory, the ruleset of that plugin will still be respected, but instead of using the YoastCS install in the `vendor` directory of the plugin, your cloned YoastCS install will be used. | ||
|
||
If you also (want to) contribute to WordPressCS, PHPCS and/or other standards, contact [@jrfnl](http://github.com/jrfnl/) to discuss an even more robust setup. |
This file contains 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 |
---|---|---|
@@ -0,0 +1,85 @@ | ||
name: CS | ||
|
||
on: | ||
# Run on all pushes and on all pull requests. | ||
# Prevent the build from running when there are only irrelevant changes. | ||
push: | ||
paths-ignore: | ||
- '**.md' | ||
pull_request: | ||
# Allow manually triggering the workflow. | ||
workflow_dispatch: | ||
|
||
jobs: | ||
checkcs: | ||
name: 'Basic CS and QA checks' | ||
runs-on: ubuntu-latest | ||
|
||
env: | ||
XMLLINT_INDENT: ' ' | ||
|
||
steps: | ||
- name: Checkout code | ||
uses: actions/checkout@v2 | ||
|
||
- name: Install PHP | ||
uses: shivammathur/setup-php@v2 | ||
with: | ||
php-version: '7.4' | ||
coverage: none | ||
tools: cs2pr | ||
|
||
# Validate the composer.json file. | ||
# @link https://getcomposer.org/doc/03-cli.md#validate | ||
- name: Validate Composer installation | ||
run: composer validate --no-check-all --strict | ||
|
||
- name: 'Composer: adjust dependencies' | ||
run: | | ||
# The sniff stage doesn't run the unit tests, so no need for PHPUnit. | ||
composer remove --no-update --dev phpunit/phpunit --no-scripts | ||
# Using PHPCS `master` as an early detection system for bugs upstream. | ||
composer require --no-update --no-scripts squizlabs/php_codesniffer:"dev-master" | ||
# Using WPCS `master` (=stable). This can be changed back to `dev-develop` after the WPCS 3.0.0 release. | ||
composer require --no-update --no-scripts wp-coding-standards/wpcs:"dev-master" | ||
# Install dependencies and handle caching in one go. | ||
# @link https://github.com/marketplace/actions/install-composer-dependencies | ||
- name: Install Composer dependencies | ||
uses: ramsey/composer-install@v1 | ||
|
||
- name: Install xmllint | ||
run: sudo apt-get install --no-install-recommends -y libxml2-utils | ||
|
||
# Show XML violations inline in the file diff. | ||
# @link https://github.com/marketplace/actions/xmllint-problem-matcher | ||
- uses: korelstar/xmllint-problem-matcher@v1 | ||
|
||
# Validate the ruleset XML file. | ||
# @link http://xmlsoft.org/xmllint.html | ||
- name: Validate ruleset against XML schema | ||
run: xmllint --noout --schema ./vendor/squizlabs/php_codesniffer/phpcs.xsd ./Yoast/ruleset.xml | ||
|
||
# Validate the Docs XML files. | ||
# @link http://xmlsoft.org/xmllint.html | ||
# For the build to properly error when validating against a scheme, these each have to be in their own condition. | ||
- name: Lint the XML sniff docs | ||
run: xmllint --noout ./Yoast/Docs/*/*Standard.xml | ||
|
||
# Check the code-style consistency of the XML ruleset files. | ||
- name: Check XML ruleset code style | ||
run: diff -B --tabsize=4 ./Yoast/ruleset.xml <(xmllint --format "./Yoast/ruleset.xml") | ||
|
||
# Check the codestyle of the files within YoastCS. | ||
# The results of the CS check will be shown inline in the PR via the CS2PR tool. | ||
# @link https://github.com/staabm/annotate-pull-request-from-checkstyle/ | ||
- name: Check PHP code style | ||
continue-on-error: true | ||
run: composer check-cs -- --report-full --report-checkstyle=./phpcs-report.xml | ||
|
||
- name: Show PHPCS results in PR | ||
run: cs2pr ./phpcs-report.xml | ||
|
||
# Check that the sniffs available are feature complete. | ||
- name: Check sniff feature completeness | ||
run: composer check-complete |
This file contains 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 |
---|---|---|
@@ -0,0 +1,88 @@ | ||
name: Quicktest | ||
|
||
on: | ||
# Run on pushes, including merges, to all branches except `master`. | ||
push: | ||
branches-ignore: | ||
- master | ||
paths-ignore: | ||
- '**.md' | ||
# Allow manually triggering the workflow. | ||
workflow_dispatch: | ||
|
||
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. | ||
quicktest: | ||
runs-on: ubuntu-latest | ||
|
||
strategy: | ||
matrix: | ||
include: | ||
- php_version: 'latest' | ||
phpcs_version: 'dev-master' | ||
wpcs_version: 'dev-master' | ||
- php_version: 'latest' | ||
phpcs_version: '3.6.0' | ||
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.6.0' | ||
wpcs_version: 'dev-master' | ||
|
||
name: "QTest${{ matrix.phpcs_version == 'dev-master' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - PHPCS ${{ matrix.phpcs_version }} - WPCS ${{ matrix.wpcs_version }}" | ||
|
||
steps: | ||
- name: Checkout code | ||
uses: actions/checkout@v2 | ||
|
||
# On stable PHPCS versions, 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 | ||
echo '::set-output name=PHP_INI::error_reporting=E_ALL & ~E_DEPRECATED, display_errors=On' | ||
else | ||
echo '::set-output name=PHP_INI::error_reporting=E_ALL, display_errors=On' | ||
fi | ||
- name: Install PHP | ||
uses: shivammathur/setup-php@v2 | ||
with: | ||
php-version: ${{ matrix.php_version }} | ||
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 }}" | ||
# Set the WPCS version to test against. | ||
composer require --no-update --no-scripts wp-coding-standards/wpcs:"${{ matrix.wpcs_version }}" | ||
# Install dependencies and handle caching in one go. | ||
# @link https://github.com/marketplace/actions/install-composer-dependencies | ||
- name: Install Composer dependencies - normal | ||
if: ${{ startsWith( matrix.php_version, '8' ) == false && matrix.php_version != 'latest' }} | ||
uses: ramsey/composer-install@v1 | ||
|
||
# For the PHP 8.0 and higher, we need to install with ignore platform reqs as not all dependencies allow it. | ||
- name: Install Composer dependencies - with ignore platform | ||
if: ${{ startsWith( matrix.php_version, '8' ) || matrix.php_version == 'latest' }} | ||
uses: ramsey/composer-install@v1 | ||
with: | ||
composer-options: --ignore-platform-reqs | ||
|
||
- name: Verify installed standards | ||
run: vendor/bin/phpcs -i | ||
|
||
- name: Lint against parse errors | ||
if: matrix.phpcs_version == 'dev-master' | ||
run: composer lint | ||
|
||
- name: Run the unit tests | ||
run: composer test |
Oops, something went wrong.