New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify Composer scripts #1629

Closed
wants to merge 11 commits into
base: develop
from
Copy path View file
@@ -51,21 +51,14 @@ before_install:
- phpenv config-rm xdebug.ini || echo 'No xdebug config.'
- export XMLLINT_INDENT=" "
- export PHPUNIT_DIR=/tmp/phpunit
- composer require squizlabs/php_codesniffer:${PHPCS_BRANCH} --update-no-dev --no-suggest --no-scripts
- |
if [[ "$SNIFF" == "1" ]]; then
composer install --dev --no-suggest
# The `dev` required DealerDirect Composer plugin takes care of the installed_paths.
else
# The above require already does the install.
$(pwd)/vendor/bin/phpcs --config-set installed_paths $(pwd)
fi
- composer install
- composer require squizlabs/php_codesniffer:${PHPCS_BRANCH} --no-suggest --no-scripts

script:
# Lint the PHP files against parse errors.
- if [[ "$LINT" == "1" ]]; then if find . -path ./vendor -prune -o -path ./bin -prune -o -name "*.php" -exec php -l {} \; | grep "^[Parse error|Fatal error]"; then exit 1; fi; fi
# Run the unit tests.
- phpunit --filter WordPress --bootstrap="$(pwd)/vendor/squizlabs/php_codesniffer/tests/bootstrap.php" $(pwd)/vendor/squizlabs/php_codesniffer/tests/AllTests.php
- composer run-test
# Test for fixer conflicts by running the auto-fixers of the complete WPCS over the test case files.
# This is not an exhaustive test, but should give an early indication for typical fixer conflicts.
# For the first run, the exit code will be 1 (= all fixable errors fixed).
Copy path View file
@@ -29,16 +29,16 @@
"minimum-stability": "RC",
"scripts": {
"check-cs": [
"@php ./vendor/squizlabs/php_codesniffer/bin/phpcs"
"@php ./vendor/bin/phpcs"
],
"fix-cs": [
"@php ./vendor/squizlabs/php_codesniffer/bin/phpcbf"
"@php ./vendor/bin/phpcbf"
],
"install-codestandards": [
"Dealerdirect\\Composer\\Plugin\\Installers\\PHPCodeSniffer\\Plugin::run"
],
"run-tests": [
"@php ./vendor/phpunit/phpunit/phpunit --filter WordPress --bootstrap=\"./vendor/squizlabs/php_codesniffer/tests/bootstrap.php\" ./vendor/squizlabs/php_codesniffer/tests/AllTests.php"
This conversation was marked as resolved by kasparsd

This comment has been minimized.

@kasparsd

kasparsd Jan 18, 2019

Author

Specify the bootstrap file in phpunit.xml.dist.

This comment has been minimized.

@jrfnl

jrfnl Jan 18, 2019

Member

No. This won't work. You are presuming that all WPCS devs work off a Composer based install.
This is not the case and undesirable as that would make it a lot more fiddly to test things with anything other than a stable PHPCS release.

Read the unit test instructions in the CONTRIBUTING.md file to learn why and how devs can have different setups.

This comment has been minimized.

@kasparsd

kasparsd Jan 18, 2019

Author

@jrfnl It was my understanding that phpunit is always looking for configuration in phpunit.xml.dist, even if installed globally and without Composer. See the comment on the phpunit.xml.dist diff.

This comment has been minimized.

@jrfnl

jrfnl Jan 18, 2019

Member

Yes, PHPUnit will pick up on the config file without problem, however the bootstrap file in the vendor directory will not exist in non-Composer installs and therefore running the unit tests will be broken for anyone using a non-Composer based dev setup.

"@php ./vendor/bin/phpunit --filter WordPress --bootstrap ./vendor/squizlabs/php_codesniffer/tests/bootstrap.php ./vendor/squizlabs/php_codesniffer/tests/AllTests.php"
]
},
"support": {
ProTip! Use n and p to navigate between commits in a pull request.