-
Notifications
You must be signed in to change notification settings - Fork 3k
TCP/TLS Socket tests will skip if TCP is not supported #10037
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
TCP/TLS Socket tests will skip if TCP is not supported #10037
Conversation
ae110a7
to
a206df9
Compare
Uh.. You completely lost me with those macros.. I was hoping a way more simpler solution, like:
Add extra 3 lines per TCP test case.. should not be too much. |
@michalpasztamobica, thank you for your changes. |
I did not consider this option to avoid influencing test results by creating, opening and closing an extra socket before every (or at least one) test routine. In this case however, I would put it on top of #9959 and add it to the test case setup function, rather than copy it into the code of every test function... |
Review astyle travis failing job |
a206df9
to
690cfc5
Compare
@SeppoTakalo , unfortunately I could not put the SKIP macro inside the testcase's setup function, because the macro returns void and the wrapper needs to return a value. But I did the second best thing possible - I added the function as you suggested and wrapped it with a simple macro, which got added to every test case. To test the changes I locally broke LWIP::socket_open() function to return NSAPI_ERROR_UNSUPPORTED when TCP protocol is in use. All TCP and TLS tests printed the SKIP message and showed up as passing in the summary. Reviewers, please review :) |
690cfc5
to
69b22a4
Compare
@michalpasztamobica Could you check the astyle job results? |
Also, it looks like the preceeding PR has been merged. |
69b22a4
to
48a3a3e
Compare
astyle fixed and the preceding PR is indeed merged (although it turned out I cannot make use of it in the end). |
CI started |
…unsupported TCP/TLS Socket tests will skip if TCP is not supported
Test run: SUCCESSSummary: 6 of 6 test jobs passed |
@SeppoTakalo @michalpasztamobica Should we wait on others to chime in? |
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.
Looka good.
@cmonr , I think there was enough time, I'd suggest to merge in. |
48a3a3e
to
09183c9
Compare
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.
Thank you!
CI started |
Test run: SUCCESSSummary: 6 of 6 test jobs passed |
Description
I generally avoid using macros, but I couldn't come up with a better way in this case.
Two reasons why macros are used to wrap the open calls:
The architectures which could make use of this (for example UBLOX_C030_N211) are not available in any RAAS server, but I agreed with @AriParkkila that he will make sure this executes correctly.
Pull request type
Reviewers
@SeppoTakalo
@KariHaapalehto
@VeijoPesonen
@mtomczykmobica
@tymoteuszblochmobica
@AriParkkila