-
Notifications
You must be signed in to change notification settings - Fork 16
Fix emit inconsitencies and add tests #30
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
base: main
Are you sure you want to change the base?
Conversation
f9df804 to
34d12cb
Compare
|
Thanks for the patch! I think it is time to add unit test by parsing and emitting captured |
|
https://github.com/rust-netlink/netlink-packet-route/?tab=readme-ov-file#development has steps to capture netlink packet. And that project also has unit test example on parsing and emitting packet. |
c5518da to
4ce8f54
Compare
|
If I'd split out the testing, we could at least merge the emit fixes. WDYT @cathay4t? |
4ce8f54 to
258e00c
Compare
|
In general, it looks good. Just two cents:
None of them is blocking the merge. Feel free to merge this PR if I am not responding. |
258e00c to
f1ee1e3
Compare
|
I fixed the issue as side-effect of #55 Could you change this PR to only include test case? Please try not use fancy stuff for test cases, there is a balance between |
f1ee1e3 to
583b134
Compare
|
I've moved the tests into single files, removed the use of macros and rebased onto the current main branch. |
583b134 to
45ae877
Compare
|
I am very sorry. Overlap again. Please check unit test cases in #58 . |
45ae877 to
e5a9e44
Compare
While trying to convert the parsed payload of information elements into the binary format, I've noticed inconsistencies regarding some emit implementations.
I've fixed some of them (which I needed) mostly based on the assumption, that the
Parsableimplementation is correct.On exception is the
Nl80211RateAndSelectorimplementation, where the comments from this type andNl80211Element::SupportedRatesAndSelectorswhere already inconsistent.Also I've added some roudtrip test, using macros to reduce the repetition. I thought about adding more tests, but I wanted to propose this as a starting point for now.