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

Tests: expand the unit test suite #146

Open
jrfnl opened this issue Dec 9, 2023 · 1 comment
Open

Tests: expand the unit test suite #146

jrfnl opened this issue Dec 9, 2023 · 1 comment

Comments

@jrfnl
Copy link
Member

jrfnl commented Dec 9, 2023

As of PR #144, code coverage will now be measured and recorded for every PR and push.

The resulting reports can be found here: https://coveralls.io/github/PHPCSStandards/PHP_CodeSniffer

As can be seen there, code coverage for the sniffs is pretty good, but for most other areas of the code... not so much.

The test suite should be expanded to allow for merging with (more) confidence.

Some thoughts on this:

  • While there are tests covering parts of the PHP_CodeSniffer\Tokenizers\PHP class, these tests don't currently register code coverage due to the test set up.
    This may be fixable with some tweaks to the existing AbstractMethodUnitTest test case, but may also require a separate test case class to get this working.
    I will probably look into this myself.
    Fixed via Tests: fix recording code coverage for Tokenizer tests #314
  • For both the Generators as well as the Reports, a new abstract test case may need to be introduced to get those tests set up more easily.

Getting started

  • Adding additional tests for individual sniffs will likely be most straight forward for new contributors. The CONTRIBUTING guide contains information about writing sniff related tests.
    • When creating additional tests for sniffs, don't get too focused on covering uncovered lines.
      Think about what the sniff is checking for and get creative with writing horribly formatted code to make sure the sniff detects this correctly.
    • Also keep in mind that some sniffs are pretty "old" and may or may not take all new PHP syntaxes into account correctly.
      Adding tests with modern PHP syntaxes is a huge help, cause even if the sniff already handles this syntax correctly, the tests will safeguard that it continues to do so, even when the code in the sniff changes.
    • When adding tests for sniff code which is related to parse errors, keep the guidelines outlined in Tests: parse error tests for sniffs should be in their own file #143 in mind.
  • Another area which should be reasonably straight forward to add tests for are some of the methods in the src/Util directory, like those in the Timing and Common classes.

Writing new tests for non-sniff code will be more complicated and may require some creativeness in setting those up. When in doubt on whether your approach is the right one, please discuss before execute.

When writing tests, keep in mind that the tests need to be able to run on PHP 5.4 (PHPUnit 4.x) up to PHP 8.3/4 (PHPUnit 9.x).

PRs related to this task should preferably only touch the tests for one sniff/one class/one function per PR.

@jrfnl
Copy link
Member Author

jrfnl commented Feb 16, 2024

PR #340 for the Squiz.Commenting.ClosingDeclarationComment sniff needs a follow up which can be addressed as part of this issue.

See #340 (comment) for a list of things which need to be addressed in that sniff.

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