Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove need to use RoboElectric for creating tests using MockRxAndroidBle #311

Closed
streetsofboston opened this issue Nov 8, 2017 · 7 comments

Comments

@streetsofboston
Copy link

streetsofboston commented Nov 8, 2017

Summary

Remove the need for RoboElectric when writing tests using MockRxAndroidBle
RoboElectric causes the startup of tests to be quite slow. Removing the dependency
on RoboElectric and being able to use only Mockito, for example, would improve the
process of TDD because tests can be run faster.

Library version

1.4.1

Issue found in MockRxAndroidBle implementation

When trying to run tests that use the MockRxAndroidBle class without RoboElectric, they fail because the test-code creates instances of classes from the Android SDK.

Our test-suite reports failures of usages of these classes:
BluetoothGattService
BluetoothGattCharacteristic
BluetoothGattDescriptor

Using PowerMock and using it to create mock-instances of these three classes makes these failures go away and the tests in our own project run fine!

However, using PowerMock requires this annotation at the head of the test-suite-class:
(snippet in Kotlin)

@PrepareForTest(
        value = [(RxBleClientMock.CharacteristicsBuilder::class), (RxBleClientMock.DeviceBuilder::class)],
        fullyQualifiedNames = ["com.polidea.rxandroidble.mockrxandroidble.RxBleConnectionMock\$21"]
)

Needless to say, this annotation is given knowledge of internal and private implementation details of MockRxAndroidBle. There's no guarantee that a future version of `MockRxAndroidBle' won't break this annotation configuration.....

Request for change in MockRxAndroidBle implementation.

Currently (version 1.4.1), the RxBleClientMock.CharacteristicsBuilder, RxBleClientMock.DeviceBuilder and RxBleConnectionMock$21 (anonymous class) create instance of BluetoothGattService, BluetoothGattCharacteristic or BluetoothGattDescriptordirectly.

  1. Either a factory pattern could be used to create these classes allowing users (writers of unit-tests) to inject mocked versions of these instances.
  2. Or the implementation of MockRxAndroidBle could create mocked version instead (and thereby requiring only Mockito).
@uKL
Copy link
Collaborator

uKL commented Nov 9, 2017

Unfortunately, I agree :)

The current implementation is not the best. Let's leave this issue open as an entry point to mock redesign.

@mzgreen
Copy link
Collaborator

mzgreen commented Nov 13, 2017

Actually mock was not written with unit testing in mind. It's purpose is to allow working on Bluetooth stuff without having the actual device, but I agree that it could be better and making it work in unit tests would be great :)

@dariuszseweryn
Copy link
Owner

This issue is a part of #213 and #239. The new API aims for fully wrapping the Android OS objects.

I am also thinking if there is really a need for the MockClient in the unit tests. It should be possible to inject your own mock implementations of RxBleDevice and RxBleConnection. As @mzgreen has mentioned above the MockClient was created as a quick drop-in replacement for the library if the peripheral you want to use is not available but still you would like to use mock it in the application.

It could be put up for a discussion if we should remove this part of Readme.md:

## Unit testing
Using RxAndroidBle enables you to unit test your application easily. For examples how to use mocking head to [MockRxAndroidBle](https://github.com/Polidea/RxAndroidBle/tree/master/mockrxandroidble).

Note: Using MockRxAndroidBle in unit tests needs [Robolectric](https://github.com/robolectric/robolectric).

@uKL
Copy link
Collaborator

uKL commented Nov 13, 2017

I'd keep it as it is possible to use it in unit tests. Certainly, it would be helpful if the framework would have some mocking/assertion engine. For example
xxx.assertCharacteristicValue(xxxxx) or similar.

@dariuszseweryn
Copy link
Owner

I think that this kind of a test would effectively test the MockClient behaviour where the user's application logic should be the subject.

@streetsofboston
Copy link
Author

I entered this test-enhancement, the removal of the need of RoboElectric, since we have a bunch end-to-end tests written as unit-tests (no need to start them on a device). In these tests, the MockRxAndroidBle client serves as a a dummy device/ble-server serving up responses for the ble-client code being under test. We use PowerMock to get around the need of RoboElectric and it all works great, but the tests are fragile if we ever change the version of MockRxAndroidBle.

Thank you all for looking into this!

@uKL
Copy link
Collaborator

uKL commented Mar 15, 2018

I think this should be closed in favor of #370. Feel free to reopen if you think otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants