WordPress.org

Make WordPress Core

Changeset 51831


Ignore:
Timestamp:
09/20/2021 07:58:09 PM (4 weeks ago)
Author:
hellofromTonya
Message:

Build/Test Tools: Fix null handling and string type casting in WP_UnitTestCase_Base::assertSameIgnoreEOL().

Basically, the whole assertSameIgnoreEOL() assertion was fundamentally flawed. The assertion contends that it checks that the expected and actual values are of the same type and value, but the reality was very different.

  • The function uses map_deep() to potentially handle all sorts of inputs.
  • map_deep() handles arrays and objects with special casing, but will call the callback on everything else without further distinction.
  • The callback used passes the expected/actual value on to the str_replace() function to remove potential new line differences.
  • And the str_replace() function will - with a non-array input for the $subject - always return a string.
  • The output of these calls to map_deep() will therefore have "normalized" _all properties_ in objects, _all values_ in arrays and _all non-object, non-array values_ to strings.
  • And a call to assertSame() will therefore NEVER do a proper type check as the type of all input has already, unintentionally, been "normalized" to string.

Aside from this clear flaw in the design of the assertion, PHP 8.1 now exposes a further issue as a null value for an object property, an array value or a plain value, will now yield a str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated notice.

To fix both these issues, the fix in this PR ensures that the call to str_replace() will now only be made if the input is a text string.
All other values passed to the callback are left in their original type.

This ensures that a proper value AND type comparison can be done as well as prevents the PHP 8.1 deprecation notices.

Ref:

This commit:

  • Fixes type-casting of non-string values to string (the flawed part of this assertion) by invoking str_replace() when the value is of string type.
  • Fixes the PHP 8.1 str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated deprecation notice.
  • Micro-optimization: skips map_deep() when actual and/or expected are null (no need to process).
  • Adjusts the method documentation for both this method and the assertEqualsIgnoreEOL() alias method to document that the $expected and $actual parameters can be of any type.

Follow-up to [48937], [51135], [51478].

Props jrf, hellofromTonya.
See #53363, #53635.

File:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/tests/phpunit/includes/abstract-testcase.php

    r51698 r51831  
    725725     * @since 5.9.0 Added the `$message` parameter.
    726726     *
    727      * @param string|array $expected The expected value.
    728      * @param string|array $actual   The actual value.
    729      * @param string       $message  Optional. Message to display when the assertion fails.
     727     * @param mixed $expected The expected value.
     728     * @param mixed $actual   The actual value.
     729     * @param string $message  Optional. Message to display when the assertion fails.
    730730     */
    731731    public function assertSameIgnoreEOL( $expected, $actual, $message = '' ) {
    732         $expected = map_deep(
    733             $expected,
    734             static function ( $value ) {
    735                 return str_replace( "\r\n", "\n", $value );
    736             }
    737         );
    738 
    739         $actual = map_deep(
    740             $actual,
    741             static function ( $value ) {
    742                 return str_replace( "\r\n", "\n", $value );
    743             }
    744         );
     732        if ( null !== $expected ) {
     733            $expected = map_deep(
     734                $expected,
     735                static function ( $value ) {
     736                    if ( is_string( $value ) ) {
     737                        return str_replace( "\r\n", "\n", $value );
     738                    }
     739
     740                    return $value;
     741                }
     742            );
     743        }
     744
     745        if ( null !== $actual ) {
     746            $actual = map_deep(
     747                $actual,
     748                static function ( $value ) {
     749                    if ( is_string( $value ) ) {
     750                        return str_replace( "\r\n", "\n", $value );
     751                    }
     752
     753                    return $value;
     754                }
     755            );
     756        }
    745757
    746758        $this->assertSame( $expected, $actual, $message );
     
    754766     * @since 5.9.0 Added the `$message` parameter.
    755767     *
    756      * @param string $expected The expected value.
    757      * @param string $actual   The actual value.
     768     * @param mixed $expected The expected value.
     769     * @param mixed $actual   The actual value.
    758770     * @param string $message  Optional. Message to display when the assertion fails.
    759771     */
Note: See TracChangeset for help on using the changeset viewer.