test cases for server directed error handling code.

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

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/test/integration/sync_test.h"
 
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/stringprintf.h"
 #include "base/string_util.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/task.h"
 #include "base/test/test_timeouts.h"
 #include "base/threading/platform_thread.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/password_manager/encryptor.h"
 #include "chrome/browser/profiles/profile.h"
 #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"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "content/common/net/url_fetcher.h"
@@ skipping 567 matching lines @@
 }
 
 void SyncTest::TriggerTransientError() {
   ASSERT_TRUE(ServerSupportsErrorTriggering());
   std::string path = "chromiumsync/transienterror";
   ui_test_utils::NavigateToURL(browser(), sync_server_.GetURL(path));
   ASSERT_EQ("Transient error",
             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";
   ui_test_utils::NavigateToURL(browser(), sync_server_.GetURL(path));
   ASSERT_EQ("Sync Tabs",
             UTF16ToASCII(browser()->GetSelectedTabContents()->GetTitle()));
 }
 
 void SyncTest::SetProxyConfig(net::URLRequestContextGetter* context_getter,
                               const net::ProxyConfig& proxy_config) {
   base::WaitableEvent done(false, false);
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
       new SetProxyConfigTask(&done,
                              context_getter,
                              proxy_config));
   done.Wait();
 }