Improve testability

Given the current code, we are very limited on how much we can test our BleManager descendants.

• We can't unit test it because the constructor initializes a Handler from the main looper, so we are forced to use AndroidJUnit4 runner
• When running on AndroidJUnit4, Mockito can't mock final classes. I'd like to not include PowerMock, but I may end up doing that.
  • We can't test our BleManagerGattCallback, which receives BluetoothGatt as a parameter. And we can't move the implementation to an independent class because we have to fetch & store the GattCharacteristics to be used later
  • We can't test our bluetooth operations, which throw InvalidRequestException because mBluetoothDevice is null. And that's probably only the tip of the iceberg, I haven't gone past that point.

Making the code testable would be a daunting task, but maybe we could start with something simple such as exposing a constructor that takes a Handler as parameter. There's a VisibleForTesting annotation that will flag the usage as a lint error if used outside from test sourceset. VisibleForTesting lives in an independent package (com.android.support:support-annotations:XXX, maybe it's androidx now?)

I'm eager to listen to other ideas, I'm sure I'm not the only one having this issue.

Thanks!

[philips77]

Hi,
That's so true. The lib has zero tests and it's, as you wrote, written in non-testable way. I'm open for any proposals.
So far, testing on our side is based on manual tests on multiple phones, apps and devices + issues on GitHub, but that's far from ideal. Changing one thing may easily break others. There are other issues with the lib. It has a lot of legacy code. Initially it was written inside nRF Toolbox, used for 2-3 profiles. Then it was extended to all, then externalized, now the version 2 with changed API and deprecated the old one. But still the BleManager is around 2000 lines too long, should be cut, request implementation should be moved to requests, etc. Then it should be implemented with tests in mind, where each module should be testable separately. A lot of work, but I can't wait to start. To be honest, I don't want to add tests to this version, as it will for sure change in some time.

But some small changes, as you said, are very welcome. Please, fork the repo and send us PR. I'm sure that adding a constructor with the handler will not be enough and I have no time to look into it right now.