[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