this is a fix for 498145557 (Arista Failing FNT: feature/qos/ingress_policer/otg_tests/ingress_police_nhg)#5387
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a specific failure observed in the ingress_police_nhg test suite on Arista hardware. By adjusting the weight parameter in the NextHopGroup entry, the configuration is now aligned with expected device behavior, ensuring stability for the associated quality of service tests. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request modifies the configureGribi function in the ingress policer NHG test to use a constant weight of 1 for next hop group members instead of a dynamic weight. The reviewer noted that this change, while likely addressing a platform-specific failure, should be applied consistently to other parts of the test suite, such as programBasicEntries, to ensure uniform behavior.
| WithNetworkInstance(deviations.DefaultNetworkInstance(dut)). | ||
| WithID(nhgID). | ||
| AddNextHop(nhID, uint64(i+1)), | ||
| AddNextHop(nhID, 1), |
There was a problem hiding this comment.
The weight for the Next Hop Group member is changed from uint64(i+1) to 1. While this change likely addresses the failure on Arista for this specific configuration, a similar pattern of using non-unit weights for single-member Next Hop Groups exists in programBasicEntries (line 471). If the platform issue is related to weights in single-member groups, programBasicEntries should also be updated to ensure the test passes consistently across the entire suite.
References
- Maintaining consistency in code patterns across the test suite is important, especially when applying a fix for a platform-specific issue that likely affects multiple locations using the same pattern.
https://partnerissuetracker.corp.google.com/u/2/issues/498145557
Adding a fix for the above BUG