-
Notifications
You must be signed in to change notification settings - Fork 2.1k
tests/gnrc_tcp: provide regression tests for fixed issues #12369
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
Conversation
@nmeum @brummer-simon please have a look. |
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.
Thanks for providing additional tests to the latest issues.
By looking at the given flake8 config line length up to 119 characters are allowed. Would you please look over the code reformat function calls that are split over multiple lines. It is just easier to read and RIOT deviates from PEP8 in terms of max line length anyway. Aside from that, good job.
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.
ACK by me.
Aside from code review I tested the PR on 'native'.
EDIT: I'll revert my ACK see next comment.
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.
I found something small while executing the test on "nucleo-f401re".
ACK from my side. Tested here on nucleo-f401re |
1780e36
to
ef52600
Compare
Rebased to current master to include #12368. |
@RIOT-OS/maintainers this also needs a proxy-ACK. |
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.
Proxy-ACK (also had a look and found no major issues codewise).
You can squash.
@aabadie, so I will squash. Can you maybe also test on |
ef52600
to
e14765d
Compare
e14765d
to
c3a85bb
Compare
Provided fix for a typo |
Just following the README and testing on EDIT: the following output is with sudo -E BOARD=samr21-xpro make -C tests/gnrc_tcp test
|
This test is not introduced here (the tests here only use |
Why is there output in your script btw Oo? |
I set |
In that case If I limit the test to
|
👍 I'll see if I can reproduce the other test. |
(yes I am, but for different cases than @fjmolinas was able to and only in some cases... @brummer-simon I think there is still some stabilization work required to be done to the tests commands...) |
Contribution description
This provides regression tests for some issues that were found in the past. The respective issues are quoted in the comments. I actually already found a regression when implementing these (see #12368).
Testing procedure
Issues/PRs references
Require #12368 to work.