Diff
Modified: trunk/Tools/ChangeLog (159846 => 159847)
--- trunk/Tools/ChangeLog 2013-11-28 16:35:28 UTC (rev 159846)
+++ trunk/Tools/ChangeLog 2013-11-28 16:55:28 UTC (rev 159847)
@@ -1,3 +1,29 @@
+2013-11-28 Commit Queue <commit-qu...@webkit.org>
+
+ Unreviewed, rolling out r159839.
+ http://trac.webkit.org/changeset/159839
+ https://bugs.webkit.org/show_bug.cgi?id=124974
+
+ run-webkit-tests doesn't generate pretty diff (Requested by
+ Ossy on #webkit).
+
+ * Scripts/webkitpy/common/prettypatch.py:
+ (PrettyPatch.__init__):
+ (PrettyPatch.pretty_diff):
+ * Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:
+ (TestResultWriter.create_text_diff_and_write_result):
+ * Scripts/webkitpy/layout_tests/models/test_run_results.py:
+ (summarize_results):
+ * Scripts/webkitpy/port/base.py:
+ (Port.__init__):
+ (Port.pretty_patch_available):
+ (Port.check_pretty_patch):
+ (Port.variable):
+ (Port.variable.pretty_patch_text):
+ * Scripts/webkitpy/port/base_unittest.py:
+ (PortTest.test_pretty_patch_os_error):
+ (PortTest.test_pretty_patch_script_error):
+
2013-11-28 Antti Koivisto <an...@apple.com>
Remove feature: CSS variables
Modified: trunk/Tools/Scripts/webkitpy/common/prettypatch.py (159846 => 159847)
--- trunk/Tools/Scripts/webkitpy/common/prettypatch.py 2013-11-28 16:35:28 UTC (rev 159846)
+++ trunk/Tools/Scripts/webkitpy/common/prettypatch.py 2013-11-28 16:55:28 UTC (rev 159847)
@@ -26,25 +26,15 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-import errno
-import logging
import os
import tempfile
-from webkitpy.common.system.executive import ScriptError
-_log = logging.getLogger(__name__)
-
class PrettyPatch(object):
# FIXME: PrettyPatch should not require checkout_root.
- def __init__(self, executive, checkout_root, filesystem=None):
+ def __init__(self, executive, checkout_root):
self._executive = executive
- self._filesystem = filesystem
self._checkout_root = checkout_root
- self._pretty_patch_path = os.path.join(self._checkout_root, "Websites", "bugs.webkit.org", "PrettyPatch")
- self._prettify_path = os.path.join(self._pretty_patch_path, "prettify.rb")
- self.pretty_patch_error_html = "Failed to run PrettyPatch, see error log."
- self.ppatch_available = None
def pretty_diff_file(self, diff):
# Diffs can contain multiple text files of different encodings
@@ -62,59 +52,16 @@
if not diff:
return ""
+ pretty_patch_path = os.path.join(self._checkout_root,
+ "Websites", "bugs.webkit.org",
+ "PrettyPatch")
+ prettify_path = os.path.join(pretty_patch_path, "prettify.rb")
args = [
"ruby",
"-I",
- self._pretty_patch_path,
- self._prettify_path,
+ pretty_patch_path,
+ prettify_path,
]
# PrettyPatch does not modify the encoding of the diff output
# so we can't expect it to be utf-8.
return self._executive.run_command(args, input=diff, decode_output=False)
-
- def pretty_patch_available(self):
- if self.ppatch_available is None:
- self.ppatch_available = self.check_pretty_patch(logging=False)
- return self.ppatch_available
-
- def check_pretty_patch(self, logging=True):
- """Checks whether we can use the PrettyPatch ruby script."""
- try:
- _ = self._executive.run_command(['ruby', '--version'])
- except OSError, e:
- if e.errno in [errno.ENOENT, errno.EACCES, errno.ECHILD]:
- if logging:
- _log.warning("Ruby is not installed; can't generate pretty patches.")
- _log.warning('')
- return False
-
- if not self._filesystem.exists(self._pretty_patch_path):
- if logging:
- _log.warning("Unable to find %s; can't generate pretty patches." % self._pretty_patch_path)
- _log.warning('')
- return False
-
- return True
-
- def pretty_patch_text(self, diff_path):
- if self.ppatch_available is None:
- self.ppatch_available = self.check_pretty_patch(logging=False)
- if not self.ppatch_available:
- return self.pretty_patch_error_html
- command = ("ruby", "-I", self._filesystem.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.
- self.ppatch_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.
- self.ppatch_available = False
- _log.error("Failed to run PrettyPatch (%s):\n%s" % (command, e.message_with_output()))
- return self.pretty_patch_error_html
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py (159846 => 159847)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py 2013-11-28 16:35:28 UTC (rev 159846)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py 2013-11-28 16:55:28 UTC (rev 159847)
@@ -196,8 +196,8 @@
self._write_binary_file(wdiff_filename, wdiff)
# Use WebKit's PrettyPatch.rb to get an HTML diff.
- if self._port.pretty_patch.pretty_patch_available():
- pretty_patch = self._port.pretty_patch.pretty_patch_text(diff_filename)
+ if self._port.pretty_patch_available():
+ pretty_patch = self._port.pretty_patch_text(diff_filename)
pretty_patch_filename = self.output_filename(self.FILENAME_SUFFIX_PRETTY_PATCH)
self._write_binary_file(pretty_patch_filename, pretty_patch)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py (159846 => 159847)
--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py 2013-11-28 16:35:28 UTC (rev 159846)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py 2013-11-28 16:55:28 UTC (rev 159847)
@@ -243,7 +243,7 @@
results['interrupted'] = initial_results.interrupted # Does results.html have enough information to compute this itself? (by checking total number of results vs. total number of tests?)
results['layout_tests_dir'] = port_obj.layout_tests_dir()
results['has_wdiff'] = port_obj.wdiff_available()
- results['has_pretty_patch'] = port_obj.pretty_patch.pretty_patch_available()
+ results['has_pretty_patch'] = port_obj.pretty_patch_available()
results['pixel_tests_enabled'] = port_obj.get_option('pixel_tests')
try:
Modified: trunk/Tools/Scripts/webkitpy/port/base.py (159846 => 159847)
--- trunk/Tools/Scripts/webkitpy/port/base.py 2013-11-28 16:35:28 UTC (rev 159846)
+++ trunk/Tools/Scripts/webkitpy/port/base.py 2013-11-28 16:55:28 UTC (rev 159847)
@@ -51,7 +51,6 @@
from webkitpy.common import find_files
from webkitpy.common import read_checksum_from_png
from webkitpy.common.memoized import memoized
-from webkitpy.common.prettypatch import PrettyPatch
from webkitpy.common.system import path
from webkitpy.common.system.executive import ScriptError
from webkitpy.common.system.systemhost import SystemHost
@@ -117,7 +116,6 @@
self._filesystem = host.filesystem
self._webkit_finder = WebKitFinder(host.filesystem)
self._config = port_config.Config(self._executive, self._filesystem, self.port_name)
- self.pretty_patch = PrettyPatch(self._executive, self.path_from_webkit_base(), self._filesystem)
self._helper = None
self._http_server = None
@@ -141,6 +139,10 @@
# http://bugs.python.org/issue3210
self._wdiff_available = None
+ # FIXME: prettypatch.py knows this path, why is it copied here?
+ self._pretty_patch_path = self.path_from_webkit_base("Websites", "bugs.webkit.org", "PrettyPatch", "prettify.rb")
+ self._pretty_patch_available = None
+
if not hasattr(options, 'configuration') or not options.configuration:
self.set_option_default('configuration', self.default_configuration())
self._test_configuration = None
@@ -176,6 +178,11 @@
self._wdiff_available = self.check_wdiff(logging=False)
return self._wdiff_available
+ def pretty_patch_available(self):
+ if self._pretty_patch_available is None:
+ self._pretty_patch_available = self.check_pretty_patch(logging=False)
+ return self._pretty_patch_available
+
def should_retry_crashes(self):
return False
@@ -274,6 +281,25 @@
return False
return True
+ def check_pretty_patch(self, logging=True):
+ """Checks whether we can use the PrettyPatch ruby script."""
+ try:
+ _ = self._executive.run_command(['ruby', '--version'])
+ except OSError, e:
+ if e.errno in [errno.ENOENT, errno.EACCES, errno.ECHILD]:
+ if logging:
+ _log.warning("Ruby is not installed; can't generate pretty patches.")
+ _log.warning('')
+ return False
+
+ if not self._filesystem.exists(self._pretty_patch_path):
+ if logging:
+ _log.warning("Unable to find %s; can't generate pretty patches." % self._pretty_patch_path)
+ _log.warning('')
+ return False
+
+ return True
+
def check_wdiff(self, logging=True):
if not self._path_to_wdiff():
# Don't need to log here since this is the port choosing not to use wdiff.
@@ -1128,6 +1154,32 @@
return ""
raise
+ # 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):
+ if self._pretty_patch_available is None:
+ self._pretty_patch_available = self.check_pretty_patch(logging=False)
+ if not self._pretty_patch_available:
+ return self._pretty_patch_error_html
+ command = ("ruby", "-I", self._filesystem.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.
+ 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.
+ 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 default_configuration(self):
return self._config.default_configuration()
Modified: trunk/Tools/Scripts/webkitpy/port/base_unittest.py (159846 => 159847)
--- trunk/Tools/Scripts/webkitpy/port/base_unittest.py 2013-11-28 16:35:28 UTC (rev 159846)
+++ trunk/Tools/Scripts/webkitpy/port/base_unittest.py 2013-11-28 16:55:28 UTC (rev 159847)
@@ -90,24 +90,24 @@
port = self.make_port(executive=executive_mock.MockExecutive2(exception=OSError))
oc = OutputCapture()
oc.capture_output()
- self.assertEqual(port.pretty_patch.pretty_patch_text("patch.txt"),
- port.pretty_patch.pretty_patch_error_html)
+ self.assertEqual(port.pretty_patch_text("patch.txt"),
+ port._pretty_patch_error_html)
# This tests repeated calls to make sure we cache the result.
- self.assertEqual(port.pretty_patch.pretty_patch_text("patch.txt"),
- port.pretty_patch.pretty_patch_error_html)
+ self.assertEqual(port.pretty_patch_text("patch.txt"),
+ port._pretty_patch_error_html)
oc.restore_output()
def test_pretty_patch_script_error(self):
# FIXME: This is some ugly white-box test hacking ...
port = self.make_port(executive=executive_mock.MockExecutive2(exception=ScriptError))
- port.pretty_patch.ppatch_available = True
- self.assertEqual(port.pretty_patch.pretty_patch_text("patch.txt"),
- port.pretty_patch.pretty_patch_error_html)
+ port._pretty_patch_available = True
+ self.assertEqual(port.pretty_patch_text("patch.txt"),
+ port._pretty_patch_error_html)
# This tests repeated calls to make sure we cache the result.
- self.assertEqual(port.pretty_patch.pretty_patch_text("patch.txt"),
- port.pretty_patch.pretty_patch_error_html)
+ self.assertEqual(port.pretty_patch_text("patch.txt"),
+ port._pretty_patch_error_html)
def integration_test_run_wdiff(self):
executive = Executive()
