BLE: Gatt client unit tests #6086

pan- · 2018-02-13T17:44:15Z

Description

Add units tests running on host for BLE. The current set of tests target the GattClient; these tests are build around gtest and gmock and solely run on a host:

cd features/FEATURE_BLE/tests
mkdir build
cd build
cmake ..
make
./gatt-client-tests

Migrations

NO

These tests are build around gtest and gmock and solely run on a host; cmake is used as a build system: - cd features/FEATURE_BLE/tests - mkdir build - cd build - cmake .. - make - ./gatt-client-tests

0xc0170 · 2018-02-14T16:29:17Z

Please review travis failures

0xc0170 · 2018-02-19T16:23:35Z

/morph build

mbed-ci · 2018-02-19T16:55:07Z

Build : SUCCESS

Build number : 1174
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6086/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-02-19T19:39:51Z

Exporter Build : SUCCESS

Build number : 851
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6086/

mbed-ci · 2018-02-19T22:18:30Z

Test : SUCCESS

Build number : 979
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6086/979

donatieng

Really good to add these :). Needs a README to and generally more doc (high level view describing different mocks and tests cases) at this stage.

donatieng · 2018-02-26T14:47:49Z

features/FEATURE_BLE/tests/CMakeLists.txt

+################################
+
+# Download and unpack googletest at configure time
+configure_file(CMakeLists.txt.in googletest-download/CMakeLists.txt)


Maybe rename CMakeLists.txt.in to something more explicit as it's a bit confusing having CMakeLists.txt.in and a CMakeLists.txt in the same directory.

I've followed instructions provided by google test. What name do you want ?

Something like googletest-CMakeLists.txt.in or similar.

donatieng · 2018-02-26T14:48:46Z

features/FEATURE_BLE/tests/generic/GattClient/TestCharacteristicDesctiptorDiscovery.cpp

+	}
+};
+
+class TestGattClientDescriptorDiscovery : public ::testing::Test {


Need a bit of doc on what the purpose of that class is

donatieng · 2018-02-26T14:59:46Z

features/FEATURE_BLE/tests/generic/GattClient/TestCharacteristicDesctiptorDiscovery.cpp

+	EXPECT_EQ(err, BLE_ERROR_NONE);
+}
+
+typedef tuple<uint16_t, uint16_t, vector<vector<pair<uint16_t, UUID>>>> test_param_t;


Needs description, maybe more explicit naming

donatieng · 2018-02-26T15:04:12Z

features/FEATURE_BLE/tests/generic/GattClient/TestCharacteristicDesctiptorDiscovery.cpp

+ * When the client call terminateCharacteristicDescriptorDiscovery the termination
+ * callback is called immediately.
+ */
+


Missing test case here?

Nope, this test is present line 160. That's an edition mistake.

pan- · 2018-02-27T11:22:03Z

@donatieng I've added a README.md file containing the instructions to run the tests and more doc in the tests.

cmonr · 2018-03-06T03:40:47Z

@donatieng Mind doing a re-review?
@paul-szczepanek-arm Ping?

donatieng

Great progress there - would want @OPpuolitaival's review

OPpuolitaival

Looks good

OPpuolitaival · 2018-03-07T14:44:51Z

features/FEATURE_BLE/tests/generic/GattClient/TestDiscoverAllServices.cpp

+		// if counter didn't wrap then send an error
+		if (next_attribute_handle != 0) {
+			// set expectation on the discovery function
+			//Log::info() << "expect discover_primary_service_by_service_uuid(" << connection_handle << ", " << next_attribute_handle << "," << service_filter << ") "


Why these log lines are commented out?

They can be removed; they are a residue of test developments.

Assuming that the log comments won't be removed.

If they still will be, then we can restart CI once that commit is in.

cmonr · 2018-03-21T02:39:14Z

/morph build

mbed-ci · 2018-03-21T03:17:22Z

Build : SUCCESS

Build number : 1501
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6086/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-03-21T06:21:55Z

Exporter Build : SUCCESS

Build number : 1144
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6086/

mbed-ci · 2018-03-21T07:36:15Z

Test : FAILURE

Build number : 1284
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6086/1284

0xc0170 · 2018-03-21T11:21:01Z

/morph test

mbed-ci · 2018-03-21T19:34:16Z

Test : FAILURE

Build number : 1290
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6086/1290

cmonr · 2018-03-21T22:06:34Z

Had to replace a F429ZI device.

Retesting.
/morph test

mbed-ci · 2018-03-22T11:09:21Z

Test : FAILURE

Build number : 1299
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6086/1299

0xc0170 · 2018-03-22T13:09:23Z

/morph test

mbed-ci · 2018-03-22T18:39:51Z

Test : SUCCESS

Build number : 1303
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6086/1303

donatieng · 2018-03-23T10:40:51Z

Great stuff - Let's get this to master :)
@OPpuolitaival this could be a good showcase example for unit-testing within mbed OS!

OPpuolitaival · 2018-03-23T10:45:00Z

@donatieng yes this is. I just would like to see one common way to do unit testing instead that all teams create own approach. But yes google test looks much better tool than cpputest.

sg- · 2018-03-26T10:20:51Z

Are the docs in the handbook for how Mbed OS unit testing works?
Why is this based on CMake? Nothing in Mbed OS is based on CMake
Why is this part of a patch release?

Glad to see this progressing but from first glance this looks far from complete but hope I'm just missing something.

pan- added 3 commits February 13, 2018 17:01

BLE: Add unit test for the Generic GattClient.

aeb992e

These tests are build around gtest and gmock and solely run on a host; cmake is used as a build system: - cd features/FEATURE_BLE/tests - mkdir build - cd build - cmake .. - make - ./gatt-client-tests

Enable travis for ble host tests.

734b5ec

BLE: Fix make invocation for tests builds by travis.

cc5b192

cmonr added the needs: work label Feb 13, 2018

Travis: add gcc6 as dependency and use it in ble testing

b689a59

0xc0170 added needs: CI and removed needs: work labels Feb 19, 2018

0xc0170 requested review from donatieng and paul-szczepanek-arm February 20, 2018 15:06

0xc0170 added needs: review and removed needs: CI labels Feb 20, 2018

donatieng reviewed Feb 26, 2018

View reviewed changes

pan- added 2 commits February 27, 2018 11:15

BLE: Improve generic gatt client tests documentation.

c6bca57

BLE: Add host test instructions.

871ebb6

donatieng reviewed Mar 7, 2018

View reviewed changes

OPpuolitaival approved these changes Mar 7, 2018

View reviewed changes

cmonr added needs: CI and removed needs: review labels Mar 21, 2018

0xc0170 approved these changes Mar 23, 2018

View reviewed changes

0xc0170 added ready for merge and removed needs: CI labels Mar 23, 2018

cmonr added the release-version: 5.8.1 label Mar 23, 2018

cmonr merged commit 387027d into ARMmbed:master Mar 23, 2018

cmonr removed the ready for merge label Mar 23, 2018

0xc0170 added release-version: 5.9.0-rc1 and removed release-version: 5.8.1 release-version: 5.9.0-rc1 labels Mar 26, 2018

0xc0170 mentioned this pull request Mar 26, 2018

Revert "BLE: Gatt client unit tests" #6452

Merged

BLE: Gatt client unit tests #6086

BLE: Gatt client unit tests #6086

Uh oh!

Conversation

pan- commented Feb 13, 2018

Description

Migrations

Uh oh!

0xc0170 commented Feb 14, 2018

Uh oh!

0xc0170 commented Feb 19, 2018

Uh oh!

mbed-ci commented Feb 19, 2018

Triggering tests

Uh oh!

mbed-ci commented Feb 19, 2018

Uh oh!

mbed-ci commented Feb 19, 2018

Uh oh!

donatieng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pan- commented Feb 27, 2018

Uh oh!

cmonr commented Mar 6, 2018

Uh oh!

donatieng left a comment

Choose a reason for hiding this comment

Uh oh!

OPpuolitaival left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmonr commented Mar 21, 2018

Uh oh!

mbed-ci commented Mar 21, 2018

Triggering tests

Uh oh!

mbed-ci commented Mar 21, 2018

Uh oh!

mbed-ci commented Mar 21, 2018

Uh oh!

0xc0170 commented Mar 21, 2018

Uh oh!

mbed-ci commented Mar 21, 2018

Uh oh!

cmonr commented Mar 21, 2018

Uh oh!

mbed-ci commented Mar 22, 2018

Uh oh!

0xc0170 commented Mar 22, 2018

Uh oh!

mbed-ci commented Mar 22, 2018

Uh oh!

donatieng commented Mar 23, 2018

Uh oh!

OPpuolitaival commented Mar 23, 2018

Uh oh!

sg- commented Mar 26, 2018

Uh oh!

Uh oh!