Bluetooth: macOS: Adding counter for service discovery callbacks.

device/bluetooth/bluetooth_device_unittest.cc

 // Copyright 2014 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 "device/bluetooth/bluetooth_device.h"
 
 #include <stddef.h>
 
 #include "base/macros.h"
 #include "base/run_loop.h"
@@ skipping 381 matching lines @@
   //    the device holds and notify of device changed.
   gatt_connections_[0]->Disconnect();
   SimulateGattDisconnection(device);
   ASSERT_FALSE(device->IsGattConnected());
 
   EXPECT_EQ(1, observer.device_changed_count());
   EXPECT_TRUE(device->GetUUIDs().empty());
 }
 #endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
+#if defined(OS_MACOSX)
+TEST_F(BluetoothTest, DoubleDidModifyServicesEvent) {

[ortuno, 2017/01/23 06:12:38]

This is a good test for the case in which we get two DidModifyServices but if I
understood you correctly this morning we need a test for when DidConnectGatt and
DidModifyServices are called one right after the other.

So:

// Starts one discovery procedure.
SimulateGattConnection()
// Starts another discovery procedure.
SimulateGattServicesChanged(...);

SimulateGattServicesDiscovered(...);
SimulateGattServicesDiscovered(...);

[jlebel, 2017/01/23 18:49:38]

Done.

+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
+
+  InitWithFakeAdapter();
+  TestBluetoothAdapterObserver observer(adapter_);
+
+  BluetoothDevice* device = SimulateLowEnergyDevice(1);
+  device->CreateGattConnection(GetGattConnectionCallback(Call::EXPECTED),
+                               GetConnectErrorCallback(Call::NOT_EXPECTED));
+  SimulateGattConnection(device);
+
+  SimulateGattServicesDiscovered(device, {} /* services */);
+  EXPECT_EQ(2, observer.device_changed_count());
+  EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
+
+  observer.Reset();
+  SimulateGattServicesChanged(device);
+  SimulateGattServicesChanged(device);
+  EXPECT_EQ(2, observer.device_changed_count());
+  EXPECT_FALSE(device->IsGattServicesDiscoveryComplete());
+
+  observer.Reset();
+  SimulateGattServicesDiscovered(device, {} /* services */);
+  SimulateGattServicesDiscovered(device, {} /* services */);

[ortuno, 2017/01/23 06:12:38]

Is there any way we can test that DiscoveryCharacteristics was not called for
the first SimulateGattServicesDiscovered?

[jlebel, 2017/01/23 18:49:38]

I guess the simplest way to do that would be to add a counter in
BluetoothTestMac, and make the MockCBPeripheral increment the counter.
MockCBPeripheral instance already know about the BluetoothTestMac instance. So
it would not be too hard. But it doesn't go exactly like the observer pattern
that already exist.
Would it work for you with this simple way?

[ortuno, 2017/01/23 20:44:02]

SGTM

+  EXPECT_EQ(1, observer.device_changed_count());
+  EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
+}
+#endif  // defined(OS_MACOSX)
+
 #if defined(OS_ANDROID) || defined(OS_MACOSX)
 // Tests Advertisement Data is updated correctly when we start discovery
 // during a connection.
 TEST_F(BluetoothTest, AdvertisementData_DiscoveryDuringConnection) {
   if (!PlatformSupportsLowEnergy()) {
     LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
     return;
   }
 
   InitWithFakeAdapter();
@@ skipping 922 matching lines @@
 
   BluetoothDevice* device2 = SimulateLowEnergyDevice(6);
   EXPECT_EQ(BLUETOOTH_TRANSPORT_DUAL, device2->GetType());
 
   BluetoothDevice* device3 = SimulateClassicDevice();
   EXPECT_EQ(BLUETOOTH_TRANSPORT_CLASSIC, device3->GetType());
 }
 #endif  // defined(OS_CHROMEOS) || defined(OS_LINUX)
 
 }  // namespace device