[wp-trac] [WordPress Trac] #53364: Tests: Reduce usage of assertEquals for 5.9
WordPress Trac
noreply at wordpress.org
Wed Oct 20 02:10:16 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):
= Overview
I've taken a closer look at the test suite and usage of `assertEquals()`.
My thoughts on the stages to take are below.
==== Stage 1:
'Clean' replacements.
PR 1768 replaces some instances of `assertEquals()` with `assertSame()`.
More specifically, it makes replacements that appear to be 'clean'
replacements, requiring no adjustment of `Tests` or `Core` code. All
PHPUnit tests pass.
==== Stage 2:
I have looked at the cases where the returned value is `float` rather than
`int`.
As @SergeyBiryukov noted
[https://core.trac.wordpress.org/ticket/38266#comment:19 here]:
> In other cases, this points to minor bugs in core, e.g. using `ceil()`
for values that are documented as integers, but without explicitly casting
to `int`, results in them being of type `float` instead. This affects some
properties like `max_num_pages`, `max_pages`, `total_pages` in various
classes, or functions like `get_comment_pages_count()`,
`wp_embed_defaults()`, `get_oembed_response_data()`.
A related ticket further down has a lengthy discussion about ways to
resolve this.
- I've tested one of the proposed solutions, a simple cast to `int` using
`(int)` before the calls to `ceil()`, followed by changing
`assertEquals()` to `assertSame()` for relevant tests. All PHPUnit tests
pass.
- With coverage incomplete this could, theoretically, be a breaking
change. However, we are yet to come across a real-world example of this,
or even an example scenario where these methods should return a `float`.
==== Stage 3
I have looked at the cases where the returned value is `string` rather
than `int`.
After investigation, the actual values are a numeric `string` for
compatibility reasons and are documented as such. For this reason, the
expected value should be a numeric `string`, rather than changing the code
in `src` and breaking compatibility.
==== Stage 4
The returned value is `array` and `assertEquals()` is being used instead
of `assertSameSets()`. It should be noted that an `array` containing an
`object` will not return the same signature, thus cannot be tested using
`assertSameSets()` or `assertSameSetsWithIndex()`.
==== Stage 5
The returned value is always `bool` rather than `int`. In these cases, the
expected value should be changed to a `bool`.
This can be done directly e.g. `0` becomes `false`, or by conditional if
comparing variables, e.g. instead of comparing `$some_int`, compare the
result of `$expected = ( 0 === $some_int )`.
This one requires particularly close attention to the methods being
tested, their possible return values and the accuracy of the current
tests.
==== Stage 6
The returned value is `float` rather than `int`. This requires
investigation to determine why the value is `float` and whether this is
correct, should be changed in `src`, or must remain `float` for
compatibility reasons, thus the expected value in the tests should be
changed to `float`.
==== Stage 7
**N.B.** This may go outside the scope of this ticket.
The returned value is `array` and `assertEqualSets()` or
`assertEqualSetsWithIndex()` is being used instead of `assertSameSets()`
and `assertSameSetsWithIndex()` respectively. It should be noted that an
`array` containing an `object` will not return the same signature, thus
cannot be tested using `assertSameSets()` or `assertSameSetsWithIndex()`.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/53364#comment:2>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list