Make WordPress Core

Changes between Version 1 and Version 4 of Ticket #56070


Ignore:
Timestamp:
06/26/2022 01:32:56 PM (2 years ago)
Author:
jrf
Comment:

Thanks for the ping @SergeyBiryukov !

These inconsistencies increase cognitive load when writing new tests or reviewing test PRs to bring them in line with existing tests.

I fully agree and applaud this initiative.

Once the format has been settled upon, please consider sending in a PR to the Coding Standards handbook repo to add this format to the PHP Documentation Standards.

Not sure if you already have, but it might be worth it to have a look through the draft PSR5 and PSR19 guidelines to see if the format as currently proposed is at least in line with those. This is, of course, not a "must-have", but it would be a "nice-to-have" to not increase the barrier of entry for contributing to WP.

As for feedback on the proposed format:

  • I would move the @see and @link tags higher up - just below (or even above) the @ticket and @since tags as this information will normally be related to the situation being tested as described in the description.
  • Along the same lines, I would reverse the order between the @covers block and the @group block as the information in the @covers tag is about what is being tested, while the information in the @group tag is about potentially selectively running the tests.
  • @group could even potentially be moved below @requires.

As discussed in another ticket previously, the @global tag should be considered discouraged as it doesn't actually add any functional value and the same information will be contained within the test (and API documentation won't be generated for tests anyhow and modern generation tools don't need the @global tag anymore either).

Along the same lines, aside from some very specific exceptions - the groups being run separately in CI -, IMO the @group tag also should be considered discouraged as the `--filter` CLI argument of PHPUnit has come a long way and is generally much more versatile than using @group.

For documenting the format, it might be a good idea to also mention the `@coversNothing` tag in the @covers block as this is a tag which will at times be used.

Additional PHPUnit specific tag to consider adding to the format: `@doesNotPerformAssertions`.

For the record, also note that the @ticket tag is actually an alias for the PHPUnit @group tag.

Ref: https://phpunit.readthedocs.io/en/9.5/annotations.html


As a final note: once I have some breathing room (may still be a while), I intend to greatly improve the WordPress-Docs standard and verifying a standardized tag order would definitely be one of the things I'd like to add to WPCS.

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #56070

    • Property Keywords has-patch has-unit-tests added
  • Ticket #56070 – Description

    v1 v4  
    2929
    3030as well as some [https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#annotations test-specific annotations]:
    31 * `@ticket`
    32 * `@group`
    33 * `@covers`
    34 * `@depends`
    35 * `@requires`
    36 * `@dataProvider`
     31* [https://phpunit.readthedocs.io/en/9.5/annotations.html#ticket `@ticket`]
     32* [https://phpunit.readthedocs.io/en/9.5/annotations.html#group `@group`]
     33* [https://phpunit.readthedocs.io/en/9.5/annotations.html#covers `@covers`]
     34* [https://phpunit.readthedocs.io/en/9.5/annotations.html#depends `@depends`]
     35* [https://phpunit.readthedocs.io/en/9.5/annotations.html#requires `@requires`]
     36* [https://phpunit.readthedocs.io/en/9.5/annotations.html#dataprovider `@dataProvider`]
    3737* `@expectedDeprecated`
    3838* `@expectedIncorrectUsage`