Tests: fix some test failures for Windows OS #7364
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| * | ||
| * @return string | ||
| */ | ||
| public function normalizeDirectorySeparatorsInPath( $path ) { |
There was a problem hiding this comment.
As this method is not an expect or assert, shouldn't it use the WordPress name convention?
| public function normalizeDirectorySeparatorsInPath( $path ) { | |
| public function normalize_directory_separators_in_path( $path ) { |
There was a problem hiding this comment.
The use of camelCase is deliberate for consistency with other expectation/assertion methods in the TestCase file.
There was a problem hiding this comment.
But in this case, it's a public helper function and not an expectation or assertion. It is used by a new assertion, yes. But it could also be used within a test class / method as a helper for setting up conditions before doing an assertion or expectation, such as being done in this PR in test_get_block_templates_paths_dir_exists().
As it's a helper, I'm wondering if it should follow snake_case, similar to other helper methods.
There was a problem hiding this comment.
Well, as I hinted at before, quite a lot of those helpers in the TestCase use camelCase, so I still think this is most consistent. Some examples:
skipOnAutomatedBranches()expectDeprecated()setExpectedIncorrectUsage()knownWPBug()prepareTemplate()
Note: AFAICS none of these are required to be camelCase by PHPUnit, still they are.
There was a problem hiding this comment.
You're right. There are mixed naming conventions for the helpers, i.e. some are camelCase (as you note) and others are snake_case.
I don't have strong opinions. Given precedence, seems okay to use camelCase.
Note: Though out of scope for this PR, might be worth establishing a code standard for test case helper methods and updating the handbook.
There was a problem hiding this comment.
Note: Though out of scope for this PR, might be worth establishing a code standard for test case helper methods and updating the handbook.
I vaguely remember we opened a ticket about this quite a while back (including about naming conventions for data providers and such). Would need to search for it, but I think it should still be open.
jrfnl
force-pushed
the
trac-61530/tests-assertsamepathignoringdirectoryseparators
branch
from
74c6a86
to
e9c5b25
Compare
Introduces `WP_UnitTestCase_Base::assertSamePathIgnoringDirectorySeparators()` and an associated helper method `WP_UnitTestCase_Base::normalizeDirectorySeparatorsInPath()` to allow for comparing two file path strings independently of OS-specific differences. The normalization is done in a separate method to also allow this method to be used for path normalization within test methods themselves, like for normalizing a group of paths in an array. The pretty specific method name for the helper (`normalizeDirectorySeparatorsInPath()`) is an attempt to prevent naming conflicts with methods which may exist in plugin test suites build on top of the WP Core test suite.
jrfnl
force-pushed
the
trac-61530/tests-assertsamepathignoringdirectoryseparators
branch
from
e9c5b25
to
309ce81
Compare
|
hellofromtonya
deleted the
trac-61530/tests-assertsamepathignoringdirectoryseparators
branch
WP_UnitTestCase_Base: introduce a new assertion and a new helper method
Introduces
WP_UnitTestCase_Base::assertSamePathIgnoringDirectorySeparators()and an associated helper methodWP_UnitTestCase_Base::normalizeDirectorySeparatorsInPath()to allow for comparing two file path strings independently of OS-specific differences.The normalization is done in a separate method to also allow this method to be used for path normalization within test methods themselves, like for normalizing a group of paths in an array.
The pretty specific method name for the helper (
normalizeDirectorySeparatorsInPath()) is an attempt to prevent naming conflicts with methods which may exist in plugin test suites build on top of the WP Core test suite.Tests: start using the new assertion and helper method
Trac ticket: https://core.trac.wordpress.org/ticket/61530
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.