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

Ethernet tests update #5576

Merged
merged 5 commits into from Dec 12, 2017

Conversation

Projects
None yet
6 participants
@jeromecoutant
Contributor

jeromecoutant commented Nov 24, 2017

Description

Some tests have been updated:

tests-netsocket-gethostbyname:

  • MBED_DNS_TEST_HOST define is replaced by MBED_CONF_APP_DNS_TEST_HOST to allow user to change host name for local tests

tests-netsocket-udp_echo:

  • UUID lines are removed as they were not used
  • If MBED_CONF_APP_ECHO_SERVER_ADDR and MBED_CONF_APP_ECHO_SERVER_PORT are not defined (default case), test is using Greentea to get server information (code before OS 5.6.1 version)

tests-netsocket-tcp_echo:

  • UUID lines are removed as they were not used
  • If MBED_CONF_APP_ECHO_SERVER_ADDR and MBED_CONF_APP_ECHO_SERVER_PORT are not defined (default case), test is using Greentea to get server information (code before OS 5.6.1 version)
  • TCP_ECHO_PREFIX is no more a mandatory feature

tests-netsocket-tcp_hello_world:

  • HTTP_SERVER_NAME and HTTP_SERVER_FILE_PATH are replaced by MBED_CONF_APP_HTTP_SERVER_NAME and MBED_CONF_APP_HTTP_SERVER_FILE_PATH to allow user to make local tests

Status

READY

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Nov 24, 2017

Added update for tests-netsocket-socket_sigio,
as test was always OK even with no connection!

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Nov 28, 2017

@SeppoTakalo @sarahmarshy
Any update ?
Thx

@sarahmarshy

This comment has been minimized.

Contributor

sarahmarshy commented Nov 28, 2017

I'll get to this tomorrow. I've been out on holiday, and in travelling home today. One immediate question: why did you remove part of the EthernetInterface config? That is what most targets use if they have LWIP, so you'll leave a lot undefined.

@geky can also review

@@ -122,10 +126,13 @@ void test_socket_attach() {
sock.send(buffer, strlen(buffer));
// wait for recv data
recvd.wait();

result = true;

This comment has been minimized.

@sarahmarshy

sarahmarshy Nov 29, 2017

Contributor

as test was always OK even with no connection!

That should not be the case -- this line will attach the get_data function. If get_data never gets called because of an incorrect implementation of sigio, this test case will time out. If get_data does get called, the success of the test depends on the getting some data from the server here

This comment has been minimized.

@jeromecoutant

jeromecoutant Nov 30, 2017

Contributor

No...
With my local test env, test was always OK whereas it is not possible to connect ARM server....

tools/test_configs/EthernetInterface.json Outdated
@@ -10,18 +10,6 @@
"connect-statement" : {
"help" : "Must use 'net' variable name",
"value" : "((EthernetInterface *)net)->connect()"
},

This comment has been minimized.

@sarahmarshy

sarahmarshy Nov 29, 2017

Contributor

These changes will cause mbed OS tests to fail. I believe this should continue to be the default test configuration file. If echo server changes are necessary, then that person should use a custom test configuration file and specify it with the --test-config option.

This comment has been minimized.

@jeromecoutant

jeromecoutant Nov 30, 2017

Contributor

No...
Check the updates in test c files. You can keep these setting in your files, there will be no change for you.
If user doesn't want to set manually the settings and use greentea feature, it is now possible

This comment has been minimized.

@sarahmarshy

sarahmarshy Nov 30, 2017

Contributor

Yes. By default, LWIP targets use this file. The echo server is not set to ublox echo server in this file, nor in the cpp files.

This comment has been minimized.

@jeromecoutant

jeromecoutant Dec 4, 2017

Contributor

Change your test setup...
Default configuration should be the basic configuration without any specific setting.
If you have a specific test environment, use a specific json configuration file.

This comment has been minimized.

@sarahmarshy

sarahmarshy Dec 4, 2017

Contributor

By and large, the Ublox server will be accessible on a network. It is a special case that is is not accessible. We want to provide a sensible default, and it is, unless you have a special case.

jeromecoutant added some commits Nov 24, 2017

tests-netsocket-gethostbyname
MBED_DNS_TEST_HOST define is replaced by MBED_CONF_APP_DNS_TEST_HOST to allow user to change host name for local tests
tests-netsocket-udp_echo
UUID lines are removed as they were not used

default case:
If MBED_CONF_APP_ECHO_SERVER_ADDR and MBED_CONF_APP_ECHO_SERVER_PORT are not defined
test is using Greentea to get server information (code before OS 5.6.1 version)
tests-netsocket-tcp_echo
UUID lines are removed as they were not used

default case:
If MBED_CONF_APP_ECHO_SERVER_ADDR and MBED_CONF_APP_ECHO_SERVER_PORT are not defined
test is using Greentea to get server information (code before OS 5.6.1 version)

TCP_ECHO_PREFIX is no more a mandatory step
tests-netsocket-tcp_hello_world
HTTP_SERVER_NAME and HTTP_SERVER_FILE_PATH are replaced by
  MBED_CONF_APP_HTTP_SERVER_NAME and MBED_CONF_APP_HTTP_SERVER_FILE_PATH
  to allow user to make local tests
tests-netsocket-socket_sigio
HTTP_SERVER_NAME and HTTP_SERVER_FILE_PATH are replaced by
  MBED_CONF_APP_HTTP_SERVER_NAME and MBED_CONF_APP_HTTP_SERVER_FILE_PATH
  to allow user to make local tests

Test on HTTP connect added as test was always OK even with no connection...

@jeromecoutant jeromecoutant force-pushed the jeromecoutant:PR_IP branch to d8cc5a3 Dec 4, 2017

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Dec 4, 2017

Hi @sarahmarshy

json files updates removed and rebase done
There should be no change in your test env, and it's OK in my local test env.

Regards

@sarahmarshy

This comment has been minimized.

Contributor

sarahmarshy commented Dec 4, 2017

/morph test

@mbed-ci

This comment has been minimized.

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Dec 4, 2017

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Dec 4, 2017

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 5, 2017

Thanks @sarahmarshy , I assume you are happy with as it is now.

@SeppoTakalo Please can you review this PR

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 7, 2017

Just to be certain

/morph test

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 merged commit 57a5735 into ARMmbed:master Dec 12, 2017

10 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter 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/events Local events testing has passed
Details
travis-ci/littlefs Local littlefs testing has passed
Details
travis-ci/mbed2 Local mbed2 testing has passed
Details
travis-ci/tools Local tools testing has passed
Details

@jeromecoutant jeromecoutant deleted the jeromecoutant:PR_IP branch Dec 13, 2017

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