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..00266cb5f510fedddda8c7bb0a6a43c6f88eaf9d 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"
@@ -146,6 +147,8 @@ class TetherConnectorTest : public NetworkStateTest {
     fake_host_scan_cache_ = base::MakeUnique<FakeHostScanCache>();
     fake_notification_presenter_ =
         base::MakeUnique<FakeNotificationPresenter>();
+    mock_host_connection_metrics_logger_ =
+        base::MakeUnique<MockHostConnectionMetricsLogger>();
 
     result_.clear();
 
@@ -155,7 +158,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();
   }
@@ -246,6 +250,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<MockHostConnectionMetricsLogger>

[Kyle Horimoto, 2017/07/05 21:49:13]

Please make this a std::unique_ptr<StrictMock<MockHostConnectionMetricsLogger>>
so that we can ensure the correct mock functions are called in all cases.

You should add a "using testing::StrictMock;" at the top of the test file.

[Ryan Hansberry, 2017/07/07 02:33:16]

Done.

+      mock_host_connection_metrics_logger_;
 
   std::string result_;
 
@@ -303,6 +309,11 @@ TEST_F(TetherConnectorTest, TestCancelWhileOperationActive) {
 }
 
 TEST_F(TetherConnectorTest, TestConnectTetheringOperationFails) {
+  EXPECT_CALL(*mock_host_connection_metrics_logger_,
+              RecordConnectionToHostResult(
+                  HostConnectionMetricsLogger::ConnectionToHostResult::
+                      CONNECTION_RESULT_FAILURE_UNKNOWN_ERROR));
+
   CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
   EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTING,
             fake_active_host_->GetActiveHostStatus());
@@ -330,6 +341,11 @@ TEST_F(TetherConnectorTest, TestConnectTetheringOperationFails) {
 }
 
 TEST_F(TetherConnectorTest, TestConnectTetheringOperationFails_SetupRequired) {
+  EXPECT_CALL(*mock_host_connection_metrics_logger_,
+              RecordConnectionToHostResult(
+                  HostConnectionMetricsLogger::ConnectionToHostResult::
+                      CONNECTION_RESULT_FAILURE_UNKNOWN_ERROR));
+
   EXPECT_FALSE(
       fake_notification_presenter_->is_setup_required_notification_shown());
 
@@ -355,6 +371,78 @@ TEST_F(TetherConnectorTest, TestConnectTetheringOperationFails_SetupRequired) {
   EXPECT_EQ(NetworkConnectionHandler::kErrorConnectFailed, GetResultAndReset());
 }
 
+TEST_F(TetherConnectorTest, TestProvisioningFailureOnHost) {

[Kyle Horimoto, 2017/07/05 21:49:13]

This seems like TestConnectTetheringOperationFails() and
TestTetheringTimeoutOnHost() except with a different error value. Can you make a
helper function and call it from these tests?

[Ryan Hansberry, 2017/07/07 02:33:16]

Good suggestion. Done.

+  EXPECT_CALL(*mock_host_connection_metrics_logger_,
+              RecordConnectionToHostResult(
+                  HostConnectionMetricsLogger::ConnectionToHostResult::
+                      CONNECTION_RESULT_PROVISIONING_FAILED));
+
+  CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
+
+  fake_tether_host_fetcher_->InvokePendingCallbacks();
+
+  fake_operation_factory_->created_operations()[0]->SendFailedResponse(
+      ConnectTetheringResponse_ResponseCode::
+          ConnectTetheringResponse_ResponseCode_PROVISIONING_FAILED);
+
+  EXPECT_EQ(NetworkConnectionHandler::kErrorConnectFailed, GetResultAndReset());
+}
+
+TEST_F(TetherConnectorTest, TestTetheringTimeoutOnHost) {
+  EXPECT_CALL(
+      *mock_host_connection_metrics_logger_,
+      RecordConnectionToHostResult(
+          HostConnectionMetricsLogger::ConnectionToHostResult::
+              CONNECTION_RESULT_FAILURE_TETHERING_TIMED_OUT_FIRST_TIME_SETUP_WAS_NOT_REQUIRED));
+
+  CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
+
+  fake_tether_host_fetcher_->InvokePendingCallbacks();
+
+  EXPECT_FALSE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
+  EXPECT_FALSE(
+      fake_operation_factory_->created_operations()[0]->setup_required());
+
+  fake_operation_factory_->created_operations()[0]->SendFailedResponse(
+      ConnectTetheringResponse_ResponseCode::
+          ConnectTetheringResponse_ResponseCode_TETHERING_TIMEOUT);
+
+  EXPECT_EQ(NetworkConnectionHandler::kErrorConnectFailed, GetResultAndReset());
+}
+
+TEST_F(TetherConnectorTest, TestTetheringTimeoutOnHost_SetupRequired) {

[Kyle Horimoto, 2017/07/05 21:49:13]

Likewise, this seems like the same thing as
TestConnectTetheringOperationFails_SetupRequired() except with a different error
value. Can you share code?

[Ryan Hansberry, 2017/07/07 02:33:16]

Done.

+  EXPECT_CALL(
+      *mock_host_connection_metrics_logger_,
+      RecordConnectionToHostResult(
+          HostConnectionMetricsLogger::ConnectionToHostResult::
+              CONNECTION_RESULT_FAILURE_TETHERING_TIMED_OUT_FIRST_TIME_SETUP_WAS_REQUIRED));
+
+  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());
+
+  fake_operation_factory_->created_operations()[0]->SendFailedResponse(
+      ConnectTetheringResponse_ResponseCode::
+          ConnectTetheringResponse_ResponseCode_TETHERING_TIMEOUT);
+
+  EXPECT_FALSE(
+      fake_notification_presenter_->is_setup_required_notification_shown());
+
+  EXPECT_EQ(NetworkConnectionHandler::kErrorConnectFailed, GetResultAndReset());
+}
+
 TEST_F(TetherConnectorTest, TestConnectingToWifiFails) {
   CallConnect(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
   EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTING,
@@ -466,6 +554,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());