Issue 551135: Cleanup the unittest helpers in load_log_unittest.h....

Issue 551135: Cleanup the unittest helpers in load_log_unittest.h.... (Closed)

Created:
10 years, 11 months ago by eroman

Modified:
9 years, 6 months ago

Reviewers:
willchan no longer on Chromium

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

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

Visibility:
Public.

Description

Cleanup the unittest helpers in load_log_unittest.h. Consolidates all the callers to using the testing::AssertionResult() flavor, and renames them to have "Event" in the name. This rename is in anticipation of adding testers for other entry types. BUG=NONE TEST=existing Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=37501

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rename overload to LogContainsEventAtTime #

Patch Set 3 : Sync and merge conflicts #

Created: 10 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+310 lines, -330 lines)			Patch
M	net/base/host_resolver_impl_unittest.cc	View	1 2	5 chunks	+19 lines, -25 lines	0 comments	Download
M	net/base/load_log_unittest.h	View	1 2	2 chunks	+47 lines, -28 lines	0 comments	Download
M	net/base/load_log_unittest.cc	View	1 2	5 chunks	+23 lines, -21 lines	0 comments	Download
M	net/flip/flip_network_transaction_unittest.cc	View	1 2	1 chunk	+2 lines, -3 lines	0 comments	Download
M	net/http/http_cache_unittest.cc	View	1 2	3 chunks	+32 lines, -32 lines	0 comments	Download
M	net/proxy/init_proxy_resolver_unittest.cc	View	1 2	3 chunks	+37 lines, -40 lines	0 comments	Download
M	net/proxy/proxy_resolver_v8_unittest.cc	View	1 2	1 chunk	+12 lines, -12 lines	0 comments	Download
M	net/proxy/proxy_service_unittest.cc	View	1 2	3 chunks	+14 lines, -13 lines	0 comments	Download
M	net/socket/client_socket_pool_base_unittest.cc	View	1 2	6 chunks	+61 lines, -61 lines	0 comments	Download
M	net/socket/socks5_client_socket_unittest.cc	View	1 2	5 chunks	+12 lines, -21 lines	0 comments	Download
M	net/socket/socks_client_socket_unittest.cc	View	1 2	7 chunks	+15 lines, -27 lines	0 comments	Download
M	net/socket/ssl_client_socket_unittest.cc	View	1 2	5 chunks	+18 lines, -18 lines	0 comments	Download
M	net/socket/tcp_client_socket_unittest.cc	View	1 2	1 chunk	+6 lines, -6 lines	0 comments	Download
M	net/socket_stream/socket_stream_unittest.cc	View	1 2	1 chunk	+4 lines, -7 lines	0 comments	Download
M	net/url_request/url_request_unittest.cc	View	1 2	1 chunk	+8 lines, -16 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

eroman

I wanted to do this cleanup before adding the unit-test you asked for.

10 years, 11 months ago (2010-01-23 03:43:23 UTC) #1

willchan no longer on Chromium

http://codereview.chromium.org/551135/diff/1/13 File net/base/load_log_unittest.h (right): http://codereview.chromium.org/551135/diff/1/13#newcode59 net/base/load_log_unittest.h:59: inline ::testing::AssertionResult LogContainsEvent( Overloads are generally disallowed by Google ...

10 years, 11 months ago (2010-01-27 19:51:15 UTC) #2

eroman

10 years, 11 months ago (2010-01-28 05:11:16 UTC) #3

willchan no longer on Chromium

10 years, 11 months ago (2010-01-28 23:10:35 UTC) #4

LGTM!

On 2010/01/28 05:11:16, eroman wrote:
> http://codereview.chromium.org/551135/diff/1/13
> File net/base/load_log_unittest.h (right):
> 
> http://codereview.chromium.org/551135/diff/1/13#newcode59
> net/base/load_log_unittest.h:59: inline ::testing::AssertionResult
> LogContainsEvent(
> On 2010/01/27 19:51:15, willchan wrote:
> > Overloads are generally disallowed by Google C++ style guide.  You should
> > probably rename this to LogContainsEventAtTime(), in which case you might
want
> > to make |expected_time| be the last argument.
> 
> Done. Renamed to  LogContainsEventAtTime().

Expand Messages | Collapse Messages