<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[51831] trunk/tests/phpunit/includes/abstract-testcase.php: Build/Test Tools: Fix null handling and string type casting in `WP_UnitTestCase_Base::assertSameIgnoreEOL()`.</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { white-space: pre-line; overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta" style="font-size: 105%">
<dt style="float: left; width: 6em; font-weight: bold">Revision</dt> <dd><a style="font-weight: bold" href="https://core.trac.wordpress.org/changeset/51831">51831</a><script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","description":"Review this Commit","action":{"@type":"ViewAction","url":"https://core.trac.wordpress.org/changeset/51831","name":"Review Commit"}}</script></dd>
<dt style="float: left; width: 6em; font-weight: bold">Author</dt> <dd>hellofromTonya</dd>
<dt style="float: left; width: 6em; font-weight: bold">Date</dt> <dd>2021-09-20 19:58:09 +0000 (Mon, 20 Sep 2021)</dd>
</dl>
<pre style='padding-left: 1em; margin: 2em 0; border-left: 2px solid #ccc; line-height: 1.25; font-size: 105%; font-family: sans-serif'>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 <a href="https://core.trac.wordpress.org/ticket/3">#3</a> ($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 <a href="https://core.trac.wordpress.org/ticket/3">#3</a> ($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 <a href="https://core.trac.wordpress.org/changeset/48937">[48937]</a>, <a href="https://core.trac.wordpress.org/changeset/51135">[51135]</a>, <a href="https://core.trac.wordpress.org/changeset/51478">[51478]</a>.
Props jrf, hellofromTonya.
See <a href="https://core.trac.wordpress.org/ticket/53363">#53363</a>, <a href="https://core.trac.wordpress.org/ticket/53635">#53635</a>.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunktestsphpunitincludesabstracttestcasephp">trunk/tests/phpunit/includes/abstract-testcase.php</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunktestsphpunitincludesabstracttestcasephp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/tests/phpunit/includes/abstract-testcase.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/tests/phpunit/includes/abstract-testcase.php 2021-09-20 18:48:47 UTC (rev 51830)
+++ trunk/tests/phpunit/includes/abstract-testcase.php 2021-09-20 19:58:09 UTC (rev 51831)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -724,25 +724,37 @@
</span><span class="cx" style="display: block; padding: 0 10px"> * @since 5.8.0 Added support for nested arrays.
</span><span class="cx" style="display: block; padding: 0 10px"> * @since 5.9.0 Added the `$message` parameter.
</span><span class="cx" style="display: block; padding: 0 10px"> *
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * @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.
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * @param mixed $expected The expected value.
+ * @param mixed $actual The actual value.
+ * @param string $message Optional. Message to display when the assertion fails.
</ins><span class="cx" style="display: block; padding: 0 10px"> */
</span><span class="cx" style="display: block; padding: 0 10px"> public function assertSameIgnoreEOL( $expected, $actual, $message = '' ) {
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- $expected = map_deep(
- $expected,
- static function ( $value ) {
- return str_replace( "\r\n", "\n", $value );
- }
- );
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ if ( null !== $expected ) {
+ $expected = map_deep(
+ $expected,
+ static function ( $value ) {
+ if ( is_string( $value ) ) {
+ return str_replace( "\r\n", "\n", $value );
+ }
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- $actual = map_deep(
- $actual,
- static function ( $value ) {
- return str_replace( "\r\n", "\n", $value );
- }
- );
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ return $value;
+ }
+ );
+ }
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ if ( null !== $actual ) {
+ $actual = map_deep(
+ $actual,
+ static function ( $value ) {
+ if ( is_string( $value ) ) {
+ return str_replace( "\r\n", "\n", $value );
+ }
+
+ return $value;
+ }
+ );
+ }
+
</ins><span class="cx" style="display: block; padding: 0 10px"> $this->assertSame( $expected, $actual, $message );
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -753,8 +765,8 @@
</span><span class="cx" style="display: block; padding: 0 10px"> * @since 5.6.0 Turned into an alias for `::assertSameIgnoreEOL()`.
</span><span class="cx" style="display: block; padding: 0 10px"> * @since 5.9.0 Added the `$message` parameter.
</span><span class="cx" style="display: block; padding: 0 10px"> *
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * @param string $expected The expected value.
- * @param string $actual The actual value.
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * @param mixed $expected The expected value.
+ * @param mixed $actual The actual value.
</ins><span class="cx" style="display: block; padding: 0 10px"> * @param string $message Optional. Message to display when the assertion fails.
</span><span class="cx" style="display: block; padding: 0 10px"> */
</span><span class="cx" style="display: block; padding: 0 10px"> public function assertEqualsIgnoreEOL( $expected, $actual, $message = '' ) {
</span></span></pre>
</div>
</div>
</body>
</html>