Diff
Modified: trunk/Tools/ChangeLog (124853 => 124854)
--- trunk/Tools/ChangeLog 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/ChangeLog 2012-08-07 06:07:10 UTC (rev 124854)
@@ -1,3 +1,41 @@
+2012-08-06 Sheriff Bot <[email protected]>
+
+ Unreviewed, rolling out r124801.
+ http://trac.webkit.org/changeset/124801
+ https://bugs.webkit.org/show_bug.cgi?id=93338
+
+ It broke NRWT (Requested by Ossy on #webkit).
+
+ * Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
+ (SingleTestRunner._compare_image):
+ (SingleTestRunner._compare_output_with_reference):
+ * Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:
+ (write_test_result):
+ * Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py:
+ (TestResultWriterTest.test_reftest_diff_image.ImageDiffTestPort.diff_image):
+ (TestResultWriterTest):
+ * Scripts/webkitpy/layout_tests/port/base.py:
+ (Port.diff_image):
+ * Scripts/webkitpy/layout_tests/port/chromium.py:
+ (ChromiumPort.diff_image):
+ * Scripts/webkitpy/layout_tests/port/chromium_port_testcase.py:
+ (ChromiumPortTestCase.test_diff_image):
+ * Scripts/webkitpy/layout_tests/port/image_diff.py:
+ (ImageDiffer.diff_image):
+ (ImageDiffer._read):
+ * Scripts/webkitpy/layout_tests/port/image_diff_unittest.py:
+ (TestImageDiffer.test_diff_image):
+ * Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py:
+ (MockDRTPortTest.test_diff_image):
+ * Scripts/webkitpy/layout_tests/port/port_testcase.py:
+ (PortTestCase.test_diff_image):
+ * Scripts/webkitpy/layout_tests/port/server_process_mock.py:
+ (MockServerProcess.__init__):
+ * Scripts/webkitpy/layout_tests/port/test.py:
+ (TestPort.diff_image):
+ * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
+ (MainTest.test_tolerance.ImageDiffTestPort.diff_image):
+
2012-08-06 Wei James <[email protected]>
[Chromium]duplicated command line options in Android LayoutTest
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -264,18 +264,12 @@
failures.append(test_failures.FailureMissingImageHash())
elif driver_output.image_hash != expected_driver_output.image_hash:
diff_result = self._port.diff_image(driver_output.image, expected_driver_output.image)
- err_str = diff_result[2]
- if err_str:
- _log.warning(' %s : %s' % (self._test_name, err_str))
- failures.append(test_failures.FailureImageHashMismatch())
- driver_output.error = (driver_output.error or '') + err_str
+ driver_output.image_diff = diff_result[0]
+ if driver_output.image_diff:
+ failures.append(test_failures.FailureImageHashMismatch(diff_result[1]))
else:
- driver_output.image_diff = diff_result[0]
- if driver_output.image_diff:
- failures.append(test_failures.FailureImageHashMismatch(diff_result[1]))
- else:
- # See https://bugs.webkit.org/show_bug.cgi?id=69444 for why this isn't a full failure.
- _log.warning(' %s -> pixel hash failed (but pixel test still passes)' % self._test_name)
+ # See https://bugs.webkit.org/show_bug.cgi?id=69444 for why this isn't a full failure.
+ _log.warning(' %s -> pixel hash failed (but pixel test still passes)' % self._test_name)
return failures
def _run_reftest(self):
@@ -323,8 +317,4 @@
failures.append(test_failures.FailureReftestMismatchDidNotOccur(reference_filename))
elif driver_output1.image_hash != driver_output2.image_hash:
failures.append(test_failures.FailureReftestMismatch(reference_filename))
-
- # recompute in case we added to stderr during diff_image
- has_stderr = driver_output1.has_stderr() or driver_output2.has_stderr()
-
return TestResult(self._test_name, failures, total_test_time, has_stderr)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -69,7 +69,7 @@
# FIXME: This work should be done earlier in the pipeline (e.g., when we compare images for non-ref tests).
# FIXME: We should always have 2 images here.
if driver_output.image and expected_driver_output.image:
- diff_image, diff_percent, err_str = port.diff_image(driver_output.image, expected_driver_output.image, tolerance=0)
+ diff_image, diff_percent = port.diff_image(driver_output.image, expected_driver_output.image, tolerance=0)
if diff_image:
writer.write_image_diff_files(diff_image)
failure.diff_percent = diff_percent
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -42,7 +42,7 @@
class ImageDiffTestPort(TestPort):
def diff_image(self, expected_contents, actual_contents, tolerance=None):
used_tolerance_values.append(tolerance)
- return (True, 1, None)
+ return (True, 1)
host = MockHost()
port = ImageDiffTestPort(host)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -318,7 +318,7 @@
return expected_audio != actual_audio
def diff_image(self, expected_contents, actual_contents, tolerance=None):
- """Compare two images and return a tuple of an image diff, a percentage difference (0-100), and an error string.
+ """Compare two images and return a tuple of an image diff, and a percentage difference (0-100).
|tolerance| should be a percentage value (0.0 - 100.0).
If it is omitted, the port default tolerance value is used.
@@ -326,9 +326,9 @@
If an error occurs (like ImageDiff isn't found, or crashes, we log an error and return True (for a diff).
"""
if not actual_contents and not expected_contents:
- return (None, 0, None)
+ return (None, 0)
if not actual_contents or not expected_contents:
- return (True, 0, None)
+ return (True, 0)
if not self._image_differ:
self._image_differ = image_diff.ImageDiffer(self)
self.set_option_default('tolerance', 0.1)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -190,16 +190,18 @@
override_step, logging)
def diff_image(self, expected_contents, actual_contents, tolerance=None):
+ # FIXME: need unit tests for this.
+
# tolerance is not used in chromium. Make sure caller doesn't pass tolerance other than zero or None.
assert (tolerance is None) or tolerance == 0
# If only one of them exists, return that one.
if not actual_contents and not expected_contents:
- return (None, 0, None)
+ return (None, 0)
if not actual_contents:
- return (expected_contents, 0, None)
+ return (expected_contents, 0)
if not expected_contents:
- return (actual_contents, 0, None)
+ return (actual_contents, 0)
tempdir = self._filesystem.mkdtemp()
@@ -219,23 +221,29 @@
comand = [executable, '--diff', native_actual_filename, native_expected_filename, native_diff_filename]
result = None
- err_str = None
try:
exit_code = self._executive.run_command(comand, return_exit_code=True)
if exit_code == 0:
# The images are the same.
result = None
- elif exit_code == 1:
- result = self._filesystem.read_binary_file(native_diff_filename)
+ elif exit_code != 1:
+ _log.error("image diff returned an exit code of %s" % exit_code)
+ # Returning None here causes the script to think that we
+ # successfully created the diff even though we didn't.
+ # FIXME: Consider raising an exception here, so that the error
+ # is not accidentally overlooked while the test passes.
+ result = None
+ except OSError, e:
+ if e.errno == errno.ENOENT or e.errno == errno.EACCES:
+ _compare_available = False
else:
- err_str = "image diff returned an exit code of %s" % exit_code
- except OSError, e:
- err_str = 'error running image diff: %s' % str(e)
+ raise
finally:
+ if exit_code == 1:
+ result = self._filesystem.read_binary_file(native_diff_filename)
self._filesystem.rmtree(str(tempdir))
+ return (result, 0) # FIXME: how to get % diff?
- return (result, 0, err_str or None) # FIXME: how to get % diff?
-
def path_from_chromium_base(self, *comps):
"""Returns the full path to path made by joining the top of the
Chromium source tree and the list of path components in |*comps|."""
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_port_testcase.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_port_testcase.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_port_testcase.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -158,11 +158,6 @@
exception_raised = True
self.assertFalse(exception_raised)
- def test_diff_image_crashed(self):
- port = ChromiumPortTestCase.TestLinuxPort()
- port._executive = MockExecutive2(exit_code=2)
- self.assertEquals(port.diff_image("EXPECTED", "ACTUAL"), (None, 0, 'image diff returned an exit code of 2'))
-
def test_expectations_files(self):
port = self.make_port()
port.port_name = 'chromium'
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/image_diff.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/image_diff.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/image_diff.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -62,7 +62,8 @@
len(expected_contents), expected_contents))
return self._read()
except IOError as exception:
- return (None, 0, "Failed to compute an image diff: %s" % str(exception))
+ _log.error("Failed to compute an image diff: %s" % str(exception))
+ return (True, 0)
def _start(self, tolerance):
command = [self._port._path_to_image_diff(), '--tolerance', str(tolerance)]
@@ -76,7 +77,7 @@
output = None
output_image = ""
- while not self._process.timed_out and not self._process.has_crashed():
+ while True:
output = self._process.read_stdout_line(deadline)
if self._process.timed_out or self._process.has_crashed() or not output:
break
@@ -92,14 +93,12 @@
break
stderr = self._process.pop_all_buffered_stderr()
- err_str = ''
if stderr:
- err_str += "ImageDiff produced stderr output:\n" + stderr
+ _log.warn("ImageDiff produced stderr output:\n" + stderr)
if self._process.timed_out:
- err_str += "ImageDiff timed out\n"
+ _log.error("ImageDiff timed out")
if self._process.has_crashed():
- err_str += "ImageDiff crashed\n"
-
+ _log.error("ImageDiff crashed")
# FIXME: There is no need to shut down the ImageDiff server after every diff.
self._process.stop()
@@ -110,7 +109,7 @@
return [None, 0]
diff_percent = float(m.group(1))
- return (output_image, diff_percent, err_str or None)
+ return (output_image, diff_percent)
def stop(self):
if self._process:
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/image_diff_unittest.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/image_diff_unittest.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/image_diff_unittest.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -49,4 +49,4 @@
def test_diff_image(self):
port = FakePort(['diff: 100% failed\n'])
image_differ = ImageDiffer(port)
- self.assertEquals(image_differ.diff_image('foo', 'bar', 0.1), ('', 100.0, None))
+ self.assertEquals(image_differ.diff_image('foo', 'bar', 0.1), ('', 100.0))
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -60,9 +60,6 @@
def test_diff_image(self):
pass
- def test_diff_image_crashed(self):
- pass
-
def test_uses_apache(self):
pass
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -259,32 +259,19 @@
port._server_process_constructor = make_proc
port.setup_test_run()
- self.assertEquals(port.diff_image('foo', 'bar'), ('', 100.0, None))
+ self.assertEquals(port.diff_image('foo', 'bar'), ('', 100.0))
self.assertEquals(self.proc.cmd[1:3], ["--tolerance", "0.1"])
- self.assertEquals(port.diff_image('foo', 'bar', None), ('', 100.0, None))
+ self.assertEquals(port.diff_image('foo', 'bar', None), ('', 100.0))
self.assertEquals(self.proc.cmd[1:3], ["--tolerance", "0.1"])
- self.assertEquals(port.diff_image('foo', 'bar', 0), ('', 100.0, None))
+ self.assertEquals(port.diff_image('foo', 'bar', 0), ('', 100.0))
self.assertEquals(self.proc.cmd[1:3], ["--tolerance", "0"])
port.clean_up_test_run()
self.assertTrue(self.proc.stopped)
self.assertEquals(port._image_differ, None)
- def test_diff_image_crashed(self):
- port = self.make_port()
- self.proc = None
-
- def make_proc(port, nm, cmd, env):
- self.proc = MockServerProcess(port, nm, cmd, env, crashed=True)
- return self.proc
-
- port._server_process_constructor = make_proc
- port.setup_test_run()
- self.assertEquals(port.diff_image('foo', 'bar'), ('', 0, 'ImageDiff crashed\n'))
- port.clean_up_test_run()
-
def test_check_wdiff(self):
port = self.make_port()
port.check_wdiff()
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process_mock.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process_mock.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process_mock.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -28,10 +28,10 @@
class MockServerProcess(object):
- def __init__(self, port_obj=None, name=None, cmd=None, env=None, universal_newlines=False, lines=None, crashed=False):
+ def __init__(self, port_obj=None, name=None, cmd=None, env=None, universal_newlines=False, lines=None):
self.timed_out = False
self.lines = lines or []
- self.crashed = crashed
+ self.crashed = False
self.writes = []
self.cmd = cmd
self.env = env
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -402,8 +402,8 @@
def diff_image(self, expected_contents, actual_contents, tolerance=None):
diffed = actual_contents != expected_contents
if diffed:
- return ("< %s\n---\n> %s\n" % (expected_contents, actual_contents), 1, None)
- return (None, 0, None)
+ return ["< %s\n---\n> %s\n" % (expected_contents, actual_contents), 1]
+ return (None, 0)
def layout_tests_dir(self):
return LAYOUT_TEST_DIR
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py (124853 => 124854)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2012-08-07 05:58:49 UTC (rev 124853)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2012-08-07 06:07:10 UTC (rev 124854)
@@ -783,7 +783,7 @@
class ImageDiffTestPort(TestPort):
def diff_image(self, expected_contents, actual_contents, tolerance=None):
self.tolerance_used_for_diff_image = self._options.tolerance
- return (True, 1, None)
+ return (True, 1)
def get_port_for_run(args):
options, parsed_args = run_webkit_tests.parse_args(args)
