[webkit-reviews] review denied: [Bug 62251] webkitpy: add integration tests for new-run-webkit-httpd, stop calling shut_down_http_server : [Attachment 96519] update w/ review feedback

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 62251: webkitpy: add integration tests for new-run-webkit-httpd, stop
calling shut_down_http_server
https://bugs.webkit.org/show_bug.cgi?id=62251

Attachment 96519: update w/ review feedback
https://bugs.webkit.org/attachment.cgi?id=96519&action=review

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

> Tools/Scripts/webkitpy/layout_tests/port/http_server_integrationtest.py:1
> +# Copyright (C) 2010 Google Inc. All rights reserved.

2011

> Tools/Scripts/webkitpy/layout_tests/port/http_server_integrationtest.py:44
> +    HTTP_PORTS = [8000, 8080, 8443]

Nit: Use a tuple instead of a list.

> Tools/Scripts/webkitpy/layout_tests/port/http_server_integrationtest.py:73
> +    def integration_test_http_server__normal(self):
> +	   self.assert_servers_are_down('localhost', self.HTTP_PORTS)

Are these integration_tests only run when invoking the file directly?  Are they
run on the bots?  Is anyone working on the FIXME in port_testcase.py to merge
this into test-webkitpy?

> Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py:148
> +    def integration_test_lighttpd_server__normal(self):

We only use lighttpd on Chromium Win.  It looks like we only skip the test on
chromium mac.  Is that intended?  Maybe the lighttpd tests should just move
into chromium_win_unittest.py.

> Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py:189
> +    def integration_test_apache_server__fails(self):
> +	   port =
self.make_port(options=mocktool.MockOptions(configuration='Release',
use_apache=True))

Do the apache tests pass on chromium windows?