Use a consistent order of annotations in the test suite

[SergeyBiryukov]

WordPress core has an ​established DocBlock format for inline documentation:

/**
 * Summary.
 *
 * Description.
 *
 * @since x.x.x
 *
 * @see Function/method/class relied on
 * @link URL
 *
 * @global type $varname Description.
 * @global type $varname Description.
 *
 * @param type $var Description.
 * @param type $var Optional. Description. Default.
 * @return type Description.
 */

This is more or less consistently applied in core, which is helpful for reusing this template for newly added functions without the guesswork of where to put each particular tag.

Unit tests also use some of these tags:

• @since
• @see
• @global
• @param
• @return (for tests with dependencies)

as well as some ​test-specific annotations:

• ​`@ticket`
• ​`@group`
• ​`@covers`
• ​`@depends`
• ​`@requires`
• ​`@dataProvider`
• @expectedDeprecated
• @expectedIncorrectUsage

However, the order of these annotations differs in various test classes and can be almost random even in test methods of the same class. These inconsistencies increase cognitive load when writing new tests or reviewing test PRs to bring them in line with existing tests.

I would like to propose a DocBlock template that can be consistently applied across the test suite. Something like:

/**
 * Summary.
 *
 * Description.
 *
 * @since x.x.x
 * @ticket 12345
 *
 * @group group_name_1
 * @group group_name_2
 *
 * @covers function_name_1
 * @covers function_name_2
 *
 * @requires function function_name
 *
 * @expectedDeprecated
 * @expectedIncorrectUsage
 *
 * @see Function/method/class relied on
 * @link URL
 *
 * @depends test_name
 * @dataProvider data_provider_name
 *
 * @global type $varname Description.
 * @global type $varname Description.
 *
 * @param type $var Description.
 * @param type $var Optional. Description. Default.
 * @return type Description.
 */

Notes:

• All of these annotations are optional and may not be present on a particular test, so most of the time the DocBlock would be much shorter. But if they are present, the order should be consistent across the test suite.
• @since and @ticket are grouped together because they are both related to when a test was introduced.
• @group and @covers are separated into their own sections for better readability when a test belongs to multiple groups and/or covers multiple functions.
• @depends and @dataProvider are grouped together and moved closer to globals and parameters, because they are both related to passing data to the test. When reviewing the current usage of @depends in the test suite, I found some instances that don't pass any data but seem to (incorrectly) assume that this annotation defines the order in which the tests are executed. That can be addressed separately.

Any thoughts on using this DocBlock format or any suggestions for improving it are welcome.

Trac ticket: https://core.trac.wordpress.org/ticket/56070

[davidbaumwald]

Thank you for flagging this. It would be nice to update the handbook with this new standard also.

[jrf]

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.

[dingo_d]

I love seeing more structure in the inline docs, kudos for creating this ticket to bring more consistency across the codebase!

I have one question about checking docblocks.

Are we checking inline docblocks for the structure/order? And do we intend to do so using tools like PHPCS?

I also agree with Juliette that we should see if we can align our docblocks with the official proposals for the docblocks in the PHP community. Even though those are still drafts (should also keep an eye on PER standards if they will add inline docs to their list).

[desrosj]

Beta 1 is in a few hours. This one is strictly test and doc related, so it can remain in 6.1 as long as someone still plans on wrapping this up in the next few weeks.

@SergeyBiryukov are you still anticipating that you'll be able to work on this one?

[SergeyBiryukov]

Let's move to 6.2 for now, this still needs some time to address the feedback from comment:4 and apply the changes to the existing test classes.

[peterwilsoncc]

@SergeyBiryukov @jrf @dingo_d I agree this is a great idea, is it possible to put together a proposal to post on make/core?

It looks like there is still some discussion about order to be finalised, so having that wrapped up would be lovely.

[costdev]

This ticket was discussed in the recent bug scrub. As this ticket can be committed at any time during the cycle, and has support from Core developers involved in the discussion, we agreed to change this ticket's type to Task (blessed).