Issue 320003: Refactor: Don't send the quit command before killing test server for ProxyScr...

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 320003: Refactor: Don't send the quit command before killing test server for ProxyScr... (Closed)

Created:
11 years, 2 months ago by eroman

Modified:
9 years, 6 months ago

Reviewers:
wtc

CC:
chromium-reviews_googlegroups.com, darin (slow to review), Paweł Hajdan Jr.

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Refactor: Don't send the quit command before killing test server for ProxyScriptFetcherTest.NoCache. Instead we just murder the process directly. Calling HttpTestServer::SendQuit() adds unnecessary latency since we need to wait for the sever to respond. But more importantly it is adding noise to my valgrind race investigation, since SendQuit() spins up its own IO thread to issue requests on (making the test have multiple threads that handle requests). Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=29826

Patch Set 1 #

Total comments: 1

Created: 11 years, 2 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+1 line, -2 lines)			Patch
	M	net/proxy/proxy_script_fetcher_unittest.cc	View		1 chunk	+1 line, -2 lines	1 comment	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

wtc

11 years, 2 months ago (2009-10-22 21:38:58 UTC) #2

LGTM.

http://codereview.chromium.org/320003/diff/1/2
File net/proxy/proxy_script_fetcher_unittest.cc (right):

http://codereview.chromium.org/320003/diff/1/2#newcode196
Line 196: EXPECT_TRUE(server->Stop());  // Verify it shutdown synchronously.
Nit: shutdown => shut down (past tense?)

Expand Messages | Collapse Messages