[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
dpranke at chromium.org
dpranke at chromium.org
Wed Dec 22 14:39:27 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit fe6f58635a90058e626eb6e2063ff01088f6731a
Author: dpranke at chromium.org <dpranke at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Thu Oct 14 23:22:53 2010 +0000
2010-10-14 Dirk Pranke <dpranke at chromium.org>
Reviewed by Eric Seidel.
new-run-webkit-tests will now handle missing Ruby installs (or
missing PrettyPatch scripts) more cleanly - previously this
would be detected when we actually tried to create the diff, and
the error message was obscure. Now we'll log a warning up front
and otherwise be silent.
This change also refactors some global variables to be class or
instance variables to be slightly more testable and more
modular. There are no cases where we create lots of port objects
and can't afford to test for configurations repeatedly, so
there's no performance concern here.
https://bugs.webkit.org/show_bug.cgi?id=47466
* Scripts/webkitpy/layout_tests/port/base.py:
* Scripts/webkitpy/layout_tests/port/base_unittest.py:
* Scripts/webkitpy/layout_tests/port/chromium.py:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@69820 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 1f42c86..03b8e6a 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,25 @@
+2010-10-14 Dirk Pranke <dpranke at chromium.org>
+
+ Reviewed by Eric Seidel.
+
+ new-run-webkit-tests will now handle missing Ruby installs (or
+ missing PrettyPatch scripts) more cleanly - previously this
+ would be detected when we actually tried to create the diff, and
+ the error message was obscure. Now we'll log a warning up front
+ and otherwise be silent.
+
+ This change also refactors some global variables to be class or
+ instance variables to be slightly more testable and more
+ modular. There are no cases where we create lots of port objects
+ and can't afford to test for configurations repeatedly, so
+ there's no performance concern here.
+
+ https://bugs.webkit.org/show_bug.cgi?id=47466
+
+ * Scripts/webkitpy/layout_tests/port/base.py:
+ * Scripts/webkitpy/layout_tests/port/base_unittest.py:
+ * Scripts/webkitpy/layout_tests/port/chromium.py:
+
2010-10-08 Martin Robinson <mrobinson at igalia.com>
Reviewed by Xan Lopez.
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
index 38b982b..b15edb2 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
@@ -55,22 +55,6 @@ from webkitpy.common.system.user import User
_log = logutils.get_logger(__file__)
-# Python's Popen has a bug that causes any pipes opened to a
-# process that can't be executed to be leaked. Since this
-# code is specifically designed to tolerate exec failures
-# to gracefully handle cases where wdiff is not installed,
-# the bug results in a massive file descriptor leak. As a
-# workaround, if an exec failure is ever experienced for
-# wdiff, assume it's not available. This will leak one
-# file descriptor but that's better than leaking each time
-# wdiff would be run.
-#
-# http://mail.python.org/pipermail/python-list/
-# 2008-August/505753.html
-# http://bugs.python.org/issue3210
-_wdiff_available = True
-_pretty_patch_available = True
-
# FIXME: This class should merge with webkitpy.webkit_port at some point.
class Port(object):
"""Abstract class for Port-specific hooks for the layout_test package.
@@ -95,6 +79,25 @@ class Port(object):
self._websocket_server = None
self._http_lock = None
+ # Python's Popen has a bug that causes any pipes opened to a
+ # process that can't be executed to be leaked. Since this
+ # code is specifically designed to tolerate exec failures
+ # to gracefully handle cases where wdiff is not installed,
+ # the bug results in a massive file descriptor leak. As a
+ # workaround, if an exec failure is ever experienced for
+ # wdiff, assume it's not available. This will leak one
+ # file descriptor but that's better than leaking each time
+ # wdiff would be run.
+ #
+ # http://mail.python.org/pipermail/python-list/
+ # 2008-August/505753.html
+ # http://bugs.python.org/issue3210
+ self._wdiff_available = True
+
+ self._pretty_patch_path = self.path_from_webkit_base("BugsSite",
+ "PrettyPatch", "prettify.rb")
+ self._pretty_patch_available = True
+
def default_child_processes(self):
"""Return the number of DumpRenderTree instances to use for this
port."""
@@ -127,6 +130,27 @@ class Port(object):
"""This routine is used to check whether image_diff binary exists."""
raise NotImplementedError('Port.check_image_diff')
+ def check_pretty_patch(self):
+ """Checks whether we can use the PrettyPatch ruby script."""
+
+ # check if Ruby is installed
+ try:
+ result = self._executive.run_command(['ruby', '--version'])
+ except OSError, e:
+ if e.errno in [errno.ENOENT, errno.EACCES, errno.ECHILD]:
+ _log.error("Ruby is not installed; "
+ "can't generate pretty patches.")
+ _log.error('')
+ return False
+
+ if not self.path_exists(self._pretty_patch_path):
+ _log.error('Unable to find %s .' % self._pretty_patch_path)
+ _log.error("Can't generate pretty patches.")
+ _log.error('')
+ return False
+
+ return True
+
def compare_text(self, expected_text, actual_text):
"""Return whether or not the two strings are *not* equal. This
routine is used to diff text output.
@@ -638,8 +662,7 @@ class Port(object):
"""Returns a string of HTML indicating the word-level diff of the
contents of the two filenames. Returns an empty string if word-level
diffing isn't available."""
- global _wdiff_available # See explaination at top of file.
- if not _wdiff_available:
+ if not self._wdiff_available:
return ""
try:
# It's possible to raise a ScriptError we pass wdiff invalid paths.
@@ -647,33 +670,33 @@ class Port(object):
except OSError, e:
if e.errno in [errno.ENOENT, errno.EACCES, errno.ECHILD]:
# Silently ignore cases where wdiff is missing.
- _wdiff_available = False
+ self._wdiff_available = False
return ""
raise
- _pretty_patch_error_html = "Failed to run PrettyPatch, see error console."
+ # This is a class variable so we can test error output easily.
+ _pretty_patch_error_html = "Failed to run PrettyPatch, see error log."
def pretty_patch_text(self, diff_path):
- # FIXME: Much of this function could move to prettypatch.rb
- global _pretty_patch_available
- if not _pretty_patch_available:
+ if not self._pretty_patch_available:
return self._pretty_patch_error_html
- pretty_patch_path = self.path_from_webkit_base("BugsSite", "PrettyPatch")
- prettify_path = os.path.join(pretty_patch_path, "prettify.rb")
- command = ["ruby", "-I", pretty_patch_path, prettify_path, diff_path]
+ command = ("ruby", "-I", os.path.dirname(self._pretty_patch_path),
+ self._pretty_patch_path, diff_path)
try:
# Diffs are treated as binary (we pass decode_output=False) as they
# may contain multiple files of conflicting encodings.
return self._executive.run_command(command, decode_output=False)
except OSError, e:
# If the system is missing ruby log the error and stop trying.
- _pretty_patch_available = False
+ self._pretty_patch_available = False
_log.error("Failed to run PrettyPatch (%s): %s" % (command, e))
return self._pretty_patch_error_html
except ScriptError, e:
- # If ruby failed to run for some reason, log the command output and stop trying.
- _pretty_patch_available = False
- _log.error("Failed to run PrettyPatch (%s):\n%s" % (command, e.message_with_output()))
+ # If ruby failed to run for some reason, log the command
+ # output and stop trying.
+ self._pretty_patch_available = False
+ _log.error("Failed to run PrettyPatch (%s):\n%s" % (command,
+ e.message_with_output()))
return self._pretty_patch_error_html
def _webkit_build_directory(self, args):
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py
index e66c64d..40eb984 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py
@@ -139,11 +139,11 @@ class PortTest(unittest.TestCase):
expected_wdiff = "<head><style>.del { background: #faa; } .add { background: #afa; }</style></head><pre><span class=del>foo</span><span class=add>bar</span></pre>"
self.assertEqual(wdiff, expected_wdiff)
# Running the full wdiff_text method should give the same result.
- base._wdiff_available = True # In case it's somehow already disabled.
+ port._wdiff_available = True # In case it's somehow already disabled.
wdiff = port.wdiff_text(actual.name, expected.name)
self.assertEqual(wdiff, expected_wdiff)
# wdiff should still be available after running wdiff_text with a valid diff.
- self.assertTrue(base._wdiff_available)
+ self.assertTrue(port._wdiff_available)
actual.close()
expected.close()
@@ -151,7 +151,7 @@ class PortTest(unittest.TestCase):
self.assertRaises(ScriptError, port._run_wdiff, "/does/not/exist", "/does/not/exist2")
self.assertRaises(ScriptError, port.wdiff_text, "/does/not/exist", "/does/not/exist2")
# wdiff will still be available after running wdiff_text with invalid paths.
- self.assertTrue(base._wdiff_available)
+ self.assertTrue(port._wdiff_available)
base._wdiff_available = True
# If wdiff does not exist _run_wdiff should throw an OSError.
@@ -161,8 +161,7 @@ class PortTest(unittest.TestCase):
# wdiff_text should not throw an error if wdiff does not exist.
self.assertEqual(port.wdiff_text("foo", "bar"), "")
# However wdiff should not be available after running wdiff_text if wdiff is missing.
- self.assertFalse(base._wdiff_available)
- base._wdiff_available = True
+ self.assertFalse(port._wdiff_available)
def test_diff_text(self):
port = base.Port()
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py
index 301d4b1..0fae62f 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py
@@ -115,6 +115,10 @@ class ChromiumPort(base.Port):
result = self.check_image_diff(
'To override, invoke with --no-pixel-tests') and result
+ # It's okay if pretty patch isn't available, but we will at
+ # least log a message.
+ self.check_pretty_patch()
+
return result
def check_sys_deps(self, needs_http):
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list