Issue 8418037: chromeos: Fix UpdateScreenTest and NetworkScreenTest for Debug build.

Issue 8418037: chromeos: Fix UpdateScreenTest and NetworkScreenTest for Debug build. (Closed)

Created:
9 years, 1 month ago by satorux1

Modified:
9 years, 1 month ago

Reviewers:
oshima

CC:
chromium-reviews, stevenjb+watch_chromium.org, nkostylev+watch_chromium.org, davemoore+watch_chromium.org, Paweł Hajdan Jr.

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

chromeos: Fix UpdateScreenTest and NetworkScreenTest for Debug build. These tests have been failing since crrev.com/107314 due to the hacky workaround I added. Along the way, rework other tests to use MockDBusThreadManager. These weren't failing but using the mock would make the tests faster and more reliable. For signed_settings_unittest.cc to pass, add expectations for StorePolicy(). We didn't need this before, as we were using stub implementation that always ran callback.Run(true), but we are now using mocks, so we should add this behavior manually. BUG=chromium-os:22183 TEST=Run UpdateScreenTest, NetworkScreenTest, ExistingUserControllerTest, SignedSettingsHelperTest with the debug build. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107865

Patch Set 1 #

Patch Set 2 : remove set_session_manager_client #

Patch Set 3 : update mock #

Patch Set 4 : update #

Patch Set 5 : update #

Patch Set 6 : reupload #

Total comments: 6

Patch Set 7 : address comments #

Created: 9 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+78 lines, -69 lines)			Patch
M	chrome/browser/chromeos/dbus/dbus_thread_manager.h	View	1 5	1 chunk	+0 lines, -8 lines	0 comments	Download
M	chrome/browser/chromeos/dbus/dbus_thread_manager.cc	View	1 2 3 4 5	1 chunk	+0 lines, -6 lines	0 comments	Download
M	chrome/browser/chromeos/dbus/mock_dbus_thread_manager.h	View	1 2 5	1 chunk	+0 lines, -2 lines	0 comments	Download
M	chrome/browser/chromeos/login/existing_user_controller_browsertest.cc	View	1 2 3 4 5 6	4 chunks	+8 lines, -9 lines	0 comments	Download
M	chrome/browser/chromeos/login/network_screen_browsertest.cc	View	1 2 3 4 5 6	3 chunks	+8 lines, -11 lines	0 comments	Download
M	chrome/browser/chromeos/login/signed_settings_unittest.cc	View	1 2 3	17 chunks	+55 lines, -23 lines	0 comments	Download
M	chrome/browser/chromeos/login/update_screen_browsertest.cc	View		3 chunks	+7 lines, -10 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

oshima

LGTM with nits http://codereview.chromium.org/8418037/diff/1005/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc File chrome/browser/chromeos/login/existing_user_controller_browsertest.cc (right): http://codereview.chromium.org/8418037/diff/1005/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc#newcode119 chrome/browser/chromeos/login/existing_user_controller_browsertest.cc:119: DBusThreadManager::InitializeForTesting(mock_dbus_thread_manager); See my comment on http://codereview.chromium.org/8343069/ ...

9 years, 1 month ago (2011-10-29 00:07:02 UTC) #2

satorux1

9 years, 1 month ago (2011-10-29 04:57:35 UTC) #3

http://codereview.chromium.org/8418037/diff/1005/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
(right):

http://codereview.chromium.org/8418037/diff/1005/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/existing_user_controller_browsertest.cc:119:
DBusThreadManager::InitializeForTesting(mock_dbus_thread_manager);
On 2011/10/29 00:07:02, oshima wrote:
> See my comment on http://codereview.chromium.org/8343069/
> Up to you though.

Thanks, but I decided to keep it as-is.

http://codereview.chromium.org/8418037/diff/1005/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/existing_user_controller_browsertest.cc:126: =
mock_dbus_thread_manager->mock_session_manager_client();
On 2011/10/29 00:07:02, oshima wrote:
> move = above

Done.

http://codereview.chromium.org/8418037/diff/1005/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/network_screen_browsertest.cc (right):

http://codereview.chromium.org/8418037/diff/1005/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/network_screen_browsertest.cc:61: =
mock_dbus_thread_manager->mock_session_manager_client();
On 2011/10/29 00:07:02, oshima wrote:
> move = above 

Done.

Expand Messages | Collapse Messages