From 0101984767187482544885a172e3160262f8840a Mon Sep 17 00:00:00 2001 From: "rlarocque@chromium.org" Date: Fri, 23 Aug 2013 04:11:16 +0000 Subject: sync: Improve testing of download functions Refactor the download functions so the commit to the server can be separated from the building of the commit message. This allows us to test these functions in download_unittest.cc without needing to use the heavyweight MockConnectionManager. It also lets us improve our testing of AppendClientDebugInfoIfNeeded by using less mocks and more real code. An important side benefit of this change is that it makes progress towards the removal of ModelTypeInvalidationMap, which is a step towards implementing crbug.com/233437. Also included in this change is a new TRACE_EVENT. Though it's not really new, since it was removed (accidentally) in the Syncer refactor (r209867). It's not related to this change, but it's a one-liner and I happen to be modifying nearby code. BUG=233437 Review URL: https://chromiumcodereview.appspot.com/22849021 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@219209 0039d316-1c4b-4281-b951-d872f2087c98 --- sync/test/engine/mock_connection_manager.cc | 6 ---- sync/test/engine/mock_connection_manager.h | 6 ---- sync/test/engine/syncer_command_test.cc | 49 +++++++++++++++++++++---- sync/test/engine/syncer_command_test.h | 55 ++++++++--------------------- 4 files changed, 57 insertions(+), 59 deletions(-) (limited to 'sync/test') diff --git a/sync/test/engine/mock_connection_manager.cc b/sync/test/engine/mock_connection_manager.cc index 65998e955..5bf2b4b 100644 --- a/sync/test/engine/mock_connection_manager.cc +++ b/sync/test/engine/mock_connection_manager.cc @@ -502,12 +502,6 @@ void MockConnectionManager::ProcessGetUpdates( GetProgressMarkerForType(gu.from_progress_marker(), model_type); EXPECT_EQ(expected_filter_.Has(model_type), (progress_marker != NULL)) << "Syncer requested_types differs from test expectation."; - if (progress_marker) { - EXPECT_EQ((expected_states_.count(model_type) > 0 ? - expected_states_[model_type].payload : - std::string()), - progress_marker->notification_hint()); - } } // Verify that the items we're about to send back to the client are of diff --git a/sync/test/engine/mock_connection_manager.h b/sync/test/engine/mock_connection_manager.h index 9f7cdb1..bab81ae 100644 --- a/sync/test/engine/mock_connection_manager.h +++ b/sync/test/engine/mock_connection_manager.h @@ -234,10 +234,6 @@ class MockConnectionManager : public ServerConnectionManager { expected_filter_ = expected_filter; } - void ExpectGetUpdatesRequestStates(const ModelTypeInvalidationMap& states) { - expected_states_ = states; - } - void SetServerReachable(); void SetServerNotReachable(); @@ -394,8 +390,6 @@ class MockConnectionManager : public ServerConnectionManager { ModelTypeSet expected_filter_; - ModelTypeInvalidationMap expected_states_; - int num_get_updates_requests_; std::string next_token_; diff --git a/sync/test/engine/syncer_command_test.cc b/sync/test/engine/syncer_command_test.cc index 68edbbd..3baf2d1 100644 --- a/sync/test/engine/syncer_command_test.cc +++ b/sync/test/engine/syncer_command_test.cc @@ -9,6 +9,49 @@ namespace syncer { const unsigned int kMaxMessages = 10; const unsigned int kMaxMessageSize = 5 * 1024; +void SyncerCommandTestBase::OnThrottled( + const base::TimeDelta& throttle_duration) { + FAIL() << "Should not get silenced."; +} + +void SyncerCommandTestBase::OnTypesThrottled( + ModelTypeSet types, + const base::TimeDelta& throttle_duration) { + FAIL() << "Should not get silenced."; +} + +bool SyncerCommandTestBase::IsCurrentlyThrottled() { + return false; +} + +void SyncerCommandTestBase::OnReceivedLongPollIntervalUpdate( + const base::TimeDelta& new_interval) { + FAIL() << "Should not get poll interval update."; +} + +void SyncerCommandTestBase::OnReceivedShortPollIntervalUpdate( + const base::TimeDelta& new_interval) { + FAIL() << "Should not get poll interval update."; +} + +void SyncerCommandTestBase::OnReceivedSessionsCommitDelay( + const base::TimeDelta& new_delay) { + FAIL() << "Should not get sessions commit delay."; +} + +void SyncerCommandTestBase::OnReceivedClientInvalidationHintBufferSize( + int size) { + FAIL() << "Should not get hint buffer size."; +} + +void SyncerCommandTestBase::OnShouldStopSyncingPermanently() { + FAIL() << "Shouldn't be called."; +} + +void SyncerCommandTestBase::OnSyncProtocolError( + const sessions::SyncSessionSnapshot& session) { + return; +} SyncerCommandTestBase::SyncerCommandTestBase() : traffic_recorder_(kMaxMessages, kMaxMessageSize) { } @@ -41,10 +84,4 @@ void SyncerCommandTest::TearDown() { dir_maker_.TearDown(); } -MockDebugInfoGetter::MockDebugInfoGetter() { -} - -MockDebugInfoGetter::~MockDebugInfoGetter() { -} - } // namespace syncer diff --git a/sync/test/engine/syncer_command_test.h b/sync/test/engine/syncer_command_test.h index f0904cc..6e1f672 100644 --- a/sync/test/engine/syncer_command_test.h +++ b/sync/test/engine/syncer_command_test.h @@ -14,6 +14,7 @@ #include "base/message_loop/message_loop.h" #include "sync/engine/model_changing_syncer_command.h" #include "sync/engine/traffic_recorder.h" +#include "sync/internal_api/debug_info_event_listener.h" #include "sync/internal_api/public/engine/model_safe_worker.h" #include "sync/sessions/debug_info_getter.h" #include "sync/sessions/sync_session.h" @@ -26,17 +27,8 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" -using ::testing::NiceMock; - namespace syncer { -class MockDebugInfoGetter : public sessions::DebugInfoGetter { - public: - MockDebugInfoGetter(); - virtual ~MockDebugInfoGetter(); - MOCK_METHOD1(GetAndClearDebugInfo, void(sync_pb::DebugInfo* debug_info)); -}; - // A test fixture that simplifies writing unit tests for individual // SyncerCommands, providing convenient access to a test directory // and a syncer session. @@ -45,39 +37,21 @@ class SyncerCommandTestBase : public testing::Test, public: // SyncSession::Delegate implementation. virtual void OnThrottled( - const base::TimeDelta& throttle_duration) OVERRIDE { - FAIL() << "Should not get silenced."; - } + const base::TimeDelta& throttle_duration) OVERRIDE; virtual void OnTypesThrottled( ModelTypeSet types, - const base::TimeDelta& throttle_duration) OVERRIDE { - FAIL() << "Should not get silenced."; - } - virtual bool IsCurrentlyThrottled() OVERRIDE { - return false; - } + const base::TimeDelta& throttle_duration) OVERRIDE; + virtual bool IsCurrentlyThrottled() OVERRIDE; virtual void OnReceivedLongPollIntervalUpdate( - const base::TimeDelta& new_interval) OVERRIDE { - FAIL() << "Should not get poll interval update."; - } + const base::TimeDelta& new_interval) OVERRIDE; virtual void OnReceivedShortPollIntervalUpdate( - const base::TimeDelta& new_interval) OVERRIDE { - FAIL() << "Should not get poll interval update."; - } + const base::TimeDelta& new_interval) OVERRIDE; virtual void OnReceivedSessionsCommitDelay( - const base::TimeDelta& new_delay) OVERRIDE { - FAIL() << "Should not get sessions commit delay."; - } - virtual void OnReceivedClientInvalidationHintBufferSize(int size) OVERRIDE { - FAIL() << "Should not get hint buffer size."; - } - virtual void OnShouldStopSyncingPermanently() OVERRIDE { - FAIL() << "Shouldn't be called."; - } + const base::TimeDelta& new_delay) OVERRIDE; + virtual void OnReceivedClientInvalidationHintBufferSize(int size) OVERRIDE; + virtual void OnShouldStopSyncingPermanently() OVERRIDE; virtual void OnSyncProtocolError( - const sessions::SyncSessionSnapshot& session) OVERRIDE { - return; - } + const sessions::SyncSessionSnapshot& session) OVERRIDE; std::vector GetWorkers() { std::vector workers; @@ -117,7 +91,7 @@ class SyncerCommandTestBase : public testing::Test, mock_server_.get(), directory(), GetWorkers(), extensions_activity_.get(), std::vector(), - &mock_debug_info_getter_, + &debug_info_event_listener_, &traffic_recorder_, true, // enable keystore encryption false, // force enable pre-commit GU avoidance experiment @@ -147,10 +121,9 @@ class SyncerCommandTestBase : public testing::Test, return mock_server_.get(); } - MockDebugInfoGetter* mock_debug_info_getter() { - return &mock_debug_info_getter_; + DebugInfoEventListener* debug_info_event_listener() { + return &debug_info_event_listener_; } - // Helper functions to check command.GetGroupsToChange(). void ExpectNoGroupsToChange(const ModelChangingSyncerCommand& command) { @@ -193,7 +166,7 @@ class SyncerCommandTestBase : public testing::Test, scoped_ptr session_; std::vector > workers_; ModelSafeRoutingInfo routing_info_; - NiceMock mock_debug_info_getter_; + DebugInfoEventListener debug_info_event_listener_; scoped_refptr extensions_activity_; TrafficRecorder traffic_recorder_; DISALLOW_COPY_AND_ASSIGN(SyncerCommandTestBase); -- cgit v1.1