NRWT spins up and down the WebSocket server when running a single HTTP test from the command line

• Component: Tools / Tests
• Status: RESOLVED FIXED

[WebKit Review Bot]

NRWT spins up and down the WebSocket server when running a single HTTP test from the command line
Requested by abarth on #webkit.

[Dirk Pranke]

Created attachment 148228 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 148228 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148228&action=review

> Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:974
>          self._printer.print_update('Starting HTTP server ...')
>          self._port.start_http_server()

So we start the http server unconditionatlly?  Or do we somewhere check _http_tests before staring that?  I'm confused why the if checks aren't right next to each other?  (or is it that websocket tests are a subset of http tests and the http if check is outsidde this function?

[Dirk Pranke]

(In reply to comment #2)
> (From update of attachment 148228 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148228&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:974
> >          self._printer.print_update('Starting HTTP server ...')
> >          self._port.start_http_server()
> 
> So we start the http server unconditionatlly?  Or do we somewhere check _http_tests before staring that?  I'm confused why the if checks aren't right next to each other?  (or is it that websocket tests are a subset of http tests and the http if check is outsidde this function?

Good question. Historically, since the websocket tests also require http, if a test required a lock it had to mean that http was needed (so the check was outside the function). 

However, we now run perf tests under the lock as well, and so this doesn't have to be true, i.e., it's a bug. I'll add a check.

[Dirk Pranke]

Created attachment 148359 [details]
do not start http server unless necessary, either

[Tony Chang]

Comment on attachment 148359 [details]
do not start http server unless necessary, either

In manager.py, around line 763, we call start_servers_with_lock() if we have locked_shareds.  Should we change that condition? E.g., if I run only perf tests, we still acquire the http lock (although after this change, we will no longer start up the http servers).

[Dirk Pranke]

(In reply to comment #5)
> (From update of attachment 148359 [details])
> In manager.py, around line 763, we call start_servers_with_lock() if we have locked_shareds.  Should we change that condition? E.g., if I run only perf tests, we still acquire the http lock (although after this change, we will no longer start up the http servers).

Good point. I will update accordingly.

[Dirk Pranke]

Committed r120795: <http://trac.webkit.org/changeset/120795>