diff options
author | scottmg@chromium.org <scottmg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-10 01:42:49 +0000 |
---|---|---|
committer | scottmg@chromium.org <scottmg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-10 01:42:49 +0000 |
commit | 8ff3cddc8a9b817ae7c9ccba743cab0d25eccd33 (patch) | |
tree | 0a9d91ad6c0b4b8dfe3ab3b254e66e7146a05296 /sync/js | |
parent | ee060d4042c1a7803510c6ed31f4840d1eb9d080 (diff) | |
download | chromium_src-8ff3cddc8a9b817ae7c9ccba743cab0d25eccd33.zip chromium_src-8ff3cddc8a9b817ae7c9ccba743cab0d25eccd33.tar.gz chromium_src-8ff3cddc8a9b817ae7c9ccba743cab0d25eccd33.tar.bz2 |
Revert 239615 "Retry: Clean up TestProfileSyncService tests"
Failed on CrOS:
http://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20%28x86%29/builds/18496/steps/BuildPackages/logs/stdio
chromeos-chrome-33.0.1734.0_alpha-r1: sync/protocol/proto_value_conversions.cc:417:11: error: 'MetaInfo' in namespace 'sync_pb' does not name a type
chromeos-chrome-33.0.1734.0_alpha-r1: const sync_pb::MetaInfo& proto) {
chromeos-chrome-33.0.1734.0_alpha-r1: ^
chromeos-chrome-33.0.1734.0_alpha-r1: sync/protocol/proto_value_conversions.cc:417:30: error: ISO C++ forbids declaration of 'proto' with no type [-fpermissive]
chromeos-chrome-33.0.1734.0_alpha-r1: const sync_pb::MetaInfo& proto) {
chromeos-chrome-33.0.1734.0_alpha-r1: ^
chromeos-chrome-33.0.1734.0_alpha-r1: sync/protocol/proto_value_conversions.cc: In function 'base::DictionaryValue* syncer::MetaInfoToValue(const int&)':
chromeos-chrome-33.0.1734.0_alpha-r1: sync/protocol/proto_value_conversions.cc:419:13: error: request for member 'has_key' in 'proto', which is of non-class type 'const int'
chromeos-chrome-33.0.1734.0_alpha-r1: SET_STR(key);
chromeos-chrome-33.0.1734.0_alpha-r1: ^
chromeos-chrome-33.0.1734.0_alpha-r1: sync/protocol/proto_value_conversions.cc:419:72: error: request for member 'key' in 'proto', which is of non-class type 'const int'
chromeos-chrome-33.0.1734.0_alpha-r1: SET_STR(key);
chromeos-chrome-33.0.1734.0_alpha-r1: ^
chromeos-chrome-33.0.1734.0_alpha-r1: sync/protocol/proto_value_conversions.cc:420:13: error: request for member 'has_value' in 'proto', which is of non-class type 'const int'
chromeos-chrome-33.0.1734.0_alpha-r1: SET_STR(value);
chromeos-chrome-33.0.1734.0_alpha-r1: ^
chromeos-chrome-33.0.1734.0_alpha-r1: sync/protocol/proto_value_conversions.cc:420:76: error: request for member 'value' in 'proto', which is of non-class type 'const int'
chromeos-chrome-33.0.1734.0_alpha-r1: SET_STR(value);
chromeos-chrome-33.0.1734.0_alpha-r1: ^
chromeos-chrome-33.0.1734.0_alpha-r1: sync/protocol/proto_value_conversions.cc: In function 'base::DictionaryValue* syncer::BookmarkSpecificsToValue(const sync_pb::BookmarkSpecifics&)':
chromeos-chrome-33.0.1734.0_alpha-r1: sync/protocol/proto_value_conversions.cc:432:51: error: 'const class sync_pb::BookmarkSpecifics' has no member named 'meta_info'
chromeos-chrome-33.0.1734.0_alpha-r1: SET_REP(meta_info, &MetaInfoToValue);
chromeos-chrome-33.0.1734.0_alpha-r1: ^
chromeos-chrome-33.0.1734.0_alpha-r1: make: *** [c/Release/obj.target/sync_core/sync/protocol/proto_value_conversions.o] Error 1
chromeos-chrome-33.0.1734.0_alpha-r1: make: *** Waiting for unfinished jobs....
chromeos-chrome-33.0.1734.0_alpha-r1: * ERROR: chromeos-base/chromeos-chrome-33.0.1734.0_alpha-r1 failed (compile phase):
chromeos-chrome-33.0.1734.0_alpha-r1: * emake failed
chromeos-chrome-33.0.1734.0_alpha-r1: *
> Retry: Clean up TestProfileSyncService tests
>
> This is a retry of r238348, which was reverted in r238368. I believe
> the build failure that led to the revert, though related to my change,
> was a flake caused by stale object files on the build bot.
>
> This CL refactors many of the tests in profile_sync_service_unittest.cc.
> It continues the refactoring work begun in r233533, r235661, and
> r235854.
>
> The JsController tests have been deleted. There is not much point in
> testing the JsController here; it can be more easily tested on its own
> in sync_js_controller_uniittest.cc. The SyncJsController unit tests have
> been updated so they now cover the few cases that were formerly only
> exercised in the ProfileSyncService unit tests.
>
> It converts all remaining uncoverted tests from relying on the
> TestProfileSyncService to using a real ProfileSyncService with an
> injected backend. The injected backend makes it easier to create the
> scenarios we want to test. We can inject a specially crafted SBH rather
> than fiddling with "synchronous init" and "fail initial download" flags.
>
> Since the TestProfileSyncService no longer needs to support the wide
> variety of test scenarios required by the tests in
> profile_sync_service_unittest.cc, we can greatly simplify its
> implementation. Many of its parameters and associated code have been
> removed.
>
> BUG=140354,312994
>
> Review URL: https://codereview.chromium.org/102513002
TBR=rlarocque@chromium.org
Review URL: https://codereview.chromium.org/110993003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239635 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync/js')
-rw-r--r-- | sync/js/sync_js_controller_unittest.cc | 55 |
1 files changed, 17 insertions, 38 deletions
diff --git a/sync/js/sync_js_controller_unittest.cc b/sync/js/sync_js_controller_unittest.cc index eca617c..ac46935 100644 --- a/sync/js/sync_js_controller_unittest.cc +++ b/sync/js/sync_js_controller_unittest.cc @@ -30,15 +30,10 @@ class SyncJsControllerTest : public testing::Test { base::MessageLoop message_loop_; }; -ACTION_P(ReplyToMessage, reply_name) { - arg2.Call(FROM_HERE, &JsReplyHandler::HandleJsReply, reply_name, JsArgList()); -} - TEST_F(SyncJsControllerTest, Messages) { InSequence dummy; // |mock_backend| needs to outlive |sync_js_controller|. StrictMock<MockJsBackend> mock_backend; - StrictMock<MockJsReplyHandler> mock_reply_handler; SyncJsController sync_js_controller; base::ListValue arg_list1, arg_list2; @@ -46,23 +41,17 @@ TEST_F(SyncJsControllerTest, Messages) { arg_list2.Append(new base::FundamentalValue(5)); JsArgList args1(&arg_list1), args2(&arg_list2); + // TODO(akalin): Write matchers for WeakHandle and use them here + // instead of _. EXPECT_CALL(mock_backend, SetJsEventHandler(_)); - EXPECT_CALL(mock_backend, ProcessJsMessage("test1", HasArgs(args2), _)) - .WillOnce(ReplyToMessage("test1_reply")); - EXPECT_CALL(mock_backend, ProcessJsMessage("test2", HasArgs(args1), _)) - .WillOnce(ReplyToMessage("test2_reply")); + EXPECT_CALL(mock_backend, ProcessJsMessage("test1", HasArgs(args2), _)); + EXPECT_CALL(mock_backend, ProcessJsMessage("test2", HasArgs(args1), _)); sync_js_controller.AttachJsBackend(mock_backend.AsWeakHandle()); - sync_js_controller.ProcessJsMessage("test1", - args2, - mock_reply_handler.AsWeakHandle()); - sync_js_controller.ProcessJsMessage("test2", - args1, - mock_reply_handler.AsWeakHandle()); - - // The replies should be waiting on our message loop. - EXPECT_CALL(mock_reply_handler, HandleJsReply("test1_reply", _)); - EXPECT_CALL(mock_reply_handler, HandleJsReply("test2_reply", _)); + sync_js_controller.ProcessJsMessage("test1", args2, + WeakHandle<JsReplyHandler>()); + sync_js_controller.ProcessJsMessage("test2", args1, + WeakHandle<JsReplyHandler>()); PumpLoop(); // Let destructor of |sync_js_controller| call RemoveBackend(). @@ -71,7 +60,6 @@ TEST_F(SyncJsControllerTest, Messages) { TEST_F(SyncJsControllerTest, QueuedMessages) { // |mock_backend| needs to outlive |sync_js_controller|. StrictMock<MockJsBackend> mock_backend; - StrictMock<MockJsReplyHandler> mock_reply_handler; SyncJsController sync_js_controller; base::ListValue arg_list1, arg_list2; @@ -80,29 +68,20 @@ TEST_F(SyncJsControllerTest, QueuedMessages) { JsArgList args1(&arg_list1), args2(&arg_list2); // Should queue messages. - sync_js_controller.ProcessJsMessage( - "test1", - args2, - mock_reply_handler.AsWeakHandle()); - sync_js_controller.ProcessJsMessage( - "test2", - args1, - mock_reply_handler.AsWeakHandle()); + sync_js_controller.ProcessJsMessage("test1", args2, + WeakHandle<JsReplyHandler>()); + sync_js_controller.ProcessJsMessage("test2", args1, + WeakHandle<JsReplyHandler>()); - // Should do nothing. - PumpLoop(); Mock::VerifyAndClearExpectations(&mock_backend); - - // Should call the queued messages. + // TODO(akalin): Write matchers for WeakHandle and use them here + // instead of _. EXPECT_CALL(mock_backend, SetJsEventHandler(_)); - EXPECT_CALL(mock_backend, ProcessJsMessage("test1", HasArgs(args2), _)) - .WillOnce(ReplyToMessage("test1_reply")); - EXPECT_CALL(mock_backend, ProcessJsMessage("test2", HasArgs(args1), _)) - .WillOnce(ReplyToMessage("test2_reply")); - EXPECT_CALL(mock_reply_handler, HandleJsReply("test1_reply", _)); - EXPECT_CALL(mock_reply_handler, HandleJsReply("test2_reply", _)); + EXPECT_CALL(mock_backend, ProcessJsMessage("test1", HasArgs(args2), _)); + EXPECT_CALL(mock_backend, ProcessJsMessage("test2", HasArgs(args1), _)); + // Should call the queued messages. sync_js_controller.AttachJsBackend(mock_backend.AsWeakHandle()); PumpLoop(); |