If requested, report network interfaces to management server.

chrome/browser/chromeos/policy/device_status_collector_browsertest.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 "chrome/browser/chromeos/policy/device_status_collector.h"
 
 #include "base/environment.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/testing_pref_service.h"
+#include "base/run_loop.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/cros_settings_names.h"
 #include "chrome/browser/chromeos/settings/cros_settings_provider.h"
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chrome/browser/chromeos/settings/stub_cros_settings_provider.h"
 #include "chrome/browser/chromeos/system/mock_statistics_provider.h"
 #include "chrome/browser/chromeos/system/statistics_provider.h"
 #include "chrome/browser/policy/proto/cloud/device_management_backend.pb.h"
 #include "chrome/common/pref_names.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/shill_device_client.h"
+#include "chromeos/network/network_handler.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/geolocation_provider.h"
 #include "content/public/test/test_browser_thread.h"
 #include "content/public/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 
 using ::testing::DoAll;
 using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::SetArgPointee;
 using ::testing::_;
 using base::Time;
 using base::TimeDelta;
 
 namespace em = enterprise_management;
@@ skipping 536 matching lines @@
 
   // Check that after enabling location reporting again, an error is reported
   // if no valid fix is available.
   SetMockPositionToReturnNext(invalid_fix);
   cros_settings_->SetBoolean(chromeos::kReportDeviceLocation, true);
   // Allow the new pref to propagate to the status collector.
   message_loop_.RunUntilIdle();
   CheckThatALocationErrorIsReported();
 }
 
+// Fake device state.
+struct FakeDeviceData {
+  const char* device_path;
+  const char* type;
+  const char* object_path;
+  const char* mac_address;
+  const char* meid;
+};
+
+static const FakeDeviceData kFakeDevices[] = {
+  { "/device/ethernet", flimflam::kTypeEthernet, "ethernet",
+    "112233445566", NULL },

[pneubeck (no reviews), 2013/07/04 15:51:48]

nit: maybe test also a device without mac_address

[Mattias Nissler (ping if slow), 2013/07/11 17:41:46]

Done.

+  { "/device/cellular", flimflam::kTypeCellular, "cellular",
+    "abcdefabcdef", "meid", },
+  { "/device/wifi", flimflam::kTypeWifi, "wifi",
+    "aabbccddeeff", NULL, },
+};
+
+class DeviceStatusCollectorNetworkInterfacesTest
+    : public DeviceStatusCollectorTest {
+ protected:
+  virtual void SetUp() OVERRIDE {
+    chromeos::DBusThreadManager::InitializeWithStub();
+    chromeos::NetworkHandler::Initialize();
+    chromeos::ShillDeviceClient::TestInterface* test_device_client =
+        chromeos::DBusThreadManager::Get()->GetShillDeviceClient()->
+            GetTestInterface();
+    test_device_client->ClearDevices();
+    for (size_t i = 0; i < arraysize(kFakeDevices); ++i) {
+      const FakeDeviceData& dev = kFakeDevices[i];
+      test_device_client->AddDevice(dev.device_path, dev.type,
+                                    dev.object_path);
+      if (dev.mac_address) {
+        base::StringValue value(dev.mac_address);

[pneubeck (no reviews), 2013/07/04 15:51:48]

nit: no need for local variable declaration |value|

[Mattias Nissler (ping if slow), 2013/07/11 17:41:46]

Done.

Clang used to complain in these cases. That was because of the copy constructor
required to pass the temporary not existing, which is no longer the case.

+        test_device_client->SetDeviceProperty(
+            dev.device_path, flimflam::kAddressProperty, value);
+      }
+      if (dev.meid) {
+        base::StringValue value(dev.meid);

[pneubeck (no reviews), 2013/07/04 15:51:48]

ditto

[Mattias Nissler (ping if slow), 2013/07/11 17:41:46]

Done.

+        test_device_client->SetDeviceProperty(
+            dev.device_path, flimflam::kMeidProperty, value);
+      }
+    }
+
+    // Flush out pending state updates.
+    base::RunLoop().RunUntilIdle();
+  }
+
+  virtual void TearDown() OVERRIDE {
+    chromeos::NetworkHandler::Shutdown();
+    chromeos::DBusThreadManager::Shutdown();
+  }
+};
+
+TEST_F(DeviceStatusCollectorNetworkInterfacesTest, NetworkInterfaces) {
+  // No interfaces should be reported if the policy is off.
+  GetStatus();
+  EXPECT_EQ(0, status_.network_interface_size());
+
+  // Switch the policy on and verify the interface list is present.
+  cros_settings_->SetBoolean(chromeos::kReportDeviceNetworkInterfaces, true);
+  GetStatus();
+
+  ASSERT_EQ(static_cast<int>(arraysize(kFakeDevices)),
+            status_.network_interface_size());
+  for (size_t i = 0; i < arraysize(kFakeDevices); ++i) {
+    const em::NetworkInterface& iface = status_.network_interface(i);
+    const FakeDeviceData& dev = kFakeDevices[i];
+    EXPECT_EQ(dev.type, iface.type());
+
+    if (dev.mac_address)

[pneubeck (no reviews), 2013/07/04 15:51:48]

you're testing the logic in the implementation with the same logic reimplemented
in the test.

wouldn't it have been an alternative to serialize the protobuf message to a JSON
string and compare that to an expected string?

that could also be used in other tests of this kind.

[Mattias Nissler (ping if slow), 2013/07/11 17:41:46]

That doesn't make a difference, I'd just be swapping one format of specifying
test expectations for another without gaining any test coverage quality.
Ideally, I'd test this against what shill gives me, but that'd require an
integration test with Chrome OS that can't be done as browser_test.

+      EXPECT_EQ(dev.mac_address, iface.mac_address());
+    else
+      EXPECT_FALSE(iface.has_mac_address());
+
+    if (dev.meid)
+      EXPECT_EQ(dev.meid, iface.meid());
+    else
+      EXPECT_FALSE(iface.has_meid());
+  }
+}
+
 }  // namespace policy