[wp-trac] [WordPress Trac] #53364: Tests: Reduce usage of assertEquals for 5.9
WordPress Trac
noreply at wordpress.org
Sun Dec 12 11:27:58 UTC 2021
#53364: Tests: Reduce usage of assertEquals for 5.9
-------------------------------------------+---------------------
Reporter: desrosj | Owner: (none)
Type: task (blessed) | Status: new
Priority: normal | Milestone: 5.9
Component: Build/Test Tools | Version:
Severity: normal | Resolution:
Keywords: php8 has-patch has-unit-tests | Focuses:
-------------------------------------------+---------------------
Comment (by costdev):
**Update**
[https://github.com/WordPress/wordpress-develop/pull/1768 The PR]
currently reduces `assertEquals()` usage from ~558 to ~191 through various
changes to assertions. There are currently no changes in source.
Regarding the remaining ~191:
As we know, many of them are comparing objects, so using `assertEquals()`
is ''fine''. But, it's a pain to review those every release to make sure
that we're not missing anything. It also gives us a false perception of
the estimated number of unacceptable uses of `assertEquals()` for us to
deal with for that release.
What about two additional assertions that are just aliases of
`assertEquals()`?
{{{#!php
// For two objects.
// We can't strictly compare objects in most cases anyway,
// so this name should be safe and the implied comparison
// is accurate in all but object id.
public function assertSameObjects( $expected, $actual, $message = '' ) {
$this->assertEquals( $expected, $actual, $message );
}
// For arrays containing objects.
public function assertSameSetsWithObjects( $expected, $actual, $message =
'' ) {
$this->assertEquals( $expected, $actual, $message );
}
}}}
The intended benefit is a clearer picture of possible loose comparison
with `assertEquals()` by excluding known acceptable usage. There's also a
sort of ''maybe benefit'' that by offering the assertion, it reinforces
''the idea of'' stricter testing. This would reduce the remaining ~191 by
quite a bit and minimize the work involved with each release.
For the remainder, these are largely due to `float` being compared to
`int`. For nearly all of these, this is an issue with `ceil()`. It has
been suggested before that we can cast the result of `ceil()` to `int` to
remedy this when the value is documented as an `int`.
So, a call for opinions:
1. Should we add the two aliases to make our lives easier going forward?
2. Should we cast `ceil()` to `int` where appropriate to help us implement
stricter comparison?
--
Ticket URL: <https://core.trac.wordpress.org/ticket/53364#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list