WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.54k stars 479 forks source link

PHPUnit is not included in the Composer dependencies #766

Closed stevegrunwell closed 6 years ago

stevegrunwell commented 7 years ago

According to the Contributing doc, PHPUnit should be available via /vendor/bin/phpunit, but the binary is not available.

Looking through the complete history of composer.json, it appears that the file has never included PHPUnit, but is instead relying on it being included through PHP_CodeSniffer. Unfortunately, (to my knowledge) Composer doesn't recursively load development dependencies; a contributor would have to explicitly cd into vendor/squizlabs/php_codesniffer/ and run composer install. Even after doing so, the tests/AllTests.php file referenced in the documentation doesn't appear to exist in the project.

It may be worthwhile for one of the maintainers — someone able to execute the test suite — tries a clean installation and updates the contributing guidelines.

GaryJones commented 7 years ago

From the documentation (my emphasis):

If you have installed phpcs and the WordPress-Coding-Standards as noted in the README, then you can navigate to the directory where the phpcs repo is checked out and do

So, for my situation, I have phpcs checked out at ~/code/phpcs, and wpcs at ~/code/wpcs. So, even though I'd want to run the unit tests covering WPCS, I'd cd into the ~/code/phpcs, and run ./vendor/bin/phpunit from there.

screenshot 2017-01-01 19 13 25

At no point do we say phpunit is available locally under the wpcs repo dependencies - that's not how sniff tests work.

stevegrunwell commented 7 years ago

Ugh, that would explain a lot. My apologies for apparently not drinking enough coffee before reading the documentation.

Since PHP_CodeSniffer doesn't package the tests/ directory in the distribution, however (see #548), contributors to WPCS need to use Composer's --prefer-source flag when installing locally for development.

I've opened #767 with some minor tweaks to the contributing doc.

jrfnl commented 6 years ago

Closing as #767 was closed as sufficiently addressed.