[wp-trac] [WordPress Trac] #56800: Tests: Reduce usage of assertEquals for 6.2

WordPress Trac noreply at wordpress.org
Fri Mar 3 03:26:32 UTC 2023


#56800: Tests: Reduce usage of assertEquals for 6.2
--------------------------------------+---------------------
 Reporter:  desrosj                   |       Owner:  (none)
     Type:  defect (bug)              |      Status:  new
 Priority:  normal                    |   Milestone:  6.2
Component:  Build/Test Tools          |     Version:
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:
--------------------------------------+---------------------

Comment (by costdev):

 I've submitted [https://github.com/WordPress/wordpress-develop/pull/4166
 PR 4166] to replace some occurrences of `assertEquals()` with
 `assertSame()`. These are switcharoo changes that don't involve type-
 coercion or any other change.

 It seems that only 7 new instances of `assertEquals()` have been
 introduced since last cycle (improvement!), with an additional one that
 was missed during the last attempts to reduce `assertEquals()` usage.

 Pinging @hellofromTonya for review of [https://github.com/WordPress
 /wordpress-develop/pull/4166 PR 4166].

 ------

 Note that there are 462 uses of `$this->assertEquals()` remaining in the
 test suite. Many of these are testing objects, and are therefore fine.
 However, I think we should pursue the intention to add two new assertions
 in 6.3:

 - `assertSimilarObject()` - A wrapper for `assertIsObject()` and
 `assertEquals()` when comparing objects.
 - `assertArrayEqualsWithObject()` - A wrapper for `assertIsArray()` and
 `assertEquals()` when comparing arrays containing at least one object.

 Why?
 Every cycle, we have to sift through these types of assertions to
 determine whether their usage is fine. This is a huge time sink, and as
 shown by the data above, meant that putting PR 4166 together required
 reviewing 470 instances of `assertEquals()`, rather than 8. The above
 assertions would remove this significant overhead, as well as add some
 context to the assertions.

 When?
 I can submit a PR for these assertions as soon as we branch for 6.3. There
 was support for these in previous cycles, but we never got around to it.

 -----

 Other remaining instances include:
 - `$expected` is not the same type as `$actual` (usually for `int` or
 `float` when `$actual` is `string`).
   - There is no need for these to be `assertEquals()` when a simple type
 change will make the test accurate.
 - `$actual` is returning a `float` when it's documented to return `int`.
   - This is essentially an issue with some `ceil()` usage in Core that
 needs appropriately rectified. Note: `(int)` casts alone are not always
 appropriate.
 - The assertion should be changed to one that isn't `assertSame()`. These
 include:
   - `assertTrue()`, `assertFalse()`, `assertNull()`,
 `assertSameSetsWithIndex()`, `assertEqualSetsWithIndex()`* and
 `assertEqualSets()`*.

 `*` Requires further review if the two new assertions are implemented.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/56800#comment:5>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list