Opened 2 years ago
Last modified 16 months ago
#56846 new enhancement
All test classes that use the "wp" prefix should be renamed for consistency
Reported by: | 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)
#3
in reply to:
↑ description
@
2 years ago
Replying to antonvlasenko:
Examples:
wp
:Tests_Formatting_wpParseStr
Wp
:Tests_Category_WpListCategories
According to this document, the correct spelling isWp
.
So, all test classes that use thewp
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
@
2 years 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.
#5
@
2 years 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
@
2 years ago
wp
appears to be more common currently:
class [A-Za-z_]*_wp
: 175 matches in 174 filesclass [A-Za-z_]*_Wp
: 34 matches in 34 files
#7
@
2 years 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:
- Use
Wp
to be consistent with the handbook and other classes in the test suite. - 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
@
2 years 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.
2 years ago
This ticket was mentioned in PR #4897 on WordPress/wordpress-develop by @dmsnell.
16 months ago
#11
- Keywords has-patch has-unit-tests added
@Bernhard Reiter commented on PR #4897:
16 months ago
#14
Thanks both. Committed to Core in https://core.trac.wordpress.org/changeset/56299.
Fully agreed @antonvlasenko! Been hoping to address this for a while, thanks for opening the ticket!