Reduce greentea socket tests failures related to network issues #10330
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ciarmcom
requested review from
KariHaapalehto,
mtomczykmobica,
SeppoTakalo,
tymoteuszblochmobica,
VeijoPesonen and
a team
|
@michalpasztamobica, thank you for your changes. |
VeijoPesonen
approved these changes
Apr 8, 2019
0xc0170
approved these changes
Apr 8, 2019
SeppoTakalo
reviewed
Apr 8, 2019
| @@ -859,9 +859,6 @@ Within each loop, one `recvfrom()` may return the received packet size | |||
| When `NSAPI_ERROR_WOULD_BLOCK` is received, check that time consumed is | |||
| more that 100 milliseconds but less than 200 milliseconds. | |||
|
|
|||
| After repeating for 10 times, at least 5 packets must have been | |||
| received. | |||
There was a problem hiding this comment.
50 % packet loss is quite a big.
If there is 100 % packet loss, and we accept it, how is this going to test anything?
SeppoTakalo
approved these changes
Apr 8, 2019
michalpasztamobica
force-pushed
the
greentea_test_fixes
branch
from
4bef448 to
872edd4
Compare
|
Ci started |
SeppoTakalo
approved these changes
Apr 8, 2019
Test run: FAILEDSummary: 1 of 6 test jobs failed Failed test jobs:
|
|
CI job restarted: K66F issues don't appear to be related to PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Netsocket greentea tests are meant to test Socket API. However they highly depend on the quality of the underlying network connection, especially when it comes to using the echo server.
I tried to identify the most common and fixable failures, which seem to result from the underlying infrastructure rather than the socket operation itself. These failures obscure the overall view of the CI results and might in result hide some actual errors.
The first improvement is in UDPSOCKET_RECV_TIMEOUT. The test was checking that at least 5 out of 10 UDP packets were received from the echo server. I do not see how this is related to socket timeout. We have separate tests UDPSOCKET_ECHO* for UDP sockets to test echo responses and that test can handle WOULD_BLOCK responses in a more reasonable way and will also check if the data in echo responses match.
I suggest we remove the packet success ratio expectation from this test to avoid random failures due to network glitches.
The second potential improvement is in TLSSOCKET_RECV_TIMEOUT. Here we sometimes fail to receive a packet within 20 seconds, especially in the tests using wifi, which we know is rather unreliable. There is a separate mechanism in this test, which guarantees that we will not exceed half of the total time for the test suite, so increasing the sigio timeout is safe and might reduce the number of failures due to network issues.
Pull request type
Reviewers
@SeppoTakalo
@VeijoPesonen
@KariHaapalehto
@mtomczykmobica
@tymoteuszblochmobica