Skip to content

restructure the setup and teardown functions for network.py scripts (Bugfix)#1342

Merged
Hook25 merged 19 commits intomainfrom
network_backup_iperf
Sep 9, 2024
Merged

restructure the setup and teardown functions for network.py scripts (Bugfix)#1342
Hook25 merged 19 commits intomainfrom
network_backup_iperf

Conversation

@stanley31huang
Copy link
Copy Markdown
Collaborator

@stanley31huang stanley31huang commented Jul 18, 2024

Description

There are some improvement in this PR as following:

  1. restructure the setup and teardown functions with context manager library.
  2. not bring up the network interfaces if it was down before testing.
  3. not shutdown the conduit network interface (master network interface) while testing with an user network interface (slave network interface) (for Linux DSA network)

Resolved issues

#233
#401

Documentation

Reference:

Tests

Tested on system with multiple physical network

Tested on system with DSA network

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 18, 2024

Codecov Report

Attention: Patch coverage is 92.50000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 45.99%. Comparing base (b92edc5) to head (8fac712).
Report is 132 commits behind head on main.

Files with missing lines Patch % Lines
providers/base/bin/network.py 92.50% 4 Missing and 5 partials ⚠️
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     
Flag Coverage Δ
provider-base 20.66% <91.66%> (+0.74%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stanley31huang stanley31huang marked this pull request as ready for review July 22, 2024 06:20
@stanley31huang stanley31huang changed the title restructure the setup and teardown functions (Bugfix) restructure the setup and teardown functions for network.py scripts (Bugfix) Aug 5, 2024
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
Copy link
Copy Markdown
Collaborator

@Hook25 Hook25 left a 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!)

stanley31huang and others added 2 commits September 2, 2024 10:53
Co-authored-by: Massimiliano <massimiliano.girardi@canonical.com>
fix issue
@fernando79513 fernando79513 added the waiting-for-changes The review has been completed but the PR is waiting for changes from the author label Sep 2, 2024
@stanley31huang stanley31huang marked this pull request as draft September 4, 2024 02:54
fix unit tests
@stanley31huang stanley31huang marked this pull request as ready for review September 4, 2024 06:41
@stanley31huang
Copy link
Copy Markdown
Collaborator Author

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?

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.

This is not ran during sru right?

I think so, it's not include in the SRU test plan as this is only include in the stress test plan.

(Oh and fantastic job with the context manager, I really like that!)

fix black issue
@Hook25 Hook25 merged commit d7d5c8c into main Sep 9, 2024
@Hook25 Hook25 deleted the network_backup_iperf branch September 9, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-changes The review has been completed but the PR is waiting for changes from the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants