-
Notifications
You must be signed in to change notification settings - Fork 50
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
restructure the setup and teardown functions for network.py scripts (Bugfix) #1342
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1342 +/- ##
==========================================
+ Coverage 45.72% 45.99% +0.26%
==========================================
Files 367 367
Lines 39134 39184 +50
Branches 6618 6629 +11
==========================================
+ Hits 17894 18021 +127
+ Misses 20565 20482 -83
- Partials 675 681 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
restructure the setup and teardown functions with context manager library. Another improvement is not bring up the network interfaces if it was down before testing.
fix issue that the wait_for_iface_up function will not return correct status
correct the function name for checking the underspeed issue
fix the ruturn value of check_underspeed function
fix check_underspeed
update error messages
correct the condition to shutdown all other netifs
correct conduit network interface
update variable name
revise variable name
fix calledprocess error
add unittest for network.py
fix unittest failed on python3.5
add more unit tests
fix unit test
48f9848
to
9f12e47
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.
For me it is very hard to understand if this is ok actually, I have a few nitpicks about the code but nothing major. The submissions you provide actually have 2 failed tests, so I'm not sure what is going on there. Could this just randomly fail on some machines? Did you test on just two? Can you please test this on more? This is not ran during sru right?
(Oh and fantastic job with the context manager, I really like that!)
Co-authored-by: Massimiliano <massimiliano.girardi@canonical.com>
fix unit tests
I am sorry for the confusion on those failed test cases, it's trying to simulate some test failed scenario and make sure the network configuration could be restore. I have added more description for those submissions with failed tests, and attached a new submission without failed tests.
I think so, it's not include in the SRU test plan as this is only include in the stress test plan.
|
fix black issue
19cbf71
to
8fac712
Compare
Description
There are some improvement in this PR as following:
Resolved issues
#233
#401
Documentation
Reference:
Tests
Tested on system with multiple physical network
https://certification.canonical.com/hardware/202405-34042/submission/391153/
https://certification.canonical.com/hardware/202405-34042/submission/381308/
Tested on system with DSA network
https://certification.canonical.com/hardware/202405-34042/submission/381472/