Skip to content
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 check to make sure that sniffs are "feature complete" #2364

Closed

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jan 11, 2019

This PR adds a custom script which will check whether each and every sniff is accompanied by a documentation XML file (warning) as well as unit test files (error).

Notes:

  • To only show errors, the script can be run in quiet mode by passing the -q flag on the command line.
  • The script has been set up in such a way that it is not only suitable for use by PHPCS itself, but also for use by external standards which use the PHPCS native unit test suite.
    For external standards to use the tool, run it from the root of the external standards repo like so:
    php -f "path/to/PHP_CodeSniffer/scripts/check-sniff-completeness.php"
  • The list of sniffs to check completeness for, is determined by getting a list of all files in Sniffs directories, where the files have a name ending with Sniff.php and not prefixed with Abstract.
  • The "has tests" check verifies that a UnitTest.php file is present in the standard's Tests directory and at least one test case file.
  • The "has docs" check verifies that a Standard.xml file is present in the standard's Docs directory.

This check only needs to be run on one build as the results won't change between PHP versions.
For that reason, I've elected to add it to the fastest PHP version run to balance out the runs some more.

As quite a large number of sniffs at this moment do not have documentation files, quiet mode is currently enabled for the check in Travis.
If/when the missing documentation files have been added, the -q flag should be removed and having unit tests as well as documentation should be enforced for all sniffs.

Note: the build for this won't pass until all sniffs are accompanied by unit tests.

Currently, the following sniffs do not have unit tests:

Related: #2345 (comment)

@jrfnl jrfnl force-pushed the feature/travis-verify-sniff-completeness branch 4 times, most recently from 9a14d02 to 6366821 Compare January 17, 2019 08:22
@jrfnl
Copy link
Contributor Author

jrfnl commented Jan 17, 2019

Rebased & updated to account for the changes made in 6ff6b5e

@jrfnl jrfnl force-pushed the feature/travis-verify-sniff-completeness branch from 6366821 to f199d35 Compare January 29, 2019 22:53
@jrfnl
Copy link
Contributor Author

jrfnl commented Jan 29, 2019

I've rebased this PR now all the PRs for missing unit test files have been merged.

The only sniff still missing unit tests is MySource.Channels.IncludeOwnSystem. @gsherwood How would you like to address this ?

@jrfnl jrfnl closed this Jan 29, 2019
@jrfnl jrfnl deleted the feature/travis-verify-sniff-completeness branch January 29, 2019 23:10
@jrfnl jrfnl restored the feature/travis-verify-sniff-completeness branch January 29, 2019 23:11
@jrfnl jrfnl reopened this Jan 29, 2019
This PR adds a custom script which will check whether each and every sniff is accompanied by a documentation XML file (warning) as well as unit test files (error).

Notes:
* To only show errors, the script can be run in `quiet` mode by passing the `-q` flag on the command line.
* The script has been set up in such a way that it is not only suitable for use by PHPCS itself, but also for use by external standards which use the PHPCS native unit test suite.
    For external standards to use the tool, run it from the root of the external standards repo like so:
    `php -f "path/to/PHP_CodeSniffer/scripts/check-sniff-completeness.php"`
* The list of sniffs to check completeness for is determined by getting a list of all files in `Sniffs` directories, where the files have a name ending with `Sniff.php` and not prefixed with `Abstract`.
* The "has tests" check verifies that a `UnitTest.php` file is present in the standard's `Tests` directory and _at least_ one test case file.
* The "has docs" check verifies that a `Standard.xml` file is present in the standard's `Docs` directory.

This check only needs to be run on one build as the results won't change between PHP versions.
For that reason, I've elected to add it to the fastest PHP version run to balance out the runs some more.

As quite a large number of sniffs at this moment do not have documentation files, `quiet` mode is currently enabled for the check in Travis.
If/when the missing documentation files have been added, the `-q` flag should be removed and having unit tests as well as documentation should be enforced for all sniffs.
@jrfnl jrfnl force-pushed the feature/travis-verify-sniff-completeness branch from f199d35 to ba2fc66 Compare May 6, 2019 01:23
@jrfnl
Copy link
Contributor Author

jrfnl commented May 6, 2019

Rebased to fix the merge conflict state.

@jrfnl
Copy link
Contributor Author

jrfnl commented Jul 31, 2019

@gsherwood Any chance this can be added to the 3.5.0 milestone ? This tool would be very useful for external standards as well.

@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 15, 2019

Closing in favour of a much improved version of this script now available via https://github.com/PHPCSStandards/PHPCSDevTools

If PHPCS still has any interest in using a script like this, I suggest adding the above mentioned repo as a dev dependency and using the script from PHPCSDevTools.

For maintainers of external PHPCS standards: see the readme to learn how to add this check to your CI-toolchain.

@jrfnl jrfnl closed this Aug 15, 2019
@jrfnl jrfnl deleted the feature/travis-verify-sniff-completeness branch August 15, 2019 01:27
@gsherwood gsherwood removed this from the 3.5.0 milestone Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants