Log Message
webkitpy: Pass ImageDiff commands in a single write https://bugs.webkit.org/show_bug.cgi?id=206194 <rdar://problem/58578775>
Reviewed by Stephanie Lewis. ImageDiff can encounter race conditions if it is fed content in multiple writes, instead of in a single block. * Scripts/webkitpy/port/image_diff.py: (ImageDiffer.diff_image): Pass ImageDiff data in a single write command. * Scripts/webkitpy/port/port_testcase.py: (PortTestCase.test_diff_image__missing_both): diff_image only accepts byte arrays. (PortTestCase.test_diff_image__missing_actual): Ditto. (PortTestCase.test_diff_image__missing_expected): Ditto. (PortTestCase.test_diff_image): Ditto. (PortTestCase.test_diff_image_passed): Ditto. (PortTestCase.test_diff_image_failed): Ditto. (PortTestCase.test_diff_image_crashed): Ditto.
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (254531 => 254532)
--- trunk/Tools/ChangeLog 2020-01-14 21:24:01 UTC (rev 254531)
+++ trunk/Tools/ChangeLog 2020-01-14 21:32:11 UTC (rev 254532)
@@ -1,3 +1,25 @@
+2020-01-14 Jonathan Bedard <jbed...@apple.com>
+
+ webkitpy: Pass ImageDiff commands in a single write
+ https://bugs.webkit.org/show_bug.cgi?id=206194
+ <rdar://problem/58578775>
+
+ Reviewed by Stephanie Lewis.
+
+ ImageDiff can encounter race conditions if it is fed content in multiple writes,
+ instead of in a single block.
+
+ * Scripts/webkitpy/port/image_diff.py:
+ (ImageDiffer.diff_image): Pass ImageDiff data in a single write command.
+ * Scripts/webkitpy/port/port_testcase.py:
+ (PortTestCase.test_diff_image__missing_both): diff_image only accepts byte arrays.
+ (PortTestCase.test_diff_image__missing_actual): Ditto.
+ (PortTestCase.test_diff_image__missing_expected): Ditto.
+ (PortTestCase.test_diff_image): Ditto.
+ (PortTestCase.test_diff_image_passed): Ditto.
+ (PortTestCase.test_diff_image_failed): Ditto.
+ (PortTestCase.test_diff_image_crashed): Ditto.
+
2020-01-14 Antti Koivisto <an...@apple.com>
[LFC][Integration] Support the feature flag in DumpRenderTree
Modified: trunk/Tools/Scripts/webkitpy/port/image_diff.py (254531 => 254532)
--- trunk/Tools/Scripts/webkitpy/port/image_diff.py 2020-01-14 21:24:01 UTC (rev 254531)
+++ trunk/Tools/Scripts/webkitpy/port/image_diff.py 2020-01-14 21:32:11 UTC (rev 254532)
@@ -35,7 +35,7 @@
import time
from webkitpy.port import server_process
-from webkitpy.common.unicode_compatibility import decode_for
+from webkitpy.common.unicode_compatibility import BytesIO, decode_for, encode_if_necessary
_log = logging.getLogger(__name__)
@@ -58,10 +58,12 @@
if not self._process:
self._start(tolerance)
# Note that although we are handed 'old', 'new', ImageDiff wants 'new', 'old'.
- self._process.write('Content-Length: {}\n'.format(len(actual_contents)))
- self._process.write(actual_contents)
- self._process.write('Content-Length: {}\n'.format(len(expected_contents)))
- self._process.write(expected_contents)
+ buffer = BytesIO()
+ buffer.write(encode_if_necessary('Content-Length: {}\n'.format(len(actual_contents))))
+ buffer.write(actual_contents)
+ buffer.write(encode_if_necessary('Content-Length: {}\n'.format(len(expected_contents))))
+ buffer.write(expected_contents)
+ self._process.write(buffer.getvalue())
return self._read()
except IOError as exception:
return (None, 0, "Failed to compute an image diff: %s" % str(exception))
Modified: trunk/Tools/Scripts/webkitpy/port/port_testcase.py (254531 => 254532)
--- trunk/Tools/Scripts/webkitpy/port/port_testcase.py 2020-01-14 21:24:01 UTC (rev 254531)
+++ trunk/Tools/Scripts/webkitpy/port/port_testcase.py 2020-01-14 21:32:11 UTC (rev 254532)
@@ -259,20 +259,20 @@
def test_diff_image__missing_both(self):
port = self.make_port()
self.assertFalse(port.diff_image(None, None)[0])
- self.assertFalse(port.diff_image(None, '')[0])
- self.assertFalse(port.diff_image('', None)[0])
+ self.assertFalse(port.diff_image(None, b'')[0])
+ self.assertFalse(port.diff_image(b'', None)[0])
- self.assertFalse(port.diff_image('', '')[0])
+ self.assertFalse(port.diff_image(b'', b'')[0])
def test_diff_image__missing_actual(self):
port = self.make_port()
- self.assertTrue(port.diff_image(None, 'foo')[0])
- self.assertTrue(port.diff_image('', 'foo')[0])
+ self.assertTrue(port.diff_image(None, b'foo')[0])
+ self.assertTrue(port.diff_image(b'', b'foo')[0])
def test_diff_image__missing_expected(self):
port = self.make_port()
- self.assertTrue(port.diff_image('foo', None)[0])
- self.assertTrue(port.diff_image('foo', '')[0])
+ self.assertTrue(port.diff_image(b'foo', None)[0])
+ self.assertTrue(port.diff_image(b'foo', b'')[0])
def test_diff_image(self):
port = self.make_port()
@@ -292,11 +292,11 @@
# First test the case of not using the JHBuild wrapper.
self.assertFalse(port._should_use_jhbuild())
- self.assertEqual(port.diff_image('foo', 'bar'), (b'', 100.0, None))
+ self.assertEqual(port.diff_image(b'foo', b'bar'), (b'', 100.0, None))
self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0.1"])
- self.assertEqual(port.diff_image('foo', 'bar', None), (b'', 100.0, None))
+ self.assertEqual(port.diff_image(b'foo', b'bar', None), (b'', 100.0, None))
self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0.1"])
- self.assertEqual(port.diff_image('foo', 'bar', 0), (b'', 100.0, None))
+ self.assertEqual(port.diff_image(b'foo', b'bar', 0), (b'', 100.0, None))
self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0"])
# Now test the case of using JHBuild wrapper.
@@ -303,11 +303,11 @@
port._filesystem.maybe_make_directory(port.path_from_webkit_base('WebKitBuild', 'Dependencies%s' % port.port_name.upper()))
self.assertTrue(port._should_use_jhbuild())
- self.assertEqual(port.diff_image('foo', 'bar'), (b'', 100.0, None))
+ self.assertEqual(port.diff_image(b'foo', b'bar'), (b'', 100.0, None))
self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0.1"])
- self.assertEqual(port.diff_image('foo', 'bar', None), (b'', 100.0, None))
+ self.assertEqual(port.diff_image(b'foo', b'bar', None), (b'', 100.0, None))
self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0.1"])
- self.assertEqual(port.diff_image('foo', 'bar', 0), (b'', 100.0, None))
+ self.assertEqual(port.diff_image(b'foo', b'bar', 0), (b'', 100.0, None))
self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0"])
port.clean_up_test_run()
@@ -318,13 +318,13 @@
port = self.make_port()
port._server_process_constructor = lambda port, nm, cmd, env: MockServerProcess(lines=['diff: 0% passed\n'])
image_differ = ImageDiffer(port)
- self.assertEqual(image_differ.diff_image('foo', 'bar', 0.1), (None, 0, None))
+ self.assertEqual(image_differ.diff_image(b'foo', b'bar', 0.1), (None, 0, None))
def test_diff_image_failed(self):
port = self.make_port()
port._server_process_constructor = lambda port, nm, cmd, env: MockServerProcess(lines=['diff: 100% failed\n'])
image_differ = ImageDiffer(port)
- self.assertEqual(image_differ.diff_image('foo', 'bar', 0.1), (b'', 100.0, None))
+ self.assertEqual(image_differ.diff_image(b'foo', b'bar', 0.1), (b'', 100.0, None))
def test_diff_image_crashed(self):
port = self.make_port()
@@ -340,7 +340,7 @@
port._server_process_constructor = make_proc
port.setup_test_run()
- self.assertEqual(port.diff_image('foo', 'bar'), (b'', 0, 'ImageDiff crashed\n'))
+ self.assertEqual(port.diff_image(b'foo', b'bar'), (b'', 0, 'ImageDiff crashed\n'))
port.clean_up_test_run()
def integration_test_websocket_server__normal(self):
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
