Tests: Reduce usage of assertEquals for 6.2

[desrosj]

Follow-up to:

• #55654 (6.1)
• #54726 (6.0)
• #53364 (5.9)
• #52482 (5.8)
• #38266 (5.7)

The assertEquals() test method does not check that the types of the expected and actual values match. This can hide subtle bugs especially when the values are falsey.

Tasks:

• Switch to using assertSame() when the type of the value in the assertion is important
• Replace overall usage of assertEquals() with type-strict assertion methods, with the aim of potentially removing its usage altogether

To help ease the effort of backporting tests, changes should also be made upstream in the Gutenberg repository.

This PR updates the test test_wp_count_attachments_should_cache_the_result to check the properties of the object returned instead of checking the object with assertEquals. That way, we guarantee the expected properties of the object.

Trac ticket: https://core.trac.wordpress.org/ticket/56800

[hellofromTonya]

Opened #57855 for the ongoing work into the 6.3 cycle.

This ensures that not only the return values match the expected results, but also that their type is the same.

Trac ticket: https://core.trac.wordpress.org/ticket/56800

[costdev]

I've submitted ​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 ​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 assertion conventions in 6.3:

• assertSimilarObject() - A new assertion that acts as a wrapper for assertIsObject() and assertEquals() when comparing objects.
• assertArrayEquals() - An existing assertion for use 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(), many of which were testing objects/arrays with objects. The above assertions would remove some of this significant overhead, as well as add some context to the assertions.

When?
I can submit a PR for the new assertSimilarObject() assertion as soon as we branch for 6.3. There was support for using these assertions 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.

[hellofromTonya]

Thank you @costdev. Reviewing now.

[hellofromTonya]

​PR 4166 is ready for commit. Prepping now.

[hellofromTonya]

In 55465:

Build/Test Tooling: Use assertSame() in WP_Date_Query tests.

Change from assertEquals() to assertSame(). Why? To ensure both the return value and data type match the expected results.

Follow-up to [54530].

Props costdev, peterwilsoncc, mukesh27, ankitmaru.
See #56800.

[hellofromTonya]

In 55466:

Build/Test Tooling: Use assertSame() in Tests_Comment::test_update_comment_from_privileged_user_by_privileged_user().

Change from assertEquals() to assertSame(). Why? To ensure both the return value and data type match the expected results.

Follow-up to [c].

Props costdev, peterwilsoncc, mukesh27, ankitmaru.
See #56800.

Follow-up to PR #4166.

Changes from assertEquals() to assertSame(). Why? To ensure both the return value and data type match the expected results.

Changes the expected height and width from string to integer data types. Why integer? getimagesize() (within wp_getimagesize()) will return an integer for both height and weight.

PR is a confidence check before committing.

Trac ticket: https://core.trac.wordpress.org/ticket/56800

[hellofromTonya]

In 55467:

Tests: Improve Tests_Media::test_wp_generate_attachment_metadata_doesnt_generate_sizes_for_150_square_image().

Changes:

• from assertEquals() to assertSame(). Why? To ensure both the return value and data type match the expected results.

• the expected height and width from string to integer data types. Why integer? getimagesize() (within wp_getimagesize()) will return an integer for both height and weight.

• adds the ticket annotation.

• adds assertion failure messages. Why? To denote which assertion failed, which aids in debugging efforts.

Follow-up to [55278].

Props costdev, peterwilsoncc, mukesh27, ankitmaru, hellofromTonya.
See #56800, #57370.

Committed via https://core.trac.wordpress.org/changeset/55467.

[hellofromTonya]

In 55468:

Tests: Use assertSame() in Tests_Theme_wpThemeJson.

Change from assertEquals() to assertSame(). Why? To ensure both the return value and data type match the expected results.

Follow-up to [55216].

Props costdev, peterwilsoncc, mukesh27, ankitmaru.
See #56800, #57621.

Closing as changes were committed.

[hellofromTonya]

Closing for 6.2. If more commits are needed and can happen before RC1 (in 2 days), then please reopen.