Make WordPress Core

Opened 18 months ago

Last modified 9 months ago

#56846 new enhancement

All test classes that use the "wp" prefix should be renamed for consistency

Reported by: antonvlasenko's profile antonvlasenko Owned by:
Milestone: Awaiting Review Priority: normal
Severity: minor Version:
Component: General Keywords: has-patch has-unit-tests
Focuses: coding-standards Cc:

Description

About 170 PHP test classes use lowercase w and only 34 test classes use capital W in the wp prefix.
Examples:
wp : Tests_Formatting_wpParseStr
Wp : Tests_Category_WpListCategories
According to this document, the correct spelling is Wp.
So, all test classes that use the wp prefix should be renamed for consistency.

Change History (15)

#1 @costdev
18 months ago

Fully agreed @antonvlasenko! Been hoping to address this for a while, thanks for opening the ticket!

#2 @antonvlasenko
18 months ago

Thanks, @costdev. I'm glad to hear I'm not the only one concerned about it. 😉

#3 in reply to: ↑ description @SergeyBiryukov
18 months ago

Replying to antonvlasenko:

Examples:
wp : Tests_Formatting_wpParseStr
Wp : Tests_Category_WpListCategories
According to this document, the correct spelling is Wp.
So, all test classes that use the wp prefix should be renamed for consistency.

I was hoping to do it the other way around and standardize on wp :)

It's probably my personal preference, but WpListCategories reminds me of capital_P_dangit(), and I find wpListCategories easier to read. That said, I guess I can get used to Wp if that's the consensus here :)

#4 @costdev
18 months ago

WpListCategories is compliant with PSR-1, although we do have numerous WP_Something classes in Core that aren't.

As the Handbook states the correct naming is WpListCategories, I think we should use this and address this topic when we begin work on reorganising the test suite as a whole.

Last edited 18 months ago by costdev (previous) (diff)

#5 @antonvlasenko
18 months ago

From my point of view, it's not very important whether to use wp or Wp.
But it is crucial that this be consistent across the project and reflected in the documentation.

#6 @SergeyBiryukov
18 months ago

wp appears to be more common currently:

  • class [A-Za-z_]*_wp: 175 matches in 174 files
  • class [A-Za-z_]*_Wp: 34 matches in 34 files

#7 @costdev
18 months ago

I don't have any strong feelings on the "right" decision on this, except that we achieve consistency.

wp is more common, although that does show that people haven't been paying attention to the handbook - a problem in itself.

I see two options for consistency:

  1. Use Wp to be consistent with the handbook and other classes in the test suite.
  2. Make wp consistent for test classes that start with those letters. Note: This would make these classes an exception, and therefore inconsistent with the handbook and other classes in the test suite.

#8 @ironprogrammer
18 months ago

+1 on consistency with Wp.

If there is a wider objective for consistency beyond this specific prefix, consider that most test classes do adhere to the documented Tests_Path_TitleCase practice, so I feel those offer the largest sampling to work from.

An important aspect of this policy is to make naming decisions easier by following a simple pattern, which works well in most cases. There are certainly awkward applications of the policy, for instance if classname Tests_oEmbed_HTTP_Headers in tests/oembed/headers.php were to be revised, since it strays from the policy in multiple ways.

Acronym usage in names also isn't covered in the unit test docs, though it is in WPCS. If there are to be exceptions to the current docs, it would help to touch on this as well. Tests_XMLRPC_* is a good example.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


18 months ago

#10 @desrosj
18 months ago

  • Version 6.1 deleted

This ticket was mentioned in PR #4897 on WordPress/wordpress-develop by @dmsnell.


9 months ago
#11

  • Keywords has-patch has-unit-tests added

@dmsnell commented on PR #4897:


9 months ago
#12

cc: @costdev

#13 @Bernhard Reiter
9 months ago

In 56299:

HTML API: Change wp infix in test classes to Wp.

In order to comply with the test class naming scheme set forth in #56846, rename the test classes covering the HTML API by changing the wp infix to Wp.

Props dmsnell, costdev.
Fixes #58899. See #56846.

@Bernhard Reiter commented on PR #4897:


9 months ago
#14

Thanks both. Committed to Core in https://core.trac.wordpress.org/changeset/56299.

@dmsnell commented on PR #4897:


9 months ago
#15

Thank you both!

Note: See TracTickets for help on using tickets.