Clean up shill fake implementations

chromeos/network/shill_property_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/shill_property_handler.h"
 
 #include <map>
 #include <set>
 #include <string>
 
@@ skipping 60 matching lines @@
 
   virtual void UpdateIPConfigProperties(
       ManagedState::ManagedType type,
       const std::string& path,
       const std::string& ip_config_path,
       const base::DictionaryValue& properties) OVERRIDE {
     AddPropertyUpdate(shill::kIPConfigsProperty, ip_config_path);
   }
 
   virtual void TechnologyListChanged() OVERRIDE {
+    VLOG(1) << "TechnologyListChanged.";
     ++technology_list_updates_;
   }
 
   virtual void CheckPortalListChanged(
       const std::string& check_portal_list) OVERRIDE {
   }
 
   virtual void ManagedStateListChanged(
       ManagedState::ManagedType type) OVERRIDE {
     AddStateListUpdate(GetTypeString(type));
   }
 
   virtual void DefaultNetworkServiceChanged(
       const std::string& service_path) OVERRIDE {
   }
 
   std::vector<std::string>& entries(const std::string& type) {
     return entries_[type];
   }
   std::map<std::string, int>& property_updates(const std::string& type) {
     return property_updates_[type];
   }
   std::map<std::string, int>& initial_property_updates(
       const std::string& type) {
     return initial_property_updates_[type];
   }
   int list_updates(const std::string& type) { return list_updates_[type]; }
-  void reset_list_updates() { list_updates_.clear(); }
   int technology_list_updates() { return technology_list_updates_; }
+  void reset_list_updates() {
+    VLOG(1) << "=== RESET LIST UPDATES ===";
+    list_updates_.clear();
+    technology_list_updates_ = 0;
+  }
   int errors() { return errors_; }
 
  private:
   std::string GetTypeString(ManagedState::ManagedType type) {
     if (type == ManagedState::MANAGED_TYPE_NETWORK) {
       return shill::kServicesProperty;
     } else if (type == ManagedState::MANAGED_TYPE_FAVORITE) {
       return shill::kServiceCompleteListProperty;
     } else if (type == ManagedState::MANAGED_TYPE_DEVICE) {
       return shill::kDevicesProperty;
@@ skipping 11 matching lines @@
          iter != entries.end(); ++iter) {
       std::string path;
       if ((*iter)->GetAsString(&path))
         entries_[type].push_back(path);
     }
   }
 
   void AddPropertyUpdate(const std::string& type, const std::string& path) {
     if (type.empty())
       return;
+    VLOG(2) << "AddPropertyUpdate: " << type;
     property_updates(type)[path] += 1;
   }
 
   void AddInitialPropertyUpdate(const std::string& type,
                                 const std::string& path) {
     if (type.empty())
       return;
+    VLOG(2) << "AddInitialPropertyUpdate: " << type;
     initial_property_updates(type)[path] += 1;
   }
 
   void AddStateListUpdate(const std::string& type) {
     if (type.empty())
       return;
     list_updates_[type] += 1;
   }
 
   // Map of list-type -> paths
@@ skipping 42 matching lines @@
   }
 
   virtual void TearDown() OVERRIDE {
     shill_property_handler_.reset();
     listener_.reset();
     DBusThreadManager::Shutdown();
   }
 
   void AddDevice(const std::string& type, const std::string& id) {
     ASSERT_TRUE(IsValidType(type));
-    device_test_->AddDevice(id, type, std::string("/device/" + id));
+    device_test_->AddDevice(id, type, id);
   }
 
   void RemoveDevice(const std::string& id) {
     device_test_->RemoveDevice(id);
   }
 
   void AddService(const std::string& type,
                   const std::string& id,
-                  const std::string& state,
-                  bool add_to_watch_list) {

[stevenjb, 2014/05/28 22:47:22]

We no longer pay attention to the watch list, so I've removed any "testing" of
it.

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

Could add_to_watch_list then also be removed from the client's test interface?

[stevenjb, 2014/06/05 20:38:19]

Yes. I was originally going to avoid that big of a change, but since I separated
this out anyway, I should do that cleanup here also.
Done.

+                  const std::string& state) {
     ASSERT_TRUE(IsValidType(type));
     service_test_->AddService(id, id, type, state,
-                              true /* visible */, add_to_watch_list);
+                              true /* visible */,
+                              true /* add_to_watch_list */);
   }
 
   void AddServiceWithIPConfig(const std::string& type,
                               const std::string& id,
                               const std::string& state,
-                              const std::string& ipconfig_path,
-                              bool add_to_watch_list) {
+                              const std::string& ipconfig_path) {
     ASSERT_TRUE(IsValidType(type));
     service_test_->AddServiceWithIPConfig(id, /* service_path */
                                           "" /* guid */,
                                           id /* name */,
                                           type,
                                           state,
                                           ipconfig_path,
                                           true /* visible */,
-                                          add_to_watch_list);
+                                          true /* add_to_watch_list */);
   }
 
   void AddServiceToProfile(const std::string& type,
                            const std::string& id,
                            bool visible) {
     service_test_->AddService(id, id, type, shill::kStateIdle,
-                              visible, false /* watch */);
+                              visible, true /* watch */);
     std::vector<std::string> profiles;
     profile_test_->GetProfilePaths(&profiles);
     ASSERT_TRUE(profiles.size() > 0);
     base::DictionaryValue properties;  // Empty entry
     profile_test_->AddService(profiles[0], id);
   }
 
   void RemoveService(const std::string& id) {
     service_test_->RemoveService(id);
   }
@@ skipping 17 matching lines @@
             type == shill::kTypeVPN);
   }
 
  protected:
   void SetupDefaultShillState() {
     message_loop_.RunUntilIdle();  // Process any pending updates
     device_test_->ClearDevices();
     AddDevice(shill::kTypeWifi, "stub_wifi_device1");
     AddDevice(shill::kTypeCellular, "stub_cellular_device1");
     service_test_->ClearServices();
-    const bool add_to_watchlist = true;
-    AddService(shill::kTypeEthernet, "stub_ethernet",
-               shill::kStateOnline, add_to_watchlist);
-    AddService(shill::kTypeWifi, "stub_wifi1",
-               shill::kStateOnline, add_to_watchlist);
-    AddService(shill::kTypeWifi, "stub_wifi2",
-               shill::kStateIdle, add_to_watchlist);
-    AddService(shill::kTypeCellular, "stub_cellular1",
-               shill::kStateIdle, add_to_watchlist);
+    AddService(shill::kTypeEthernet, "stub_ethernet", shill::kStateOnline);
+    AddService(shill::kTypeWifi, "stub_wifi1", shill::kStateOnline);
+    AddService(shill::kTypeWifi, "stub_wifi2", shill::kStateIdle);
+    AddService(shill::kTypeCellular, "stub_cellular1", shill::kStateIdle);
   }
 
   base::MessageLoopForUI message_loop_;
   scoped_ptr<TestListener> listener_;
   scoped_ptr<internal::ShillPropertyHandler> shill_property_handler_;
   ShillManagerClient::TestInterface* manager_test_;
   ShillDeviceClient::TestInterface* device_test_;
   ShillServiceClient::TestInterface* service_test_;
   ShillProfileClient::TestInterface* profile_test_;
 
@@ skipping 12 matching lines @@
             listener_->entries(shill::kServicesProperty).size());
 
   EXPECT_EQ(0, listener_->errors());
 }
 
 TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerTechnologyChanged) {
   const int initial_technology_updates = 2;  // Available and Enabled lists
   EXPECT_EQ(initial_technology_updates, listener_->technology_list_updates());
 
   // Remove an enabled technology. Updates both the Available and Enabled lists.
+  listener_->reset_list_updates();
   manager_test_->RemoveTechnology(shill::kTypeWifi);
   message_loop_.RunUntilIdle();
-  EXPECT_EQ(initial_technology_updates + 2,
-            listener_->technology_list_updates());
+  EXPECT_EQ(2, listener_->technology_list_updates());
 
   // Add a disabled technology.
+  listener_->reset_list_updates();
   manager_test_->AddTechnology(shill::kTypeWifi, false);
   message_loop_.RunUntilIdle();
-  EXPECT_EQ(initial_technology_updates + 3,
-            listener_->technology_list_updates());
+  EXPECT_EQ(1, listener_->technology_list_updates());
   EXPECT_TRUE(shill_property_handler_->IsTechnologyAvailable(
       shill::kTypeWifi));
   EXPECT_FALSE(shill_property_handler_->IsTechnologyEnabled(shill::kTypeWifi));
 
   // Enable the technology.
+  listener_->reset_list_updates();
   DBusThreadManager::Get()->GetShillManagerClient()->EnableTechnology(
       shill::kTypeWifi,
       base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
   message_loop_.RunUntilIdle();
-  EXPECT_EQ(initial_technology_updates + 4,
-            listener_->technology_list_updates());
+  EXPECT_EQ(1, listener_->technology_list_updates());
   EXPECT_TRUE(shill_property_handler_->IsTechnologyEnabled(shill::kTypeWifi));
 
   EXPECT_EQ(0, listener_->errors());
 }
 
 TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerDevicePropertyChanged) {
   const size_t kNumShillManagerClientStubImplDevices = 2;
   EXPECT_EQ(kNumShillManagerClientStubImplDevices,
             listener_->entries(shill::kDevicesProperty).size());
   // Add a device.
   listener_->reset_list_updates();
   const std::string kTestDevicePath("test_wifi_device1");
   AddDevice(shill::kTypeWifi, kTestDevicePath);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(1, listener_->list_updates(shill::kDevicesProperty));
   EXPECT_EQ(kNumShillManagerClientStubImplDevices + 1,
             listener_->entries(shill::kDevicesProperty).size());
-  // Device changes are not observed.
+
   // Remove a device
   listener_->reset_list_updates();
   RemoveDevice(kTestDevicePath);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(1, listener_->list_updates(shill::kDevicesProperty));
   EXPECT_EQ(kNumShillManagerClientStubImplDevices,
             listener_->entries(shill::kDevicesProperty).size());
 
   EXPECT_EQ(0, listener_->errors());
 }
 
 TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServicePropertyChanged) {
   const size_t kNumShillManagerClientStubImplServices = 4;
   EXPECT_EQ(kNumShillManagerClientStubImplServices,
             listener_->entries(shill::kServicesProperty).size());
 
-  // Add an unwatched service.
+  // Add a service.
   listener_->reset_list_updates();
   const std::string kTestServicePath("test_wifi_service1");
-  AddService(shill::kTypeWifi, kTestServicePath, shill::kStateIdle, false);
+  AddService(shill::kTypeWifi, kTestServicePath, shill::kStateIdle);
   message_loop_.RunUntilIdle();
-  // Watched and unwatched services trigger a service list update.
+  // Add should trigger a service list update and update entries.
   EXPECT_EQ(1, listener_->list_updates(shill::kServicesProperty));
   EXPECT_EQ(kNumShillManagerClientStubImplServices + 1,
             listener_->entries(shill::kServicesProperty).size());
   // Service receives an initial property update.
   EXPECT_EQ(1, listener_->initial_property_updates(
       shill::kServicesProperty)[kTestServicePath]);
   // Change a property.
   base::FundamentalValue scan_interval(3);
   DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
       dbus::ObjectPath(kTestServicePath),
       shill::kScanIntervalProperty,
       scan_interval,
       base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
   message_loop_.RunUntilIdle();
-  // Property change triggers an update.
+  // Property change triggers an update (but not a service list update).
   EXPECT_EQ(1, listener_->property_updates(
       shill::kServicesProperty)[kTestServicePath]);
 
-  // Add the existing service to the watch list.
-  listener_->reset_list_updates();
-  AddService(shill::kTypeWifi, kTestServicePath, shill::kStateIdle, true);
-  message_loop_.RunUntilIdle();
-  // Service list update should be received when watch list changes.
-  EXPECT_EQ(1, listener_->list_updates(shill::kServicesProperty));
-  // Number of services shouldn't change.
-  EXPECT_EQ(kNumShillManagerClientStubImplServices + 1,
-            listener_->entries(shill::kServicesProperty).size());
-
-  // Change a property.
-  DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
-      dbus::ObjectPath(kTestServicePath),
-      shill::kScanIntervalProperty,
-      scan_interval,
-      base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
-  message_loop_.RunUntilIdle();
-  // Property change should trigger another update.
-  EXPECT_EQ(2, listener_->property_updates(
-      shill::kServicesProperty)[kTestServicePath]);
-
-  // Remove a service
+  // Remove a service.
   listener_->reset_list_updates();
   RemoveService(kTestServicePath);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(1, listener_->list_updates(shill::kServicesProperty));
   EXPECT_EQ(kNumShillManagerClientStubImplServices,
             listener_->entries(shill::kServicesProperty).size());
 
   EXPECT_EQ(0, listener_->errors());
 }
 
@@ skipping 21 matching lines @@
   base::StringValue gateway("192.0.0.1");
   DBusThreadManager::Get()->GetShillIPConfigClient()->SetProperty(
       dbus::ObjectPath(kTestIPConfigPath),
       shill::kGatewayProperty, gateway,
       base::Bind(&DoNothingWithCallStatus));
   message_loop_.RunUntilIdle();
 
   // Add a service with an empty ipconfig and then update
   // its ipconfig property.
   const std::string kTestServicePath1("test_wifi_service1");
-  AddService(shill::kTypeWifi, kTestServicePath1, shill::kStateIdle, true);
+  AddService(shill::kTypeWifi, kTestServicePath1, shill::kStateIdle);
   message_loop_.RunUntilIdle();
   // This is the initial property update.
   EXPECT_EQ(1, listener_->initial_property_updates(
       shill::kServicesProperty)[kTestServicePath1]);
   DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
       dbus::ObjectPath(kTestServicePath1),
       shill::kIPConfigProperty,
       base::StringValue(kTestIPConfigPath),
       base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
   message_loop_.RunUntilIdle();
   // IPConfig property change on the service should trigger an IPConfigs update.
   EXPECT_EQ(1, listener_->property_updates(
       shill::kIPConfigsProperty)[kTestIPConfigPath]);
 
-  // Now, Add a new watched service with the IPConfig already set.
+  // Now, Add a new service with the IPConfig already set.
   const std::string kTestServicePath2("test_wifi_service2");
   AddServiceWithIPConfig(shill::kTypeWifi, kTestServicePath2,
-                         shill::kStateIdle, kTestIPConfigPath, true);
+                         shill::kStateIdle, kTestIPConfigPath);
   message_loop_.RunUntilIdle();
-  // A watched service with the IPConfig property already set should trigger an
+  // A service with the IPConfig property already set should trigger an
   // additional IPConfigs update.
   EXPECT_EQ(2, listener_->property_updates(
       shill::kIPConfigsProperty)[kTestIPConfigPath]);
 }
 
 TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServiceCompleteList) {
   // Add a new entry to the profile only (triggers a Services update).
   const std::string kTestServicePath1("stub_wifi_profile_only1");
-  AddServiceToProfile(shill::kTypeWifi, kTestServicePath1, false);
+  AddServiceToProfile(shill::kTypeWifi, kTestServicePath1, false /* visible */);
   message_loop_.RunUntilIdle();
 
   // Update the Manager properties. This should trigger a single list update
   // for both Services and ServiceCompleteList, and a single property update
   // for ServiceCompleteList.
   listener_->reset_list_updates();
   shill_property_handler_->UpdateManagerProperties();
   message_loop_.RunUntilIdle();
   EXPECT_EQ(1, listener_->list_updates(shill::kServicesProperty));
   EXPECT_EQ(1, listener_->list_updates(shill::kServiceCompleteListProperty));
@@ skipping 38 matching lines @@
   message_loop_.RunUntilIdle();
   // Property change should trigger an update for the Network only; no
   // property updates pushed by Shill affect Favorites.
   EXPECT_EQ(2, listener_->property_updates(
       shill::kServicesProperty)[kTestServicePath2]);
   EXPECT_EQ(0, listener_->property_updates(
       shill::kServiceCompleteListProperty)[kTestServicePath2]);
 }
 
 }  // namespace chromeos