<!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>[213918] trunk/Tools</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 { 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">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/213918">213918</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2017-03-14 11:22:06 -0700 (Tue, 14 Mar 2017)</dd>
</dl>

<h3>Log Message</h3>
<pre>Nwtr ignores ImageDiff's errors for ref tests
https://bugs.webkit.org/show_bug.cgi?id=168033

Patch by Fujii Hironori &lt;Hironori.Fujii@sony.com&gt; on 2017-03-14
Reviewed by Alexey Proskuryakov.

Nwtr checks ImageDiff's errors only for pixel tests, but for ref
tests. Those errors of ref tests also should be checked.

In the current implementation of expected mismatch ref tests,
diff_image was called if the image hashes match. This is useless
because two images are ensured identical in that case. Calling
image_hash is considered unnecessary for expected mismatch ref
tests. Do not call diff_image for them.

As the result, check the error only for expected match ref tests.

* Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
(SingleTestRunner._compare_image): Rename a variable 'err_str' to 'error_string'.
(SingleTestRunner._compare_output_with_reference): Do not call
diff_image for expected mismatch ref tests. Check the error and
marked the test failed for expected match ref tests.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsScriptswebkitpylayout_testscontrollerssingle_test_runnerpy">trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (213917 => 213918)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog        2017-03-14 18:19:47 UTC (rev 213917)
+++ trunk/Tools/ChangeLog        2017-03-14 18:22:06 UTC (rev 213918)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+2017-03-14  Fujii Hironori  &lt;Hironori.Fujii@sony.com&gt;
+
+        Nwtr ignores ImageDiff's errors for ref tests
+        https://bugs.webkit.org/show_bug.cgi?id=168033
+
+        Reviewed by Alexey Proskuryakov.
+
+        Nwtr checks ImageDiff's errors only for pixel tests, but for ref
+        tests. Those errors of ref tests also should be checked.
+
+        In the current implementation of expected mismatch ref tests,
+        diff_image was called if the image hashes match. This is useless
+        because two images are ensured identical in that case. Calling
+        image_hash is considered unnecessary for expected mismatch ref
+        tests. Do not call diff_image for them.
+
+        As the result, check the error only for expected match ref tests.
+
+        * Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
+        (SingleTestRunner._compare_image): Rename a variable 'err_str' to 'error_string'.
+        (SingleTestRunner._compare_output_with_reference): Do not call
+        diff_image for expected mismatch ref tests. Check the error and
+        marked the test failed for expected match ref tests.
+
</ins><span class="cx"> 2017-03-14  Brady Eidson  &lt;beidson@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         REGRESSION (r213877): WebKit2.CookieManager fails.
</span></span></pre></div>
<a id="trunkToolsScriptswebkitpylayout_testscontrollerssingle_test_runnerpy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py (213917 => 213918)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py        2017-03-14 18:19:47 UTC (rev 213917)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py        2017-03-14 18:22:06 UTC (rev 213918)
</span><span class="lines">@@ -273,11 +273,11 @@
</span><span class="cx">             failures.append(test_failures.FailureMissingImageHash())
</span><span class="cx">         elif driver_output.image_hash != expected_driver_output.image_hash:
</span><span class="cx">             diff_result = self._port.diff_image(expected_driver_output.image, driver_output.image)
</span><del>-            err_str = diff_result[2]
-            if err_str:
-                _log.warning('  %s : %s' % (self._test_name, err_str))
</del><ins>+            error_string = diff_result[2]
+            if error_string:
+                _log.warning('  %s : %s' % (self._test_name, error_string))
</ins><span class="cx">                 failures.append(test_failures.FailureImageHashMismatch())
</span><del>-                driver_output.error = (driver_output.error or '') + err_str
</del><ins>+                driver_output.error = (driver_output.error or '') + error_string
</ins><span class="cx">             else:
</span><span class="cx">                 driver_output.image_diff = diff_result[0]
</span><span class="cx">                 if driver_output.image_diff:
</span><span class="lines">@@ -331,17 +331,19 @@
</span><span class="cx">         if not reference_driver_output.image_hash and not actual_driver_output.image_hash:
</span><span class="cx">             failures.append(test_failures.FailureReftestNoImagesGenerated(reference_filename))
</span><span class="cx">         elif mismatch:
</span><ins>+            # Calling image_hash is considered unnecessary for expected mismatch ref tests.
</ins><span class="cx">             if reference_driver_output.image_hash == actual_driver_output.image_hash:
</span><del>-                diff_result = self._port.diff_image(reference_driver_output.image, actual_driver_output.image, tolerance=0)
-                if not diff_result[0]:
-                    failures.append(test_failures.FailureReftestMismatchDidNotOccur(reference_filename))
-                else:
-                    _log.warning(&quot;  %s -&gt; ref test hashes matched but diff failed&quot; % self._test_name)
-
</del><ins>+                failures.append(test_failures.FailureReftestMismatchDidNotOccur(reference_filename))
</ins><span class="cx">         elif reference_driver_output.image_hash != actual_driver_output.image_hash:
</span><ins>+            # ImageDiff has a hard coded color distance threshold even though tolerance=0 is specified.
</ins><span class="cx">             diff_result = self._port.diff_image(reference_driver_output.image, actual_driver_output.image, tolerance=0)
</span><del>-            if diff_result[0]:
</del><ins>+            error_string = diff_result[2]
+            if error_string:
+                _log.warning('  %s : %s' % (self._test_name, error_string))
</ins><span class="cx">                 failures.append(test_failures.FailureReftestMismatch(reference_filename))
</span><ins>+                actual_driver_output.error = (actual_driver_output.error or '') + error_string
+            elif diff_result[0]:
+                failures.append(test_failures.FailureReftestMismatch(reference_filename))
</ins><span class="cx">             else:
</span><span class="cx">                 _log.warning(&quot;  %s -&gt; ref test hashes didn't match but diff passed&quot; % self._test_name)
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>