Skip to content

Conversation

@yonatan-linik
Copy link
Contributor

No description provided.

@yonatan-linik
Copy link
Contributor Author

I see the tests failed because of one of the timers changing between the 2 commands running.
I am not sure what to do with this so the tests won't be flakey, if you have a suggestion, let me know.
I also wanted to run some commands to create interfaces before the tests start, but I am guessing it doesn't work, because of permissions (to run ip).
Should I just add a script that does that and call it with sudo in the Makefile?

@yonatan-linik yonatan-linik force-pushed the support-more-interfaces branch from 07d63cb to 96e404c Compare October 14, 2025 15:52
@cathay4t
Copy link
Member

I do not have time for cleaning this up. I am blindly accepting contributions for now. Will clean up in my weekend (sorry, not my business project).

sudo is OK.

Please rebase to last which has bridge covered somehow.

@cathay4t
Copy link
Member

For the CI test cases, please us similar stuff like https://github.com/nispor/nispor/blob/base/src/lib/integ_tests/veth.rs#L26

To me this ctor::ctor is a hack and very hard to understand for new contributor. Please use panic::catch_unwind().

And next time, be cautious on introducing new dependency.

@yonatan-linik yonatan-linik force-pushed the support-more-interfaces branch from 96e404c to fcc28a5 Compare October 25, 2025 14:31
Setup for tests is now part of the Makefile.
@yonatan-linik yonatan-linik force-pushed the support-more-interfaces branch from fcc28a5 to 3a63935 Compare October 25, 2025 14:38
@yonatan-linik
Copy link
Contributor Author

@qinqon I removed some of the fields you added in your last PR, as they broke the tests.
The commit with the changes is this one, if you have a suggestion on how to make the tests pass, let me know.
We can do a revert later, to get the functionality back.

@yonatan-linik
Copy link
Contributor Author

@cathay4t No rush, but it is ready for review/merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants