Sync FakeServer cleanup: CHECKs and logging info

This CL removes unnecessary DCHECKs, switches useful DCHECKs
to CHECKS, and adds logging statements.

BUG=NONE

Review URL: https://codereview.chromium.org/327593003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@276171 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 20 insertions, 22 deletions

diff --git a/sync/test/fake_server/fake_server.cc b/sync/test/fake_server/fake_server.cc
index 25ec36a..d881080 100644
--- a/sync/test/fake_server/fake_server.cc
+++ b/sync/test/fake_server/fake_server.cc
@@ -115,7 +115,8 @@ class UpdateSieve {
 
 scoped_ptr<UpdateSieve> UpdateSieve::Create(
     const sync_pb::GetUpdatesMessage& get_updates_message) {
-  DCHECK_GT(get_updates_message.from_progress_marker_size(), 0);
+  CHECK_GT(get_updates_message.from_progress_marker_size(), 0)
+      << "A GetUpdates request must have at least one progress marker.";
 
   UpdateSieve::ModelTypeToVersionMap request_from_version;
   int64 min_version = std::numeric_limits<int64>::max();
@@ -128,7 +129,7 @@ scoped_ptr<UpdateSieve> UpdateSieve::Create(
     // first request for this type).
     if (marker.has_token() && !marker.token().empty()) {
       bool parsed = base::StringToInt64(marker.token(), &version);
-      DCHECK(parsed);
+      CHECK(parsed) << "Unable to parse progress marker token.";
     }
 
     ModelType model_type = syncer::GetModelTypeFromSpecificsFieldNumber(
@@ -223,7 +224,7 @@ void FakeServer::HandleCommand(const string& request,
 
   sync_pb::ClientToServerMessage message;
   bool parsed = message.ParseFromString(request);
-  DCHECK(parsed);
+  CHECK(parsed) << "Unable to parse the ClientToServerMessage.";
 
   sync_pb::SyncEnums_ErrorType error_code;
   sync_pb::ClientToServerResponse response_proto;
diff --git a/sync/test/fake_server/fake_server_entity.cc b/sync/test/fake_server/fake_server_entity.cc
index 4d96a80..b2aa6f9 100644
--- a/sync/test/fake_server/fake_server_entity.cc
+++ b/sync/test/fake_server/fake_server_entity.cc
@@ -87,8 +87,6 @@ FakeServerEntity::FakeServerEntity(const string& id,
 
 void FakeServerEntity::SerializeBaseProtoFields(
     sync_pb::SyncEntity* sync_entity) {
-  DCHECK(sync_entity);
-
   // FakeServerEntity fields
   sync_entity->set_id_string(id_);
   sync_entity->set_version(version_);
diff --git a/sync/test/fake_server/permanent_entity.cc b/sync/test/fake_server/permanent_entity.cc
index 480e380..3ceb5c8 100644
--- a/sync/test/fake_server/permanent_entity.cc
+++ b/sync/test/fake_server/permanent_entity.cc
@@ -29,13 +29,14 @@ FakeServerEntity* PermanentEntity::Create(const ModelType& model_type,
                                           const string& server_tag,
                                           const string& name,
                                           const string& parent_server_tag) {
-  DCHECK(model_type != syncer::UNSPECIFIED);
-  DCHECK(!server_tag.empty());
-  DCHECK(!name.empty());
-  DCHECK(!parent_server_tag.empty());
-  if (parent_server_tag == kRootParentTag) {
-    return NULL;
-  }
+  CHECK(model_type != syncer::UNSPECIFIED) << "The entity's ModelType is "
+                                           << "invalid.";
+  CHECK(!server_tag.empty()) << "A PermanentEntity must have a server tag.";
+  CHECK(!name.empty()) << "The entity must have a non-empty name.";
+  CHECK(!parent_server_tag.empty()) << "A PermanentEntity must have a parent "
+                                    << "server tag.";
+  CHECK(parent_server_tag != kRootParentTag) << "Top-level entities should not "
+                                             << "be created with this factory.";
 
   string id = FakeServerEntity::CreateId(model_type, server_tag);
   string parent_id = FakeServerEntity::CreateId(model_type, parent_server_tag);
@@ -50,8 +51,9 @@ FakeServerEntity* PermanentEntity::Create(const ModelType& model_type,
 }
 
 // static
-FakeServerEntity* PermanentEntity::CreateTopLevel(
-    const ModelType& model_type) {
+FakeServerEntity* PermanentEntity::CreateTopLevel(const ModelType& model_type) {
+  CHECK(model_type != syncer::UNSPECIFIED) << "The entity's ModelType is "
+                                           << "invalid.";
   string server_tag = syncer::ModelTypeToRootTag(model_type);
   string name = syncer::ModelTypeToString(model_type);
   string id = FakeServerEntity::CreateId(model_type, server_tag);
@@ -70,9 +72,8 @@ FakeServerEntity* PermanentEntity::CreateUpdatedNigoriEntity(
     const sync_pb::SyncEntity& client_entity,
     FakeServerEntity* current_server_entity) {
   ModelType model_type = current_server_entity->GetModelType();
-  if (model_type != syncer::NIGORI) {
-    return NULL;
-  }
+  CHECK(model_type == syncer::NIGORI) << "This factory only supports NIGORI "
+                                      << "entities.";
 
   return new PermanentEntity(current_server_entity->GetId(),
                              model_type,
diff --git a/sync/test/fake_server/permanent_entity.h b/sync/test/fake_server/permanent_entity.h
index ea84cfc..391b70b 100644
--- a/sync/test/fake_server/permanent_entity.h
+++ b/sync/test/fake_server/permanent_entity.h
@@ -31,8 +31,7 @@ class PermanentEntity : public FakeServerEntity {
   static FakeServerEntity* CreateTopLevel(const syncer::ModelType& model_type);
 
   // Factory function for creating an updated version of a PermanentEntity.
-  // This function should only be called for the Nigori entity and will return
-  // NULL if any other type is specified.
+  // This function should only be called for the Nigori entity.
   static FakeServerEntity* CreateUpdatedNigoriEntity(
       const sync_pb::SyncEntity& client_entity,
       FakeServerEntity* current_server_entity);
diff --git a/sync/test/fake_server/unique_client_entity.cc b/sync/test/fake_server/unique_client_entity.cc
index 693b24a..e9ce2e9 100644
--- a/sync/test/fake_server/unique_client_entity.cc
+++ b/sync/test/fake_server/unique_client_entity.cc
@@ -23,9 +23,8 @@ UniqueClientEntity::~UniqueClientEntity() { }
 // static
 FakeServerEntity* UniqueClientEntity::CreateNew(
     const sync_pb::SyncEntity& client_entity) {
-  DCHECK(client_entity.has_client_defined_unique_tag());
-  DCHECK(!client_entity.folder());
-  DCHECK(!client_entity.deleted());
+  CHECK(client_entity.has_client_defined_unique_tag())
+      << "A UniqueClientEntity must have a client-defined unique tag.";
   ModelType model_type =
       syncer::GetModelTypeFromSpecifics(client_entity.specifics());
   string id = client_entity.version() == 0 ?