Tests now each run in their own netns#20
Conversation
yonatan-linik
force-pushed
the
netns_per_test
branch
from
0869b2f to
d16a187
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the test suite to utilize network namespaces for better isolation, replacing legacy command execution helpers with a new NetnsGuard utility. It also updates the bond interface data model to use optional fields and refines the vxlan display output. Key feedback includes correcting a syntax error in a test helper's return type, addressing a potential string slicing issue in timer normalization, and cleaning up commented-out code.
|
|
||
| // Skip whitespace | ||
| let whitespace_len = | ||
| remaining.chars().take_while(|c| c.is_whitespace()).count(); |
There was a problem hiding this comment.
Using chars().count() to determine a byte index for slicing is dangerous because it returns the number of Unicode scalar values, not the number of bytes. While it may work here if the characters are all ASCII, it is better practice to use byte-oriented methods like find to ensure the index is valid for slicing.
let whitespace_len = remaining.find(|c: char| !c.is_whitespace()).unwrap_or(remaining.len());
yonatan-linik
force-pushed
the
netns_per_test
branch
3 times, most recently
from
41bf1f4 to
4650e3d
Compare
|
@yonatan-linik Before you spend more time on this. Could you explain why we need to run tests in isolated namespace and why we cannot live with current test design? |
@cathay4t Running each test in its own namespace makes sure there are no name collisions (or other unique values) and allows for guaranteed cleanup of all resources with netns deletion. |
|
Good point. Now we need to wrap it into simpler design to make follow-up contributors life easier. |
|
@cathay4t Simpler than what I have in this PR? |
The spaces were in a place that can cause a double space if some fields
are missing.
Like this:
```
vxlan id 6671 srcport 0 0 dstport 25758 ttl auto ageing 300 addrgenmode eui64
^^
```
yonatan-linik
force-pushed
the
netns_per_test
branch
from
4650e3d to
e8955e5
Compare
|
It is good enough. Since you are using namespace for each tests now, please modify the tests to use the same test interface name. |
|
@cathay4t I already did, unless you see I missed some file. |
|
All set. Thanks for the great work! |
2 participants
Also adds convenience functions to make tests shorter