Cleanup the NetworkDeviceHandler tests.

Removes the need for a *ForTest function and friend declarations in NetworkDeviceHandler.
Fixes the tests for error_callback invocation, which weren't correctly tested before.
While there, does some other minor cleanups.

BUG=323537

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

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

3 files changed, 67 insertions, 186 deletions

diff --git a/chromeos/network/network_device_handler.cc b/chromeos/network/network_device_handler.cc
index 75d4c7d..4814d85 100644
--- a/chromeos/network/network_device_handler.cc
+++ b/chromeos/network/network_device_handler.cc
@@ -127,9 +127,6 @@ const char NetworkDeviceHandler::kErrorPinBlocked[] = "pin-blocked";
 const char NetworkDeviceHandler::kErrorPinRequired[] = "pin-required";
 const char NetworkDeviceHandler::kErrorUnknown[] = "unknown";
 
-NetworkDeviceHandler::NetworkDeviceHandler() {
-}
-
 NetworkDeviceHandler::~NetworkDeviceHandler() {
 }
 
@@ -254,13 +251,7 @@ void NetworkDeviceHandler::ChangePin(
       base::Bind(&HandleShillCallFailure, device_path, error_callback));
 }
 
-void NetworkDeviceHandler::HandleShillCallFailureForTest(
-    const std::string& device_path,
-    const network_handler::ErrorCallback& error_callback,
-    const std::string& shill_error_name,
-    const std::string& shill_error_message) {
-  HandleShillCallFailure(
-      device_path, error_callback, shill_error_name, shill_error_message);
+NetworkDeviceHandler::NetworkDeviceHandler() {
 }
 
 }  // namespace chromeos
diff --git a/chromeos/network/network_device_handler.h b/chromeos/network/network_device_handler.h
index 7cb2f52..d675cfa 100644
--- a/chromeos/network/network_device_handler.h
+++ b/chromeos/network/network_device_handler.h
@@ -8,7 +8,6 @@
 #include <string>
 
 #include "base/callback.h"
-#include "base/gtest_prod_util.h"
 #include "base/memory/weak_ptr.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/network/network_handler.h"
@@ -195,16 +194,9 @@ class CHROMEOS_EXPORT NetworkDeviceHandler {
  private:
   friend class NetworkHandler;
   friend class NetworkDeviceHandlerTest;
-  FRIEND_TEST_ALL_PREFIXES(NetworkDeviceHandlerTest, ErrorTranslation);
 
   NetworkDeviceHandler();
 
-  void HandleShillCallFailureForTest(
-      const std::string& device_path,
-      const network_handler::ErrorCallback& error_callback,
-      const std::string& error_name,
-      const std::string& error_message);
-
   DISALLOW_COPY_AND_ASSIGN(NetworkDeviceHandler);
 };
 
diff --git a/chromeos/network/network_device_handler_unittest.cc b/chromeos/network/network_device_handler_unittest.cc
index dd9667a..372b50c 100644
--- a/chromeos/network/network_device_handler_unittest.cc
+++ b/chromeos/network/network_device_handler_unittest.cc
@@ -18,6 +18,7 @@ namespace chromeos {
 namespace {
 
 const char kDefaultCellularDevicePath[] = "stub_cellular_device";
+const char kUnknownCellularDevicePath[] = "unknown_cellular_device";
 const char kDefaultWifiDevicePath[] = "stub_wifi_device";
 const char kResultSuccess[] = "success";
 
@@ -30,15 +31,6 @@ class NetworkDeviceHandlerTest : public testing::Test {
 
   virtual void SetUp() OVERRIDE {
     DBusThreadManager::InitializeWithStub();
-    message_loop_.RunUntilIdle();
-    success_callback_ = base::Bind(&NetworkDeviceHandlerTest::SuccessCallback,
-                                   base::Unretained(this));
-    properties_success_callback_ =
-        base::Bind(&NetworkDeviceHandlerTest::PropertiesSuccessCallback,
-                   base::Unretained(this));
-    error_callback_ = base::Bind(&NetworkDeviceHandlerTest::ErrorCallback,
-                                 base::Unretained(this));
-    network_device_handler_.reset(new NetworkDeviceHandler());
 
     ShillDeviceClient::TestInterface* device_test =
         DBusThreadManager::Get()->GetShillDeviceClient()->GetTestInterface();
@@ -48,15 +40,23 @@ class NetworkDeviceHandlerTest : public testing::Test {
     device_test->AddDevice(kDefaultWifiDevicePath, shill::kTypeWifi, "wifi1");
 
     base::FundamentalValue allow_roaming(false);
-    device_test->SetDeviceProperty(
-        kDefaultCellularDevicePath,
-        shill::kCellularAllowRoamingProperty,
-        allow_roaming);
+    device_test->SetDeviceProperty(kDefaultCellularDevicePath,
+                                   shill::kCellularAllowRoamingProperty,
+                                   allow_roaming);
 
     base::ListValue test_ip_configs;
     test_ip_configs.AppendString("ip_config1");
     device_test->SetDeviceProperty(
         kDefaultWifiDevicePath, shill::kIPConfigsProperty, test_ip_configs);
+
+    success_callback_ = base::Bind(&NetworkDeviceHandlerTest::SuccessCallback,
+                                   base::Unretained(this));
+    properties_success_callback_ =
+        base::Bind(&NetworkDeviceHandlerTest::PropertiesSuccessCallback,
+                   base::Unretained(this));
+    error_callback_ = base::Bind(&NetworkDeviceHandlerTest::ErrorCallback,
+                                 base::Unretained(this));
+    network_device_handler_.reset(new NetworkDeviceHandler);
   }
 
   virtual void TearDown() OVERRIDE {
@@ -64,17 +64,6 @@ class NetworkDeviceHandlerTest : public testing::Test {
     DBusThreadManager::Shutdown();
   }
 
-  base::Closure GetErrorInvokingCallback(
-      const std::string& device_path,
-      const std::string& error_name) {
-    return base::Bind(&NetworkDeviceHandlerTest::InvokeDBusErrorCallback,
-                      base::Unretained(this),
-                      device_path,
-                      base::Bind(&NetworkDeviceHandlerTest::ErrorCallback,
-                                 base::Unretained(this)),
-                      error_name);
-  }
-
   void ErrorCallback(const std::string& error_name,
                      scoped_ptr<base::DictionaryValue> error_data) {
     result_ = error_name;
@@ -90,14 +79,6 @@ class NetworkDeviceHandlerTest : public testing::Test {
     properties_.reset(properties.DeepCopy());
   }
 
-  void InvokeDBusErrorCallback(
-      const std::string& device_path,
-      const network_handler::ErrorCallback& callback,
-      const std::string& error_name) {
-    network_device_handler_->HandleShillCallFailureForTest(
-        device_path, callback, error_name, "Error message.");
-  }
-
  protected:
   std::string result_;
 
@@ -112,60 +93,9 @@ class NetworkDeviceHandlerTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(NetworkDeviceHandlerTest);
 };
 
-TEST_F(NetworkDeviceHandlerTest, ErrorTranslation) {
-  EXPECT_TRUE(result_.empty());
-  network_handler::ErrorCallback callback =
-      base::Bind(&NetworkDeviceHandlerTest::ErrorCallback,
-                 base::Unretained(this));
-
-  network_device_handler_->HandleShillCallFailureForTest(
-      kDefaultCellularDevicePath,
-      callback,
-      "org.chromium.flimflam.Error.Failure",
-      "Error happened.");
-  EXPECT_EQ(NetworkDeviceHandler::kErrorFailure, result_);
-
-  network_device_handler_->HandleShillCallFailureForTest(
-      kDefaultCellularDevicePath,
-      callback,
-      "org.chromium.flimflam.Error.IncorrectPin",
-      "Incorrect pin.");
-  EXPECT_EQ(NetworkDeviceHandler::kErrorIncorrectPin, result_);
-
-  network_device_handler_->HandleShillCallFailureForTest(
-      kDefaultCellularDevicePath,
-      callback,
-      "org.chromium.flimflam.Error.NotSupported",
-      "Operation not supported.");
-  EXPECT_EQ(NetworkDeviceHandler::kErrorNotSupported, result_);
-
-  network_device_handler_->HandleShillCallFailureForTest(
-      kDefaultCellularDevicePath,
-      callback,
-      "org.chromium.flimflam.Error.PinBlocked",
-      "PIN is blocked.");
-  EXPECT_EQ(NetworkDeviceHandler::kErrorPinBlocked, result_);
-
-  network_device_handler_->HandleShillCallFailureForTest(
-      kDefaultCellularDevicePath,
-      callback,
-      "org.chromium.flimflam.Error.PinRequired",
-      "A PIN error has occurred.");
-  EXPECT_EQ(NetworkDeviceHandler::kErrorPinRequired, result_);
-
-  network_device_handler_->HandleShillCallFailureForTest(
-      kDefaultCellularDevicePath,
-      callback,
-      "org.chromium.flimflam.Error.WorldExploded",
-      "The earth is no more.");
-  EXPECT_EQ(NetworkDeviceHandler::kErrorUnknown, result_);
-}
-
 TEST_F(NetworkDeviceHandlerTest, GetDeviceProperties) {
   network_device_handler_->GetDeviceProperties(
-      kDefaultWifiDevicePath,
-      properties_success_callback_,
-      error_callback_);
+      kDefaultWifiDevicePath, properties_success_callback_, error_callback_);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kResultSuccess, result_);
   std::string type;
@@ -175,10 +105,9 @@ TEST_F(NetworkDeviceHandlerTest, GetDeviceProperties) {
 
 TEST_F(NetworkDeviceHandlerTest, SetDeviceProperty) {
   // Check that GetDeviceProperties returns the expected initial values.
-  network_device_handler_->GetDeviceProperties(
-      kDefaultCellularDevicePath,
-      properties_success_callback_,
-      error_callback_);
+  network_device_handler_->GetDeviceProperties(kDefaultCellularDevicePath,
+                                               properties_success_callback_,
+                                               error_callback_);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kResultSuccess, result_);
   bool allow_roaming;
@@ -199,10 +128,9 @@ TEST_F(NetworkDeviceHandlerTest, SetDeviceProperty) {
   EXPECT_EQ(kResultSuccess, result_);
 
   // GetDeviceProperties should return the value set by SetDeviceProperty.
-  network_device_handler_->GetDeviceProperties(
-      kDefaultCellularDevicePath,
-      properties_success_callback_,
-      error_callback_);
+  network_device_handler_->GetDeviceProperties(kDefaultCellularDevicePath,
+                                               properties_success_callback_,
+                                               error_callback_);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kResultSuccess, result_);
   EXPECT_TRUE(properties_->GetBooleanWithoutPathExpansion(
@@ -211,7 +139,7 @@ TEST_F(NetworkDeviceHandlerTest, SetDeviceProperty) {
 
   // Set property on an invalid path.
   network_device_handler_->SetDeviceProperty(
-      "/device/invalid_path",
+      kUnknownCellularDevicePath,
       shill::kCellularAllowRoamingProperty,
       allow_roaming_value,
       success_callback_,
@@ -222,9 +150,7 @@ TEST_F(NetworkDeviceHandlerTest, SetDeviceProperty) {
 
 TEST_F(NetworkDeviceHandlerTest, RequestRefreshIPConfigs) {
   network_device_handler_->RequestRefreshIPConfigs(
-      kDefaultWifiDevicePath,
-      success_callback_,
-      error_callback_);
+      kDefaultWifiDevicePath, success_callback_, error_callback_);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kResultSuccess, result_);
   // TODO(stevenjb): Add test interface to ShillIPConfigClient and test
@@ -236,49 +162,37 @@ TEST_F(NetworkDeviceHandlerTest, SetCarrier) {
 
   // Test that the success callback gets called.
   network_device_handler_->SetCarrier(
-      kDefaultCellularDevicePath,
-      kCarrier,
-      success_callback_,
-      error_callback_);
+      kDefaultCellularDevicePath, kCarrier, success_callback_, error_callback_);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kResultSuccess, result_);
 
-  // Test that the shill error gets properly translated and propagates to the
-  // error callback.
+  // Test that the shill error propagates to the error callback.
   network_device_handler_->SetCarrier(
-      kDefaultCellularDevicePath,
-      kCarrier,
-      GetErrorInvokingCallback(kDefaultCellularDevicePath,
-                               "org.chromium.flimflam.Error.NotSupported"),
-      error_callback_);
+      kUnknownCellularDevicePath, kCarrier, success_callback_, error_callback_);
   message_loop_.RunUntilIdle();
-  EXPECT_EQ(NetworkDeviceHandler::kErrorNotSupported, result_);
+  EXPECT_EQ(NetworkDeviceHandler::kErrorFailure, result_);
 }
 
 TEST_F(NetworkDeviceHandlerTest, RequirePin) {
   const char kPin[] = "1234";
 
   // Test that the success callback gets called.
-  network_device_handler_->RequirePin(
-      kDefaultCellularDevicePath,
-      true,
-      kPin,
-      success_callback_,
-      error_callback_);
+  network_device_handler_->RequirePin(kDefaultCellularDevicePath,
+                                      true,
+                                      kPin,
+                                      success_callback_,
+                                      error_callback_);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kResultSuccess, result_);
 
-  // Test that the shill error gets properly translated and propagates to the
-  // error callback.
-  network_device_handler_->RequirePin(
-      kDefaultCellularDevicePath,
-      true,
-      kPin,
-      GetErrorInvokingCallback(kDefaultCellularDevicePath,
-                               "org.chromium.flimflam.Error.IncorrectPin"),
-      error_callback_);
+  // Test that the shill error propagates to the error callback.
+  network_device_handler_->RequirePin(kUnknownCellularDevicePath,
+                                      true,
+                                      kPin,
+                                      success_callback_,
+                                      error_callback_);
   message_loop_.RunUntilIdle();
-  EXPECT_EQ(NetworkDeviceHandler::kErrorIncorrectPin, result_);
+  EXPECT_EQ(NetworkDeviceHandler::kErrorFailure, result_);
 }
 
 TEST_F(NetworkDeviceHandlerTest, EnterPin) {
@@ -286,23 +200,15 @@ TEST_F(NetworkDeviceHandlerTest, EnterPin) {
 
   // Test that the success callback gets called.
   network_device_handler_->EnterPin(
-      kDefaultCellularDevicePath,
-      kPin,
-      success_callback_,
-      error_callback_);
+      kDefaultCellularDevicePath, kPin, success_callback_, error_callback_);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kResultSuccess, result_);
 
-  // Test that the shill error gets properly translated and propagates to the
-  // error callback.
+  // Test that the shill error propagates to the error callback.
   network_device_handler_->EnterPin(
-      kDefaultCellularDevicePath,
-      kPin,
-      GetErrorInvokingCallback(kDefaultCellularDevicePath,
-                               "org.chromium.flimflam.Error.IncorrectPin"),
-      error_callback_);
+      kUnknownCellularDevicePath, kPin, success_callback_, error_callback_);
   message_loop_.RunUntilIdle();
-  EXPECT_EQ(NetworkDeviceHandler::kErrorIncorrectPin, result_);
+  EXPECT_EQ(NetworkDeviceHandler::kErrorFailure, result_);
 }
 
 TEST_F(NetworkDeviceHandlerTest, UnblockPin) {
@@ -310,26 +216,22 @@ TEST_F(NetworkDeviceHandlerTest, UnblockPin) {
   const char kPin[] = "1234";
 
   // Test that the success callback gets called.
-  network_device_handler_->UnblockPin(
-      kDefaultCellularDevicePath,
-      kPin,
-      kPuk,
-      success_callback_,
-      error_callback_);
+  network_device_handler_->UnblockPin(kDefaultCellularDevicePath,
+                                      kPin,
+                                      kPuk,
+                                      success_callback_,
+                                      error_callback_);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kResultSuccess, result_);
 
-  // Test that the shill error gets properly translated and propagates to the
-  // error callback.
-  network_device_handler_->UnblockPin(
-      kDefaultCellularDevicePath,
-      kPin,
-      kPuk,
-      GetErrorInvokingCallback(kDefaultCellularDevicePath,
-                               "org.chromium.flimflam.Error.PinRequired"),
-      error_callback_);
+  // Test that the shill error propagates to the error callback.
+  network_device_handler_->UnblockPin(kUnknownCellularDevicePath,
+                                      kPin,
+                                      kPuk,
+                                      success_callback_,
+                                      error_callback_);
   message_loop_.RunUntilIdle();
-  EXPECT_EQ(NetworkDeviceHandler::kErrorPinRequired, result_);
+  EXPECT_EQ(NetworkDeviceHandler::kErrorFailure, result_);
 }
 
 TEST_F(NetworkDeviceHandlerTest, ChangePin) {
@@ -337,26 +239,22 @@ TEST_F(NetworkDeviceHandlerTest, ChangePin) {
   const char kNewPin[] = "1234";
 
   // Test that the success callback gets called.
-  network_device_handler_->ChangePin(
-      kDefaultCellularDevicePath,
-      kOldPin,
-      kNewPin,
-      success_callback_,
-      error_callback_);
+  network_device_handler_->ChangePin(kDefaultCellularDevicePath,
+                                     kOldPin,
+                                     kNewPin,
+                                     success_callback_,
+                                     error_callback_);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kResultSuccess, result_);
 
-  // Test that the shill error gets properly translated and propagates to the
-  // error callback.
-  network_device_handler_->ChangePin(
-      kDefaultCellularDevicePath,
-      kOldPin,
-      kNewPin,
-      GetErrorInvokingCallback(kDefaultCellularDevicePath,
-                               "org.chromium.flimflam.Error.PinBlocked"),
-      error_callback_);
+  // Test that the shill error propagates to the error callback.
+  network_device_handler_->ChangePin(kUnknownCellularDevicePath,
+                                     kOldPin,
+                                     kNewPin,
+                                     success_callback_,
+                                     error_callback_);
   message_loop_.RunUntilIdle();
-  EXPECT_EQ(NetworkDeviceHandler::kErrorPinBlocked, result_);
+  EXPECT_EQ(NetworkDeviceHandler::kErrorFailure, result_);
 }
 
 }  // namespace chromeos