Opened 4 years ago
Last modified 6 weeks ago
#53010 assigned task (blessed)
Tests: introduce namespacing for the test classes
Reported by: | jrf | Owned by: | hellofromTonya |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | has-patch has-unit-tests |
Focuses: | docs, coding-standards | Cc: |
Description (last modified by )
Introducing namespaces in the production code for WordPress Core is a hot topic, so I very purposely do NOT want to touch that in this ticket.
However, for the test suite, which doesn't get shipped with the WordPress production code, it's a whole other matter.
Benefits
Using namespaces in the test suite provides us with the following benefits:
- If used consistently and providing they follow a set pattern (more about this below), they will make it very easy for contributors to find the test files/classes they are looking for.
- It will allow for shorter file and test class names, while those will still be descriptive.
- And... it will allow for mocking PHP native functions by declaring a namespaced version of that same function in the test class.
- It will also allow more easily for multiple test classes to be created to test one particular feature/function, which the current naming scheme does not allow for. This will allow for tests for the same functionality, but which need different fixtures (setup/teardown) to be encapsulated in their own test classes.
Caveats:
As the WordPress Core test suite is used not only by Core, but also by plugins and themes for integration tests, the test class namespacing should be reserved for actual test classes and - for now - not be applied to test utility classes / Abstract base test classes (i.e. the tests/phpunit/includes
directory should NOT be touched for now).
Proposed pattern
The current directory structure for tests is, to put it mildly, confusing and inconsistent.
To solve that, I would like to propose the following pattern:
- File paths:
tests/phpunit/tests/wp-[includes|admin]/[SubFolder/]*Class_Under_Test/FunctionUnderTest[OptionalSubsetIndicator]Test.php
- Namespace:
WordPress\Tests\WP_[Includes|Admin]\[SubFolder\]*Class_Under_Test
- Classname:
FunctionUnderTest[OptionalSubsetIndicator]Test
For WP Core files which only contain functions outside of a class structure, the following pattern is proposed:
- File paths:
tests/phpunit/tests/wp-[includes|admin]/[SubFolder/]*Functions_FileName/FunctionUnderTest[OptionalSubsetIndicator]Test.php
- Namespace:
WordPress\Tests\WP_[Includes|Admin]\[SubFolder\]*Functions_FileName
- Classname:
FunctionUnderTest[OptionalSubsetIndicator]Test
The pattern I'm proposing does imply a re-organisation of the test suite directory and file structure, but that IMO is for the better.
It also follows a PSR4-like pattern which will be more intuitive for new contributors to work with, as well as follow the PHPUnit recommended test class name pattern with having the Test
as the end of the class name.
This will also allow for using PSR-4 autoloading for the tests classes and adding the autoload-dev
directive to the composer.json
file.
Planning
This should be regarded as a small project and not all renaming needs to be done at the same time.
New tests should start following the above proposed pattern as soon as consensus has been reached about this proposal.
Existing tests can be gradually switched over to the new pattern over time.
Additional tasks associated with this project
- [ ] Updating the contributors handbook for Core.
- [ ] Verify that the WordPressCS sniffs will validate this pattern correctly.
- [ ] Write a Make post about the decision once consensus has been reached.
Change History (52)
This ticket was mentioned in Slack in #core by ayesh. View the logs.
4 years ago
#6
@
3 years ago
I've opened https://github.com/WordPress/WordPress-Coding-Standards/issues/1995 to adjust the WordPress Coding Standards for the above.
#7
@
3 years ago
- Milestone changed from Awaiting Review to 5.9
- Owner set to hellofromTonya
- Status changed from new to assigned
#8
@
3 years ago
One thing which isn't addressed yet in the above proposal, but which we should probably have a think about: what to do with test specific "fixtures", mocks and doubles ?
What I mean by this, are additional files which may contain a supporting class or just some text, which are needed for a particular test.
Where should those live in the directory structure ?
If we look at other large projects, there are often three different structures to be seen:
- One "fixtures" directory at test top-level and sub-directories within that directory which follow the path to the test which the fixture applies to.
- One fixtures directory at test top-level and some form of organisation within, but not necessarily one which follows the test directory structure. Think: organisation by test groups for instance.
- A fixtures subdirectory within the (sub)directory a test lives in with the fixtures for the tests in that directory. In practices that would mean that the fixtures would be close to the tests they apply to and easy to find, but also that there will be a multitude of fixtures directories.
At this time, I'm not going to propose a direction yet. I'm just posting this here as a reminder that we need to think about this and should probably discuss this/take a decision about this before the reorganization starts.
#9
@
3 years ago
- Keywords 2nd-opinion added
As the WordPress Core test suite is used not only by Core, but also by plugins and themes for integration tests, the test class namespacing should be reserved for ...
Yeah, I'm not really sure introducing "partial" namespacing will have any benefits. As far as I understand it will create more of a "messy" situation where some names can be repeated and other cannot. This doesn't seem helpful to anybody.
Looking at the benefits of namespacing as mentioned in the PHP manual, they aren't very appealing in many cases:
- Name collisions between code you create, and internal PHP classes/functions/constants or third-party classes/functions/constants.
- Ability to alias (or shorten) Extra_Long_Names designed to alleviate the first problem, improving readability of source code.
So, at the basic level namespaces are good when:
- You want to name your function or class the same as an existing built-in function or class.
- You want to avoid using longer, more descriptive names.
Both of these cases would signify a bad naming practice by promoting confusing, sloppy names and would make the code less self-documenting.
Looking at the "benefits" listed in this ticket description:
- This seems to be a non-issue. Finding a class or a function works very well in all IDEs and IDE type text editors I've seen. Most of them would also auto-complete the name for you.
- "Shorted file names" are not better! :) That's a disadvantage if anything.
- Has there ever been a need to mock PHP built-in functions in "local" code? What would that accomplish?
- This is a non-issue too. It will probably be easier to read and understand if the multiple test classes are named accordingly to what they do, i.e. self-documenting, descriptive names not short, non-descriptive, repeating names.
In that terms I don't think introducing partial namespaces in the WP PHP tests is a good idea.
#10
@
3 years ago
- Milestone changed from 5.9 to Future Release
Moving to the next cycle, though that milestone isn't yet available. For now, moving to Future Release
.
#11
@
3 years ago
- Milestone changed from Future Release to 6.0
Hey 6.0 is now available for selection. Moving this into the 6.0 cycle.
This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.
3 years ago
This ticket was mentioned in Slack in #core by costdev. View the logs.
3 years ago
#14
@
3 years ago
- Milestone changed from 6.0 to Future Release
Per the discussion in the bug scrub, I'm moving this ticket to Future Release.
#15
@
2 years ago
This seems like a good proposal which would help organize the tests and make it easier to find what you're looking for.
#16
follow-up:
↓ 39
@
18 months ago
May be missing something but it seems the proposed "File paths" suggest that each class should be split in up to 50 separate files, one per method. Is that correct?
If yes, could you please explain what are the benefits of having such "file and directory layout" with several thousands of files instead of 100-200? Seems this would trigger a lot of needless disk i/o and slow down running of the tests as most of the files will have to be loaded every time the tests are run.
Also seems this is not recommended (compatible?) by PSR4. It states that:
A fully qualified class name has the following form:
\<NamespaceName>(\<SubNamespaceNames>)*\<ClassName>
(where "The terminating class name corresponds to a file name ending in .php
.").
In any case, thinking that an example of the proposed namespaces, and list of the file paths and file names would be great to have. For example: what would https://core.trac.wordpress.org/browser/tags/6.2.2/tests/phpunit/tests/blocks/register.php look like if this proposal is implemented?
This ticket was mentioned in PR #6235 on WordPress/wordpress-develop by @jrf.
9 months ago
#17
- Keywords has-patch has-unit-tests added
### Tests/admin/wpListTable: create a testcase for tests for WP_List_Table
Most tests test testing the WP_List_Table
class will need the same set up/tear down fixtures, so moving those fixtures from the actual test class to a separate abstract TestCase
class will allow for expanding the tests more easily.
While doing this, we ran the tests in isolation using the --filter
CLI argument and discovered that there was a previously unrecognized presumption in the fixtures that the $hook_suffix
global would always be set.
This presumption proved incorrect as was demonstrated by the Undefined global variable $hook_suffix
warning being thrown when running the tests.
This warning has been fixed now in the new Admin_WpListTable_TestCase
by adding some additional defensive coding and making sure that the global state is always brought back to its original state, including if that original state means that the $hook_suffix
global variable was not set.
### Tests/admin/wpListTable: split off tests for the get_column_info() method to separate test class
This will allow the test class to be compatible with PHPUnit 11 using the new Admin_WpListTable_TestCase
class as introduced in the previous commit.
Includes removing method level @covers
tags in favour of class level @covers
tags to allow for supporting PHPUnit 11 attributes instead of annotations.
### Tests/admin/wpListTable: split off tests for the get_views_links() method to separate test class
This will allow the test class to be compatible with PHPUnit 11 using the new Admin_WpListTable_TestCase
class as introduced in the previous commit.
Includes removing method level @covers
tags in favour of class level @covers
tags to allow for supporting PHPUnit 11 attributes instead of annotations.
### Tests/admin/wpListTable: rename the original test class to be more descriptive for the remaining tests
The remaining tests in the original test class all cover the magic methods and those are very closely related, so it is perfectly fine for those tests to be grouped together in one test class.
This commit, however, does rename the test class to use the new naming conventions, both for the class name as well as the file name.
It also includes removing method level @covers
tags in favour of class level @covers
tags to allow for supporting PHPUnit 11 attributes instead of annotations.
This will allow the test class to be compatible with PHPUnit 11.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6272 on WordPress/wordpress-develop by @afercia.
9 months ago
#20
## Tests/admin/wpPluginInstallListTable: rename the original test class to be more descriptive and adjust covers notaiton.
This will allow the test class to be compatible with PHPUnit 11.
Includes removing method level @covers tags in favour of class level @covers tags to allow for supporting PHPUnit 11 attributes instead of annotations.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6273 on WordPress/wordpress-develop by @afercia.
9 months ago
#21
## Tests/admin/wpPluginsListTable: split the original test class and adjust covers notaitons.
This will allow the test class to be compatible with PHPUnit 11.
Includes removing method level https://github.com/Covers tags in favour of class level https://github.com/Covers tags to allow for supporting PHPUnit 11 attributes instead of annotations.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6274 on WordPress/wordpress-develop by @afercia.
9 months ago
#22
## Tests/admin/ExportWp: rename the original test class.
This will allow the test class to be compatible with PHPUnit 11.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6276 on WordPress/wordpress-develop by @poena.
9 months ago
#23
Rename the test class Tests_Admin_wpPostCommentsListTable
to Admin_WpPostCommentsListTable_GetViews_Test
so that the name includes both the class and the method that the test covers.
Moves @covers
to the class level.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6277 on WordPress/wordpress-develop by @poena.
9 months ago
#24
Tests/admin/wpMediaListTable:
The tests for WP_Media_List_Table
use the same set up, tear down and helper methods. By moving them to a new abstract TestCase class, Admin_WpMediaListTable_TestCase
, they can be reused by multiple tests.
The test for prepare_items
is moved to a new test class; Admin_WpMediaListTable_PrepareItems_Test
and the @covers
is moved to the class level.
This will allow the test class to be compatible with PHPUnit 11 using the new Admin_WpListMediaTable_TestCase
class.
Tests_Admin_wpMediaListTable
is renamed to Admin_WpMediaListTable_GetRowActions_Test
to reflect that the remaining test is for _get_row_actions
. The @covers
is moved to the class level.
Trac ticket:
https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6296 on WordPress/wordpress-develop by @poena.
9 months ago
#25
…ive and move @covers
Renames IncludesComment
to include the path, file name, and function name: Admin_Includes_Comment_CommentExists_Test
Removes duplicate @covers
and moves @covers
to the test class level.
Tests/Admin/IncludesComment: Rename the test file to be more descriptive and move @covers
.
Renames IncludesComment
to include the path, file name, and function name: Admin_Includes_Comment_CommentExists_Test
Removes duplicate @covers
and moves @covers
to the test class level.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6297 on WordPress/wordpress-develop by @poena.
9 months ago
#26
Test/Admin/IncludesFile: Split the test class into one file and test class for get_home_path
and one for download_url
.
The test for get_home_path
is moved to Admin_Includes_File_GetHomePath_Test
.
The file- and class name for the tests for download_url
is renamed from IncludesFile
to Admin_Includes_File_DownloadUrl_Test
.
Duplicate the @covers
tags are removed.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6384 on WordPress/wordpress-develop by @SergeyBiryukov.
8 months ago
#27
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6446 on WordPress/wordpress-develop by @poena.
7 months ago
#28
Renames IncludesUser
to include the path, file name, and function name, to better indicate which function the test tests: Admin_Includes_User_WpIsAuthorizeApplicationPasswordRequestValid_Test
Moves @covers
to the test class level.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6447 on WordPress/wordpress-develop by @poena.
7 months ago
#29
Move the setup functions from Tests_Admin_WpAutomaticUpdater
to a new abstract TestCase class, so that they can be used by multiple tests for the WpAutomaticUpdater
class.
Splits the existing test class Tests_Admin_WpAutomaticUpdater
into three classes. This will allow these test classes to be compatible with PHPUnit 11.
class Tests_Admin_WpAutomaticUpdater
is renamed toAdmin_WpAutomaticUpdater_SendPluginThemeEmail_Test
and coversWP_Automatic_Updater::send_plugin_theme_email
- Tests that covers
WP_Automatic_Updater::is_allowed_dir
are moved to a new test class:Admin_WpAutomaticUpdater_IsAllowedDir_Test
- The test that covers
WP_Automatic_Updater::is_vcs_checkout
is moved to a new test class:Admin_WpAutomaticUpdater_IsVcsCheckout_Test
- Duplicate
@covers
are removed, the remaining@covers
are moved to the test class level.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6481 on WordPress/wordpress-develop by @poena.
7 months ago
#30
This PR splits the existing test class wpPrivacyRequestsTable
into two, and creates a new TestCase.
The purpose is to make the test classes compatible with PHPUnit 11.
Move tear down and helper functions from wpPrivacyRequestsTable
to a new abstract TestCase so that they can be reused in multiple tests.
Split off the test for WP_Privacy_Requests_Table::get_views
into a separate test class,
Admin_WpPrivacyRequestsTable_GetViews_Test
.
Correct the covers
for `WP_Privacy_Requests_Table::get_views:
Previously @covers WP_Privacy_Requests_List_Table::get_views
.
Updated @covers WP_Privacy_Requests_Table::get_views
.
The previous was possibly a typo.
Rename the remaining test Tests_Admin_wpPrivacyRequestsTable
and test file to Admin_WpPrivacyRequestsTable_PrepareItems_Test
, to clarify which method that is being tested. Move the covers to the class level.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6483 on WordPress/wordpress-develop by @poena.
7 months ago
#31
Rename the test class Tests_Admin_wpUsersListTable
and the file to Admin_WpUsersListTable_GetViews_Test
to clarify which method is being tested.
Moves the @covers
to the class level.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
This ticket was mentioned in PR #6644 on WordPress/wordpress-develop by @poena.
6 months ago
#32
Trac ticket:
https://core.trac.wordpress.org/ticket/53010
The test class Tests_Admin_WpUpgrader (in tests/phpunit/tests/admin/wpUpgrader.php) contained tests for several methods.
This PR splits the test class into a new test class for each method, to indicate which methods are being tested, and to improve compatibility with PHPUnit 11.
Each of the split test classes has one covers
annotation at the class level.
A new abstract TestCase
is created, so that the set up and tear down functionality for WP_Upgrader
can be reused in multiple tests. The name if the test case is Admin_WpUpgrader_TestCase
This ticket was mentioned in PR #6679 on WordPress/wordpress-develop by @afercia.
6 months ago
#33
…ss to be more descriptive and adjust covers annotations.
Trac ticket: https://core.trac.wordpress.org/ticket/53010
#34
follow-up:
↓ 37
@
3 months ago
Related: #62004
Note: while yes/no namespacing is still up for debate, the rest of the proposals in this ticket MUST be addressed to allow the test suite to update to PHPUnit 11/12, so this ticket is no longer just a suggestion. It is a requirement.
#35
@
3 months ago
I've opened up a ticket in WPCS for this now too: https://github.com/WordPress/WordPress-Coding-Standards/issues/2484
This ticket was mentioned in Slack in #core by jrf. View the logs.
3 months ago
#37
in reply to:
↑ 34
@
3 months ago
Replying to jrf:
Related: #62004
Note: while yes/no namespacing is still up for debate, the rest of the proposals in this ticket MUST be addressed to allow the test suite to update to PHPUnit 11/12, so this ticket is no longer just a suggestion. It is a requirement.
This ticket is now part of #62004. The renaming and splitting tasks are now a requirement and prerequisite:
- [ ] Split the tests into 1 test class per each function or method. See the explanation here.
- [ ] Rename the test class to remove
Tests_
prefix and add_Test
suffix. See the explanation here. - [ ] Rename each test class file to exactly match the class name. See the explanation here.
As all of the test classes will need to be touched/changed/split, what is proposed in this ticket now aligns well with these needs as @jrf explains:
As all test classes will need to be touched/changed/split to address problem 4 anyway, I would strongly recommend for the new test classes to be made fully compatible with the PHPUnit naming conventions.
This will prevent having to re-do this exercise yet again in the future if the bypass for the first rule would be made impossible.
The PRs already attached to this ticket are good examples of these tasks.
#39
in reply to:
↑ 16
;
follow-up:
↓ 47
@
3 months ago
Hey @azaozz, fast-forwarding to today, this proposal is now part of #62004 "Test suite: update the tests for PHPUnit 10/11 and get ready for PHPUnit 12".
As I pointed out in comment:37, the renaming and splitting are now requirements.
Replying to azaozz:
May be missing something but it seems the proposed "File paths" suggest that each class should be split in up to 50 separate files, one per method. Is that correct?
If yes, could you please explain what are the benefits of having such "file and directory layout" with several thousands of files instead of 100-200? Seems this would trigger a lot of needless disk i/o and slow down running of the tests as most of the files will have to be loaded every time the tests are run.
Let's start with the splitting of tests into 1 function / method per test class / file.
Test coverage changes in PHPUnit are now the primary driver for splitting the tests. The full explanation is found here.
tl;dr
we don't just need to split the test classes up by class, in actual fact, we'll need to split them up to individual test classes for each method in a class to still be able to measure code coverage correctly.
What are the test coverage changes driving this effort?
The full explanation is found here.
Replying to azaozz:
Also seems this is not recommended (compatible?) by PSR4. It states that:
A fully qualified class name has the following form:
\<NamespaceName>(\<SubNamespaceNames>)*\<ClassName>
(where "The terminating class name corresponds to a file name ending in
.php
.").
Fast-forwarding to today, the recommendation now is to fully compatible with PHPUnit naming conventions, which are "comply largely with PSR4 file name regulations".
Why is renaming a requirement?
As explained here, tests will not run on PHPUnit 11+ if they do not comply with the naming conventions:
- Class name
[SomethingUnderTest]Test
- take note of the Test suffix. - The file name for the test class MUST match the class name EXACTLY in a case-sensitive manner.
An example:
Consider the test class Tests_Compat_ArrayKeyLast
which is located in a file named called tests/compat/arrayKeyLast.php
.
What needs to change?
- This class will need to be renamed to
Compat_ArrayKeyLast_Test
. - The file will need to be renamed to
tests/compat/Compat_ArrayKeyLast_Test.php
.
Does that make sense?
#40
@
3 months ago
Hi All
So we don't have to touch the files twice as we spit them
Can we please agree on the namespacing
Paul
#41
@
3 months ago
Scope of Work that can start now
The required changes for #62004 are:
- For tests that include more than one function or method, split each function and method into a separate test class and file.
- Rename each test class.
- Rename each test file to exactly match the test class name.
Contributors:
Work can (and should) start on doing only this part of the scope. Currently, there are PRs/patches you can use as examples, such as https://github.com/WordPress/wordpress-develop/pull/6235.
Scope of Work that is still being discussed
The following parts of the proposal are not required for PHPUnit 11+, but are ideal to address now to avoid touching the files more than once:
- Namespace.
- Reorganize the test suite directory and file structure.
Currently, there's not yet consensus on these remaining parts.
Contributors:
Please don't yet make these changes in the PRs submitted.
Committers
Ideally, consensus is reached on the remaining parts of this proposal before the commits are done. Why? To avoid touching the files more than once and to minimize disruptions to existing patches/PRs.
Let's work towards getting consensus on whether to move or not with the remaining parts of the proposal (i.e. namespace and directory reorganization).
This ticket was mentioned in PR #7406 on WordPress/wordpress-develop by @pbearne.
2 months ago
#42
#43
@
2 months ago
I worked it out I got it wrong
I made a script to do the renaming
I have not done any spiting yet
Can you confirm the example renames is right?
Paul
This ticket was mentioned in PR #7424 on WordPress/wordpress-develop by @pbearne.
2 months ago
#44
Created new PHPUnit test classes for get_avatar
, get_avatar_data
, and get_avatar_url
functions. These changes improve test organization and maintainability by grouping related tests together. Also, renamed avatar.php
to Functions_GetAvatarUrl_Test.php
to better reflect its content.
#45
@
2 months ago
Hi All
I have started to commit to the https://github.com/pbearne/wordpress-develop/tree/rename-test
I used a script to rename all the tests with a single @covers in it, renaming the file and class to match the @covers value.
There are about 500 files in the set :-)
I am working through to QA and fix/spilt as needed, like the first 3 files.
Can someone confirm that the naming is correct?
#46
@
2 months ago
@pbearne To be blunt: every automated patch for this ticket is automatically disqualified.
The whole point of this exercise in the context of #62004 is to have a human who understands the task at hand:
- Verify that any existing
@covers
tags are actually correct. - Decide based on those
@covers
tags if and if so, how to split the test class into multiple classes. Depending on the test class, this could end up being a split into dozens of new classes, which will each need careful review of what set up/tear down they need, what test case they should extend etc. - Make sure the
@covers
tags are no longer at method level, but at class level. - Decide what the right class name should be based on what is actually being tested.
- If there are no
@covers
tags or only partial@covers
tags (for some test methods in a class, not for others), figure out what is really being tested (and no, that can't be done based on what function/methods are called in the test method, there are a lot more things at play - this needs examining the actual code coverage reports and understanding the impact of setup/teardown on those, it needs history tracing viagit blame
etc).
Honestly, if I thought, for even a moment, that any form of automation for this task would be helpful, I would have written a sniff with an auto-fixer for this. The fact that I didn't, should tell you enough.
In other words, the renaming is the least of our problems and should only be done as an after-thought of the test class splitting. Not the other way around.
#47
in reply to:
↑ 39
@
2 months ago
Replying to hellofromTonya:
As I pointed out in comment:37, the renaming and splitting are now requirements.
...
For tests that include more than one function or method, split each function and method into a separate test class and file.
...
Test coverage changes in PHPUnit are now the primary driver for splitting the tests.
So basically according to PHPUnit there can only be one test case per class if test coverage is to be tracked (example)? Do I understand this correctly? If yes, this seems soooo inefficient? Is PHPUnit trying to be as slow as possible and wear out the hard disks of the computers it is running on, lol?
Joking of course, but thought PHPUnit was supposed to be PSR-4 compliant, right? Limiting classes to only one method is not compliant.
I'm also starting to think whether this splitting is worth it. Seems like a pretty bad degradation. Perhaps can settle down for partial/somewhat undefined coverage if that means tests will run faster and cause less problems?
the recommendation now is to fully compatible with PHPUnit naming conventions
Sure, makes sense.
The following parts of the proposal are not required for PHPUnit 11+, but are ideal to address now to avoid touching the files more than once:
- Namespace.
- Reorganize the test suite directory and file structure.
I don't really care about namespacing. Don't think it would fix or improve anything, just some overhead and very unlikely but nevertheless possible back-compat issues? But if that's your personal preference, may as well add them.
If adding namespaces, imho changing the directory structure won't break anything more :)
Frankly I'm starting to wonder if WP should just fork PHPUnit? Perhaps other older PHP projects would be happy to use/help maintain a fork? As far as I see there are always problems when it needs to be updated, and WP has to "jump through hoops" to be able to use it. Seems PHPUnit doesn't care much about its users and what are the use cases/existing code for it :(
#48
@
2 months ago
@hellofromTonya
The script is just to help with a few automation and to make sure I get the steps right.
I check every file and run the test on every.
I am confused as the what I should set he first part of the name to
Is the folder the test is in?
or the file the function is in
That said, we can now add namespace.
Could we create folders/namespace where the filename is the top folder e.g./wp_includes/functions?
And only add a prefix if the function is in a class?
I notice that the pull requests in this ticket are for a single function is this how you what the requests.
Or will you take a pull with 400 - 500 files/changes to get use started?
Paul
This ticket was mentioned in Slack in #core-test by ironprogrammer. View the logs.
2 months ago
#50
@
8 weeks ago
- Keywords 2nd-opinion removed
This ticket is now unblocked. Removing 2nd-opinion
. This ticket is a subtask of and requirement for #62004.
Plus note, the namespace part of the ticket is not a requirement for #62004, but is recommended.
Thinking the ticket's summary needs to be updated as this ticket is not solely about introducing namespacing. Rather, it's about reorganizing and rearchitecting the test suites, which has a subtasks recommendation of introducing namespacing.
#51
@
8 weeks ago
Thinking the ticket's summary needs to be updated as this ticket is not solely about introducing namespacing. Rather, it's about reorganizing and rearchitecting the test suites, which has a subtasks recommendation of introducing namespacing.
On Oct 14th, @jrf and I plan to update / rewrite this ticket's summary and description, i.e. for it to provide a clear scope of work and guidance of subtasks for contributors and committers.
Note: this proposal to follow PSR4 for the tests becomes all the more relevant once the test suite needs to start supporting PHPUnit 9.x.
PHPUnit 9.1.0 deprecated support for test file names which do not match the class name of the test class within.
It also deprecates the use of multiple (test) classes within a file, though I don't think that's used in WP.
Ref: https://github.com/sebastianbergmann/phpunit/pull/4109/