[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 #70429|review?                     |review+
               Flag|                            |




--- Comment #9 from Tony Chang <tony at chromium.org>  2010-10-11 14:49:48 PST ---
(From update of attachment 70429)
View in context: https://bugs.webkit.org/attachment.cgi?id=70429&action=review

> WebKitTools/Scripts/webkitpy/layout_tests/port/base.py:48
>  import apache_http_server
>  import test_files
> +import http_lock
>  import http_server
>  import websocket_server

Nit: sort these alphabetically

> WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock.py:113
> +                self._process_lock_file_name = (self._lock_file_path_prefix +
> +                                          str(self._next_lock_number()))

Nit: The indention looks weird here.  Either line it up with the ( in the previous line or just indent 4 spaces.

> WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock_unittest.py:53
> +        if first != second:
> +            self.clean_all_lockfile()

This seems ok, although you could also use try/finally in each test.  I don't really care either way.

> WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock_unittest.py:83
> +        lock_file_list = [
> +            self.lock_file_path_prefix + "00",
> +            self.lock_file_path_prefix + "9",
> +            self.lock_file_path_prefix + "001",
> +            self.lock_file_path_prefix + "021",
> +        ]

Nit: A tuple is a bit faster than using a list.  It also prevents unintended modification.

> WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock_unittest.py:85
> +        expected_number_list = ["0", "9", "1", "21"]

Nit: tuple, and why not drop the "" and just make these ints (so you and avoid the cast below).

> WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock_unittest.py:89
> +        for i in range(len(lock_file_list)):
> +            self.assertEqual(self.http_lock_obj._extract_lock_number(lock_file_list[i]),
> +                             int(expected_number_list[i]))

Nit: You could re-write this as:
for lock_file, expected in zip(lock_file_list, expected_number_list):
    self.assertEqual(self.http_lock_obj._extract_lock_number(lock_file), int(expected_number_list))

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