[wp-trac] [WordPress Trac] #39265: Missing @covers in the comment blocks in PHPUnit tests

Sat Oct 24 23:10:37 UTC 2020

#39265: Missing @covers in the comment blocks in PHPUnit tests
------------------------------+-----------------------------
 Reporter:  pbearne           |       Owner:  SergeyBiryukov
     Type:  task (blessed)    |      Status:  accepted
 Priority:  normal            |   Milestone:  5.6
Component:  Build/Test Tools  |     Version:  4.8
 Severity:  normal            |  Resolution:
 Keywords:  php8              |     Focuses:
------------------------------+-----------------------------

Comment (by jrf):

 I've done an initial review of the submitted patches adding new covers
 tags for commit readiness.
 I have NOT verified whether the added tags are correct. I trust this has
 been thoroughly fetted by the person who created the patch. I've only left
 a remark on those here and there when what I saw triggered an instance
 response.

 Thank you all for stepping up and creating these patches! <3 This is a
 huge amount of work you've all done and getting these patches committed
 will be a great step forward!

 Please find my feedback below. Line numbers I refer to are generally the
 "new" line number. File names as per the patch.

 == functions_folder.patch

 @pbearne

 * `@covers` syntax is correct.
 * In a number of places superfluous empty docblocks lines are introduced
 at the start or end of a docblock. These need to be removed. Examples:
   - `tests/phpunit/tests/functions/allowedProtocols.php` line 19.
   - `tests/phpunit/tests/functions/canonicalCharset.php` all except the
 last one.
   - `tests/phpunit/tests/functions/wpAuthCheck.php` line 31.
 * `@covers` tags need to be in docblock comments, i.e. the comment block
 needs to start with `/**`. In file
 `tests/phpunit/tests/functions/wpListFilter.php`, changes between line 42
 and 162, this is not the case.

 == d_folders.patch

 @patopaiar

 * `@covers` syntax is correct.
 * `date/currentTime.php` - indentation of the code on line 37 seems to
 have become borked.
 * `date/dateI18n.php` - line 43 to 115 - need docblock format (see point 3
 above) and alignment of the stars.
 * `date/dateI18n.php` line 143, indentation of the comment starter is off.
 * `date/maybeDeclineDate.php` - line 65 introduces a superfluous empty
 line.
 * `date/query.php` :+1: on adding descriptions for the tests. The
 descriptions do still need a period  `.` as "end of sentence" punctuation
 and most of these fixes all need docblock format + alignment of the stars.
 * `date/query.php` line 310 + 313 introduce fixes to the actual test. This
 does not belong with this ticket and should be done in a separate patch,
 though the actual changes look good. :+1:
 * `date/query.php` line 636, 647, 670, 768 - empty comment line at start
 of docblock needs to be removed.
 * `dependencies/scripts.php`, line 684, 939 - something wonky is going on
 with the star indentation/alignment.

 == e_folders.patch

 @patopaiar

 * `@covers` syntax is correct.
 * `editor/wpEditors.php` - these all need docblock format and the last one
 also removal of the stray empty line + alignment of the stars.
 * `error-protection/recovery-mode-cookie-service.php` line 94,
 `ReflectionMethod::invoke` is a PHP native method and should not be
 included in the `@covers` tags.
 * `external-http/basic.php` - needs docblock format, alignment of the
 stars and removal of a stray blank line above the new docblock (two empty
 lines at start of class instead of one).

 == g_folders.patch

 @patopaiar

 * This patch seems to miss the path to the files. This probably has to do
 with from which folder the patch was generated. Generally speaking it is
 always a good idea to generate patches from the project root so file
 references have the full relative path.
 * `@covers` syntax is correct.
 * `document-title.php` - these all need docblock format and alignment of
 the stars.
 * `paginateLinks.php` - the new comment blockse all need docblock format.
 * `resourceHints.php`, line 224, 239 - star alignment is off.
 * `template.php`, line 418 introduces a stray blank line.

 == L_folders.patch

 @sephsekla

 * `@covers` syntax is correct.
 * `tests/phpunit/tests/link/adminUrl.php`,
 `tests/phpunit/tests/link/getAdjacentPost.php`,
 `tests/phpunit/tests/link/getAdjacentPostLink.php`,
 `tests/phpunit/tests/link/getDashboardUrl.php`,
 `tests/phpunit/tests/link/getPostCommentsFeedLink.php`,
 `tests/phpunit/tests/link/getPostTypeArchiveLink.php` - if all tests cover
 the same function, the `@covers` tag at the class level is sufficient.
 Otherwise, the `@covers` tag at class level should be removed.
   Generally speaking, having `@covers` tags at test function level is
 better as it allows for more flexibility, so I'd recommend removing the
 one(s) at the class level.
 * `tests/phpunit/tests/link/getAdjacentPostLink.php`,
 `tests/phpunit/tests/link/getPostCommentsFeedLink.php`,
 `tests/phpunit/tests/link/getPreviewPostLink.php`,
 `tests/phpunit/tests/link/getPreviousCommentsLink.php` - stray blank lines
 at the start of the docblocks need to be removed.

 == g_folders.patch

 @patopaiar

 * `@covers` syntax is correct.
 * Same as with other patches, new comment blocks need to use docblock
 format and the stars need to be aligned (nearly all files).
 * `http/functions.php` line 214 - star alignment is off.
 * `http/http.php` line 218 - introduces a stray blank line.
 * `http/http.php` line 274 - `ReflectionClass::getConstants` is a PHP
 native method and should not be included in the `@covers` tags.

 == root.patch

 @pbearne

 * This patch seems to contain unrelated changes to the `package-lock.json`
 file. Please remove these.
 * `@covers` syntax is correct in nearly all cases.
 * `tests/phpunit/tests/actions.php`, `tests/phpunit/tests/auth.php`,
 `tests/phpunit/tests/avatar.php`, `tests/phpunit/tests/cache.php`,
 `tests/phpunit/tests/comment-submission.php`,
 `tests/phpunit/tests/comment.php` etc - stray blank lines at the start of
 docblocks need to be removed.
 * `tests/phpunit/tests/adminbar.php`, line 254 - 260 - the
 `get_standard_admin_bar()` method is not a test method, but a test helper.
 This should not have a `@covers` tag. Also: stray blank lines at the start
 of the docblock.
 * `tests/phpunit/tests/adminbar.php`, line 373 - stray blank comment line
 between two `@covers` tags.
 * `tests/phpunit/tests/adminbar.php`, line 750 - superfluous blank comment
 line (two instead of one).
 * `tests/phpunit/tests/auth.php`, line 116 seems to change a test ?
 * `tests/phpunit/tests/auth.php`, line 145-146 need to be removed
 (duplicate).
 * `tests/phpunit/tests/basic.php` line 6 - there is a tag for that ;-)
 `@coversNothing` See:
 https://phpunit.readthedocs.io/en/7.0/annotations.html#coversnothing .
 * `tests/phpunit/tests/db.php` line 238, 255, 793 introduce stray blank
 comment lines which should be removed.
 * `tests/phpunit/tests/db.php`, line 1687 - missing `::` before the
 covered function name.
 * `tests/phpunit/tests/db.php`, line 1826 - 1828 seem incorrect.
 * `tests/phpunit/tests/filters.php`, line 422 introduces a stray blank
 comment line.
 * `tests/phpunit/tests/link.php`, line 131 seems to change a test ?
 * `tests/phpunit/tests/mail.php`, line 454 - PHPMailer is not included in
 code coverage reporting, so this should probably be changed to
 `@coversNothing`.
 * `tests/phpunit/tests/media.php`, line 522 seems to change a test and
 break it...
 * `tests/phpunit/tests/media.php`, line 1541 seems to change a test and
 break it...
 * `tests/phpunit/tests/media.php`, line 2658 removes the ticket number.
 This should be brought back or if the ticket number did not belong with
 the test, the whole line should be removed.
 * `tests/phpunit/tests/pluggable.php` (both tests) - these look more like
 QA tests, than unit tests as they don't actually test the functioning of
 the functions. I think these tests would be better off being marked with
 `@coversNothing`.
 * `tests/phpunit/tests/rest-api.php`, line 1021-1023 introduces a stray
 blank docblock.
 * `tests/phpunit/tests/user.php`, line 201-202 - this method looks to test
 the magic `__get()`, `__set()`, `__isset()` and `__unset()` methods.
 * `tests/phpunit/tests/user.php`, line 250, 265 - tests cannot cover
 properties. I believe both these tests are actually testing the
 `__unset()` method.
 * `tests/phpunit/tests/user.php`, line 293, 307 - tests cannot cover
 properties.
 * `tests/phpunit/tests/widgets.php`, line 45 seems to change a test and
 break it...

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/39265#comment:24>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform