-
Notifications
You must be signed in to change notification settings - Fork 5
NAT: Move tests back to "nat" crate #645
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implement PartialEq and Eq traits for struct NatTables, so we can compare different objects in tests in a future commit. We need to dreive the traits for structs PerVniTable, NatPrefixRuleTable, NatPeerRuleTable, and TrieValue. We also need the trait to be implemented for PrefixTrie, but we cannot derive it (the RTrieMap objects do not implement the trait), so we implement it manually by comparing all entries. Signed-off-by: Quentin Monnet <[email protected]>
We used to have a similar test in the module, but we recently moved it to the "mgmt" crate because it depended on some objects from "mgmt", but the "nat" crate can no longer use "mgmt" as a dependency. Restore the test in the stateless NAT submodule, but instead of building the NAT tables from the configuration code from the "mgmt" crate, just build the table manually. In a future commit, we'll remove the copy of the test from the "mgmt" crate and will instead check that the generated config object matches with the manually-initialised NatTables object from this commit. Signed-off-by: Quentin Monnet <[email protected]>
In a recent commit we copied the tests for stateless NAT from the "mgmt" crate back to the "nat" crate. We no longer need the version from the "mgmt" crate. Instead, ensure that the NatTables object generated from the config matches the reference object that we manually instantiated in the tests of the "nat" crate. In order to share and compare this object, gate it behind a "testing" feature for the "nat" crate. Signed-off-by: Quentin Monnet <[email protected]>
Fredi-raspall
approved these changes
Jun 27, 2025
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.
Looks good to me @qmonnet , thanks!
Fredi-raspall
requested changes
Jun 27, 2025
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.
Can we remove the PartialEq?
Fredi-raspall
approved these changes
Jun 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please take a look at individual commit descriptions for details.