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

TestUtils: add base sniff test class #72

Open
jrfnl opened this issue Jan 31, 2020 · 1 comment
Open

TestUtils: add base sniff test class #72

jrfnl opened this issue Jan 31, 2020 · 1 comment

Comments

@jrfnl
Copy link
Member

jrfnl commented Jan 31, 2020

... to allow external standards to more easily test their sniffs without using the PHPCS native unit test framework.

Advantages this will/can provide:

  • Tests will keep working once PHPCS 4.0 comes out.
    As of PHPCS 4.0, the distribution package of PHPCS won't ship with the test directories, so the abstract testsuite and sniff classes won't be available. This can be worked-around by using --prefer-source in the CI composer install if needs be.
    Also see: Remove tests from the composer package squizlabs/PHP_CodeSniffer#1908 and squizlabs/PHP_CodeSniffer@cf91994
  • With the PHPCS native setup, you can only use the PHPUnit --filter option to filter by standard. Just running the tests for one individual sniff or for a category is difficult to do.
    The base sniff test class I envision would allow for the PHPUnit --filter option to work properly again as expected.
  • No need anymore to work around the PHPCS native TestSuite which loads every single standard and tries to find the tests for it. The current way to ignore standards using PHPCS_IGNORE_TESTS works, but is fiddly and requires too much manual maintenance.
  • Compatibility with PHPUnit 8.x/9.x
    This has since been sorted in PHPCS itself via Tests: allow the test suite to run on PHPUnit 8.x and 9.x PHP_CodeSniffer#59.

Things to consider adding on top of the functionality which is in the PHPCS native base sniff test class:

  • Allow for tests to provide a list of all error codes included in the sniff and verify that all error codes are tested (have at least one error/warning reported)
    This could be used by standards which don't check code coverage to at least verify that all error codes are tested.
  • Allow for testing that the messages thrown are actually the messages expected.
    Just counting the number of messages may still hide changed sniff behaviour if one message is replaced by another.
    Will probably need an array with line -> errorcode -> nr of errors.
  • Allow for testing expected metrics.
    Not a good idea if those would accidentally become inaccurate, so would be great to be able to safeguard these via tests.
    This could be set up as plug and play, optional extra functionality to add to any testcase via a trait with a test method and an abstract data provider method.
  • Allow for testing the contents of select error messages.
    This is useful if the replacement values aren't just simple values, i.e. the sniff contains logic to create the replacement values which is a little more complex.
    This could be set up as plug and play, optional extra functionality to add to any testcase via a trait with a test method and an abstract data provider method.
  • Fail tests if the sniff under test contains fixers and the test case file would trigger those (= would yield a diff), but there is no .fixed version available for the test case file.

Implementation considerations

  • Implementation wise switching to the new PHPCSUtils test class should be as easy as changing the use statement of a test class and updating the CI command (removing the calling of the PHPCS TestSuite file).
    Using new/additional functionality should be optional and elective.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant