[wp-trac] [WordPress Trac] #53844: Fix four warnings in the test suite
WordPress Trac
noreply at wordpress.org
Fri Nov 19 16:01:04 UTC 2021
#53844: Fix four warnings in the test suite
------------------------------+-----------------------------
Reporter: jrf | Owner: hellofromTonya
Type: defect (bug) | Status: assigned
Priority: normal | Milestone: 6.0
Component: Build/Test Tools | Version:
Severity: normal | Resolution:
Keywords: has-patch | Focuses: rest-api
------------------------------+-----------------------------
Comment (by jrf):
@sourovroy Thank you for creating a patch for this ticket and sorry it
took me a while to get to reviewing it.
I've had a good look at your patch now and would like to suggest the
following changes:
1. Generally speaking it is preferred (and will become a hard rule in the
future) to have only one class per file. The `mock-utils.php` file looks
to be set up to potentially contain more classes in the future.
I'd like to recommend the following changes to that file/class:
* Rename the class from `Mock_REST_Invokable` to `Mock_Invokable`. There
is nothing REST specific in the mock class, so no need to give the
impression that the mock can only be used for REST API tests.
* Rename the file to match the class, i.e. `mock-invokable.php`.
* Adjust the file docblock to match.
2. Only a few methods in the test classes need this mock. The
`require_once` for the file has been put in the `set_up()` method, but the
file only needs to be includes once no matter what. What about placing the
`require_once` statement in a `set_up_before_class()` method instead ?
That way the file hit will only happen once for each test class, instead
of multiple times.
Other than that, this is 100% the sort of solution I envisioned and once
the above mentioned tweaks have been made, I think this patch can go into
Core.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/53844#comment:4>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list