Customer port verification tests for cellular #4446

hasnainvirk · 2017-06-05T15:25:05Z

Description

Basic TCP/UDP tests for PPP based onboard cellular modems.
Tests basic public APIs defined in CellularBase.h
A customer port must pass this test set, hence verifying their
particular implementation.

Status

READY

Migrations

NO

0xc0170 · 2017-06-05T15:27:11Z

features/netsocket/cellular/generic_modem_driver/TESTS/unit_tests/default/main.cpp

@@ -0,0 +1,541 @@
+#include "mbed.h"


license please

License was added @0xc0170 . i don't know why it's not visible in diff

0xc0170 · 2017-06-05T15:27:46Z

features/netsocket/cellular/generic_modem_driver/TESTS/unit_tests/default/main.cpp

+ */
+
+// The credentials of the SIM in the board.
+#ifndef MBED_CONF_APP_DEFAULT_PIN


can this be put in the header file, too much noise in the test code file?

These are coming from a json file needed to provide user configuration. If I create a header file that will be then included in every single build. Wouldn't it ?

The default value does not have to be defined in this C file. I think that's what martin is getting at.

Also, a header does not get included if you don't #include it.

If TESTS/ folder is being skipped at build time then yes we can add a header file. Otherwise, this may cause contentions with user configuration for his/her application.

The TESTS/ folder is skipped by mbed compile

Okay, great to know. I thought only mbedignore was doing that locally,

As jimmy noted, my objection was that if the default config is inhte header file, the code file stays clean and only implementation there.. scrolling down through 100x lines of ifdef config values is not user friendly . This header file could come with the test in the same folder (default/main.cpp default/default_config.h)

0xc0170 · 2017-06-05T15:28:30Z

features/netsocket/cellular/generic_modem_driver/TESTS/unit_tests/default/main.cpp

+/**
+ * Send an entire TCP data buffer until done
+ */
+static int sendAll(TCPSocket *sock, const char *data, int size)


noCamelCase send_all

0xc0170 · 2017-06-05T15:34:20Z

features/netsocket/cellular/generic_modem_driver/TESTS/unit_tests/default/main.cpp

+/**
+ * Setup Test Environment
+ */
+utest::v1::status_t test_setup(const size_t number_of_cases) {


can you fix formatting ( { on the new line, I see 8 spaces on the line 495, etc)

0xc0170 · 2017-06-05T15:34:31Z

features/netsocket/cellular/generic_modem_driver/TESTS/unit_tests/default/main.cpp

+ */
+void test_connect_credentials() {
+
+	driver->disconnect();


tab here (misaligned?)

0xc0170 · 2017-06-05T15:35:11Z

features/netsocket/cellular/generic_modem_driver/TESTS/unit_tests/default/main.cpp

+}
+
+/**
+ * Verification tests for a successful porting


worth having on the top of the file or might be overlooked in here?

hasnainvirk · 2017-06-07T10:47:53Z

@tommikas This is not passing integration tests because it can't find K64F resources in RAAS. Could you please make sure enough devices are availoable in RAAS ?

Basic TCP/UDP tests for PPP based onboard cellular modems. Tests basic public APIs defined in CellularBase.h A customer port must pass this test set, hence verifying their particular implementation.

VeliMattiLahtela · 2017-06-08T09:09:21Z

Not failing on our Jenkins side.

TuomoHautamaki · 2017-06-08T11:18:33Z

@0xc0170 any idea how we could re-trigger tests: Cam-CI uvisor Build & Test? Seems that could be temp CI issue..

TuomoHautamaki · 2017-06-08T12:00:09Z

@AlessandroA any idea how we could re-trigger tests: Cam-CI uvisor Build & Test? Seems that could be temp CI issue..

0xc0170 · 2017-06-08T12:36:09Z

retest uvisor

hasnainvirk · 2017-06-08T16:27:03Z

@sg- morph test needed here
@TuomoHautamaki yeah, it was a temporary CI issue. All tests passed.

hasnainvirk · 2017-06-13T13:30:29Z

@0xc0170 This will need CI too

TuomoHautamaki · 2017-06-20T11:56:08Z

@0xc0170 anything preventing this to be merged?

0xc0170 · 2017-06-20T12:02:57Z

@0xc0170 anything preventing this to be merged?

Review after the changes, and CI (will be queued once we get 5.5.1 tested today)

0xc0170 · 2017-06-20T12:08:08Z

features/netsocket/cellular/generic_modem_driver/TESTS/unit_tests/default/template_mbed_app.txt

@@ -0,0 +1,69 @@
+{


what's the intention with this .txt ? It is just a template for json file, that is not present but required by these tests. I dont see any defaults here?

check the docs here https://github.com/ARMmbed/mbed_OS_API_Docs/blob/5.5/docs/APIs/communication/cellular.md#port-verification-testing

You can't have a json file here. Yes, it is template file, whose contents are copied to a json file when it is desirable to run these particular tests. A tutorial is also part of the file, check lines after license.

0xc0170 · 2017-06-20T12:08:48Z

features/netsocket/cellular/generic_modem_driver/TESTS/unit_tests/default/main.cpp

+        tr_debug("Sent %d, |%*.*s|", size, size, size, test_data);
+        tr_debug("Rcvd %d, |%*.*s|", size, size, size, (char * ) recv_data);
+        // We do not assert a failure here because ublox TCP echo server doesn't send
+        // back original data. It actually constructs a ublox message string. They need to fix it as


ublox ? isnt this generic test?

We use ublox's echo server for testing. It's a public echo server. However, unfortunately their TCP server doesn't echo back the data we send. It sends a packet of the correct length but with arbitrary data. UDP server works as it should. This was reported to them already.

0xc0170 · 2017-06-20T12:09:42Z

features/netsocket/cellular/generic_modem_driver/TESTS/unit_tests/default/main.cpp

+    TEST_ASSERT(driver->is_connected());
+
+    TEST_ASSERT(ip_address != NULL);
+    tr_debug("IP address %s.", ip_address);


how does this help if assert is first , then ip address ? If ip adress is null, it would finish on the assert, would never get to print it?

Yes, its a connectivity test. If you are connected, you should have an IP address as well.

0xc0170

Can you please share the tests results on some targets?

0xc0170 · 2017-06-22T17:59:07Z

/morph test

0xc0170 · 2017-06-22T18:04:00Z

To summarise, these tests cant be automated at the moment as they require additional details that are in the template ? I am missing this type of information.

Can you share the tests results run on the platform? How to automate these? can we anyhow?

mbed-bot · 2017-06-22T18:45:36Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 615

Build failed!

TuomoHautamaki · 2017-06-26T06:17:36Z

There is no automation available out of the box. A set of tests cases and documentation are provided, allowing users to either execute tests manually, or develop a method for automating the verification.

0xc0170 · 2017-06-26T06:57:11Z

/morph test

mbed-bot · 2017-06-26T08:56:30Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 630

Test failed!

theotherjimmy · 2017-06-26T15:06:27Z

/morph test

mbed-bot · 2017-06-27T05:18:56Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 643

Test failed!

0xc0170 · 2017-06-28T12:08:31Z

retest uvisor

/morph test

mbed-bot · 2017-06-28T22:24:11Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 678

All builds and test passed!

0xc0170 · 2017-06-29T13:43:49Z

retest uvisor

0xc0170 · 2017-06-30T08:38:38Z

@hasnainvirk I believe we got one problem with this test, if all tests are run for a target, this reports as UNDEF, and causes CI to report a failure. We should have run nightly for this job, which would result with this error in this case. Please fix asap

20:06:36 	no test case report present, assuming test suite to be a single test case!
20:06:36 	test suite: features-netsocket-cellular-generic_modem_driver-tests-unit_tests-default
20:06:36 	test case: features-netsocket-cellular-generic_modem_driver-tests-unit_tests-default
20:06:36 mbedgt: test on hardware with target id: 0220000025414e450055501147e0001c30f1000097969900
20:06:36 mbedgt: test suite 'features-netsocket-cellular-generic_modem_driver-tests-unit_tests-default' ....... UNDEF in 14.79 sec
20:06:36 	test case: 'features-netsocket-cellular-generic_modem_driver-tests-unit_tests-default' ....... ERROR in 14.79 sec

internal link here for review http://mbed-ci-master-2.austin.arm.com:8081/job/test_matrix/631/target=KL46Z,toolchain=GCC_ARM/consoleFull

0xc0170 · 2017-06-30T08:51:18Z

I found what is missing, sending a patch now

0xc0170 requested changes Jun 5, 2017

View reviewed changes

0xc0170 added the needs: work label Jun 5, 2017

hasnainvirk force-pushed the add_cellular_unit_tests branch 4 times, most recently from 906f99b to c08fe08 Compare June 5, 2017 18:25

Customer port verification tests for cellular

09b07a4

Basic TCP/UDP tests for PPP based onboard cellular modems. Tests basic public APIs defined in CellularBase.h A customer port must pass this test set, hence verifying their particular implementation.

hasnainvirk force-pushed the add_cellular_unit_tests branch from c08fe08 to 09b07a4 Compare June 7, 2017 12:29

sg- added needs: CI and removed needs: work labels Jun 8, 2017

0xc0170 reviewed Jun 20, 2017

View reviewed changes

0xc0170 approved these changes Jun 20, 2017

View reviewed changes

0xc0170 added ready for merge and removed needs: CI labels Jun 29, 2017

theotherjimmy merged commit 8f42c87 into ARMmbed:master Jun 29, 2017

theotherjimmy added release-version: 5.5.2 and removed ready for merge labels Jun 29, 2017

0xc0170 mentioned this pull request Jun 30, 2017

Cellular: not-supported error if MODEM_ON_BOARD not defined #4672

Merged

adbridge added release-version: 5.5.3 and removed release-version: 5.5.2 labels Jun 30, 2017

Customer port verification tests for cellular #4446

Customer port verification tests for cellular #4446

Uh oh!

Conversation

hasnainvirk commented Jun 5, 2017

Description

Status

Migrations

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!

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!

hasnainvirk commented Jun 7, 2017

Uh oh!

VeliMattiLahtela commented Jun 8, 2017

Uh oh!

TuomoHautamaki commented Jun 8, 2017

Uh oh!

TuomoHautamaki commented Jun 8, 2017

Uh oh!

0xc0170 commented Jun 8, 2017

Uh oh!

hasnainvirk commented Jun 8, 2017

Uh oh!

hasnainvirk commented Jun 13, 2017

Uh oh!

TuomoHautamaki commented Jun 20, 2017

Uh oh!

0xc0170 commented Jun 20, 2017

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!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Jun 22, 2017

Uh oh!

0xc0170 commented Jun 22, 2017

Uh oh!

mbed-bot commented Jun 22, 2017

Output

Uh oh!

TuomoHautamaki commented Jun 26, 2017

Uh oh!

0xc0170 commented Jun 26, 2017

0xc0170 commented Jun 30, 2017 •

edited

Loading