TRANSCEIVER-1.2: wait for CD telemetry to settle after interface flap - #5974
TRANSCEIVER-1.2: wait for CD telemetry to settle after interface flap#5974snaragund wants to merge 1 commit into
Conversation
- Replace fixed SampleStream.Nexts(2) with AwaitNext so Instant/Min/Max/Avg
CD values are asserted only after they match the expected UP/DOWN range.
- It fixes intermittent failure where Instant CD was still -1 briefly after
oper-status DOWN while the test expected inactive value 0.
- Remove redundant reflect float64 type check.
"This code is a Contribution to the OpenConfig Feature Profiles project ("Work") made under the Google Software Grant and Corporate Contributor License Agreement ("CLA") and governed by the Apache License 2.0. No other rights or licenses in or to any of Nokia's intellectual property are granted for any other purpose. This code is provided on an "as is" basis without any warranties of any kind."
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 improves the reliability of CD telemetry verification in the ZRP CD test suite. By moving from a fixed-sample approach to a conditional wait mechanism, the tests now correctly handle the settling time required for telemetry after an interface flap, eliminating race conditions and intermittent failures. 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 the 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 counterproductive. 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 refactors the verifyCDValue function in zrp_cd_test.go to use AwaitNext with a predicate for validating CD telemetry values based on the interface operational status, which removes the need for reflection-based type checks and unused imports. The review feedback suggests adding an early validation check for operStatus to prevent the test from hanging for the full timeout duration when an invalid status is provided.
| func verifyCDValue(t *testing.T, dut1 *ondatra.DUTDevice, pStream *samplestream.SampleStream[float64], sensorName string, operStatus oc.E_Interface_OperStatus) float64 { | ||
| cdSampleNexts := pStream.Nexts(2) | ||
| cdSample := cdSampleNexts[1] | ||
| cdSample, ok := pStream.AwaitNext(timeout, func(v *ygnmi.Value[float64]) bool { |
There was a problem hiding this comment.
If an invalid or unexpected operStatus is passed to verifyCDValue, the predicate inside AwaitNext will always return false. This causes the test to hang and poll for the entire 10-minute timeout duration before failing. Adding an early validation check for operStatus allows the test to fail fast immediately.
if operStatus != oc.Interface_OperStatus_UP && operStatus != oc.Interface_OperStatus_DOWN {
t.Fatalf("Invalid status %v", operStatus)
}
cdSample, ok := pStream.AwaitNext(timeout, func(v *ygnmi.Value[float64]) bool {
"This code is a Contribution to the OpenConfig Feature Profiles project ("Work") made under the Google Software Grant and Corporate Contributor License Agreement ("CLA") and governed by the Apache License 2.0. No other rights or licenses in or to any of Nokia's intellectual property are granted for any other purpose. This code is provided on an "as is" basis without any warranties of any kind."