-
-
Notifications
You must be signed in to change notification settings - Fork 480
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
Travis: add a new check for early detection of ruleset problems #1430
Conversation
@@ -10,6 +10,7 @@ | |||
<!-- Exclude the code in the PHPCS 2.x test files copied in from PHPCS. --> | |||
<exclude-pattern>/Test/AllTests.php</exclude-pattern> | |||
<exclude-pattern>/Test/Standards/*.php</exclude-pattern> | |||
<exclude-pattern>/bin/class-ruleset-test.php</exclude-pattern> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bin/
is typically for shell scripts or other binaries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be happy to place it somewhere else, any suggestions on a more appropriate location ?
The /Test
directory came to mind, though all the other files in that directory will be removed in WPCS 2.x when PHPCS 2.x is dropped and the test files would need another name if placed there so as not to confuse PHPUnit.
WPCS itself uses `WordPress-Extra` + `WordPress-Docs` for its CS, which includes `WordPress-Core`. Those rulesets were therefore "tested" on each build, just by running them against the WPCS code. However, the WPCS native CS ruleset also excludes some things and the CS check ignores warnings for the Travis run and it also left the `WordPress-VIP` and `WordPress` rulesets _untested_. This PR intends to fix this by adding a minimal test file which should trigger most sniffs and testing each ruleset against it. This should, from now on, prevent the ruleset issues we've seen related to the moving and deprecating of sniffs. This will also serve as an early warning system in case any of the upstream sniffs which are included in the rulesets would be removed/renamed or otherwise have a change in behaviour which WPCS will need to take into account. Notes: * This is complimentary to the unit tests as those are run in a ruleset-agnostic manner, i.e. the ruleset isn't loaded and any settings in the ruleset are ignored. * At this moment, the test file is incomplete, but complete enough for our purposes. Further improvement of the test file in the future to make sure that all sniffs are triggered is recommended. * The check only needs to be run once against every PHPCS version supported/tested. With that in mind, "old-style" ignore annotations have been used. Once the minimum PHPCS version WPCS supports goes beyond PHPCS 3.2.0, those should be swopped out for selective ignores instead. * The check should be run on a high PHP version to prevent triggering the `Generic.PHP.Syntax` sniff when modern PHP code is included in the test file to ensure that this triggers the appropriate sniffs. At this moment, the most modern syntax included in nullable types which was introduced in PHP 7.1 which is why that is the PHP version against which these tests are currently run.
90fcca8
to
3d92e2f
Compare
Rebased so we can get a passing build. I'll merge once the build is green. |
WPCS itself uses
WordPress-Extra
+WordPress-Docs
for its CS, which includesWordPress-Core
.Those rulesets were therefore "tested" on each build, just by running them against the WPCS code.
However, the WPCS native CS ruleset also excludes some things and the CS check ignores warnings for the Travis run and it also left the
WordPress-VIP
andWordPress
rulesets untested.This PR intends to fix this by adding a minimal test file which should trigger most sniffs and testing each ruleset against it.
This should, from now on, prevent the ruleset issues we've seen related to the moving and deprecating of sniffs, i.e. #1325 and #1425.
This will also serve as an early warning system in case any of the upstream sniffs which are included in the rulesets would be removed/renamed or otherwise have a change in behaviour which WPCS will need to take into account.
Notes:
With that in mind, "old-style" ignore annotations have been used.
Once the minimum PHPCS version WPCS supports goes beyond PHPCS 3.2.0, those should be swopped out for selective ignores instead.
Generic.PHP.Syntax
sniff when modern PHP code is included in the test file to ensure that this triggers the appropriate sniffs.At this moment, the most modern syntax included in nullable types which was introduced in PHP 7.1 which is why that is the PHP version against which these tests are currently run.
N.B.: The builds for this PR won't pass until the current issue with the VIP deprecation warnings (#1425) is fixed, which is, of course, exactly the point of this PR ;-)