[Webkit-unassigned] [Bug 47072] Implement http locking in NRWT

[bugzilla-daemon at webkit.org]

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


Tony Chang <tony at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #69679|review?                     |review-
               Flag|                            |




--- Comment #7 from Tony Chang <tony at chromium.org>  2010-10-04 15:27:56 PST ---
(From update of attachment 69679)
View in context: https://bugs.webkit.org/attachment.cgi?id=69679&action=review

r- mainly because there are no tests

> WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:1609
> +        optparse.make_option("--no-wait-for-httpd", action="store_false",
> +            dest="wait_for_httpd", help="Do not use http locks."),

I realize you're just trying to match ORWT, but do you think we could get by without --no-wait-for-httpd ?

> WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:413
> +                    elif self._http_server_running:
> +                        self._clean_up_http_lock()

Why was this added?  Doesn't this say other threads should stop the http server?

> WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:560
> +    def _clean_up_http_lock(self):

Do we need to call _clean_up_http_lock when the "test_to_http_lock" are done running?

> WebKitTools/Scripts/webkitpy/layout_tests/layout_package/http_lock.py:38
> +class HttpLock(object):
> +

I agree with Dirk that this file needs unittests and should include a class docstring.

> WebKitTools/Scripts/webkitpy/layout_tests/layout_package/http_lock.py:98
> +                self._process_lock_file_name = self._lock_file_path_prefix + \
> +                                          str(self._next_lock_number())

Nit: () instead of \.  Also the indention looks a bit off.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.