Bug 64489

Summary:

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

Product:

WebKit

Reporter:

WebKit Review Bot <webkit.review.bot>

Component:

Tools / Tests

Assignee:

Dirk Pranke <dpranke>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dpranke, eric, ojan, tony

Priority:

Keywords:

NRWT

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

64491

Attachments:

Description	Flags
Patch	none
do not start http server unless necessary, either	tony: review+

WebKit Review Bot

Reported 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.

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Dirk Pranke

Comment 1 2012-06-18 19:39:28 PDT

Created attachment 148228 [details] Patch

Eric Seidel (no email)

Comment 2 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?

Dirk Pranke

Comment 3 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.

Dirk Pranke

Comment 4 2012-06-19 10:54:52 PDT

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

Tony Chang

Comment 5 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).

Dirk Pranke

Comment 6 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.

Dirk Pranke

Comment 7 2012-06-19 21:09:26 PDT

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

Note You need to log in before you can comment on or make changes to this bug.