Bug 64489 - NRWT spins up and down the WebSocket server when running a single HTTP test from the command line
Summary: NRWT spins up and down the WebSocket server when running a single HTTP test f...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords: NRWT
Depends on:
Blocks: 64491
  Show dependency treegraph
 
Reported: 2011-07-13 15:50 PDT by WebKit Review Bot
Modified: 2012-06-19 21:09 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.26 KB, patch)
2012-06-18 19:39 PDT, Dirk Pranke
no flags Details | Formatted Diff | Diff
do not start http server unless necessary, either (6.25 KB, patch)
2012-06-19 10:54 PDT, Dirk Pranke
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description WebKit Review Bot 2011-07-13 15:50:47 PDT
NRWT spins up and down the WebSocket server when running a single HTTP test from the command line
Requested by abarth on #webkit.
Comment 1 Dirk Pranke 2012-06-18 19:39:28 PDT
Created attachment 148228 [details]
Patch
Comment 2 Eric Seidel (no email) 2012-06-19 09:06:17 PDT
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?
Comment 3 Dirk Pranke 2012-06-19 09:12:33 PDT
(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.
Comment 4 Dirk Pranke 2012-06-19 10:54:52 PDT
Created attachment 148359 [details]
do not start http server unless necessary, either
Comment 5 Tony Chang 2012-06-19 15:56:00 PDT
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).
Comment 6 Dirk Pranke 2012-06-19 16:20:23 PDT
(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.
Comment 7 Dirk Pranke 2012-06-19 21:09:26 PDT
Committed r120795: <http://trac.webkit.org/changeset/120795>