[webkit-reviews] review granted: [Bug 88134] new-run-webkit-tests should spin-up enough httpd processes to avoid timeouts : [Attachment 148155] add missing unittest file

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 88134: new-run-webkit-tests should spin-up enough httpd processes to avoid
timeouts
https://bugs.webkit.org/show_bug.cgi?id=88134

Attachment 148155: add missing unittest file
https://bugs.webkit.org/attachment.cgi?id=148155&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=148155&action=review


I was able to repro Ami's results on my machine. I think the key was to use a
debug build of DRT.

On an unrelated note, new-run-webkit-httpd is broken (it always uses lighttpd),
but that's a separate bug.

> Tools/Scripts/webkitpy/common/system/executive_mock.py:40
> -    def __init__(self, stdout='MOCK STDOUT\n'):
> +    def __init__(self, stdout='MOCK STDOUT\n', stderr=''):
>	   self.pid = 42
>	   self.stdout = StringIO.StringIO(stdout)
> +	   self.stderr = StringIO.StringIO(stderr)

Why is this change necessary?  Can you mention it in the ChangeLog?

> Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:763
> +	       self.start_servers_with_lock(num_servers=2 * min(num_workers,
len(locked_shards)))

Nit: It's weird to me to name the param when it's not an optional param.  It
you want to make it clear what this is for, you could make num_servers a local
var.

> Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:972
>	   self._printer.print_update('Starting HTTP server ...')
> -	   self._port.start_http_server()
> +	   self._port.start_http_server(num_servers=num_servers)
>	   self._printer.print_update('Starting WebSocket server ...')

You probably need to rebase.

> Tools/Scripts/webkitpy/layout_tests/port/base.py:790
> +    def start_http_server(self, additional_dirs=None, num_servers=None):

Nit: num_servers -> number_of_servers

> Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server_unittest.py:31

> +import unittest
> +import re
> +import sys

Nit: sort.

> Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server_unittest.py:35

> +from webkitpy.common.system.outputcapture import OutputCapture
> +from webkitpy.common.host_mock import MockHost

Nit: sort.