Add unit test for ArcBluetoothBridge and TypeConverter

components/arc/bluetooth/arc_bluetooth_bridge_unittest.cc

Index: components/arc/bluetooth/arc_bluetooth_bridge_unittest.cc
diff --git a/components/arc/bluetooth/arc_bluetooth_bridge_unittest.cc b/components/arc/bluetooth/arc_bluetooth_bridge_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..2115057bdafcb5df5603e82f86d2cbf9198481c0
--- /dev/null
+++ b/components/arc/bluetooth/arc_bluetooth_bridge_unittest.cc
@@ -0,0 +1,205 @@
+// Copyright 2016 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 "components/arc/bluetooth/arc_bluetooth_bridge.h"
+
+#include <string>
+#include <vector>
+
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "components/arc/bluetooth/bluetooth_type_converters.h"
+#include "components/arc/common/bluetooth.mojom.h"
+#include "components/arc/test/fake_arc_bridge_service.h"
+#include "components/arc/test/fake_bluetooth_instance.h"
+#include "device/bluetooth/dbus/bluez_dbus_manager.h"
+#include "device/bluetooth/dbus/fake_bluetooth_adapter_client.h"
+#include "device/bluetooth/dbus/fake_bluetooth_device_client.h"
+#include "device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_client.h"
+#include "device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_client.h"
+#include "device/bluetooth/dbus/fake_bluetooth_gatt_service_client.h"
+#include "mojo/public/cpp/bindings/array.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace arc {
+
+class ArcBluetoothBridgeTest : public testing::Test {
+ protected:
+  void AddTestDevice() {
+    fake_bluetooth_device_client_->CreateDevice(
+        dbus::ObjectPath(bluez::FakeBluetoothAdapterClient::kAdapterPath),
+        dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
+    fake_bluetooth_gatt_service_client_->ExposeHeartRateService(
+        dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
+    fake_bluetooth_gatt_characteristic_client_->ExposeHeartRateCharacteristics(
+        fake_bluetooth_gatt_service_client_->GetHeartRateServicePath());
+  }
+
+  std::unique_ptr<FakeArcBridgeService> fake_arc_bridge_service_;
+  std::unique_ptr<FakeBluetoothInstance> fake_bluetooth_instance_;
+  std::unique_ptr<ArcBluetoothBridge> arc_bluetooth_bridge_;
+  scoped_refptr<device::BluetoothAdapter> adapter_;
+
+  void OnAdapterInitialized(scoped_refptr<device::BluetoothAdapter> adapter) {
+    adapter_ = adapter;
+    get_adapter_run_loop_.Quit();
+  }
+
+  void SetUp() override {
+    std::unique_ptr<bluez::BluezDBusManagerSetter> dbus_setter =
+        bluez::BluezDBusManager::GetSetterForTesting();
+    fake_bluetooth_device_client_ = new bluez::FakeBluetoothDeviceClient;
+    fake_bluetooth_device_client_->RemoveAllDevices();
+    fake_bluetooth_gatt_service_client_ =
+        new bluez::FakeBluetoothGattServiceClient;
+    fake_bluetooth_gatt_characteristic_client_ =
+        new bluez::FakeBluetoothGattCharacteristicClient;
+    fake_bluetooth_gatt_descriptor_client_ =
+        new bluez::FakeBluetoothGattDescriptorClient;
+    dbus_setter->SetBluetoothDeviceClient(
+        base::WrapUnique(fake_bluetooth_device_client_));

[Luis Héctor Chávez, 2016/07/29 23:09:09]

This pattern looks very bad (you are keeping a reference to a unique_ptr that
you already transferred ownership of).

Is it possible to write it like:

dbus_setter->SetXxx(base::MakeUnique<FakeXxx>(...));
fake_xxx_ = static_cast<FakeXxx>(xxx->GetXxx());

?

Otherwise, please add a comment about this pattern. It's ugly, but it's limited
to tests :(

[rkc, 2016/07/30 22:40:55]

The problem with this pattern is holding on to the raw pointer of a unique_ptr
owned by another class. Whether we hold on to the raw pointer from before we
passed ownership or hold on to it after getting it back from a getter doesn't
ameliorate that issue at all.

The correct solution would be to not hold on to the raw pointer and always
getting it from the getter. Though doing so in a limited controlled environment
like a unittest has generally been acceptable practice.

[puthik_chromium, 2016/08/01 18:02:48]

I changed it to use the get the FakeBTClient from DBus when we want to use it.

+    dbus_setter->SetBluetoothGattServiceClient(
+        base::WrapUnique(fake_bluetooth_gatt_service_client_));
+    dbus_setter->SetBluetoothGattCharacteristicClient(
+        base::WrapUnique(fake_bluetooth_gatt_characteristic_client_));
+    dbus_setter->SetBluetoothGattDescriptorClient(
+        base::WrapUnique(fake_bluetooth_gatt_descriptor_client_));
+
+    fake_arc_bridge_service_.reset(new FakeArcBridgeService());
+    fake_bluetooth_instance_.reset(new FakeBluetoothInstance());
+    fake_arc_bridge_service_->bluetooth()->SetInstance(
+        fake_bluetooth_instance_.get(), 2);
+    arc_bluetooth_bridge_.reset(
+        new ArcBluetoothBridge(fake_arc_bridge_service_.get()));
+
+    device::BluetoothAdapterFactory::GetAdapter(base::Bind(
+        &ArcBluetoothBridgeTest::OnAdapterInitialized, base::Unretained(this)));
+    // We will quit the loop once we get the adapter.
+    get_adapter_run_loop_.Run();
+  }
+
+  base::MessageLoop message_loop_;
+  base::RunLoop get_adapter_run_loop_;
+  bluez::FakeBluetoothDeviceClient* fake_bluetooth_device_client_;
+  bluez::FakeBluetoothGattServiceClient* fake_bluetooth_gatt_service_client_;
+  bluez::FakeBluetoothGattCharacteristicClient*
+      fake_bluetooth_gatt_characteristic_client_;
+  bluez::FakeBluetoothGattDescriptorClient*
+      fake_bluetooth_gatt_descriptor_client_;
+};
+
+// When we add device to bluez::FakeBluetoothDeviceClient, ArcBluetoothBridge
+// should send new device data to Android. This test will then check
+// the correctness of the device properties sent via arc bridge.
+TEST_F(ArcBluetoothBridgeTest, DeviceFound) {
+  EXPECT_EQ((size_t)0, fake_bluetooth_instance_->device_found_data().size());

[Luis Héctor Chávez, 2016/07/29 23:09:09]

Try to avoid C-style casts (since they are forbidden by the style guide). Does
0u help avoid the compilation error?

Another question: does `git cl lint` complain about this?

[puthik_chromium, 2016/08/01 18:02:48]

Done.
Also, git cl lint does not complain about (size_t) cast.

[Luis Héctor Chávez, 2016/08/01 18:10:57]

Maybe it should :/ I'll ask around.

+  AddTestDevice();
+  EXPECT_EQ((size_t)1, fake_bluetooth_instance_->device_found_data().size());

[rkc, 2016/07/30 22:40:55]

As Luis said, all these casts need to go. When comparing variables to literal,
please use literal suffixes (like u, or l, etc) to match the types, not a cast.

[puthik_chromium, 2016/08/01 18:02:48]

Done.

+  const mojo::Array<mojom::BluetoothPropertyPtr>& prop =
+      fake_bluetooth_instance_->device_found_data().back();
+
+  EXPECT_EQ((size_t)7, prop.size());
+  EXPECT_TRUE(prop[0]->is_bdname());
+  EXPECT_EQ(std::string(bluez::FakeBluetoothDeviceClient::kLowEnergyName),
+            prop[0]->get_bdname());
+  EXPECT_TRUE(prop[1]->is_bdaddr());
+  EXPECT_EQ(std::string(bluez::FakeBluetoothDeviceClient::kLowEnergyAddress),
+            prop[1]->get_bdaddr()->To<std::string>());
+  EXPECT_TRUE(prop[2]->is_uuids());
+  EXPECT_EQ((size_t)1, prop[2]->get_uuids().size());
+  EXPECT_EQ(bluez::FakeBluetoothGattServiceClient::kHeartRateServiceUUID,
+            prop[2]->get_uuids()[0].To<device::BluetoothUUID>().value());
+  EXPECT_TRUE(prop[3]->is_device_class());
+  EXPECT_EQ(bluez::FakeBluetoothDeviceClient::kLowEnergyClass,
+            prop[3]->get_device_class());
+  EXPECT_TRUE(prop[4]->is_device_type());
+  // bluez::FakeBluetoothDeviceClient does not return proper device type.
+  EXPECT_TRUE(prop[5]->is_remote_friendly_name());
+  EXPECT_EQ(std::string(bluez::FakeBluetoothDeviceClient::kLowEnergyName),
+            prop[5]->get_remote_friendly_name());
+  EXPECT_TRUE(prop[6]->is_remote_rssi());
+}
+
+// Invoke OnDiscoveryStarted to send cached device to BT instance,
+// and check correctness of the Advertising data sent via arc bridge.
+TEST_F(ArcBluetoothBridgeTest, LEDeviceFound) {
+  EXPECT_EQ((size_t)0, fake_bluetooth_instance_->device_found_data().size());
+  AddTestDevice();
+  EXPECT_EQ((size_t)1, fake_bluetooth_instance_->le_device_found_data().size());
+
+  const mojom::BluetoothAddressPtr& addr =
+      fake_bluetooth_instance_->le_device_found_data().back()->addr();
+  const mojo::Array<mojom::BluetoothAdvertisingDataPtr>& adv_data =
+      fake_bluetooth_instance_->le_device_found_data().back()->adv_data();
+
+  EXPECT_EQ(std::string(bluez::FakeBluetoothDeviceClient::kLowEnergyAddress),
+            addr->To<std::string>());
+  EXPECT_EQ((size_t)1, adv_data.size());
+
+  EXPECT_TRUE(adv_data[0]->is_local_name());
+  EXPECT_EQ(std::string(bluez::FakeBluetoothDeviceClient::kLowEnergyName),
+            adv_data[0]->get_local_name().To<std::string>());
+}
+
+// Invoke GetGattDB and check correctness of the GattDB sent via arc bridge.
+TEST_F(ArcBluetoothBridgeTest, GetGattDB) {
+  AddTestDevice();
+
+  arc_bluetooth_bridge_->GetGattDB(mojom::BluetoothAddress::From(
+      std::string(bluez::FakeBluetoothDeviceClient::kLowEnergyAddress)));
+  EXPECT_EQ((size_t)1, fake_bluetooth_instance_->gatt_db_result().size());
+
+  const mojom::BluetoothAddressPtr& addr =
+      fake_bluetooth_instance_->gatt_db_result().back()->remote_addr();
+  EXPECT_EQ(std::string(bluez::FakeBluetoothDeviceClient::kLowEnergyAddress),
+            addr->To<std::string>());
+
+  // HeartRateService in bluez::FakeBluetoothDeviceClient consists of
+  // Service: HeartRateService
+  //     Characteristic: HeartRateMeasurement
+  //         Descriptor: ClientCharacteristicConfiguration
+  //     Characteristic: BodySensorLocation
+  //     Characteristic: HeartRateControlPoint
+  const mojo::Array<mojom::BluetoothGattDBElementPtr>& db =
+      fake_bluetooth_instance_->gatt_db_result().back()->db();
+  EXPECT_EQ((size_t)5, db.size());
+
+  EXPECT_EQ(device::BluetoothUUID(
+                bluez::FakeBluetoothGattServiceClient::kHeartRateServiceUUID),
+            db[0]->uuid.To<device::BluetoothUUID>());
+  EXPECT_EQ(mojom::BluetoothGattDBAttributeType::BTGATT_DB_PRIMARY_SERVICE,
+            db[0]->type);
+
+  EXPECT_EQ(device::BluetoothUUID(bluez::FakeBluetoothGattCharacteristicClient::
+                                      kHeartRateMeasurementUUID),
+            db[1]->uuid.To<device::BluetoothUUID>());
+  EXPECT_EQ(mojom::BluetoothGattDBAttributeType::BTGATT_DB_CHARACTERISTIC,
+            db[1]->type);
+  EXPECT_EQ(device::BluetoothGattCharacteristic::PROPERTY_NOTIFY,
+            db[1]->properties);
+
+  EXPECT_EQ(device::BluetoothUUID(bluez::FakeBluetoothGattDescriptorClient::
+                                      kClientCharacteristicConfigurationUUID),
+            db[2]->uuid.To<device::BluetoothUUID>());
+  EXPECT_EQ(mojom::BluetoothGattDBAttributeType::BTGATT_DB_DESCRIPTOR,
+            db[2]->type);
+
+  EXPECT_EQ(device::BluetoothUUID(bluez::FakeBluetoothGattCharacteristicClient::
+                                      kBodySensorLocationUUID),
+            db[3]->uuid.To<device::BluetoothUUID>());
+  EXPECT_EQ(mojom::BluetoothGattDBAttributeType::BTGATT_DB_CHARACTERISTIC,
+            db[3]->type);
+  EXPECT_EQ(device::BluetoothGattCharacteristic::PROPERTY_READ,
+            db[3]->properties);
+
+  EXPECT_EQ(device::BluetoothUUID(bluez::FakeBluetoothGattCharacteristicClient::
+                                      kHeartRateControlPointUUID),
+            db[4]->uuid.To<device::BluetoothUUID>());
+  EXPECT_EQ(mojom::BluetoothGattDBAttributeType::BTGATT_DB_CHARACTERISTIC,
+            db[4]->type);
+  EXPECT_EQ(device::BluetoothGattCharacteristic::PROPERTY_WRITE,
+            db[4]->properties);
+}
+
+}  // namespace arc