Changeset 58279 in webkit
- Timestamp:
- Apr 26, 2010 7:33:04 PM (14 years ago)
- Location:
- trunk/WebKitTools
- Files:
-
- 2 added
- 10 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/WebKitTools/ChangeLog
r58278 r58279 1 2010-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 1 61 2010-04-26 Adam Barth <abarth@webkit.org> 2 62 -
trunk/WebKitTools/Scripts/webkitpy/layout_tests/driver_test.py
r57466 r58279 43 43 # |image_path| is a path to the image capture from the driver. 44 44 image_path = 'image_result.png' 45 driver = port.start_driver(image_path, None) 45 driver = port.create_driver(image_path, None) 46 driver.start() 46 47 for t in tests: 47 48 uri = port.filename_to_uri(os.path.join(port.layout_tests_dir(), t)) … … 59 60 print '"""' 60 61 print 62 driver.stop() 61 63 62 64 -
trunk/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py
r58076 r58279 156 156 def run(self): 157 157 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() 159 160 start = time.time() 160 161 crash, timeout, actual_checksum, output, error = \ … … 448 449 """ 449 450 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() 452 453 453 454 def _kill_dump_render_tree(self): -
trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
r58071 r58279 35 35 import errno 36 36 import os 37 import shlex 37 38 import subprocess 38 39 import sys … … 50 51 51 52 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. 65 54 _wdiff_available = True 66 55 _pretty_patch_available = True … … 428 417 raise NotImplementedError('Port.show_html_results_file') 429 418 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') 433 423 434 424 def start_helper(self): … … 534 524 raise NotImplementedError('Port.version') 535 525 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>" % wdiff548 return result549 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 560 526 def wdiff_text(self, actual_filename, expected_filename): 561 527 """Returns a string of HTML indicating the word-level diff of the 562 528 contents of the two filenames. Returns an empty string if word-level 563 529 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? 565 539 global _wdiff_available 566 if not _wdiff_available: 567 return "" 540 result = '' 568 541 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>' 571 575 except OSError, e: 572 # If the system is missing wdiff stop trying.573 _wdiff_available = False574 return ''575 except ScriptError,e:576 # If wdiff failed to run for some reason, stop trying.577 _wdiff_available = False578 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 580 584 581 585 _pretty_patch_error_html = "Failed to run PrettyPatch, see error console." … … 589 593 command = ["ruby", "-I", pretty_patch_path, prettify_path, diff_path] 590 594 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) 592 598 except OSError, e: 593 599 # If the system is missing ruby log the error and stop trying. … … 721 727 raise NotImplementedError('Driver.run_test') 722 728 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 723 747 def poll(self): 724 748 """Returns None if the Driver is still running. Returns the returncode -
trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py
r58055 r58279 174 174 uri = self.filename_to_uri(results_filename) 175 175 if self._options.use_drt: 176 # FIXME: This should use User.open_url 176 177 webbrowser.open(uri, new=1) 177 178 else: 178 179 subprocess.Popen([self._path_to_driver(), uri]) 179 180 180 def start_driver(self, image_path, options):181 def create_driver(self, image_path, options): 181 182 """Starts a new Driver and returns a handle to it.""" 182 183 if self._options.use_drt: … … 275 276 def __init__(self, port, image_path, options): 276 277 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() 277 282 self._options = options 278 self._configuration = port._options.configuration279 283 self._image_path = image_path 280 284 285 def start(self): 286 # FIXME: Should be an error to call this method twice. 281 287 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 296 293 297 294 # We need to pass close_fds=True to work around Python bug #2320 … … 305 302 stderr=subprocess.STDOUT, 306 303 close_fds=close_flag) 304 307 305 def poll(self): 308 306 return self._proc.poll() … … 310 308 def returncode(self): 311 309 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 312 334 313 335 def run_test(self, uri, timeoutms, checksum): … … 320 342 321 343 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) 338 347 339 348 while not crash and line.rstrip() != "#EOF": … … 342 351 # This is hex code 0xc000001d, which is used for abrupt 343 352 # termination. This happens if we hit ctrl+c from the prompt 344 # and we happen to be waiting on t he DumpRenderTree.353 # and we happen to be waiting on test_shell. 345 354 # sdoyon: Not sure for which OS and in what circumstances the 346 355 # above code is valid. What works for me under Linux to detect … … 370 379 error.append(line) 371 380 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) 380 382 381 383 return (crash, timeout, actual_checksum, ''.join(output), … … 391 393 # Closing stdin/stdout/stderr hangs sometimes on OS X, 392 394 # (see __init__(), above), and anyway we don't want to hang 393 # the harness if DumpRenderTreeis buggy, so we wait a couple394 # seconds to give DumpRenderTreea chance to clean up, but then395 # 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 395 397 # force-kill the process if necessary. 396 398 KILL_TIMEOUT = 3.0 -
trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py
r57871 r58279 117 117 pass 118 118 119 def start_driver(self, image_path, options):119 def create_driver(self, image_path, options): 120 120 return DryrunDriver(self, image_path, options) 121 121 … … 154 154 return (False, False, hash, text_output, None) 155 155 156 def start(self): 157 pass 158 156 159 def stop(self): 157 160 pass -
trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py
r57399 r58279 82 82 pass 83 83 84 def start_driver(self, image_path, options):84 def create_driver(self, image_path, options): 85 85 return TestDriver(image_path, options, self) 86 86 … … 133 133 return (False, False, image_hash, '', None) 134 134 135 def start(self): 136 pass 137 135 138 def stop(self): 136 139 pass -
trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py
r58139 r58279 195 195 webbrowser.open(uri, new=1) 196 196 197 def start_driver(self, image_path, options):197 def create_driver(self, image_path, options): 198 198 return WebKitDriver(self, image_path, options) 199 199 … … 350 350 def __init__(self, port, image_path, driver_options): 351 351 self._port = port 352 self._driver_options = driver_options352 # FIXME: driver_options is never used. 353 353 self._image_path = image_path 354 354 355 def start(self): 355 356 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: 372 361 command.append('--pixel-tests') 373 362 environment = os.environ … … 403 392 have_seen_content_type = False 404 393 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() 407 396 408 397 timeout = int(timeoutms) / 1000.0 … … 416 405 have_seen_content_type = True 417 406 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 420 412 line = self._server_process.read_line(timeout) 421 413 timeout = deadline - time.time() -
trunk/WebKitTools/Scripts/webkitpy/layout_tests/test_types/test_type_base.py
r58050 r58279 154 154 raise NotImplemented 155 155 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): 157 157 """This method assumes that byte_array is already encoded 158 158 into the right format.""" … … 194 194 return 195 195 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. 196 199 diff = port.diff_text(expected, output, expected_filename, actual_filename) 197 200 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) 199 202 200 203 # Shell out to wdiff to get colored inline diffs. 201 204 wdiff = port.wdiff_text(expected_filename, actual_filename) 202 205 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) 204 207 205 208 # Use WebKit's PrettyPatch.rb to get an HTML diff. 206 209 pretty_patch = port.pretty_patch_text(diff_filename) 207 210 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 76 76 # FIXME: We repeat this pattern often, we should share code. 77 77 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: 79 82 text = file.read() 83 # We could assert that the text is valid utf-8. 80 84 except IOError, e: 81 85 if errno.ENOENT != e.errno: … … 93 97 # If we're generating a new baseline, we pass. 94 98 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) 96 103 return failures 97 104 … … 105 112 # Text doesn't match, write output files. 106 113 self.write_output_files(port, filename, ".txt", output, 107 expected, encoding= "utf-8",114 expected, encoding=None, 108 115 print_text_diffs=True) 109 116
Note: See TracChangeset
for help on using the changeset viewer.