Changeset 51831

Timestamp:

09/20/2021 07:58:09 PM (3 years 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:

• ​https://developer.wordpress.org/reference/functions/map_deep/
• ​https://www.php.net/manual/en/function.str-replace.php

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:

• trunk/tests/phpunit/includes/abstract-testcase.php (modified) (2 diffs)

trunk/tests/phpunit/includes/abstract-testcase.php

@@ -725 +725 @@
      * @since 5.9.0 Added the `$message` parameter.
      *
-     * @param string|array $expected The expected value.
-     * @param string|array $actual   The actual value.
-     * @param string       $message  Optional. Message to display when the assertion fails.
+     * @param mixed  $expected The expected value.
+     * @param mixed  $actual   The actual value.
+     * @param string $message  Optional. Message to display when the assertion fails.
      */
     public function assertSameIgnoreEOL( $expected, $actual, $message = '' ) {
-        $expected = map_deep(
-            $expected,
-            static function ( $value ) {
-                return str_replace( "\r\n", "\n", $value );
-            }
-        );
-
-        $actual = map_deep(
-            $actual,
-            static function ( $value ) {
-                return str_replace( "\r\n", "\n", $value );
-            }
-        );
+        if ( null !== $expected ) {
+            $expected = map_deep(
+                $expected,
+                static function ( $value ) {
+                    if ( is_string( $value ) ) {
+                        return str_replace( "\r\n", "\n", $value );
+                    }
+
+                    return $value;
+                }
+            );
+        }
+
+        if ( null !== $actual ) {
+            $actual = map_deep(
+                $actual,
+                static function ( $value ) {
+                    if ( is_string( $value ) ) {
+                        return str_replace( "\r\n", "\n", $value );
+                    }
+
+                    return $value;
+                }
+            );
+        }
 
         $this->assertSame( $expected, $actual, $message );
@@ -754 +766 @@
      * @since 5.9.0 Added the `$message` parameter.
      *
-     * @param string $expected The expected value.
-     * @param string $actual   The actual value.
+     * @param mixed  $expected The expected value.
+     * @param mixed  $actual   The actual value.
      * @param string $message  Optional. Message to display when the assertion fails.
      */