Commit 774991a9 authored by eric@webkit.org's avatar eric@webkit.org
Browse files

2010-04-28 Eric Seidel <eric@webkit.org>

        Reviewed by Shinichiro Hamaji.

        wdiff_text throws ScriptError because wdiff returns non-zero when files differ
        https://bugs.webkit.org/show_bug.cgi?id=38246

        wdiff returns 0 when files are the same, 1 when they differ.
        run_command by default raises ScriptError if the return code is non-zero.
        Fixed this by adding a custom error handler which only raises if the
        return code is not 1.

        I broke up the huge wdiff_text() method into little pieces
        for easier unit testing.  There is only one functional change here
        and that is the addition of the custom error handler.

        * Scripts/webkitpy/layout_tests/port/base.py:
        * Scripts/webkitpy/layout_tests/port/base_unittest.py:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@58395 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent db7ccfd7
2010-04-28 Eric Seidel <eric@webkit.org>
Reviewed by Shinichiro Hamaji.
wdiff_text throws ScriptError because wdiff returns non-zero when files differ
https://bugs.webkit.org/show_bug.cgi?id=38246
wdiff returns 0 when files are the same, 1 when they differ.
run_command by default raises ScriptError if the return code is non-zero.
Fixed this by adding a custom error handler which only raises if the
return code is not 1.
I broke up the huge wdiff_text() method into little pieces
for easier unit testing. There is only one functional change here
and that is the addition of the custom error handler.
* Scripts/webkitpy/layout_tests/port/base.py:
* Scripts/webkitpy/layout_tests/port/base_unittest.py:
2010-04-28 Fumitoshi Ukai <ukai@chromium.org>
 
Unreviewed build fix.
......@@ -535,39 +535,63 @@ class Port(object):
expectations, determining search paths, and logging information."""
raise NotImplementedError('Port.version')
_WDIFF_DEL = '##WDIFF_DEL##'
_WDIFF_ADD = '##WDIFF_ADD##'
_WDIFF_END = '##WDIFF_END##'
def _format_wdiff_output_as_html(self, wdiff):
wdiff = cgi.escape(wdiff)
wdiff = wdiff.replace(self._WDIFF_DEL, "<span class=del>")
wdiff = wdiff.replace(self._WDIFF_ADD, "<span class=add>")
wdiff = wdiff.replace(self._WDIFF_END, "</span>")
html = "<head><style>.del { background: #faa; } "
html += ".add { background: #afa; }</style></head>"
html += "<pre>%s</pre>" % wdiff
return html
def _wdiff_command(self, actual_filename, expected_filename):
executable = self._path_to_wdiff()
return [executable,
"--start-delete=%s" % self._WDIFF_DEL,
"--end-delete=%s" % self._WDIFF_END,
"--start-insert=%s" % self._WDIFF_ADD,
"--end-insert=%s" % self._WDIFF_END,
actual_filename,
expected_filename]
@staticmethod
def _handle_wdiff_error(script_error):
# Exit 1 means the files differed, any other exit code is an error.
if script_error.exit_code != 1:
raise script_error
def _run_wdiff(self, actual_filename, expected_filename):
"""Runs wdiff and may throw exceptions.
This is mostly a hook for unit testing."""
# Diffs are treated as binary as they may include multiple files
# with conflicting encodings. Thus we do not decode the output.
command = self._wdiff_command(actual_filename, expected_filename)
wdiff = self._executive.run_command(command, decode_output=False,
error_handler=self._handle_wdiff_error)
return self._format_wdiff_output_as_html(wdiff)
def wdiff_text(self, actual_filename, expected_filename):
"""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."""
executable = self._path_to_wdiff()
cmd = [executable,
'--start-delete=##WDIFF_DEL##',
'--end-delete=##WDIFF_END##',
'--start-insert=##WDIFF_ADD##',
'--end-insert=##WDIFF_END##',
actual_filename,
expected_filename]
global _wdiff_available # See explaination at top of file.
result = ''
if not _wdiff_available:
return ""
try:
if _wdiff_available:
wdiff = self._executive.run_command(cmd, decode_output=False)
wdiff = cgi.escape(wdiff)
wdiff = wdiff.replace('##WDIFF_DEL##', '<span class=del>')
wdiff = wdiff.replace('##WDIFF_ADD##', '<span class=add>')
wdiff = wdiff.replace('##WDIFF_END##', '</span>')
result = '<head><style>.del { background: #faa; } '
result += '.add { background: #afa; }</style></head>'
result += '<pre>' + wdiff + '</pre>'
# It's possible to raise a ScriptError we pass wdiff invalid paths.
return self._run_wdiff(actual_filename, expected_filename)
except OSError, e:
if (e.errno == errno.ENOENT or e.errno == errno.EACCES or
e.errno == errno.ECHILD):
if e.errno in [errno.ENOENT, errno.EACCES, errno.ECHILD]:
# Silently ignore cases where wdiff is missing.
_wdiff_available = False
else:
raise e
# Diffs are treated as binary as they may include multiple files
# with conflicting encodings. Thus we do not decode the output here.
return result
return ""
raise
assert(False) # Should never be reached.
_pretty_patch_error_html = "Failed to run PrettyPatch, see error console."
......
......@@ -28,10 +28,88 @@
import base
import unittest
import tempfile
from webkitpy.common.system.executive import Executive, ScriptError
from webkitpy.thirdparty.mock import Mock
class PortTest(unittest.TestCase):
def test_format_wdiff_output_as_html(self):
output = "OUTPUT %s %s %s" % (base.Port._WDIFF_DEL, base.Port._WDIFF_ADD, base.Port._WDIFF_END)
html = base.Port()._format_wdiff_output_as_html(output)
expected_html = "<head><style>.del { background: #faa; } .add { background: #afa; }</style></head><pre>OUTPUT <span class=del> <span class=add> </span></pre>"
self.assertEqual(html, expected_html)
def test_wdiff_command(self):
port = base.Port()
port._path_to_wdiff = lambda: "/path/to/wdiff"
command = port._wdiff_command("/actual/path", "/expected/path")
expected_command = [
"/path/to/wdiff",
"--start-delete=##WDIFF_DEL##",
"--end-delete=##WDIFF_END##",
"--start-insert=##WDIFF_ADD##",
"--end-insert=##WDIFF_END##",
"/actual/path",
"/expected/path",
]
self.assertEqual(command, expected_command)
def _file_with_contents(self, contents, encoding="utf-8"):
new_file = tempfile.NamedTemporaryFile()
new_file.write(contents.encode(encoding))
new_file.flush()
return new_file
def test_run_wdiff(self):
executive = Executive()
# This may fail on some systems. We could ask the port
# object for the wdiff path, but since we don't know what
# port object to use, this is sufficient for now.
try:
wdiff_path = executive.run_command(["which", "wdiff"]).rstrip()
except Exception, e:
wdiff_path = None
port = base.Port()
port._path_to_wdiff = lambda: wdiff_path
if wdiff_path:
# "with tempfile.NamedTemporaryFile() as actual" does not seem to work in Python 2.5
actual = self._file_with_contents(u"foo")
expected = self._file_with_contents(u"bar")
wdiff = port._run_wdiff(actual.name, expected.name)
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.
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)
actual.close()
expected.close()
# Bogus paths should raise a script error.
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)
base._wdiff_available = True
# If wdiff does not exist _run_wdiff should throw an OSError.
port._path_to_wdiff = lambda: "/invalid/path/to/wdiff"
self.assertRaises(OSError, port._run_wdiff, "foo", "bar")
# 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
class DriverTest(unittest.TestCase):
def _assert_wrapper(self, wrapper_string, expected_wrapper):
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment