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

Greentea netsocket more tests #6665

Merged
merged 19 commits into from May 25, 2018

Conversation

Projects
None yet
8 participants
@VeijoPesonen
Contributor

VeijoPesonen commented Apr 18, 2018

Description

Improving Greentea netsocket test case coverage

Pull request type

[X] Test case
[ ] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@VeijoPesonen

This comment has been minimized.

Contributor

VeijoPesonen commented Apr 18, 2018

@kjbracey-arm would you please add DO NOT MERGE-label. There are more test cases to come. @SeppoTakalo, @jarlamsa, @juhaylinen, you can start reviewing if you want but it's not expected. At this stage I just want to make the stuff visible so you can see how things are proceeding.

net->disconnect();
}
/** Generate RFC 864 example pattern.

This comment has been minimized.

@VeijoPesonen

VeijoPesonen Apr 18, 2018

Contributor

Implementation provided by @SeppoTakalo

@@ -13,7 +13,7 @@
},
"echo-server-addr" : {
"help" : "IP address of echo server",
"value" : "\"52.215.34.155\""
"value" : "\"echo.mbedcloudtesting.com\""

This comment has been minimized.

@VeijoPesonen

VeijoPesonen Apr 20, 2018

Contributor

This won't work. Only IP address is accepted.

This comment has been minimized.

@SeppoTakalo

SeppoTakalo Apr 20, 2018

Contributor

Where this limitation is?
Socket API is perfectly capable of accepting domain names.

This comment has been minimized.

@VeijoPesonen

VeijoPesonen Apr 20, 2018

Contributor

Need to figure it out.

{
int err;
TCPSocket sock;
SocketAddress tcp_addr(MBED_CONF_APP_ECHO_SERVER_ADDR, 19);

This comment has been minimized.

@SeppoTakalo

SeppoTakalo Apr 20, 2018

Contributor

SocketAddress does not resolve names. It is just a placeholder for IP addresses.
Use NetworkInterface::gethostbyname() or feed the string to Socket which translates its string through the gethostbyname()

This comment has been minimized.

@SeppoTakalo

SeppoTakalo Apr 20, 2018

Contributor

So for example TCPSocket::connect(const char *host, uint16_t port) does the translation.

This comment has been minimized.

@VeijoPesonen

VeijoPesonen Apr 23, 2018

Contributor

Ok, I just need to remember to make the same change to all netsocket test cases.

@VeijoPesonen VeijoPesonen force-pushed the VeijoPesonen:greentea_netsocket_more_tests branch 8 times, most recently from d940e9d to 5b2b792 Apr 23, 2018

@VeijoPesonen VeijoPesonen force-pushed the VeijoPesonen:greentea_netsocket_more_tests branch 4 times, most recently from 4f7c503 to f69eb9e Apr 30, 2018

@VeijoPesonen VeijoPesonen force-pushed the VeijoPesonen:greentea_netsocket_more_tests branch from 501de63 to a9d0167 May 11, 2018

@VeijoPesonen

This comment has been minimized.

Contributor

VeijoPesonen commented May 11, 2018

TCPSOCKET_RECV_100K_NONBLOCK and TCPSOCKET_RECV_100K are still unstable but people can start to review the other tests.

Wrong lwIP configuration (#define TCP_QUEUE_OOSEQ 0) was causing the issue

@SeppoTakalo

Add error handling.

There are multiple cases where you call socket functions without testing the return code.
For example:

get_interface()->gethostbyname(...);
sock->open(...);
sock->connect(...);
sock->close();

Every socket call should be tested and compared against acceptable return values. Not only those that transfer the data, but also ones than translate names or create connection.

utest::v1::status_t greentea_setup(const size_t number_of_cases)
{
GREENTEA_SETUP(240, "default_auto");
_ifup();

This comment has been minimized.

@SeppoTakalo

SeppoTakalo May 14, 2018

Contributor

Does the TEST_ASSERT() propagation work in greentea_setup()?
Are we able to see whether device did not join the network and therefore tests failed to run?

This comment has been minimized.

@VeijoPesonen

VeijoPesonen May 14, 2018

Contributor

Yes, we are able to see if the problem arises from the handler. But there is still need to check the logs.

PS C:\ws\mbed-os> mbedhtrun -f .\BUILD\tests\K64F\ARM\TESTS\netsocket\udp\udp.bin -d D: -p COM14:9600
[1526317024.03][HTST][INF] host test executor ver. 1.2.0
[1526317024.03][HTST][INF] copy image onto target...
        1 file(s) copied.
[1526317032.47][MBED][WRN] Target ID not found: Skipping flash check and retry
[1526317032.47][HTST][INF] starting host test process...
[1526317033.09][CONN][INF] starting connection process...
[1526317033.09][CONN][INF] notify event queue about extra 60 sec timeout for serial port pooling
[1526317033.09][CONN][INF] initializing serial port listener...
[1526317033.09][SERI][INF] serial(port=COM14, baudrate=9600, read_timeout=0.01, write_timeout=5)
[1526317033.09][SERI][INF] reset device using 'default' plugin...
[1526317033.11][HTST][INF] setting timeout to: 60 sec
[1526317033.36][SERI][INF] waiting 1.00 sec after reset
[1526317034.36][SERI][INF] wait for it...
[1526317034.36][SERI][TXD] mbedmbedmbedmbedmbedmbedmbedmbedmbedmbed
[1526317034.36][CONN][INF] sending up to 2 __sync packets (specified with --sync=2)
[1526317034.36][CONN][INF] sending preamble '0ea1005b-0e2c-4dec-a15f-c947b6556639'
[1526317034.36][SERI][TXD] {{__sync;0ea1005b-0e2c-4dec-a15f-c947b6556639}}
[1526317034.49][CONN][RXD] mbedmbedmbedmbedmbedmbedmbedmbed
[1526317034.54][CONN][INF] found SYNC in stream: {{__sync;0ea1005b-0e2c-4dec-a15f-c947b6556639}} it is #0 sent, queued...
[1526317034.56][HTST][INF] sync KV found, uuid=0ea1005b-0e2c-4dec-a15f-c947b6556639, timestamp=1526317034.540000
[1526317034.57][CONN][INF] found KV pair in stream: {{__version;1.3.0}}, queued...
[[1526317034.59][HTST][INF] DUT greentea-client version: 1.3.0
1526317034.59][CONN][INF] found KV pair in stream: {{__timeout;120}}, queued...
[1526317034.59][HTST][INF] setting timeout to: 120 sec
[1526317034.62][CONN][INF] found KV pair in stream: {{__host_test_name;default_auto}}, queued...
[1526317034.62][HTST][INF] host test class: '<class 'mbed_host_tests.host_tests.default_auto.DefaultAuto'>'
[1526317034.62][HTST][INF] host test setup() call...
[1526317034.62][HTST][INF] CALLBACKs updated
[1526317034.62][HTST][INF] host test detected: default_auto
[1526317078.79][CONN][RXD] :46::FAIL: Expected 0 Was -3004
[1526317078.86][CONN][RXD] >>> failure with reason 'Assertion Failed' during 'Test Setup Handler'
[1526317078.90][CONN][INF] found KV pair in stream: {{max_heap_usage;0}}, queued...
[[1526317078.91][CONN][INF] found KV pair in stream: {{reserved_heap;0}}, queued...
1526317078.91][HTST][ERR] orphan event in main phase: {{max_heap_usage;0}}, timestamp=1526317078.897000
[1526317078.91][HTST][ERR] orphan event in main phase: {{reserved_heap;0}}, timestamp=1526317078.913000
[1526317078.93][CONN][INF] found KV pair in stream: {{end;failure}}, queued...
[1526317078.94][CONN][INF] found KV pair in stream: {{__exit;0}}, queued...
[1526317078.95][HTST][INF] __exit(0)
[1526317078.95][HTST][INF] __notify_complete(False)
[1526317078.95][HTST][INF] __exit_event_queue received
[1526317078.95][HTST][INF] test suite run finished after 44.36 sec...
[1526317078.96][CONN][INF] received special even '__host_test_finished' value='True', finishing
[1526317078.98][HTST][INF] CONN exited with code: 0
[1526317078.98][HTST][INF] Some events in queue
[1526317078.98][HTST][INF] stopped consuming events
[1526317078.98][HTST][INF] host test result() call skipped, received: False
[1526317078.98][HTST][INF] calling blocking teardown()
[1526317078.98][HTST][INF] teardown() finished
[1526317078.98][HTST][INF] {{result;failure}}

This comment has been minimized.

@VeijoPesonen

VeijoPesonen May 14, 2018

Contributor

I'll add the required return value checks. I wouldn't prefer not to because it will just add clutter on the test cases and assertions would reveal problems arising during setups. Part of the test is to check that the desired behavior is the result of correct setup.

sent = sock.sendto(udp_addr, tx_buffer, pkt_s);
if (sent != pkt_s) {
printf("[Round#%02d - Sender] error, returned %d\n", s_idx, sent);
continue;

This comment has been minimized.

@SeppoTakalo

SeppoTakalo May 14, 2018

Contributor

Are you ignoring the sendto() failures?

This comment has been minimized.

@VeijoPesonen

VeijoPesonen May 14, 2018

Contributor

Yes, I am because buffer comparison will reveal if data was not transmitted and received - retries are allowed. Checking for errors wouldn't make any difference.

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 14, 2018

@VeijoPesonen Was there a particular reason why this PR was give a new category?

@SeppoTakalo @kjbracey-arm Is the intention to get this in for 5.9?

@VeijoPesonen VeijoPesonen force-pushed the VeijoPesonen:greentea_netsocket_more_tests branch from 021f6cd to 5e8de8d May 14, 2018

@VeijoPesonen

This comment has been minimized.

Contributor

VeijoPesonen commented May 14, 2018

@cmonr My changes are not fixes, refactoring of existing code, breaking change to existing code, new features and do not consider new targets. I decided to add a new category to evoke this discussion. I would say there is need to have its own category for test cases. Unless new test case is to be treated like a new feature.

@VeijoPesonen VeijoPesonen force-pushed the VeijoPesonen:greentea_netsocket_more_tests branch from 79566a5 to b3c498c May 15, 2018

VeijoPesonen and others added some commits May 9, 2018

Adds TCP test case
	tcpsocket_send_repeat
Adds UDP Greentea test case
        udpsocket_sendto_repeat
Adds UDP Greentea test cases
	udpsocket_echotest
	udpsocket_echotest_nonblock
Fixes Greentea UDP test cases
	udpsocket_echotest.cpp
	udpsocket_echotest_burst.cpp
Switches Greentea netsocket test case naming convention
From lowercase to uppercase and drops 'test_'-prefix. Test case
descriptions do also match to TC names now.

Additionally all the TCs are not behing MBED_EXTENDED_TESTS
anymore.

@SeppoTakalo SeppoTakalo dismissed stale reviews from kjbracey-arm and themself via 491a7ea May 25, 2018

@SeppoTakalo SeppoTakalo force-pushed the VeijoPesonen:greentea_netsocket_more_tests branch from 87dc2d3 to 491a7ea May 25, 2018

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented May 25, 2018

Rebased on top of master.

Emac is now in "needs Preceeding PR" label should be removed.

@0xc0170 or @kjbracey-arm please start tests for this.

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented May 25, 2018

It would be nice if this could go to 5.9.0, but I think CI load may prevent it. Labelling as 5.9.1 for now.

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 25, 2018

Going to attempt to squeeze this in.

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented May 25, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr

cmonr approved these changes May 25, 2018

@cmonr cmonr merged commit 45dc00d into ARMmbed:master May 25, 2018

13 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 911 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10133 cycles (-90 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 9964B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@VeijoPesonen VeijoPesonen deleted the VeijoPesonen:greentea_netsocket_more_tests branch May 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment