-
Notifications
You must be signed in to change notification settings - Fork 18
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
Porting tests to new utest-greentea framework #62
Conversation
@bremoran @niklas-arm @PrzemekWirkus @0xc0170 @stegut01 |
@@ -41,6 +36,7 @@ void terminate(bool status, TCPEchoClient* client); | |||
|
|||
char out_buffer[] = "Hello World\n"; | |||
char buffer[256]; | |||
char out_success[] = "{{success}}\n{{end}}\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This string is confusing, is this just a payload used for test or actual control command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved it outside from onRx() in original code to sit with other strings. It indicates to the host test that target test has validated a round trip (echo) data and now terminating. It is good stop to host test from echoing back as target client closes the connection right after sending this string.
But it should not be confused with serial k,v command.
CI build failing for error
|
… thread. It helps to close the connection when test tears down.
d2d78c4
to
9337384
Compare
I might be mistaken, but isn't TCP and UDP client already part of socket-test? I did take the tests in this repo as a basis for the new tests, but with utest expressing the statefulness of these tests can be achieved much better without classes. |
@niklas-arm Yes, I asked Przemek the same question. I was told that all these public tests which are part of our modules will be updated to use the new greentea client, deprecate to use test_env. Plus, those tests you references, are not public yet. Przemek might add more details. |
@niklas-arm if tests were moved to socket-test then as part of the move they should have been deleted from this repo. So that nobody wastes time in fixing or running them. Please remove these tests from this repo if not needed here now. Anyway other purpose of updating these tests is to gain understanding of utest. You can atleast review from the perspective of utest usage. |
The tests weren't "moved", we just didn't make the new ones public, and we didn't update these.
I would have liked to have a more granular test cases, instead of one big class, similar to how socket-tests work, but if it works this way, it's fine too. |
Good to know that the effort isn't wasted. We can surely have granular tests in socket-test/test. |
@@ -21,7 +21,9 @@ | |||
"minar": "^1.0.0" | |||
}, | |||
"testDependencies": { | |||
"mbed-drivers": "~0.11.1" | |||
"mbed-drivers": "~0.11.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dependency is a little old.
A couple of minor comments. updating the dependency to mbed-drivers ~0.12.0 will fix the CI build error. |
@bremoran review comments incorporated. |
Porting tests to new utest-greentea framework
Following tests are ported to new utest-greentea framework. Also necessary changes have be done in the test code for tests to work and for clean tear down.
For details on new utest - greentea framework please see description in PR ARMmbed/greentea#78