Add test gnmi-3 Union Replace - #5419
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 introduces a new test suite to validate the gNMI Union Replace operation. The tests ensure that both OpenConfig and CLI-based configurations are handled correctly, including scenarios involving configuration overlaps, hardware mismatches, and error handling for invalid configurations. These changes aim to improve the robustness and reliability of gNMI configuration management on supported devices. 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
|
Pull Request Functional Test Report for #5419 / 3dbe350Virtual Devices
Hardware Devices
|
There was a problem hiding this comment.
Code Review
This pull request implements a new test suite for gNMI union_replace functionality, including tests for idempotent configurations, additions, deletions, and overlapping configurations across OpenConfig and CLI origins. The feedback identifies several violations of the repository's style guide regarding the use of RFC 5398 compliant ASNs. Additionally, improvements are suggested to handle asynchronous gNMI state updates more reliably using gnmi.Watch and to ensure interface name parsing is robust against different naming conventions to avoid potential panics.
|
Added new logs at: https://partnerissuetracker.corp.google.com/issues/517964859 |
|
Please do not re-write the pre-existing tests. Leave them in place. You may refactor them to be executed as table driven tests, but don't change the logic. Regarding gnmi-3.7, the requirements were updated to use a configuration containing an invalid interface. ref: featureprofiles/feature/gnmi/tests/union_replace_test/README.md Lines 122 to 152 in 4f2c803 |
|
When run in our environment I get the following results. 3.1 and 3.3.1,2 were passing. Please revert the logic on those tests. This PR #5419 on hardware EOS-4.36.1F Previous PR #5451 on hardware EOS-4.36.1F |
dplore
left a comment
There was a problem hiding this comment.
When run in our environment I get the following results. 3.1 and 3.3.1,2 were passing. Please revert the logic on those tests.
@navaneethyv can assume review after those changes are made
|
@ijernoiu - The test is failing, below is the test summary. The DUT is not returning the description "Await(t) on dut(xxxxxxx) at /interfaces/interface[name=Ethernet20/1]/state/description: context deadline exceeded" |
|
@ijernoiu can you please share the latest updates and let us know where we are ? |
|
@navaneethyv Please check https://partnerissuetracker.corp.google.com/issues/509736406 for the current status. |
|
@ijernoiu - Please take a look at the latest comments in the bug 509736406 and let me know your feedback on this one. |
ram-mac
left a comment
There was a problem hiding this comment.
With the latest changes the test is now passing for Arista. All relevant details are captured in bug 509736406
Readme location: https://github.com/openconfig/featureprofiles/blob/main/feature/gnmi/tests/union_replace_test/README.md
Issues identified: https://partnerissuetracker.corp.google.com/issues/509552131
Logs available at: https://partnerissuetracker.corp.google.com/issues/415458482#comment202