-
Notifications
You must be signed in to change notification settings - Fork 18
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
fix(evpn): unit test fixes #402
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #402 +/- ##
===========================================
+ Coverage 10.03% 33.58% +23.54%
===========================================
Files 16 17 +1
Lines 2132 2162 +30
===========================================
+ Hits 214 726 +512
+ Misses 1900 1403 -497
- Partials 18 33 +15 ☔ View full report in Codecov by Sentry. |
271a43a
to
f35afed
Compare
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.
@atulpatel261194 a great improvements of the tests! Nice to see +17.80% of the overall coverage. However, there are some comments from my side.
I appreciate your feedback, I want to acknowledge that I am currently on leave for the next two weeks. However, please be assured that I am committed to addressing the suggestions you provided as soon as I return in January'24 . I have made note of your suggestions and will prioritize implementing the necessary changes to improve the quality and effectivenes |
Sure. Enjoy your vacation! |
444a8d1
to
4c384e7
Compare
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. Just some minor comments
network/svi_test.go
Outdated
Addr: &pc.IPAddress{ | ||
Af: pc.IpAf_IP_AF_INET, | ||
V4OrV6: &pc.IPAddress_V4Addr{ | ||
V4Addr: 192<<24 | 168<<16 | 1<<8 | 1, |
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.
do we have a dedicated function in go, which could perform that conversion for us?
Signed-off-by: atulpatel261194 <atul.patel@intel.com>
4c384e7
to
b314a45
Compare
Signed-off-by: atulpatel261194 <atul.patel@intel.com>
Signed-off-by: atulpatel261194 <atul.patel@intel.com>
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.
LGTM!
No description provided.