[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