Tether: record each type of host connection result.

chromeos/components/tether/tether_connector_unittest.cc

Index: chromeos/components/tether/tether_connector_unittest.cc
diff --git a/chromeos/components/tether/tether_connector_unittest.cc b/chromeos/components/tether/tether_connector_unittest.cc
index 1398d2aa9986ace6df9d43ed59884c68d6466ff1..0a955d8e459c9807e9a14ad11bb56d66fa79eda8 100644
--- a/chromeos/components/tether/tether_connector_unittest.cc
+++ b/chromeos/components/tether/tether_connector_unittest.cc
@@ -14,6 +14,7 @@
 #include "chromeos/components/tether/fake_notification_presenter.h"
 #include "chromeos/components/tether/fake_tether_host_fetcher.h"
 #include "chromeos/components/tether/fake_wifi_hotspot_connector.h"
+#include "chromeos/components/tether/mock_host_connection_metrics_logger.h"
 #include "chromeos/components/tether/mock_tether_host_response_recorder.h"
 #include "chromeos/components/tether/tether_connector.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -27,6 +28,8 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
 
+using testing::StrictMock;
+
 namespace chromeos {
 
 namespace tether {
@@ -146,6 +149,8 @@ class TetherConnectorTest : public NetworkStateTest {
     fake_host_scan_cache_ = base::MakeUnique<FakeHostScanCache>();
     fake_notification_presenter_ =
         base::MakeUnique<FakeNotificationPresenter>();
+    mock_host_connection_metrics_logger_ =
+        base::WrapUnique(new StrictMock<MockHostConnectionMetricsLogger>);
 
     result_.clear();
 
@@ -155,7 +160,8 @@ class TetherConnectorTest : public NetworkStateTest {
         fake_ble_connection_manager_.get(),
         mock_tether_host_response_recorder_.get(),
         device_id_tether_network_guid_map_.get(), fake_host_scan_cache_.get(),
-        fake_notification_presenter_.get()));
+        fake_notification_presenter_.get(),
+        mock_host_connection_metrics_logger_.get()));
 
     SetUpTetherNetworks();
   }
@@ -225,6 +231,56 @@ class TetherConnectorTest : public NetworkStateTest {
                    base::Unretained(this)));
   }
 
+  void TestConnectTetheringOperationFails(

[Kyle Horimoto, 2017/07/12 17:23:21]

VerifyConnectTetheringOperationFails

[Ryan Hansberry, 2017/07/12 17:51:17]

Done.

+      ConnectTetheringResponse_ResponseCode response_code,
+      bool setup_required,
+      HostConnectionMetricsLogger::ConnectionToHostResult expected_event_type) {
+    EXPECT_CALL(*mock_host_connection_metrics_logger_,
+                RecordConnectionToHostResult(expected_event_type));
+
+    EXPECT_FALSE(
+        fake_notification_presenter_->is_setup_required_notification_shown());
+
+    cryptauth::RemoteDevice test_device = test_devices_[setup_required ? 1 : 0];

[Kyle Horimoto, 2017/07/12 17:23:21]

It's hard to tell why setup_required determines the index used without reading
through the whole test. Add a comment explaining that this corresponds to the
AddTetherNetwork() call in SetUpTetherNetworks() above.

[Ryan Hansberry, 2017/07/12 17:51:17]

Done.

+
+    CallConnect(GetTetherNetworkGuid(test_device.GetDeviceId()));
+    EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTING,
+              fake_active_host_->GetActiveHostStatus());
+    EXPECT_EQ(test_device.GetDeviceId(),
+              fake_active_host_->GetActiveHostDeviceId());
+    EXPECT_EQ(GetTetherNetworkGuid(test_device.GetDeviceId()),
+              fake_active_host_->GetTetherNetworkGuid());
+    EXPECT_TRUE(fake_active_host_->GetWifiNetworkGuid().empty());
+
+    EXPECT_EQ(
+        setup_required,
+        fake_notification_presenter_->is_setup_required_notification_shown());
+
+    fake_tether_host_fetcher_->InvokePendingCallbacks();
+
+    EXPECT_EQ(
+        setup_required,
+        fake_notification_presenter_->is_setup_required_notification_shown());
+    EXPECT_EQ(
+        setup_required,
+        fake_operation_factory_->created_operations()[0]->setup_required());
+
+    // Simulate a failed connection attempt (either the host cannot provide
+    // tethering at this time or a timeout occurs).
+    EXPECT_EQ(1u, fake_operation_factory_->created_operations().size());
+    fake_operation_factory_->created_operations()[0]->SendFailedResponse(
+        response_code);
+
+    EXPECT_FALSE(
+        fake_notification_presenter_->is_setup_required_notification_shown());
+
+    // The failure should have resulted in the host being disconnected.
+    EXPECT_EQ(ActiveHost::ActiveHostStatus::DISCONNECTED,
+              fake_active_host_->GetActiveHostStatus());
+    EXPECT_EQ(NetworkConnectionHandler::kErrorConnectFailed,
+              GetResultAndReset());
+  }
+
   std::string GetResultAndReset() {
     std::string result;
     result.swap(result_);
@@ -246,6 +302,8 @@ class TetherConnectorTest : public NetworkStateTest {
       device_id_tether_network_guid_map_;
   std::unique_ptr<FakeHostScanCache> fake_host_scan_cache_;
   std::unique_ptr<FakeNotificationPresenter> fake_notification_presenter_;
+  std::unique_ptr<StrictMock<MockHostConnectionMetricsLogger>>
+      mock_host_connection_metrics_logger_;
 
   std::string result_;
 
@@ -256,6 +314,12 @@ class TetherConnectorTest : public NetworkStateTest {
 };
 
 TEST_F(TetherConnectorTest, TestCannotFetchDevice) {
+  EXPECT_CALL(
+      *mock_host_connection_metrics_logger_,
+      RecordConnectionToHostResult(
+          HostConnectionMetricsLogger::ConnectionToHostResult::
+              CONNECTION_RESULT_FAILURE_CLIENT_CONNECTION_INTERNAL_ERROR));
+
   // Base64-encoded version of "nonexistentDeviceId".
   const char kNonexistentDeviceId[] = "bm9uZXhpc3RlbnREZXZpY2VJZA==";
 
@@ -277,6 +341,12 @@ TEST_F(TetherConnectorTest, TestCannotFetchDevice) {
 }
 
 TEST_F(TetherConnectorTest, TestCancelWhileOperationActive) {
+  EXPECT_CALL(
+      *mock_host_connection_metrics_logger_,
+      RecordConnectionToHostResult(
+          HostConnectionMetricsLogger::ConnectionToHostResult::
+              CONNECTION_RESULT_FAILURE_CLIENT_CONNECTION_CANCELED_BY_USER));
+
   CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
   EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTING,
             fake_active_host_->GetActiveHostStatus());
@@ -302,60 +372,61 @@ TEST_F(TetherConnectorTest, TestCancelWhileOperationActive) {
             GetResultAndReset());
 }
 
-TEST_F(TetherConnectorTest, TestConnectTetheringOperationFails) {
-  CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
-  EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTING,
-            fake_active_host_->GetActiveHostStatus());
-  EXPECT_EQ(test_devices_[0].GetDeviceId(),
-            fake_active_host_->GetActiveHostDeviceId());
-  EXPECT_EQ(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()),
-            fake_active_host_->GetTetherNetworkGuid());
-  EXPECT_TRUE(fake_active_host_->GetWifiNetworkGuid().empty());
-
-  fake_tether_host_fetcher_->InvokePendingCallbacks();
-
-  // Simulate a failed connection attempt (either the host cannot provide
-  // tethering at this time or a timeout occurs).
-  EXPECT_EQ(1u, fake_operation_factory_->created_operations().size());
-  EXPECT_FALSE(
-      fake_operation_factory_->created_operations()[0]->setup_required());
-  fake_operation_factory_->created_operations()[0]->SendFailedResponse(
+TEST_F(TetherConnectorTest,
+       TestConnectTetheringOperationFails_SetupNotRequired) {
+  TestConnectTetheringOperationFails(
       ConnectTetheringResponse_ResponseCode::
-          ConnectTetheringResponse_ResponseCode_UNKNOWN_ERROR);
-
-  // The failure should have resulted in the host being disconnected.
-  EXPECT_EQ(ActiveHost::ActiveHostStatus::DISCONNECTED,
-            fake_active_host_->GetActiveHostStatus());
-  EXPECT_EQ(NetworkConnectionHandler::kErrorConnectFailed, GetResultAndReset());
+          ConnectTetheringResponse_ResponseCode_UNKNOWN_ERROR,
+      false /* setup_required */,
+      HostConnectionMetricsLogger::ConnectionToHostResult::
+          CONNECTION_RESULT_FAILURE_UNKNOWN_ERROR);
 }
 
 TEST_F(TetherConnectorTest, TestConnectTetheringOperationFails_SetupRequired) {
-  EXPECT_FALSE(
-      fake_notification_presenter_->is_setup_required_notification_shown());
-
-  CallConnect(GetTetherNetworkGuid(test_devices_[1].GetDeviceId()));
-
-  EXPECT_TRUE(
-      fake_notification_presenter_->is_setup_required_notification_shown());
-
-  fake_tether_host_fetcher_->InvokePendingCallbacks();
-
-  EXPECT_TRUE(
-      fake_notification_presenter_->is_setup_required_notification_shown());
-  EXPECT_TRUE(
-      fake_operation_factory_->created_operations()[0]->setup_required());
+  TestConnectTetheringOperationFails(
+      ConnectTetheringResponse_ResponseCode::
+          ConnectTetheringResponse_ResponseCode_UNKNOWN_ERROR,
+      true /* setup_required */,
+      HostConnectionMetricsLogger::ConnectionToHostResult::
+          CONNECTION_RESULT_FAILURE_UNKNOWN_ERROR);
+}
 
-  fake_operation_factory_->created_operations()[0]->SendFailedResponse(
+TEST_F(TetherConnectorTest,
+       TestConnectTetheringOperationFails_ProvisioningFailed) {
+  TestConnectTetheringOperationFails(
       ConnectTetheringResponse_ResponseCode::
-          ConnectTetheringResponse_ResponseCode_UNKNOWN_ERROR);
+          ConnectTetheringResponse_ResponseCode_PROVISIONING_FAILED,
+      false /* setup_required */,
+      HostConnectionMetricsLogger::ConnectionToHostResult::
+          CONNECTION_RESULT_PROVISIONING_FAILED);
+}
 
-  EXPECT_FALSE(
-      fake_notification_presenter_->is_setup_required_notification_shown());
+TEST_F(TetherConnectorTest,
+       TestConnectTetheringOperationFails_TetheringTimeout_SetupNotRequired) {
+  TestConnectTetheringOperationFails(
+      ConnectTetheringResponse_ResponseCode::
+          ConnectTetheringResponse_ResponseCode_TETHERING_TIMEOUT,
+      false /* setup_required */,
+      HostConnectionMetricsLogger::ConnectionToHostResult::
+          CONNECTION_RESULT_FAILURE_TETHERING_TIMED_OUT_FIRST_TIME_SETUP_WAS_NOT_REQUIRED);
+}
 
-  EXPECT_EQ(NetworkConnectionHandler::kErrorConnectFailed, GetResultAndReset());
+TEST_F(TetherConnectorTest,
+       TestConnectTetheringOperationFails_TetheringTimeout_SetupRequired) {
+  TestConnectTetheringOperationFails(
+      ConnectTetheringResponse_ResponseCode::
+          ConnectTetheringResponse_ResponseCode_TETHERING_TIMEOUT,
+      true /* setup_required */,
+      HostConnectionMetricsLogger::ConnectionToHostResult::
+          CONNECTION_RESULT_FAILURE_TETHERING_TIMED_OUT_FIRST_TIME_SETUP_WAS_REQUIRED);
 }
 
 TEST_F(TetherConnectorTest, TestConnectingToWifiFails) {
+  EXPECT_CALL(*mock_host_connection_metrics_logger_,
+              RecordConnectionToHostResult(
+                  HostConnectionMetricsLogger::ConnectionToHostResult::
+                      CONNECTION_RESULT_FAILURE_CLIENT_CONNECTION_TIMEOUT));
+
   CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
   EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTING,
             fake_active_host_->GetActiveHostStatus());
@@ -392,6 +463,12 @@ TEST_F(TetherConnectorTest, TestConnectingToWifiFails) {
 }
 
 TEST_F(TetherConnectorTest, TestCancelWhileConnectingToWifi) {
+  EXPECT_CALL(
+      *mock_host_connection_metrics_logger_,
+      RecordConnectionToHostResult(
+          HostConnectionMetricsLogger::ConnectionToHostResult::
+              CONNECTION_RESULT_FAILURE_CLIENT_CONNECTION_CANCELED_BY_USER));
+
   CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
   EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTING,
             fake_active_host_->GetActiveHostStatus());
@@ -422,6 +499,11 @@ TEST_F(TetherConnectorTest, TestCancelWhileConnectingToWifi) {
 }
 
 TEST_F(TetherConnectorTest, TestSuccessfulConnection) {
+  EXPECT_CALL(*mock_host_connection_metrics_logger_,
+              RecordConnectionToHostResult(
+                  HostConnectionMetricsLogger::ConnectionToHostResult::
+                      CONNECTION_RESULT_SUCCESS));
+
   CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
   EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTING,
             fake_active_host_->GetActiveHostStatus());
@@ -466,6 +548,11 @@ TEST_F(TetherConnectorTest, TestSuccessfulConnection) {
 }
 
 TEST_F(TetherConnectorTest, TestSuccessfulConnection_SetupRequired) {
+  EXPECT_CALL(*mock_host_connection_metrics_logger_,
+              RecordConnectionToHostResult(
+                  HostConnectionMetricsLogger::ConnectionToHostResult::
+                      CONNECTION_RESULT_SUCCESS));
+
   EXPECT_FALSE(
       fake_notification_presenter_->is_setup_required_notification_shown());
 
@@ -497,6 +584,12 @@ TEST_F(TetherConnectorTest, TestSuccessfulConnection_SetupRequired) {
 
 TEST_F(TetherConnectorTest,
        TestNewConnectionAttemptDuringFetch_DifferentDevice) {
+  EXPECT_CALL(
+      *mock_host_connection_metrics_logger_,
+      RecordConnectionToHostResult(
+          HostConnectionMetricsLogger::ConnectionToHostResult::
+              CONNECTION_RESULT_FAILURE_CLIENT_CONNECTION_CANCELED_BY_USER));
+
   CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
 
   // Instead of invoking the pending callbacks on |fake_tether_host_fetcher_|,
@@ -518,6 +611,12 @@ TEST_F(TetherConnectorTest,
 
 TEST_F(TetherConnectorTest,
        TestNewConnectionAttemptDuringOperation_DifferentDevice) {
+  EXPECT_CALL(
+      *mock_host_connection_metrics_logger_,
+      RecordConnectionToHostResult(
+          HostConnectionMetricsLogger::ConnectionToHostResult::
+              CONNECTION_RESULT_FAILURE_CLIENT_CONNECTION_CANCELED_BY_USER));
+
   CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
   EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTING,
             fake_active_host_->GetActiveHostStatus());
@@ -570,6 +669,12 @@ TEST_F(TetherConnectorTest,
 
 TEST_F(TetherConnectorTest,
        TestNewConnectionAttemptDuringWifiConnection_DifferentDevice) {
+  EXPECT_CALL(
+      *mock_host_connection_metrics_logger_,
+      RecordConnectionToHostResult(
+          HostConnectionMetricsLogger::ConnectionToHostResult::
+              CONNECTION_RESULT_FAILURE_CLIENT_CONNECTION_CANCELED_BY_USER));
+
   CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
   EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTING,
             fake_active_host_->GetActiveHostStatus());