Clean up shill fake implementations

chromeos/network/network_state_handler_unittest.cc

 // Copyright (c) 2012 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 "chromeos/network/network_state_handler.h"
 
 #include <map>
 #include <set>
 #include <string>
 
@@ skipping 13 matching lines @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace {
 
 void ErrorCallbackFunction(const std::string& error_name,
                            const std::string& error_message) {
   LOG(ERROR) << "Shill Error: " << error_name << " : " << error_message;
 }
 
-const std::string kShillManagerClientStubDefaultService = "eth1";
-const std::string kShillManagerClientStubDefaultWifi = "wifi1";
-const std::string kShillManagerClientStubWifi2 = "wifi2";
-const std::string kShillManagerClientStubCellular = "cellular1";
+const std::string kShillManagerClientStubDefaultService = "/service/eth1";
+const std::string kShillManagerClientStubDefaultWifi = "/service/wifi1";
+const std::string kShillManagerClientStubWifi2 = "/service/wifi2";
+const std::string kShillManagerClientStubCellular = "/service/cellular1";
 
 using chromeos::NetworkState;
 using chromeos::NetworkStateHandler;
 
 class TestObserver : public chromeos::NetworkStateHandlerObserver {
  public:
   explicit TestObserver(NetworkStateHandler* handler)
       : handler_(handler),
         device_list_changed_count_(0),
+        device_count_(0),

[pneubeck (no reviews), 2014/06/04 09:30:27]

nit:
You could consider setting the absolute counters
{device_,network_,favorite_}count_ to a value that is clearly invalid, like -1.
Otherwise, it could appear in some test that there are indeed zero devices,
although the counts are not initialized yet (which happens e.g. in
*ListChanged()).

[stevenjb, 2014/06/04 19:49:58]

We don't ever currently test for 0 entries, and probably wouldn't. We also track
*list_changed_count for confirming that the update was called.

         network_count_(0),
         default_network_change_count_(0),
         favorite_count_(0) {
   }
 
   virtual ~TestObserver() {
   }
 
   virtual void DeviceListChanged() OVERRIDE {
+    NetworkStateHandler::DeviceStateList devices;
+    handler_->GetDeviceList(&devices);
+    device_count_ = devices.size();
     ++device_list_changed_count_;
   }
 
   virtual void NetworkListChanged() OVERRIDE {
     NetworkStateHandler::NetworkStateList networks;
     handler_->GetNetworkList(&networks);
     network_count_ = networks.size();
     if (network_count_ == 0) {
       default_network_ = "";
       default_network_connection_state_ = "";
@@ skipping 17 matching lines @@
     network_connection_state_[network->path()] = network->connection_state();
     connection_state_changes_[network->path()]++;
   }
 
   virtual void NetworkPropertiesUpdated(const NetworkState* network) OVERRIDE {
     DCHECK(network);
     property_updates_[network->path()]++;
   }
 
   size_t device_list_changed_count() { return device_list_changed_count_; }
+  size_t device_count() { return device_count_; }
   size_t network_count() { return network_count_; }
   size_t default_network_change_count() {
     return default_network_change_count_;
   }
-  void reset_network_change_count() {
-    DVLOG(1) << "ResetNetworkChangeCount";
+  void reset_change_counts() {
+    DVLOG(1) << "=== RESET CHANGE COUNTS ===";
     default_network_change_count_ = 0;
+    device_list_changed_count_ = 0;
   }
   std::string default_network() { return default_network_; }
   std::string default_network_connection_state() {
     return default_network_connection_state_;
   }
   size_t favorite_count() { return favorite_count_; }
 
   int PropertyUpdatesForService(const std::string& service_path) {
     return property_updates_[service_path];
   }
 
   int ConnectionStateChangesForService(const std::string& service_path) {
     return connection_state_changes_[service_path];
   }
 
   std::string NetworkConnectionStateForService(
       const std::string& service_path) {
     return network_connection_state_[service_path];
   }
 
  private:
   NetworkStateHandler* handler_;
   size_t device_list_changed_count_;
+  size_t device_count_;
   size_t network_count_;
   size_t default_network_change_count_;
   std::string default_network_;
   std::string default_network_connection_state_;
   size_t favorite_count_;
   std::map<std::string, int> property_updates_;
   std::map<std::string, int> connection_state_changes_;
   std::map<std::string, std::string> network_connection_state_;
 
   DISALLOW_COPY_AND_ASSIGN(TestObserver);
 };
 
 }  // namespace
 
 namespace chromeos {
 
 class NetworkStateHandlerTest : public testing::Test {
  public:
   NetworkStateHandlerTest()
       : device_test_(NULL),
         manager_test_(NULL),
         profile_test_(NULL),
         service_test_(NULL) {}
   virtual ~NetworkStateHandlerTest() {}
 
   virtual void SetUp() OVERRIDE {
     // Initialize DBusThreadManager with a stub implementation.
     DBusThreadManager::InitializeWithStub();
-    SetupNetworkStateHandler();
+    SetupDefaultShillState();
+    network_state_handler_.reset(new NetworkStateHandler);
+    test_observer_.reset(new TestObserver(network_state_handler_.get()));
+    network_state_handler_->AddObserver(test_observer_.get(), FROM_HERE);
+    network_state_handler_->InitShillPropertyHandler();
     message_loop_.RunUntilIdle();
   }
 
   virtual void TearDown() OVERRIDE {
     network_state_handler_->RemoveObserver(test_observer_.get(), FROM_HERE);
     test_observer_.reset();
     network_state_handler_.reset();
     DBusThreadManager::Shutdown();
   }
 
-  void SetupNetworkStateHandler() {
-    SetupDefaultShillState();
-    network_state_handler_.reset(new NetworkStateHandler);
-    test_observer_.reset(new TestObserver(network_state_handler_.get()));
-    network_state_handler_->AddObserver(test_observer_.get(), FROM_HERE);
-    network_state_handler_->InitShillPropertyHandler();
-  }
-
  protected:
   void AddService(const std::string& service_path,
                   const std::string& name,
                   const std::string& type,
                   const std::string& state) {
     service_test_->AddService(service_path, name, type, state,
                               true /* add_to_visible */,
                               true /* add_to_watchlist */);
   }
 
@@ skipping 16 matching lines @@
     profile_test_ =
         DBusThreadManager::Get()->GetShillProfileClient()->GetTestInterface();
     ASSERT_TRUE(profile_test_);
     profile_test_->ClearProfiles();
 
     service_test_ =
         DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
     ASSERT_TRUE(service_test_);
     service_test_->ClearServices();
     AddService(kShillManagerClientStubDefaultService,
-               kShillManagerClientStubDefaultService,
+               "eth1",
                shill::kTypeEthernet,
                shill::kStateOnline);
     AddService(kShillManagerClientStubDefaultWifi,
-               kShillManagerClientStubDefaultWifi,
+               "wifi1",
                shill::kTypeWifi,
                shill::kStateOnline);
     AddService(kShillManagerClientStubWifi2,
-               kShillManagerClientStubWifi2,
+               "wifi2",
                shill::kTypeWifi,
                shill::kStateIdle);
     AddService(kShillManagerClientStubCellular,
-               kShillManagerClientStubCellular,
+               "cellular1",
                shill::kTypeCellular,
                shill::kStateIdle);
   }
 
   void UpdateManagerProperties() {
     message_loop_.RunUntilIdle();
     network_state_handler_->UpdateManagerProperties();
     message_loop_.RunUntilIdle();
   }
 
   base::MessageLoopForUI message_loop_;
   scoped_ptr<NetworkStateHandler> network_state_handler_;
   scoped_ptr<TestObserver> test_observer_;
   ShillDeviceClient::TestInterface* device_test_;
   ShillManagerClient::TestInterface* manager_test_;
   ShillProfileClient::TestInterface* profile_test_;
   ShillServiceClient::TestInterface* service_test_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NetworkStateHandlerTest);
 };
 
 TEST_F(NetworkStateHandlerTest, NetworkStateHandlerStub) {
-  // Ensure that the network list is the expected size.
+  // Ensure that the device and network list are the expected size.
+  const size_t kNumShillManagerClientStubImplDevices = 2;
+  EXPECT_EQ(kNumShillManagerClientStubImplDevices,
+            test_observer_->device_count());
   const size_t kNumShillManagerClientStubImplServices = 4;
   EXPECT_EQ(kNumShillManagerClientStubImplServices,
             test_observer_->network_count());
   // Ensure that the first stub network is the default network.
   EXPECT_EQ(kShillManagerClientStubDefaultService,
             test_observer_->default_network());
+  ASSERT_TRUE(network_state_handler_->DefaultNetwork());
   EXPECT_EQ(kShillManagerClientStubDefaultService,
-            network_state_handler_->ConnectedNetworkByType(
-                NetworkTypePattern::Default())->path());
+            network_state_handler_->DefaultNetwork()->path());
   EXPECT_EQ(kShillManagerClientStubDefaultService,
             network_state_handler_->ConnectedNetworkByType(
                 NetworkTypePattern::Ethernet())->path());
   EXPECT_EQ(kShillManagerClientStubDefaultWifi,
             network_state_handler_->ConnectedNetworkByType(
                 NetworkTypePattern::WiFi())->path());
   EXPECT_EQ(kShillManagerClientStubCellular,
             network_state_handler_->FirstNetworkByType(
                 NetworkTypePattern::Mobile())->path());
   EXPECT_EQ(
       kShillManagerClientStubCellular,
       network_state_handler_->FirstNetworkByType(NetworkTypePattern::Cellular())
           ->path());
   EXPECT_EQ(shill::kStateOnline,
             test_observer_->default_network_connection_state());
 }
 
 TEST_F(NetworkStateHandlerTest, TechnologyChanged) {
-  // There may be several manager changes during initialization.
-  size_t initial_changed_count = test_observer_->device_list_changed_count();
-  // Disable a technology.
+  // Disable a technology. Will immediately set the state to AVAILABLE and
+  // notify observers.
+  test_observer_->reset_change_counts();

[pneubeck (no reviews), 2014/06/04 09:30:27]

maybe move this to the end of SetUp() in the fixture?

[stevenjb, 2014/06/04 19:49:58]

Done.

   network_state_handler_->SetTechnologyEnabled(
-      NetworkTypePattern::Wimax(), false, network_handler::ErrorCallback());
-  EXPECT_NE(
-      NetworkStateHandler::TECHNOLOGY_ENABLED,
-      network_state_handler_->GetTechnologyState(NetworkTypePattern::Wimax()));
-  EXPECT_EQ(initial_changed_count + 1,
-            test_observer_->device_list_changed_count());
-  // Enable a technology.
+      NetworkTypePattern::WiFi(), false, network_handler::ErrorCallback());
+  EXPECT_EQ(1u, test_observer_->device_list_changed_count());
+  EXPECT_EQ(
+      NetworkStateHandler::TECHNOLOGY_AVAILABLE,
+      network_state_handler_->GetTechnologyState(NetworkTypePattern::WiFi()));
+
+  // Run the message loop. An additional notification will be received when
+  // Shill updates the enabled technologies. The state should remain AVAILABLE.
+  test_observer_->reset_change_counts();
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(1u, test_observer_->device_list_changed_count());
+  EXPECT_EQ(
+      NetworkStateHandler::TECHNOLOGY_AVAILABLE,
+      network_state_handler_->GetTechnologyState(NetworkTypePattern::WiFi()));
+
+  // Enable a technology. Will immediately set the state to ENABLING and
+  // notify observers.
+  test_observer_->reset_change_counts();
   network_state_handler_->SetTechnologyEnabled(
-      NetworkTypePattern::Wimax(), true, network_handler::ErrorCallback());
-  // The technology state should immediately change to ENABLING and we should
-  // receive a manager changed callback.
-  EXPECT_EQ(initial_changed_count + 2,
-            test_observer_->device_list_changed_count());
+      NetworkTypePattern::WiFi(), true, network_handler::ErrorCallback());
+  EXPECT_EQ(1u, test_observer_->device_list_changed_count());
   EXPECT_EQ(
       NetworkStateHandler::TECHNOLOGY_ENABLING,
-      network_state_handler_->GetTechnologyState(NetworkTypePattern::Wimax()));
+      network_state_handler_->GetTechnologyState(NetworkTypePattern::WiFi()));
+
+  // Run the message loop. State should change to ENABLED.
+  test_observer_->reset_change_counts();
   message_loop_.RunUntilIdle();
-  // Ensure we receive 2 manager changed callbacks when the technology becomes
-  // avalable and enabled.
-  EXPECT_EQ(initial_changed_count + 4,
-            test_observer_->device_list_changed_count());
+  EXPECT_EQ(1u, test_observer_->device_list_changed_count());
   EXPECT_EQ(
       NetworkStateHandler::TECHNOLOGY_ENABLED,
-      network_state_handler_->GetTechnologyState(NetworkTypePattern::Wimax()));
+      network_state_handler_->GetTechnologyState(NetworkTypePattern::WiFi()));
 }
 
 TEST_F(NetworkStateHandlerTest, TechnologyState) {
   manager_test_->RemoveTechnology(shill::kTypeWimax);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(
       NetworkStateHandler::TECHNOLOGY_UNAVAILABLE,
       network_state_handler_->GetTechnologyState(NetworkTypePattern::Wimax()));
 
   manager_test_->AddTechnology(shill::kTypeWimax, false);
@@ skipping 99 matching lines @@
                                    connection_state_idle_value);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(2, test_observer_->ConnectionStateChangesForService(eth1));
 }
 
 TEST_F(NetworkStateHandlerTest, DefaultServiceDisconnected) {
   const std::string eth1 = kShillManagerClientStubDefaultService;
   const std::string wifi1 = kShillManagerClientStubDefaultWifi;
 
   // Disconnect ethernet.
-  test_observer_->reset_network_change_count();
+  test_observer_->reset_change_counts();
   base::StringValue connection_state_idle_value(shill::kStateIdle);
   service_test_->SetServiceProperty(eth1, shill::kStateProperty,
                                     connection_state_idle_value);
   message_loop_.RunUntilIdle();
   // Expect two changes: first when eth1 becomes disconnected, second when
   // wifi1 becomes the default.
   EXPECT_EQ(2u, test_observer_->default_network_change_count());
   EXPECT_EQ(wifi1, test_observer_->default_network());
 
   // Disconnect wifi.
-  test_observer_->reset_network_change_count();
+  test_observer_->reset_change_counts();
   service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
                                     connection_state_idle_value);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(1u, test_observer_->default_network_change_count());
   EXPECT_EQ("", test_observer_->default_network());
 }
 
 TEST_F(NetworkStateHandlerTest, DefaultServiceConnected) {
   const std::string eth1 = kShillManagerClientStubDefaultService;
   const std::string wifi1 = kShillManagerClientStubDefaultWifi;
 
   // Disconnect ethernet and wifi.
   base::StringValue connection_state_idle_value(shill::kStateIdle);
   service_test_->SetServiceProperty(eth1, shill::kStateProperty,
                                     connection_state_idle_value);
   service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
                                     connection_state_idle_value);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(std::string(), test_observer_->default_network());
 
   // Connect ethernet, should become the default network.
-  test_observer_->reset_network_change_count();
+  test_observer_->reset_change_counts();
   base::StringValue connection_state_ready_value(shill::kStateReady);
   service_test_->SetServiceProperty(eth1, shill::kStateProperty,
                                     connection_state_ready_value);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(eth1, test_observer_->default_network());
   EXPECT_EQ(shill::kStateReady,
             test_observer_->default_network_connection_state());
   EXPECT_EQ(1u, test_observer_->default_network_change_count());
 }
 
 TEST_F(NetworkStateHandlerTest, DefaultServiceChanged) {
   const std::string eth1 = kShillManagerClientStubDefaultService;
   // The default service should be eth1.
   EXPECT_EQ(eth1, test_observer_->default_network());
 
   // Change the default network by changing Manager.DefaultService.
-  test_observer_->reset_network_change_count();
+  test_observer_->reset_change_counts();
   const std::string wifi1 = kShillManagerClientStubDefaultWifi;
   base::StringValue wifi1_value(wifi1);
   manager_test_->SetManagerProperty(
       shill::kDefaultServiceProperty, wifi1_value);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(wifi1, test_observer_->default_network());
   EXPECT_EQ(1u, test_observer_->default_network_change_count());
 
   // Change the state of the default network.
-  test_observer_->reset_network_change_count();
+  test_observer_->reset_change_counts();
   base::StringValue connection_state_ready_value(shill::kStateReady);
   service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
                                    connection_state_ready_value);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(shill::kStateReady,
             test_observer_->default_network_connection_state());
   EXPECT_EQ(1u, test_observer_->default_network_change_count());
 
   // Updating a property on the default network should trigger
   // a default network change.
-  test_observer_->reset_network_change_count();
+  test_observer_->reset_change_counts();
   DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
       dbus::ObjectPath(wifi1),
       shill::kSecurityProperty, base::StringValue("TestSecurity"),
       base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
   message_loop_.RunUntilIdle();
   EXPECT_EQ(1u, test_observer_->default_network_change_count());
 
   // No default network updates for signal strength changes.
-  test_observer_->reset_network_change_count();
+  test_observer_->reset_change_counts();
   DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
       dbus::ObjectPath(wifi1),
       shill::kSignalStrengthProperty, base::FundamentalValue(32),
       base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
   message_loop_.RunUntilIdle();
   EXPECT_EQ(0u, test_observer_->default_network_change_count());
 }
 
 TEST_F(NetworkStateHandlerTest, RequestUpdate) {
   // Request an update for kShillManagerClientStubDefaultWifi.
   EXPECT_EQ(1, test_observer_->PropertyUpdatesForService(
       kShillManagerClientStubDefaultWifi));
   network_state_handler_->RequestUpdateForNetwork(
       kShillManagerClientStubDefaultWifi);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(
       kShillManagerClientStubDefaultWifi));
 }
 
 TEST_F(NetworkStateHandlerTest, NetworkGuidInProfile) {
   const std::string profile = "/profile/profile1";
-  const std::string wifi_path = "wifi_with_guid";
+  const std::string wifi_path = "/service/wifi_with_guid";
   const std::string wifi_guid = "WIFI_GUID";
   const bool is_service_configured = true;
 
   // Add a network to the default Profile with a specified GUID.
   service_test_->AddServiceWithIPConfig(
       wifi_path,
       wifi_guid,
       wifi_path  /* name */,
       shill::kTypeWifi,
       shill::kStateOnline,
@@ skipping 31 matching lines @@
   EXPECT_EQ(wifi_guid, network->guid());
 
   // Also verify FavoriteState (mostly to test the stub behavior).
   favorite = network_state_handler_->GetFavoriteStateFromServicePath(
       wifi_path, is_service_configured);
   ASSERT_TRUE(favorite);
   EXPECT_EQ(wifi_guid, favorite->guid());
 }
 
 TEST_F(NetworkStateHandlerTest, NetworkGuidNotInProfile) {
-  const std::string wifi_path = "wifi_with_guid";
+  const std::string wifi_path = "/service/wifi_with_guid";
   const bool is_service_configured = false;
 
   // Add a network without adding it to a profile.
   AddService(wifi_path, wifi_path, shill::kTypeWifi, shill::kStateOnline);
   UpdateManagerProperties();
 
   // Verify that a FavoriteState exists with an assigned GUID.
   const FavoriteState* favorite =
       network_state_handler_->GetFavoriteStateFromServicePath(
           wifi_path, is_service_configured);
@@ skipping 21 matching lines @@
   EXPECT_EQ(wifi_guid, network->guid());
 
   // Also verify FavoriteState (mostly to test the stub behavior).
   favorite = network_state_handler_->GetFavoriteStateFromServicePath(
       wifi_path, is_service_configured);
   ASSERT_TRUE(favorite);
   EXPECT_EQ(wifi_guid, favorite->guid());
 }
 
 }  // namespace chromeos