Add unit tests for BluetoothDevice::GetPrimaryServices() etc.

device/bluetooth/bluetooth_device_unittest.cc

Index: device/bluetooth/bluetooth_device_unittest.cc
diff --git a/device/bluetooth/bluetooth_device_unittest.cc b/device/bluetooth/bluetooth_device_unittest.cc
index 9dcb4a1a7be0f094445f25151c8061088db20c55..b0193943559f0df509d6b407c8def2c00286055b 100644
--- a/device/bluetooth/bluetooth_device_unittest.cc
+++ b/device/bluetooth/bluetooth_device_unittest.cc
@@ -1483,4 +1483,158 @@ TEST_F(BluetoothTest, GetDeviceTransportType) {
 }
 #endif  // defined(OS_CHROMEOS) || defined(OS_LINUX)
 
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
+TEST_F(BluetoothTest, GetPrimaryServices) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
+  InitWithFakeAdapter();

[scheib, 2017/03/17 17:38:40]

There's a lot of boilerplate for these tests. Consider if it's worth moving out
to a helper method. I think the same fake setup could be used for all these
service and characteristic tests.

[juncai, 2017/03/17 21:25:47]

I think the above 5 lines are also repeated in other places in this test. Maybe
that's on purpose?

Even though these three added tests can use the same fake setup, but if put all
of these setup code into a function, the following test code still needs to
access the |device|, |services|, it seems it will affect the readability of the
code.

[scheib, 2017/03/17 22:10:47]

Look at e.g. the test fixture setup done for the characteristics unittest. By
doing the boilerplate in the test fixture the service and characteristic objects
can be saved as member variables. Not all tests in this file have to use the
same test fixture, so it's OK for these new tests to use a text fixture just for
them. The previous tests didn't overlap as much in what they did vs the number
of tests. By the time you add these tests the copy-paste is getting excessive --
especially because all these new tests can use the exact same setup.

[juncai, 2017/03/17 23:33:54]

Done.

+  StartLowEnergyDiscoverySession();
+  BluetoothDevice* device = SimulateLowEnergyDevice(3);
+  EXPECT_FALSE(device->IsConnected());
+
+  // Connect to the device
+  ResetEventCounts();
+  device->CreateGattConnection(GetGattConnectionCallback(Call::EXPECTED),
+                               GetConnectErrorCallback(Call::NOT_EXPECTED));
+  TestBluetoothAdapterObserver observer(adapter_);
+  SimulateGattConnection(device);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(device->IsConnected());
+
+  // Discover services
+  std::vector<std::string> services;
+  services.push_back("00000000-0000-1000-8000-00805f9b34fb");
+  services.push_back("00000001-0000-1000-8000-00805f9b34fb");

[scheib, 2017/03/17 17:38:40]

multiple services, same ID. So, probably use 1 unique ID, and then 2 instances
of a 2nd ID.

[juncai, 2017/03/17 21:25:47]

Done.

+  SimulateGattServicesDiscovered(device, services);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
+
+  EXPECT_EQ(2u, device->GetPrimaryServices().size());
+
+  // Disconnect from the device

[scheib, 2017/03/17 17:38:40]

Everything after GetPrimaryServices is not needed.

[juncai, 2017/03/17 21:25:47]

Done.

+  device->DisconnectGatt();
+  SimulateGattDisconnection(device);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(device->GetPrimaryServices().empty());
+}
+
+TEST_F(BluetoothTest, GetPrimaryServicesByUUID) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
+  InitWithFakeAdapter();
+  StartLowEnergyDiscoverySession();
+  BluetoothDevice* device = SimulateLowEnergyDevice(3);
+  EXPECT_FALSE(device->IsConnected());
+
+  // Connect to the device
+  ResetEventCounts();
+  device->CreateGattConnection(GetGattConnectionCallback(Call::EXPECTED),
+                               GetConnectErrorCallback(Call::NOT_EXPECTED));
+  TestBluetoothAdapterObserver observer(adapter_);
+  SimulateGattConnection(device);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(device->IsConnected());
+
+  // Discover services
+  std::vector<std::string> services;
+  services.push_back("00000000-0000-1000-8000-00805f9b34fb");
+  services.push_back("00000001-0000-1000-8000-00805f9b34fb");
+  SimulateGattServicesDiscovered(device, services);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
+
+  EXPECT_EQ(
+      1u, device->GetPrimaryServicesByUUID(BluetoothUUID(services[0])).size());
+  EXPECT_EQ(
+      1u, device->GetPrimaryServicesByUUID(BluetoothUUID(services[1])).size());
+  EXPECT_TRUE(
+      device->GetPrimaryServicesByUUID(BluetoothUUID("non-existent")).empty());

[scheib, 2017/03/17 17:38:40]

Let's use a valid UUID that doesn't exist in the fake that was setup.

[juncai, 2017/03/17 21:25:48]

Done.

+
+  // Disconnect from the device
+  device->DisconnectGatt();
+  SimulateGattDisconnection(device);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(
+      device->GetPrimaryServicesByUUID(BluetoothUUID(services[0])).empty());
+  EXPECT_TRUE(
+      device->GetPrimaryServicesByUUID(BluetoothUUID(services[1])).empty());
+}
+
+TEST_F(BluetoothTest, GetCharacteristicsByUUID) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
+  InitWithFakeAdapter();
+  StartLowEnergyDiscoverySession();
+  BluetoothDevice* device = SimulateLowEnergyDevice(3);
+  EXPECT_FALSE(device->IsConnected());
+
+  // Connect to the device
+  ResetEventCounts();
+  device->CreateGattConnection(GetGattConnectionCallback(Call::EXPECTED),
+                               GetConnectErrorCallback(Call::NOT_EXPECTED));
+  TestBluetoothAdapterObserver observer(adapter_);
+  SimulateGattConnection(device);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(device->IsConnected());
+
+  // Discover services
+  std::vector<std::string> services;
+  services.push_back("00000000-0000-1000-8000-00805f9b34fb");
+  services.push_back("00000001-0000-1000-8000-00805f9b34fb");
+  SimulateGattServicesDiscovered(device, services);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
+
+  std::vector<BluetoothRemoteGattService*> primary_services =
+      device->GetPrimaryServices();
+
+  // 2 characteristics, 1 on each service.
+  SimulateGattCharacteristic(primary_services[0], services[0],

[scheib, 2017/03/17 17:38:41]

Let's use a unique ID for the characteristic instead of reusing the service
UUID. Also, similar to services have 1 UUID used once, and another UUID have
multiple instances.

[juncai, 2017/03/17 21:25:48]

Done.

+                             0 /* properties */);
+  SimulateGattCharacteristic(primary_services[1], services[1],
+                             0 /* properties */);
+
+  std::string service_instance_id_0 = primary_services[0]->GetIdentifier();
+  BluetoothUUID characteristic_uuid_0(services[0]);
+  EXPECT_EQ(1u, device
+                    ->GetCharacteristicsByUUID(service_instance_id_0,
+                                               characteristic_uuid_0)
+                    .size());
+
+  std::string service_instance_id_1 = primary_services[1]->GetIdentifier();
+  BluetoothUUID characteristic_uuid_1(services[1]);
+  EXPECT_EQ(1u, device
+                    ->GetCharacteristicsByUUID(service_instance_id_1,
+                                               characteristic_uuid_1)
+                    .size());
+
+  EXPECT_TRUE(
+      device->GetCharacteristicsByUUID("non-existent", characteristic_uuid_0)

[scheib, 2017/03/17 17:38:40]

Let's use a valid UUID that doesn't exist in the fake that was setup.

[juncai, 2017/03/17 21:25:47]

Done.

+          .empty());
+  EXPECT_TRUE(device
+                  ->GetCharacteristicsByUUID(service_instance_id_0,
+                                             BluetoothUUID("non-existent"))
+                  .empty());
+
+  // Disconnect from the device
+  device->DisconnectGatt();
+  SimulateGattDisconnection(device);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(device
+                  ->GetCharacteristicsByUUID(service_instance_id_0,
+                                             characteristic_uuid_0)
+                  .empty());
+  EXPECT_TRUE(device
+                  ->GetCharacteristicsByUUID(service_instance_id_1,
+                                             characteristic_uuid_1)
+                  .empty());
+}
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
+
 }  // namespace device