test cases for server directed error handling code.

chrome/browser/sync/test/integration/sync_test.cc

Index: chrome/browser/sync/test/integration/sync_test.cc
diff --git a/chrome/browser/sync/test/integration/sync_test.cc b/chrome/browser/sync/test/integration/sync_test.cc
index 293cec5e6cf009493134414273480cc998914dd2..44e3cf4cb04467639495f12e5e5f150fdc233bdf 100644
--- a/chrome/browser/sync/test/integration/sync_test.cc
+++ b/chrome/browser/sync/test/integration/sync_test.cc
@@ -23,6 +23,7 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/sync/notifier/p2p_notifier.h"
 #include "chrome/browser/sync/profile_sync_service_harness.h"
+#include "chrome/browser/sync/protocol/sync.pb.h"
 #include "chrome/browser/sync/test/integration/sync_datatype_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -610,6 +611,75 @@ void SyncTest::TriggerTransientError() {
             UTF16ToASCII(browser()->GetSelectedTabContents()->GetTitle()));
 }
 
+namespace {

[Raghu Simha, 2011/09/16 04:01:02]

Style guide seems to recommend a blank line here.

[lipalani1, 2011/09/19 18:59:13]

Done.

+sync_pb::ClientToServerResponse::ErrorType
+    SyncProtocolErrorTypeToClientToServerResponseErrorType(

[Raghu Simha, 2011/09/16 04:01:02]

Holy long name, Batman! How about GetClientToServerResponseErrorType()?

[lipalani1, 2011/09/19 18:59:13]

Done.

+       browser_sync::SyncProtocolErrorType error) {
+  switch (error) {
+    case browser_sync::SYNC_SUCCESS:
+      return sync_pb::ClientToServerResponse::SUCCESS;
+    case browser_sync::NOT_MY_BIRTHDAY:
+      return sync_pb::ClientToServerResponse::NOT_MY_BIRTHDAY;
+    case browser_sync::THROTTLED:
+      return sync_pb::ClientToServerResponse::THROTTLED;
+    case browser_sync::CLEAR_PENDING:
+      return sync_pb::ClientToServerResponse::CLEAR_PENDING;
+    case browser_sync::TRANSIENT_ERROR:
+      return sync_pb::ClientToServerResponse::TRANSIENT_ERROR;
+    case browser_sync::MIGRATION_DONE:
+      return sync_pb::ClientToServerResponse::MIGRATION_DONE;
+    case browser_sync::UNKNOWN_ERROR:
+      return sync_pb::ClientToServerResponse::UNKNOWN;
+    default:
+      NOTREACHED();
+      return sync_pb::ClientToServerResponse::UNKNOWN;
+  }
+}
+
+sync_pb::ClientToServerResponse::Error::Action
+   ClientActionToClientToServerResponseAction(

[Raghu Simha, 2011/09/16 04:01:02]

indent += 1. In this case, indent -= 3 will also work.

[Raghu Simha, 2011/09/16 04:01:02]

How about renaming this to GetServerResponseAction()?

[lipalani1, 2011/09/19 18:59:13]

Done.

[lipalani1, 2011/09/19 18:59:13]

Done.

+       const browser_sync::ClientAction& action) {
+  switch (action) {
+    case browser_sync::UPGRADE_CLIENT:
+      return sync_pb::ClientToServerResponse::Error::UPGRADE_CLIENT;
+    case browser_sync::CLEAR_USER_DATA_AND_RESYNC:
+      return sync_pb::ClientToServerResponse::Error::CLEAR_USER_DATA_AND_RESYNC;
+    case browser_sync::ENABLE_SYNC_ON_ACCOUNT:
+      return sync_pb::ClientToServerResponse::Error::ENABLE_SYNC_ON_ACCOUNT;
+    case browser_sync::STOP_AND_RESTART_SYNC:
+      return sync_pb::ClientToServerResponse::Error::STOP_AND_RESTART_SYNC;
+    case browser_sync::DISABLE_SYNC_ON_CLIENT:
+      return sync_pb::ClientToServerResponse::Error::DISABLE_SYNC_ON_CLIENT;
+    case browser_sync::UNKNOWN_ACTION:
+      return sync_pb::ClientToServerResponse::Error::UNKNOWN_ACTION;
+    default:
+      NOTREACHED();
+      return sync_pb::ClientToServerResponse::Error::UNKNOWN_ACTION;
+  }
+}

[Raghu Simha, 2011/09/16 04:01:02]

Style guide seems to recommend a blank line here.

[lipalani1, 2011/09/19 18:59:13]

Done.

+}  // namespace
+
+void SyncTest::TriggerSyncError(const browser_sync::SyncProtocolError& error) {
+  ASSERT_TRUE(ServerSupportsErrorTriggering());
+  std::string path = "chromiumsync/error";
+  int error_type =
+      static_cast<int>(SyncProtocolErrorTypeToClientToServerResponseErrorType(
+          error.error_type));
+  int action = static_cast<int>(ClientActionToClientToServerResponseAction(
+      error.action));
+
+  path.append(base::StringPrintf("?error=%d", error_type));
+  path.append(base::StringPrintf("&action=%d", action));
+
+  path += "&error_description=" + error.error_description;
+  path += "&url=" + error.url;
+
+  ui_test_utils::NavigateToURL(browser(), sync_server_.GetURL(path));
+  std::string output = UTF16ToASCII(
+      browser()->GetSelectedTabContents()->GetTitle());
+  ASSERT_TRUE(output.find("SetError: 200") != string16::npos);

[Raghu Simha, 2011/09/16 04:01:02]

Isn't output an std::string? Shouldn't you be comparing the result of find()
with std::string::npos?

[Raghu Simha, 2011/09/16 04:01:02]

From chromiumsync.py, isn't the error code 400? Or am I reading it wrong?

[lipalani1, 2011/09/19 18:59:13]

No the output is size_t. So we need to compare against npos.
On 2011/09/16 04:01:02, rsimha wrote:

[lipalani1, 2011/09/19 18:59:13]

200 is for success. I thing the .py file says the same...
On 2011/09/16 04:01:02, rsimha wrote:

[Raghu Simha, 2011/09/19 22:25:02]

Yeah, what I meant was that for consistency, we should be using
std::string::npos and not string16::npos, since |output| is an std::string and
not a string16 variable.

[Raghu Simha, 2011/09/19 22:25:02]

You're right. 400 is returned only if chromiumsync.py hits a python exception.

+}
+
 void SyncTest::TriggerSetSyncTabs() {
   ASSERT_TRUE(ServerSupportsErrorTriggering());
   std::string path = "chromiumsync/synctabs";