Changeset 56647 in webkit


Ignore:
Timestamp:
Mar 26, 2010 4:38:37 PM (14 years ago)
Author:
dpranke@chromium.org
Message:

2010-03-26 Dirk Pranke <​dpranke@chromium.org>

Reviewed by Eric Seidel.

Implement pixel tests (image diff) properly on the Mac port.

This change introduces a new "ServerPocess" class that can be used
to manage processes that the run-webkit-tests harness forks off and
expects to stay up for longer than a single request/response session.
Both DumpRenderTree and ImageDiff use this style of communication,
although the current code forks off a new ImageDiff for each diff
(We need to restructure other parts of the code to be able to do this
safely in a multi-threaded environment).

Also, now that the ServerProcess abstraction exists, we can probably
clean up and simplify some of the thread management logic in
test_shell_thread as well.

​https://bugs.webkit.org/show_bug.cgi?id=34826

  • Scripts/webkitpy/layout_tests/port/mac.py:
  • Scripts/webkitpy/layout_tests/port/server_process.py:
  • Scripts/webkitpy/layout_tests/test_types/image_diff.py:
Location:
trunk/WebKitTools
Files:
1 added
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r56645 r56647  
     12010-03-26  Dirk Pranke  <dpranke@chromium.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        Implement pixel tests (image diff) properly on the Mac port.
     6
     7        This change introduces a new "ServerPocess" class that can be used
     8        to manage processes that the run-webkit-tests harness forks off and
     9        expects to stay up for longer than a single request/response session.
     10        Both DumpRenderTree and ImageDiff use this style of communication,
     11        although the current code forks off a new ImageDiff for each diff
     12        (We need to restructure other parts of the code to be able to do this
     13        safely in a multi-threaded environment).
     14
     15        Also, now that the ServerProcess abstraction exists, we can probably
     16        clean up and simplify some of the thread management logic in
     17        test_shell_thread as well.
     18
     19        https://bugs.webkit.org/show_bug.cgi?id=34826
     20
     21        * Scripts/webkitpy/layout_tests/port/mac.py:
     22        * Scripts/webkitpy/layout_tests/port/server_process.py:
     23        * Scripts/webkitpy/layout_tests/test_types/image_diff.py:
     24
    1252010-03-26  Sergio Villar Senin  <svillar@igalia.com>
    226

  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/mac.py

    r56517 r56647  
    3030"""WebKit Mac implementation of the Port interface."""
    3131
    32 import fcntl
    3332import logging
    3433import os
    3534import pdb
    3635import platform
    37 import select
     36import re
     37import shutil
    3838import signal
    3939import subprocess
    … …  
    4343
    4444import base
     45import server_process
    4546
    4647import webkitpy
    … …  
    8687            return False
    8788
    88         # This should also validate that the ImageDiff path is valid
    89         # (once this script knows how to use ImageDiff).
    90         # https://bugs.webkit.org/show_bug.cgi?id=34826
     89        image_diff_path = self._path_to_image_diff()
     90        if not os.path.exists(image_diff_path):
     91            _log.error("ImageDiff was not found at %s" % image_diff_path)
     92            return False
     93
    9194        return True
     95
     96    def diff_image(self, expected_filename, actual_filename,
     97                   diff_filename=None):
     98        """Return True if the two files are different. Also write a delta
     99        image of the two images into |diff_filename| if it is not None."""
     100
     101        # Handle the case where the test didn't actually generate an image.
     102        actual_length = os.stat(actual_filename).st_size
     103        if actual_length == 0:
     104            if diff_filename:
     105                shutil.copyfile(actual_filename, expected_filename)
     106            return True
     107
     108        sp = self._diff_image_request(expected_filename, actual_filename)
     109        return self._diff_image_reply(sp, expected_filename, diff_filename)
     110
     111    def _diff_image_request(self, expected_filename, actual_filename):
     112        # FIXME: either expose the tolerance argument as a command-line
     113        # parameter, or make it go away and aways use exact matches.
     114        cmd = [self._path_to_image_diff(), '--tolerance', '0.1']
     115        sp = server_process.ServerProcess(self, 'ImageDiff', cmd)
     116
     117        actual_length = os.stat(actual_filename).st_size
     118        actual_file = open(actual_filename).read()
     119        expected_length = os.stat(expected_filename).st_size
     120        expected_file = open(expected_filename).read()
     121        sp.write('Content-Length: %d\n%sContent-Length: %d\n%s' %
     122                 (actual_length, actual_file, expected_length, expected_file))
     123
     124        return sp
     125
     126    def _diff_image_reply(self, sp, expected_filename, diff_filename):
     127        timeout = 2.0
     128        deadline = time.time() + timeout
     129        output = sp.read_line(timeout)
     130        while not sp.timed_out and not sp.crashed and output:
     131            if output.startswith('Content-Length'):
     132                m = re.match('Content-Length: (\d+)', output)
     133                content_length = int(m.group(1))
     134                timeout = deadline - time.time()
     135                output = sp.read(timeout, content_length)
     136                break
     137            elif output.startswith('diff'):
     138                break
     139            else:
     140                timeout = deadline - time.time()
     141                output = sp.read_line(deadline)
     142
     143        result = True
     144        if output.startswith('diff'):
     145            m = re.match('diff: (.+)% (passed|failed)', output)
     146            if m.group(2) == 'passed':
     147                result = False
     148        elif output and diff_filename:
     149            open(diff_filename, 'w').write(output)
     150        elif sp.timed_out:
     151            _log.error("ImageDiff timed out on %s" % expected_filename)
     152        elif sp.crashed:
     153            _log.error("ImageDiff crashed")
     154        sp.stop()
     155        return result
    92156
    93157    def num_cores(self):
    … …  
    254318
    255319    def _path_to_image_diff(self):
    256         return self._build_path('image_diff') # FIXME: This is wrong and should be "ImageDiff", but having the correct path causes other parts of the script to hang.
     320        return self._build_path('ImageDiff')
    257321
    258322    def _path_to_wdiff(self):
    … …  
    289353        self._port = port
    290354        self._driver_options = driver_options
    291         self._target = port._options.target
    292355        self._image_path = image_path
    293         self._stdout_fd = None
    294         self._cmd = None
    295         self._env = None
    296         self._proc = None
    297         self._read_buffer = ''
    298356
    299357        cmd = []
    … …  
    309367            # about it anyway.
    310368            cmd += self._options.wrapper.split()
    311         # FIXME: Using arch here masks any possible file-not-found errors from a non-existant driver executable.
     369
    312370        cmd += ['arch', '-i386', port._path_to_driver(), '-']
    313371
    314         # FIXME: This is a hack around our lack of ImageDiff support for now.
    315         if not self._port._options.no_pixel_tests:
    316             _log.warn("This port does not yet support pixel tests.")
    317             self._port._options.no_pixel_tests = True
    318             #cmd.append('--pixel-tests')
    319 
    320         #if driver_options:
    321         #    cmd += driver_options
     372        if image_path:
     373            cmd.append('--pixel-tests')
    322374        env = os.environ
    323375        env['DYLD_FRAMEWORK_PATH'] = self._port._build_path()
    324         self._cmd = cmd
    325         self._env = env
    326         self.restart()
     376        self._sproc = server_process.ServerProcess(self._port,
     377            "DumpRenderTree", cmd, env)
    327378
    328379    def poll(self):
    329         return self._proc.poll()
     380        return self._sproc.poll()
    330381
    331382    def restart(self):
    332         self.stop()
    333         # We need to pass close_fds=True to work around Python bug #2320
    334         # (otherwise we can hang when we kill test_shell when we are running
    335         # multiple threads). See http://bugs.python.org/issue2320 .
    336         self._proc = subprocess.Popen(self._cmd, stdin=subprocess.PIPE,
    337                                       stdout=subprocess.PIPE,
    338                                       stderr=subprocess.PIPE,
    339                                       env=self._env,
    340                                       close_fds=True)
     383        self._sproc.stop()
     384        self._sproc.start()
     385        return
    341386
    342387    def returncode(self):
    343         return self._proc.returncode
     388        return self._proc.returncode()
    344389
    345390    def run_test(self, uri, timeoutms, image_hash):
    346         output = []
    347         error = []
    348         image = ''
    349         crash = False
    350         timeout = False
    351         actual_uri = None
    352         actual_image_hash = None
    353 
    354391        if uri.startswith("file:///"):
    355392            cmd = uri[7:]
    … …  
    361398        cmd += "\n"
    362399
    363         self._proc.stdin.write(cmd)
    364         self._stdout_fd = self._proc.stdout.fileno()
    365         fl = fcntl.fcntl(self._stdout_fd, fcntl.F_GETFL)
    366         fcntl.fcntl(self._stdout_fd, fcntl.F_SETFL, fl | os.O_NONBLOCK)
    367 
    368         stop_time = time.time() + (int(timeoutms) / 1000.0)
    369         resp = ''
    370         (timeout, line) = self._read_line(timeout, stop_time)
    371         resp += line
     400        # pdb.set_trace()
     401        sp = self._sproc
     402        sp.write(cmd)
     403
    372404        have_seen_content_type = False
    373         while not timeout and line.rstrip() != "#EOF":
    374             # Make sure we haven't crashed.
    375             if line == '' and self.poll() is not None:
    376                 # This is hex code 0xc000001d, which is used for abrupt
    377                 # termination. This happens if we hit ctrl+c from the prompt
    378                 # and we happen to be waiting on the test_shell.
    379                 # sdoyon: Not sure for which OS and in what circumstances the
    380                 # above code is valid. What works for me under Linux to detect
    381                 # ctrl+c is for the subprocess returncode to be negative
    382                 # SIGINT. And that agrees with the subprocess documentation.
    383                 if (-1073741510 == self.returncode() or
    384                     - signal.SIGINT == self.returncode()):
    385                     raise KeyboardInterrupt
    386                 crash = True
    387                 break
    388 
    389             elif (line.startswith('Content-Type:') and not
    390                   have_seen_content_type):
     405        actual_image_hash = None
     406        output = ''
     407        image = ''
     408
     409        timeout = int(timeoutms) / 1000.0
     410        deadline = time.time() + timeout
     411        line = sp.read_line(timeout)
     412        while not sp.timed_out and not sp.crashed and line.rstrip() != "#EOF":
     413            if (line.startswith('Content-Type:') and not
     414                have_seen_content_type):
    391415                have_seen_content_type = True
    392                 pass
    393416            else:
    394                 output.append(line)
    395 
    396             (timeout, line) = self._read_line(timeout, stop_time)
    397             resp += line
     417                output += line
     418            line = sp.read_line(timeout)
     419            timeout = deadline - time.time()
    398420
    399421        # Now read a second block of text for the optional image data
    400         image_length = 0
    401         (timeout, line) = self._read_line(timeout, stop_time)
    402         resp += line
     422        remaining_length = -1
    403423        HASH_HEADER = 'ActualHash: '
    404424        LENGTH_HEADER = 'Content-Length: '
    405         while not timeout and not crash and line.rstrip() != "#EOF":
    406             if line == '' and self.poll() is not None:
    407                 if (-1073741510 == self.returncode() or
    408                     - signal.SIGINT == self.returncode()):
    409                     raise KeyboardInterrupt
    410                 crash = True
    411                 break
    412             elif line.startswith(HASH_HEADER):
     425        line = sp.read_line(timeout)
     426        while not sp.timed_out and not sp.crashed and line.rstrip() != "#EOF":
     427            if line.startswith(HASH_HEADER):
    413428                actual_image_hash = line[len(HASH_HEADER):].strip()
    414429            elif line.startswith('Content-Type:'):
    415430                pass
    416431            elif line.startswith(LENGTH_HEADER):
    417                 image_length = int(line[len(LENGTH_HEADER):])
    418             elif image_length:
    419                 image += line
    420 
    421             (timeout, line) = self._read_line(timeout, stop_time, image_length)
    422             resp += line
    423 
    424         if timeout:
    425             self.restart()
     432                timeout = deadline - time.time()
     433                content_length = int(line[len(LENGTH_HEADER):])
     434                image = sp.read(timeout, content_length)
     435            timeout = deadline - time.time()
     436            line = sp.read_line(timeout)
    426437
    427438        if self._image_path and len(self._image_path):
    … …  
    429440            image_file.write(image)
    430441            image_file.close()
    431 
    432         return (crash, timeout, actual_image_hash,
    433                 ''.join(output), ''.join(error))
     442        return (sp.crashed, sp.timed_out, actual_image_hash, output, sp.error)
    434443
    435444    def stop(self):
    436         if self._proc:
    437             self._proc.stdin.close()
    438             self._proc.stdout.close()
    439             if self._proc.stderr:
    440                 self._proc.stderr.close()
    441             if sys.platform not in ('win32', 'cygwin'):
    442                 # Closing stdin/stdout/stderr hangs sometimes on OS X,
    443                 # (see restart(), above), and anyway we don't want to hang
    444                 # the harness if test_shell is buggy, so we wait a couple
    445                 # seconds to give test_shell a chance to clean up, but then
    446                 # force-kill the process if necessary.
    447                 KILL_TIMEOUT = 3.0
    448                 timeout = time.time() + KILL_TIMEOUT
    449                 while self._proc.poll() is None and time.time() < timeout:
    450                     time.sleep(0.1)
    451                 if self._proc.poll() is None:
    452                     _log.warning('stopping test driver timed out, '
    453                                  'killing it')
    454                     null = open(os.devnull, "w")
    455                     subprocess.Popen(["kill", "-9",
    456                                      str(self._proc.pid)], stderr=null)
    457                     null.close()
    458 
    459     def _read_line(self, timeout, stop_time, image_length=0):
    460         now = time.time()
    461         read_fds = []
    462 
    463         # first check to see if we have a line already read or if we've
    464         # read the entire image
    465         if image_length and len(self._read_buffer) >= image_length:
    466             out = self._read_buffer[0:image_length]
    467             self._read_buffer = self._read_buffer[image_length:]
    468             return (timeout, out)
    469 
    470         idx = self._read_buffer.find('\n')
    471         if not image_length and idx != -1:
    472             out = self._read_buffer[0:idx + 1]
    473             self._read_buffer = self._read_buffer[idx + 1:]
    474             return (timeout, out)
    475 
    476         # If we've timed out, return just what we have, if anything
    477         if timeout or now >= stop_time:
    478             out = self._read_buffer
    479             self._read_buffer = ''
    480             return (True, out)
    481 
    482         (read_fds, write_fds, err_fds) = select.select(
    483             [self._stdout_fd], [], [], stop_time - now)
    484         try:
    485             if timeout or len(read_fds) == 1:
    486                 self._read_buffer += self._proc.stdout.read()
    487         except IOError, e:
    488             read = []
    489         return self._read_line(timeout, stop_time)
     445        if self._sproc:
     446            self._sproc.stop()
     447            self._sproc = None

  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/test_types/image_diff.py

    r55603 r56647  
    9191          filename: the name of the test
    9292          target: Debug or Release
     93        Returns True if the files are different, False if they match
    9394        """
    9495        diff_filename = self.output_filename(filename,
    … …  
    99100          self.FILENAME_SUFFIX_EXPECTED + '.png')
    100101
     102        result = True
    101103        try:
    102104            _compare_available = True
    … …  
    166168        self._copy_output_png(filename, expected_png_file, '-expected.png')
    167169
    168         # Even though we only use result in one codepath below but we
     170        # Even though we only use the result in one codepath below but we
    169171        # still need to call CreateImageDiff for other codepaths.
    170         result = self._create_image_diff(port, filename, target)
     172        images_are_different = self._create_image_diff(port, filename, target)
    171173        if expected_hash == '':
    172174            failures.append(test_failures.FailureMissingImageHash(self))
    173175        elif test_args.hash != expected_hash:
    174             # Hashes don't match, so see if the images match. If they do, then
    175             # the hash is wrong.
    176             if result == 0:
     176            if images_are_different:
     177                failures.append(test_failures.FailureImageHashMismatch(self))
     178            else:
    177179                failures.append(test_failures.FailureImageHashIncorrect(self))
    178             else:
    179                 failures.append(test_failures.FailureImageHashMismatch(self))
    180180
    181181        return failures
    … …  
    191191          False otherwise.
    192192        """
    193 
    194193        try:
    195             result = port.diff_image(file1, file2)
     194            return port.diff_image(file1, file2)
    196195        except ValueError, e:
    197196            return True
    198 
    199         return result == 1
Note: See TracChangeset for help on using the changeset viewer.