Changeset 58279 in webkit


Ignore:
Timestamp:
Apr 26, 2010 7:33:04 PM (14 years ago)
Author:
eric@webkit.org
Message:

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

Reviewed by Adam Barth.

[chromium] new-run-webkit-tests hangs on Chromium Bots (OS X and Linux)
https://bugs.webkit.org/show_bug.cgi?id=37987

Rolled out:
http://trac.webkit.org/changeset/58062
http://trac.webkit.org/changeset/58060
http://trac.webkit.org/changeset/58059
http://trac.webkit.org/changeset/58055
http://trac.webkit.org/changeset/58054
and parts of:
http://trac.webkit.org/changeset/58050

I also wrote some new comments and a tiny amount of new
code to help make ChromiumDriver.run_test easier to read.

In order to unit-test my new code, I had to change ChromiumDriver
to not automatically start itself when created. That ended up
being a lot of plumbing, but is hopefully easier to understand now.

There are no tests for the (restored) wdiff code. wdiff does not
exist on all systems, so for now we will assume it worked since
it is just old code being reverted.

  • Scripts/webkitpy/layout_tests/driver_test.py:
    • Use create_driver instead of start_driver, and be sure to call .stop()
  • Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
    • Use create_driver instead of start_driver
  • Scripts/webkitpy/layout_tests/port/base.py:
    • Added a comment to explain that diffs are binary files.
    • Various patch reverts relating to wdiff
    • Add Driver._command_wrapper to share code between WebKitDriver and ChromiumDriver.
    • Made _command_wrapper use shlex.split to get rid of the FIXME.
  • Scripts/webkitpy/layout_tests/port/base_unittest.py: Added.
    • test the new _command_wrapper
  • Scripts/webkitpy/layout_tests/port/chromium.py:
  • Use _command_wrapper to get rid of a bunch of ugly code.
  • Make init stop auto-starting.
  • Rename create_driver to start_driver.
  • Added _write_command_and_read_line to make it possible to put a FIXME next to read_line() w/o having to put it in two places.
  • Moved test_shell command building into _test_shell_command and tested it.
  • Fix comments to say test_shell since ChromiumDriver is test_shell only.
  • Scripts/webkitpy/layout_tests/port/chromium_unittest.py: Added.
    • Test the new test_shell_command method.
  • Scripts/webkitpy/layout_tests/port/dryrun.py:
    • Rename create_driver to start_driver.
  • Scripts/webkitpy/layout_tests/port/test.py:
    • Rename create_driver to start_driver.
  • Scripts/webkitpy/layout_tests/port/webkit.py:
    • Rename create_driver to start_driver.
    • Treat output as binary arrays.
  • Scripts/webkitpy/layout_tests/test_types/test_type_base.py:
    • Treat diff files as binary.
  • Scripts/webkitpy/layout_tests/test_types/text_diff.py:
    • Treat diff files as binary.
Location:
trunk/WebKitTools
Files:
2 added
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/WebKitTools/ChangeLog

    r58278 r58279  
     12010-04-26  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        [chromium] new-run-webkit-tests hangs on Chromium Bots (OS X and Linux)
     6        https://bugs.webkit.org/show_bug.cgi?id=37987
     7
     8        Rolled out:
     9        http://trac.webkit.org/changeset/58062
     10        http://trac.webkit.org/changeset/58060
     11        http://trac.webkit.org/changeset/58059
     12        http://trac.webkit.org/changeset/58055
     13        http://trac.webkit.org/changeset/58054
     14        and parts of:
     15        http://trac.webkit.org/changeset/58050
     16
     17        I also wrote some new comments and a tiny amount of new
     18        code to help make ChromiumDriver.run_test easier to read.
     19
     20        In order to unit-test my new code, I had to change ChromiumDriver
     21        to not automatically start itself when created.  That ended up
     22        being a lot of plumbing, but is hopefully easier to understand now.
     23
     24        There are no tests for the (restored) wdiff code.  wdiff does not
     25        exist on all systems, so for now we will assume it worked since
     26        it is just old code being reverted.
     27
     28         * Scripts/webkitpy/layout_tests/driver_test.py:
     29          - Use create_driver instead of start_driver, and be sure to call .stop()
     30         * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
     31          - Use create_driver instead of start_driver
     32         * Scripts/webkitpy/layout_tests/port/base.py:
     33          - Added a comment to explain that diffs are binary files.
     34          - Various patch reverts relating to wdiff
     35          - Add Driver._command_wrapper to share code between WebKitDriver and ChromiumDriver.
     36          - Made _command_wrapper use shlex.split to get rid of the FIXME.
     37         * Scripts/webkitpy/layout_tests/port/base_unittest.py: Added.
     38          - test the new _command_wrapper
     39         * Scripts/webkitpy/layout_tests/port/chromium.py:
     40         - Use _command_wrapper to get rid of a bunch of ugly code.
     41         - Make __init__ stop auto-starting.
     42         - Rename create_driver to start_driver.
     43         - Added _write_command_and_read_line to make it possible to
     44           put a FIXME next to read_line() w/o having to put it in two places.
     45         - Moved test_shell command building into _test_shell_command and tested it.
     46         - Fix comments to say test_shell since ChromiumDriver is test_shell only.
     47         * Scripts/webkitpy/layout_tests/port/chromium_unittest.py: Added.
     48          - Test the new test_shell_command method.
     49         * Scripts/webkitpy/layout_tests/port/dryrun.py:
     50          - Rename create_driver to start_driver.
     51         * Scripts/webkitpy/layout_tests/port/test.py:
     52          - Rename create_driver to start_driver.
     53         * Scripts/webkitpy/layout_tests/port/webkit.py:
     54          - Rename create_driver to start_driver.
     55          - Treat output as binary arrays.
     56         * Scripts/webkitpy/layout_tests/test_types/test_type_base.py:
     57          - Treat diff files as binary.
     58         * Scripts/webkitpy/layout_tests/test_types/text_diff.py:
     59          - Treat diff files as binary.
     60
    1612010-04-26  Adam Barth  <abarth@webkit.org>
    262
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/driver_test.py

    r57466 r58279  
    4343    # |image_path| is a path to the image capture from the driver.
    4444    image_path = 'image_result.png'
    45     driver = port.start_driver(image_path, None)
     45    driver = port.create_driver(image_path, None)
     46    driver.start()
    4647    for t in tests:
    4748        uri = port.filename_to_uri(os.path.join(port.layout_tests_dir(), t))
     
    5960        print '"""'
    6061        print
     62    driver.stop()
    6163
    6264
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py

    r58076 r58279  
    156156    def run(self):
    157157        test_info = self._test_info
    158         driver = self._port.start_driver(self._image_path, self._shell_args)
     158        driver = self._port.create_driver(self._image_path, self._shell_args)
     159        driver.start()
    159160        start = time.time()
    160161        crash, timeout, actual_checksum, output, error = \
     
    448449        """
    449450        if (not self._driver or self._driver.poll() is not None):
    450             self._driver = self._port.start_driver(
    451                 self._image_path, self._shell_args)
     451            self._driver = self._port.create_driver(self._image_path, self._shell_args)
     452            self._driver.start()
    452453
    453454    def _kill_dump_render_tree(self):
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py

    r58071 r58279  
    3535import errno
    3636import os
     37import shlex
    3738import subprocess
    3839import sys
     
    5051
    5152
    52 # Python's Popen has a bug that causes any pipes opened to a
    53 # process that can't be executed to be leaked.  Since this
    54 # code is specifically designed to tolerate exec failures
    55 # to gracefully handle cases where wdiff is not installed,
    56 # the bug results in a massive file descriptor leak. As a
    57 # workaround, if an exec failure is ever experienced for
    58 # wdiff, assume it's not available.  This will leak one
    59 # file descriptor but that's better than leaking each time
    60 # wdiff would be run.
    61 #
    62 # http://mail.python.org/pipermail/python-list/
    63 #    2008-August/505753.html
    64 # http://bugs.python.org/issue3210
     53# Python bug workaround.  See Port.wdiff_text() for an explanation.
    6554_wdiff_available = True
    6655_pretty_patch_available = True
     
    428417        raise NotImplementedError('Port.show_html_results_file')
    429418
    430     def start_driver(self, png_path, options):
    431         """Starts a new test Driver and returns a handle to the object."""
    432         raise NotImplementedError('Port.start_driver')
     419    def create_driver(self, png_path, options):
     420        """Return a newly created base.Driver subclass for starting/stopping
     421        the test driver."""
     422        raise NotImplementedError('Port.create_driver')
    433423
    434424    def start_helper(self):
     
    534524        raise NotImplementedError('Port.version')
    535525
    536     _WDIFF_DEL = '##WDIFF_DEL##'
    537     _WDIFF_ADD = '##WDIFF_ADD##'
    538     _WDIFF_END = '##WDIFF_END##'
    539 
    540     def _format_wdiff_output_as_html(self, wdiff):
    541         wdiff = cgi.escape(wdiff)
    542         wdiff = wdiff.replace(self._WDIFF_DEL, '<span class=del>')
    543         wdiff = wdiff.replace(self._WDIFF_ADD, '<span class=add>')
    544         wdiff = wdiff.replace(self._WDIFF_END, '</span>')
    545         html = '<head><style>.del { background: #faa; } '
    546         html += '.add { background: #afa; }</style></head>'
    547         html += "<pre>%s</pre>" % wdiff
    548         return result
    549 
    550     def _wdiff_command(self, actual_filename, expected_filename):
    551         executable = self._path_to_wdiff()
    552         return [executable,
    553                 "--start-delete=%s" % self._WDIFF_DEL,
    554                 "--end-delete=%s" % self._WDIFF_END,
    555                 "--start-insert=%s" % self._WDIFF_ADD,
    556                 "--end-insert=%s" % self._WDIFF_END,
    557                 actual_filename,
    558                 expected_filename]
    559 
    560526    def wdiff_text(self, actual_filename, expected_filename):
    561527        """Returns a string of HTML indicating the word-level diff of the
    562528        contents of the two filenames. Returns an empty string if word-level
    563529        diffing isn't available."""
    564 
     530        executable = self._path_to_wdiff()
     531        cmd = [executable,
     532               '--start-delete=##WDIFF_DEL##',
     533               '--end-delete=##WDIFF_END##',
     534               '--start-insert=##WDIFF_ADD##',
     535               '--end-insert=##WDIFF_END##',
     536               actual_filename,
     537               expected_filename]
     538        # FIXME: Why not just check os.exists(executable) once?
    565539        global _wdiff_available
    566         if not _wdiff_available:
    567             return ""
     540        result = ''
    568541        try:
    569             cmd = self._wdiff_command(actual_filename, expected_filename)
    570             return self._executive.run_command(cmd)
     542            # Python's Popen has a bug that causes any pipes opened to a
     543            # process that can't be executed to be leaked.  Since this
     544            # code is specifically designed to tolerate exec failures
     545            # to gracefully handle cases where wdiff is not installed,
     546            # the bug results in a massive file descriptor leak. As a
     547            # workaround, if an exec failure is ever experienced for
     548            # wdiff, assume it's not available.  This will leak one
     549            # file descriptor but that's better than leaking each time
     550            # wdiff would be run.
     551            #
     552            # http://mail.python.org/pipermail/python-list/
     553            #    2008-August/505753.html
     554            # http://bugs.python.org/issue3210
     555            #
     556            # It also has a threading bug, so we don't output wdiff if
     557            # the Popen raises a ValueError.
     558            # http://bugs.python.org/issue1236
     559            if _wdiff_available:
     560                try:
     561                    # FIXME: Use Executive() here.
     562                    wdiff = subprocess.Popen(cmd,
     563                        stdout=subprocess.PIPE).communicate()[0]
     564                except ValueError, e:
     565                    # Working around a race in Python 2.4's implementation
     566                    # of Popen().
     567                    wdiff = ''
     568                wdiff = cgi.escape(wdiff)
     569                wdiff = wdiff.replace('##WDIFF_DEL##', '<span class=del>')
     570                wdiff = wdiff.replace('##WDIFF_ADD##', '<span class=add>')
     571                wdiff = wdiff.replace('##WDIFF_END##', '</span>')
     572                result = '<head><style>.del { background: #faa; } '
     573                result += '.add { background: #afa; }</style></head>'
     574                result += '<pre>' + wdiff + '</pre>'
    571575        except OSError, e:
    572             # If the system is missing wdiff stop trying.
    573             _wdiff_available = False
    574             return ''
    575         except ScriptError, e:
    576             # If wdiff failed to run for some reason, stop trying.
    577             _wdiff_available = False
    578             return ""
    579 
     576            if (e.errno == errno.ENOENT or e.errno == errno.EACCES or
     577                e.errno == errno.ECHILD):
     578                _wdiff_available = False
     579            else:
     580                raise e
     581        # Diffs are treated as binary as they may include multiple files
     582        # with conflicting encodings.  Thus we do not decode the output here.
     583        return result
    580584
    581585    _pretty_patch_error_html = "Failed to run PrettyPatch, see error console."
     
    589593        command = ["ruby", "-I", pretty_patch_path, prettify_path, diff_path]
    590594        try:
    591             return self._executive.run_command(command)
     595            # Diffs are treated as binary (we pass decode_output=False) as they
     596            # may contain multiple files of conflicting encodings.
     597            return self._executive.run_command(command, decode_output=False)
    592598        except OSError, e:
    593599            # If the system is missing ruby log the error and stop trying.
     
    721727        raise NotImplementedError('Driver.run_test')
    722728
     729    # FIXME: This is static so we can test it w/o creating a Base instance.
     730    @classmethod
     731    def _command_wrapper(cls, wrapper_option):
     732        # Hook for injecting valgrind or other runtime instrumentation,
     733        # used by e.g. tools/valgrind/valgrind_tests.py.
     734        wrapper = []
     735        browser_wrapper = os.environ.get("BROWSER_WRAPPER", None)
     736        if browser_wrapper:
     737            # FIXME: There seems to be no reason to use BROWSER_WRAPPER over --wrapper.
     738            # Remove this code any time after the date listed below.
     739            _log.error("BROWSER_WRAPPER is deprecated, please use --wrapper instead.")
     740            _log.error("BROWSER_WRAPPER will be removed any time after June 1st 2010 and your scripts will break.")
     741            wrapper += [browser_wrapper]
     742
     743        if wrapper_option:
     744            wrapper += shlex.split(wrapper_option)
     745        return wrapper
     746
    723747    def poll(self):
    724748        """Returns None if the Driver is still running. Returns the returncode
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py

    r58055 r58279  
    174174        uri = self.filename_to_uri(results_filename)
    175175        if self._options.use_drt:
     176            # FIXME: This should use User.open_url
    176177            webbrowser.open(uri, new=1)
    177178        else:
    178179            subprocess.Popen([self._path_to_driver(), uri])
    179180
    180     def start_driver(self, image_path, options):
     181    def create_driver(self, image_path, options):
    181182        """Starts a new Driver and returns a handle to it."""
    182183        if self._options.use_drt:
     
    275276    def __init__(self, port, image_path, options):
    276277        self._port = port
     278        self._configuration = port._options.configuration
     279        # FIXME: _options is very confusing, because it's not an Options() element.
     280        # FIXME: These don't need to be passed into the constructor, but could rather
     281        # be passed into .start()
    277282        self._options = options
    278         self._configuration = port._options.configuration
    279283        self._image_path = image_path
    280284
     285    def start(self):
     286        # FIXME: Should be an error to call this method twice.
    281287        cmd = []
    282         # Hook for injecting valgrind or other runtime instrumentation,
    283         # used by e.g. tools/valgrind/valgrind_tests.py.
    284         wrapper = os.environ.get("BROWSER_WRAPPER", None)
    285         if wrapper != None:
    286             cmd += [wrapper]
    287         if self._port._options.wrapper:
    288             # This split() isn't really what we want -- it incorrectly will
    289             # split quoted strings within the wrapper argument -- but in
    290             # practice it shouldn't come up and the --help output warns
    291             # about it anyway.
    292             cmd += self._options.wrapper.split()
    293         cmd += [port._path_to_driver(), '--layout-tests']
    294         if options:
    295             cmd += options
     288        # FIXME: We should not be grabbing at self._port._options.wrapper directly.
     289        cmd += self._command_wrapper(self._port._options.wrapper)
     290        cmd += [self._port._path_to_driver(), '--layout-tests']
     291        if self._options:
     292            cmd += self._options
    296293
    297294        # We need to pass close_fds=True to work around Python bug #2320
     
    305302                                      stderr=subprocess.STDOUT,
    306303                                      close_fds=close_flag)
     304
    307305    def poll(self):
    308306        return self._proc.poll()
     
    310308    def returncode(self):
    311309        return self._proc.returncode
     310
     311    def _write_command_and_read_line(self, input=None):
     312        """Returns a tuple: (line, did_crash)"""
     313        try:
     314            if input:
     315                self._proc.stdin.write(input)
     316            # DumpRenderTree text output is always UTF-8.  However some tests
     317            # (e.g. webarchive) may spit out binary data instead of text so we
     318            # don't bother to decode the output (for either DRT or test_shell).
     319            line = self._proc.stdout.readline()
     320            # We could assert() here that line correctly decodes as UTF-8.
     321            return (line, False)
     322        except IOError, e:
     323            _log.error("IOError communicating w/ test_shell: " + str(e))
     324            return (None, True)
     325
     326    def _test_shell_command(self, uri, timeoutms, checksum):
     327        cmd = uri
     328        if timeoutms:
     329            cmd += ' ' + str(timeoutms)
     330        if checksum:
     331            cmd += ' ' + checksum
     332        cmd += "\n"
     333        return cmd
    312334
    313335    def run_test(self, uri, timeoutms, checksum):
     
    320342
    321343        start_time = time.time()
    322         cmd = uri
    323         if timeoutms:
    324             cmd += ' ' + str(timeoutms)
    325         if checksum:
    326             cmd += ' ' + checksum
    327         cmd += "\n"
    328 
    329         try:
    330             self._proc.stdin.write(cmd)
    331             line = self._proc.stdout.readline()
    332             # As far as I can tell, all output from test_shell
    333             # is text output, thus we know it's all utf-8.
    334             line = line.decode("utf-8")
    335         except IOError, e:
    336             _log.error("IOError communicating w/ test_shell: " + str(e))
    337             crash = True
     344
     345        cmd = self._test_shell_command(uri, timeoutms, checksum)
     346        (line, crash) = self._write_command_and_read_line(input=cmd)
    338347
    339348        while not crash and line.rstrip() != "#EOF":
     
    342351                # This is hex code 0xc000001d, which is used for abrupt
    343352                # termination. This happens if we hit ctrl+c from the prompt
    344                 # and we happen to be waiting on the DumpRenderTree.
     353                # and we happen to be waiting on test_shell.
    345354                # sdoyon: Not sure for which OS and in what circumstances the
    346355                # above code is valid. What works for me under Linux to detect
     
    370379                error.append(line)
    371380
    372             try:
    373                 line = self._proc.stdout.readline()
    374                 # As far as I can tell, all output from test_shell
    375                 # is text output, thus we know it's all utf-8.
    376                 line = line.decode("utf-8")
    377             except IOError, e:
    378                 _log.error("IOError while reading: " + str(e))
    379                 crash = True
     381            (line, crash) = self._write_command_and_read_line(input=None)
    380382
    381383        return (crash, timeout, actual_checksum, ''.join(output),
     
    391393                # Closing stdin/stdout/stderr hangs sometimes on OS X,
    392394                # (see __init__(), above), and anyway we don't want to hang
    393                 # the harness if DumpRenderTree is buggy, so we wait a couple
    394                 # seconds to give DumpRenderTree a chance to clean up, but then
     395                # the harness if test_shell is buggy, so we wait a couple
     396                # seconds to give test_shell a chance to clean up, but then
    395397                # force-kill the process if necessary.
    396398                KILL_TIMEOUT = 3.0
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py

    r57871 r58279  
    117117        pass
    118118
    119     def start_driver(self, image_path, options):
     119    def create_driver(self, image_path, options):
    120120        return DryrunDriver(self, image_path, options)
    121121
     
    154154        return (False, False, hash, text_output, None)
    155155
     156    def start(self):
     157        pass
     158
    156159    def stop(self):
    157160        pass
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py

    r57399 r58279  
    8282        pass
    8383
    84     def start_driver(self, image_path, options):
     84    def create_driver(self, image_path, options):
    8585        return TestDriver(image_path, options, self)
    8686
     
    133133        return (False, False, image_hash, '', None)
    134134
     135    def start(self):
     136        pass
     137
    135138    def stop(self):
    136139        pass
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py

    r58139 r58279  
    195195        webbrowser.open(uri, new=1)
    196196
    197     def start_driver(self, image_path, options):
     197    def create_driver(self, image_path, options):
    198198        return WebKitDriver(self, image_path, options)
    199199
     
    350350    def __init__(self, port, image_path, driver_options):
    351351        self._port = port
    352         self._driver_options = driver_options
     352        # FIXME: driver_options is never used.
    353353        self._image_path = image_path
    354354
     355    def start(self):
    355356        command = []
    356         # Hook for injecting valgrind or other runtime instrumentation,
    357         # used by e.g. tools/valgrind/valgrind_tests.py.
    358         wrapper = os.environ.get("BROWSER_WRAPPER", None)
    359         if wrapper != None:
    360             command += [wrapper]
    361         if self._port._options.wrapper:
    362             # This split() isn't really what we want -- it incorrectly will
    363             # split quoted strings within the wrapper argument -- but in
    364             # practice it shouldn't come up and the --help output warns
    365             # about it anyway.
    366             # FIXME: Use a real shell parser.
    367             command += self._options.wrapper.split()
    368 
    369         command += [port._path_to_driver(), '-']
    370 
    371         if image_path:
     357        # FIXME: We should not be grabbing at self._port._options.wrapper directly.
     358        command += self._command_wrapper(self._port._options.wrapper)
     359        command += [self._port._path_to_driver(), '-']
     360        if self._image_path:
    372361            command.append('--pixel-tests')
    373362        environment = os.environ
     
    403392        have_seen_content_type = False
    404393        actual_image_hash = None
    405         output = u''
    406         image = ''
     394        output = str()  # Use a byte array for output, even though it should be UTF-8.
     395        image = str()
    407396
    408397        timeout = int(timeoutms) / 1000.0
     
    416405                have_seen_content_type = True
    417406            else:
    418                 # We expect the text content from DumpRenderTree to be UTF-8
    419                 output += line.decode("utf-8")
     407                # Note: Text output from DumpRenderTree is always UTF-8.
     408                # However, some tests (e.g. webarchives) spit out binary
     409                # data instead of text.  So to make things simple, we
     410                # always treat the output as binary.
     411                output += line
    420412            line = self._server_process.read_line(timeout)
    421413            timeout = deadline - time.time()
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/test_types/test_type_base.py

    r58050 r58279  
    154154        raise NotImplemented
    155155
    156     def _write_into_file_at_path(self, file_path, contents, encoding="utf-8"):
     156    def _write_into_file_at_path(self, file_path, contents, encoding):
    157157        """This method assumes that byte_array is already encoded
    158158        into the right format."""
     
    194194            return
    195195
     196        # Note: We pass encoding=None for all diff writes, as we treat diff
     197        # output as binary.  Diff output may contain multiple files in
     198        # conflicting encodings.
    196199        diff = port.diff_text(expected, output, expected_filename, actual_filename)
    197200        diff_filename = self.output_filename(filename, self.FILENAME_SUFFIX_DIFF + file_type)
    198         self._write_into_file_at_path(diff_filename, diff, "utf-8")
     201        self._write_into_file_at_path(diff_filename, diff, encoding=None)
    199202
    200203        # Shell out to wdiff to get colored inline diffs.
    201204        wdiff = port.wdiff_text(expected_filename, actual_filename)
    202205        wdiff_filename = self.output_filename(filename, self.FILENAME_SUFFIX_WDIFF)
    203         self._write_into_file_at_path(wdiff_filename, wdiff, "utf-8")
     206        self._write_into_file_at_path(wdiff_filename, wdiff, encoding=None)
    204207
    205208        # Use WebKit's PrettyPatch.rb to get an HTML diff.
    206209        pretty_patch = port.pretty_patch_text(diff_filename)
    207210        pretty_patch_filename = self.output_filename(filename, self.FILENAME_SUFFIX_PRETTY_PATCH)
    208         self._write_into_file_at_path(pretty_patch_filename, pretty_patch, "utf-8")
     211        self._write_into_file_at_path(pretty_patch_filename, pretty_patch, encoding=None)
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/test_types/text_diff.py

    r58050 r58279  
    7676        # FIXME: We repeat this pattern often, we should share code.
    7777        try:
    78             with codecs.open(filename, "r", "utf-8") as file:
     78            # NOTE: -expected.txt files are ALWAYS utf-8.  However,
     79            # we do not decode the output from DRT, so we should not
     80            # decode the -expected.txt values either to allow comparisons.
     81            with codecs.open(filename, "r", encoding=None) as file:
    7982                text = file.read()
     83                # We could assert that the text is valid utf-8.
    8084        except IOError, e:
    8185            if errno.ENOENT != e.errno:
     
    9397        # If we're generating a new baseline, we pass.
    9498        if test_args.new_baseline:
    95             self._save_baseline_data(filename, output, ".txt", encoding="utf-8")
     99            # Although all test_shell/DumpRenderTree output should be utf-8,
     100            # we do not ever decode it inside run-webkit-tests.  For some tests
     101            # DumpRenderTree may not output utf-8 text (e.g. webarchives).
     102            self._save_baseline_data(filename, output, ".txt", encoding=None)
    96103            return failures
    97104
     
    105112            # Text doesn't match, write output files.
    106113            self.write_output_files(port, filename, ".txt", output,
    107                                     expected, encoding="utf-8",
     114                                    expected, encoding=None,
    108115                                    print_text_diffs=True)
    109116
Note: See TracChangeset for help on using the changeset viewer.