Merged Tether and cellular network types in System Tray.

chrome/browser/chromeos/tether/tether_service_unittest.cc

 // Copyright 2017 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/chromeos/tether/tether_service.h"
 
 #include <memory>
 
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
@@ skipping 256 matching lines @@
       chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_PROHIBITED,
       network_state_handler()->GetTechnologyState(
           chromeos::NetworkTypePattern::Tether()));
 }
 
 TEST_F(TetherServiceTest, TestBluetoothIsNotPowered) {
   ON_CALL(*mock_adapter_, IsPowered()).WillByDefault(Return(false));
 
   CreateTetherService();
 
+  base::RunLoop().RunUntilIdle();

[Ryan Hansberry, 2017/05/23 01:14:29]

Is this necessary?

[stevenjb, 2017/05/23 22:11:07]

Maybe add a comment. The NetworkHandler code is largely asyncronous, so if
CreateTetherService calls into it at all to set state, we may need to run the
message loop until all asynchronous calls have been competed.

It might be better to just do this in CreateTetherService().

[lesliewatkins, 2017/05/26 23:49:32]

This one isn't-- it got added by accident!

+
   EXPECT_EQ(
       chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_UNINITIALIZED,
       network_state_handler()->GetTechnologyState(
           chromeos::NetworkTypePattern::Tether()));
 }
 
+TEST_F(TetherServiceTest, TestCellularIsUnavailable) {
+  test_manager_client()->RemoveTechnology(shill::kTypeCellular);
+  ASSERT_EQ(
+      chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_UNAVAILABLE,
+      network_state_handler()->GetTechnologyState(
+          chromeos::NetworkTypePattern::Cellular()));
+
+  CreateTetherService();
+
+  SetTetherTechnologyStateEnabled(false);
+  EXPECT_EQ(
+      chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_AVAILABLE,
+      network_state_handler()->GetTechnologyState(
+          chromeos::NetworkTypePattern::Tether()));
+
+  SetTetherTechnologyStateEnabled(true);
+  EXPECT_EQ(chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_ENABLED,
+            network_state_handler()->GetTechnologyState(
+                chromeos::NetworkTypePattern::Tether()));
+}
+
+TEST_F(TetherServiceTest, TestCellularIsAvailable) {
+  // TODO (lesliewatkins): Investigate why cellular needs to be removed and
+  // re-added for NetworkStateHandler to return the correct TechnologyState.
+  test_manager_client()->RemoveTechnology(shill::kTypeCellular);
+  test_manager_client()->AddTechnology(shill::kTypeCellular, false);
+
+  CreateTetherService();
+
+  network_state_handler()->SetTechnologyEnabled(
+      chromeos::NetworkTypePattern::Cellular(), false,
+      chromeos::network_handler::ErrorCallback());
+
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(
+      chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_AVAILABLE,
+      network_state_handler()->GetTechnologyState(
+          chromeos::NetworkTypePattern::Cellular()));
+
+  SetTetherTechnologyStateEnabled(false);
+  EXPECT_EQ(
+      chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_UNINITIALIZED,
+      network_state_handler()->GetTechnologyState(
+          chromeos::NetworkTypePattern::Tether()));
+
+  SetTetherTechnologyStateEnabled(true);
+  EXPECT_EQ(
+      chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_UNINITIALIZED,
+      network_state_handler()->GetTechnologyState(
+          chromeos::NetworkTypePattern::Tether()));
+
+  network_state_handler()->SetTechnologyEnabled(

[Ryan Hansberry, 2017/05/23 01:14:29]

Please break out these repeated SetTechnologyEnabled and RunLoop calls into a
helper method.

[lesliewatkins, 2017/05/26 23:49:32]

Done.

+      chromeos::NetworkTypePattern::Cellular(), true,
+      chromeos::network_handler::ErrorCallback());
+  base::RunLoop().RunUntilIdle();

[Ryan Hansberry, 2017/05/23 01:14:29]

Are these RunLoop calls after calling on NetworkStateHandler necessary?

[stevenjb, 2017/05/23 22:11:07]

Se comment above. A SetTechnologyEnabled() wrapper that calls RunUntilIdle()
would make sense to me.

[lesliewatkins, 2017/05/26 23:49:32]

Yes, these are, because the call is still asynchronous. See
https://cs.chromium.org/chromium/src/chromeos/network/network_state_handler.h...

+  ASSERT_EQ(chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_ENABLED,
+            network_state_handler()->GetTechnologyState(
+                chromeos::NetworkTypePattern::Cellular()));
+
+  SetTetherTechnologyStateEnabled(false);
+  EXPECT_EQ(
+      chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_AVAILABLE,
+      network_state_handler()->GetTechnologyState(
+          chromeos::NetworkTypePattern::Tether()));
+
+  SetTetherTechnologyStateEnabled(true);
+  EXPECT_EQ(chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_ENABLED,
+            network_state_handler()->GetTechnologyState(
+                chromeos::NetworkTypePattern::Tether()));
+}
+
 TEST_F(TetherServiceTest, TestEnabled) {
   CreateTetherService();
 
   EXPECT_EQ(chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_ENABLED,
             network_state_handler()->GetTechnologyState(
                 chromeos::NetworkTypePattern::Tether()));
 
   SetTetherTechnologyStateEnabled(false);
   EXPECT_EQ(
       chromeos::NetworkStateHandler::TechnologyState::TECHNOLOGY_AVAILABLE,
@@ skipping 24 matching lines @@
   SetTetherTechnologyStateEnabled(false);
 
   EXPECT_EQ(1, tether_service_->updated_technology_state_count());
 
   SetTetherTechnologyStateEnabled(true);
   SetTetherTechnologyStateEnabled(true);
   SetTetherTechnologyStateEnabled(true);
 
   EXPECT_EQ(2, tether_service_->updated_technology_state_count());
 }