[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