[Optimization] Improve traceroute_packetin_with_vrf_selection_test run time - #5806
[Optimization] Improve traceroute_packetin_with_vrf_selection_test run time#5806mansi0803 wants to merge 3 commits into
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 optimizes the execution time of the traceroute_packetin_with_vrf_selection_test by transitioning from static wait times to event-driven telemetry monitoring. By leveraging gnmi.Watch on OTG counters, the test suite now concludes immediately upon reaching the required packet threshold, significantly reducing idle time. Additionally, the refactoring removes unnecessary configuration and protocol initialization steps, resulting in a cleaner and more efficient test setup. 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
|
Pull Request Functional Test Report for #5806 / 49eba6dVirtual Devices
Hardware Devices
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the traceroute packet-in tests to eliminate static sleeps by implementing real-time state detection via gnmi.Watch and .Await(t). It replaces duration-based traffic generation with a target packet count mechanism and removes redundant OTG configuration and protocol start steps. The review feedback recommends removing a redundant StartProtocols call in testTraffic to prevent protocol session flaps, and using t.Fatalf instead of t.Errorf when traffic fails to reach the target to fail fast.
| if !ok { | ||
| t.Errorf("Traffic flow %s did not reach the target of %d packets within the timeout", flow.Name(), targetPkts) | ||
| } |
There was a problem hiding this comment.
If the traffic flow fails to reach the target number of packets, the subsequent packet-in validations will inevitably fail or produce misleading results due to missing packets. Consider using t.Fatalf instead of t.Errorf to terminate the test immediately and avoid cascading failures.
| if !ok { | |
| t.Errorf("Traffic flow %s did not reach the target of %d packets within the timeout", flow.Name(), targetPkts) | |
| } | |
| if !ok { | |
| t.Fatalf("Traffic flow %s did not reach the target of %d packets within the timeout", flow.Name(), targetPkts) | |
| } |
References
- In tests, t.Fatalf is preferred over t.Errorf when a failure makes subsequent test steps meaningless, as this fails fast and reduces overall test execution time.
mansi0803
changed the title
mansi0803
changed the title
| flow = append(flow, args.packetIO.GetTrafficFlow(args.ate, dstMac, isIPv4, 1, 300, 50, ipv4InnerDst, flowValue)) | ||
| } | ||
| pktOut := testTraffic(t, args.top, args.ate, flow, srcEndPoint, 60, cs) | ||
| pktOut := testTraffic(t, args.top, args.ate, flow, srcEndPoint, 500, cs) |
… telemetry watch to reduce test execution time Refactor traffic waiting logic in to use on OTG telemetry counters instead of a fixed . This allows test cases to finish dynamically as soon as the target packet count (500 packets) is transmitted, significantly reducing total test execution time. In addition, cleaned up redundant , , and calls in and , ensuring OTG configuration and capture state are pushed once in after all flows are attached. - Replaced fixed sleep with on with an loop. - Cleaned up redundant OTG config push and protocol start calls.
77484a7 to
49eba6d
Compare
4 participants
traceroute_packetin_with_vrf_selection_test: Replace fixed sleep with telemetry watch to reduce test execution time
Refactor traffic waiting logic in to use on OTG telemetry counters instead of a fixed . This allows test cases to finish dynamically as soon as the target packet count (500 packets) is transmitted, significantly reducing total test execution time. In addition, cleaned up redundant , , and calls in and , ensuring OTG configuration and capture state are pushed once in after all flows are attached.