Opened 9 years ago
Closed 6 years ago
#35065 closed enhancement (reported-upstream)
Tests for docblock quality
Reported by: | johnbillion | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | has-patch ongoing |
Focuses: | docs | Cc: |
Description
Attached is a patch which implements various tests for docblocks in PHP files. The patch is experimental and is intended only to determine whether unit tests for docblocks are a good idea or not.
Currently tested for each function/method:
- Existence of docblock.
- Non-emptiness of docblock.
- Correct number of
@param
tags.
Currently tested for each parameter of a function/method:
- Non-emptiness of description.
- Name matches actual parameter name.
- Various type hint validations (not so appropriate for core).
- Existence or not of "Optional" string.
- Existence or not of "Default" string.
Currently this only tests functions/classes which are loaded in the current scope. phpDocumentor's static reflection library allows static analysis of files instead of using PHP's built-in Reflection API, which would allow for all functions and classes to be tested.
cc @DrewAPicture
Attachments (3)
Change History (17)
#2
@
9 years ago
The phpDocumentor
library that's used is PHP 5.3+, so these tests would need some logic so they only run on PHP 5.3+.
#3
follow-up:
↓ 4
@
9 years ago
Is this not the job of some kind of code sniffer rather than unit testing?
#4
in reply to:
↑ 3
;
follow-up:
↓ 6
@
9 years ago
Replying to joehoyle:
Is this not the job of some kind of code sniffer rather than unit testing?
Potentially, yes, but we don't have an infrastructure for this currently. WPCS could be used, but there's nothing in there for docblock quality and it would have to be run separately from the PHP rules in WPCS because WordPress itself (naturally) doesn't yet adhere to its standards.
#5
@
9 years ago
What about collecting data for a report regarding file location and function name to ease finding where one would need to work on?
#6
in reply to:
↑ 4
@
9 years ago
Replying to johnbillion:
Replying to joehoyle:
Is this not the job of some kind of code sniffer rather than unit testing?
Potentially, yes, but we don't have an infrastructure for this currently. WPCS could be used, but there's nothing in there for docblock quality and it would have to be run separately from the PHP rules in WPCS because WordPress itself (naturally) doesn't yet adhere to its standards.
This is actually incorrect. WPCS includes the WordPress-Docs
ruleset for checking docblock quality (thanks to @GaryJ). So you can run phpcs --standard=WordPress-Docs
and just check for docblock issues. It is possible that the rules won't match exactly what WordPress core expects, but PRs are welcome :-).
#7
@
9 years ago
The rules that exist in WordPress-Docs were just the easy wins that utilised existing Sniffs that are part of existing standards that ship with PHPCS (i.e. Squiz and Generic) - see https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/WordPress-Docs/ruleset.xml .
This is in no-way comprehensive, and doesn't check for many WP idiosyncrasies. The ruleset.xml in fact is mainly made up of exclusions from those more typical existing standards, and would therefore need custom Sniffs to add those valid checks in.
#8
@
9 years ago
docblocks.2.diff switches to static analysis instead of PHP's built-in Reflection, so classes and functions that are not loaded in the current scope can be tested too. Needs Composer to set up before running the tests:
cd tests/phpunit
composer install
See the $to_search
and $files
variables in the Tests_DocBlocks::dataReflectionTestFunctions()
method for tweaks that can be made, and also the $ignore_empty
static property on the class which silences errors caused by functions and methods which completely lack a docblock.
At this point I think it's clear that such tests don't need to be in core's tests, but having them allows the docs team to work through the issues that this patch does highlight.
See #32246 for ongoing results.
#9
@
9 years ago
docblocks.3.diff corrects the composer.json file.
This ticket was mentioned in Slack in #docs by morganestes. View the logs.
8 years ago
#13
@
8 years ago
I have a standalone repo for this: https://github.com/johnbillion/php-docs-standards and I was planning a WordPress specific set of rules based on the above.
Seeing as the static analyses approach requires a Composer dependency, maybe we can close this ticket as maybelater, or we can switch to using WPCS for docblock quality sniffing.
#14
@
6 years ago
- Milestone Awaiting Review deleted
- Resolution set to reported-upstream
- Status changed from new to closed
This was fun, but it's really the domain of WPCS.
https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/424
Usage:
$to_search
,$files
,$test_functions
, and$test_classes
variables inTests_DocBlocks::dataReflectionTestFunctions()
as appropriate.phpunit --filter Tests_DocBlocks
.