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

Add greentea tests for network interface status and connect/disconnect #7882

Merged
merged 1 commit into from Sep 19, 2018

Conversation

Projects
None yet
6 participants
@mikaleppanen
Contributor

mikaleppanen commented Aug 24, 2018

Description

Added following network interface status and connect/disconnect greentea tests:

NETWORKINTERFACE_STATUS
NETWORKINTERFACE_STATUS_NONBLOCK
NETWORKINTERFACE_STATUS_GET
NETWORKINTERFACE_CONN_DISC_REPEAT

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[X] Functionality change
[ ] Breaking change
Added greentea tests for network interface status and connect/disconnect
Added tests:
NETWORKINTERFACE_STATUS
NETWORKINTERFACE_STATUS_NONBLOCK
NETWORKINTERFACE_STATUS_GET
NETWORKINTERFACE_CONN_DISC_REPEAT

@0xc0170 0xc0170 requested a review from ARMmbed/mbed-os-ipcore Aug 24, 2018

@0xc0170 0xc0170 changed the title from Added greentea tests for network interface status and connect/disconnect to Add greentea tests for network interface status and connect/disconnect Aug 24, 2018

/*
* Test cases
*/
void NETWORKINTERFACE_STATUS();

This comment has been minimized.

@0xc0170

0xc0170 Aug 24, 2018

Member

why are these in capital letters? I assumed these are macros that somehow get overwritten somewhere?

This comment has been minimized.

@mikaleppanen

mikaleppanen Aug 27, 2018

Contributor

This is the naming convention for our socket and network tests. Test case names are in capital letters. Used in socket and dns tests already.

This comment has been minimized.

@mikaleppanen

mikaleppanen Aug 27, 2018

Contributor

Yes, older tests not using that yet. @SeppoTakalo should we update also those?

This comment has been minimized.

@SeppoTakalo

SeppoTakalo Aug 27, 2018

Contributor

I prefer the exact same name as test plan has.

Test cases are anyway a bit special, so having exception to common API guidelines is acceptable.

Having testcases in small letters has a added benefit that it is clearly visible separation from test suite (binary name)

10:18:17 mbedgt: test suite 'mbed-os-tests-netsocket-dns' ..................................................... OK in 72.73 sec
10:18:17 	test case: 'ASYNCHRONOUS_DNS' ................................................................ OK in 0.11 sec
10:18:17 	test case: 'ASYNCHRONOUS_DNS_CACHE' .......................................................... OK in 0.33 sec
10:18:17 	test case: 'ASYNCHRONOUS_DNS_CANCEL' ......................................................... OK in 5.34 sec
10:18:17 	test case: 'ASYNCHRONOUS_DNS_EXTERNAL_EVENT_QUEUE' ........................................... OK in 2.60 sec
10:18:17 	test case: 'ASYNCHRONOUS_DNS_INVALID_HOST' ................................................... OK in 0.66 sec
10:18:17 	test case: 'ASYNCHRONOUS_DNS_NON_ASYNC_AND_ASYNC' ............................................ OK in 0.56 sec
10:18:17 	test case: 'ASYNCHRONOUS_DNS_SIMULTANEOUS' ................................................... OK in 0.55 sec
10:18:17 	test case: 'ASYNCHRONOUS_DNS_SIMULTANEOUS_CACHE' ............................................. OK in 0.55 sec
10:18:17 	test case: 'ASYNCHRONOUS_DNS_SIMULTANEOUS_REPEAT' ............................................ OK in 28.41 sec
10:18:17 	test case: 'ASYNCHRONOUS_DNS_TIMEOUTS' ....................................................... OK in 2.39 sec

This comment has been minimized.

@mikaleppanen

mikaleppanen Aug 28, 2018

Contributor

0xc0170 I think we would be changing the emac/wifi tests to match socket/dns tests in an another pull requests. Can this pull request proceed?

This comment has been minimized.

@0xc0170

0xc0170 Aug 28, 2018

Member

Yes, entering needs: CI now.

Test cases are anyway a bit special, so having exception to common API guidelines is acceptable.

It would be (I would still challenge we are entering the ground where preprocessor can replace it and leads to failures. I would avoid naming anything else with capital letters but macros) but this is not the case for test cases for our tests. I found this only in these tests not anywhere else so do not see why it would be different (we can update the test plan to match the implementation). If this is something up for the change, should be documented (test cases are upper case because..) and followed?

cc @ARMmbed/mbed-os-test

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 18, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Sep 18, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 merged commit 0233c8d into ARMmbed:master Sep 19, 2018

14 checks passed

ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed , RTOS ROM(+0.0%) RAM(-0.09%)
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/cloud_client_smoke_test Test job was successful
Details
travis-ci/astyle Passed, 558 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 8998 cycles
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8372B
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment