gNOI-4.1 comparison function

[fmolinar]

Adding function to compare configs, taking into account default values for the tests:

TestPushAndVerifyBGPConfig

TestPushAndVerifyInterfaceConfig

[OpenConfigBot]

Pull Request Functional Test Report for #2888 / bfa6389860c36a3bb35d57a6277085c27efa46d0

Virtual Devices

Device	Test	Test Documentation	Job	Raw Log
Arista cEOS		gNOI-4.1: Software Upgrade
Cisco 8000E		gNOI-4.1: Software Upgrade
Cisco XRd		gNOI-4.1: Software Upgrade
Juniper ncPTX		gNOI-4.1: Software Upgrade
Nokia SR Linux		gNOI-4.1: Software Upgrade
Openconfig Lemming		gNOI-4.1: Software Upgrade

Hardware Devices

Device	Test	Test Documentation	Raw Log
Arista 7808		gNOI-4.1: Software Upgrade
Cisco 8808		gNOI-4.1: Software Upgrade
Juniper PTX10008		gNOI-4.1: Software Upgrade
Nokia 7250 IXR-10e		gNOI-4.1: Software Upgrade

Help

[coveralls]

Pull Request Test Coverage Report for Build 9657222426

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 55.5%

---

Totals
Change from base Build 9657106637:	0.0%
Covered Lines:	1983
Relevant Lines:	3573

---

💛 - Coveralls

[LimeHat]

Could you please elaborate on the reasoning behind this change?

[fmolinar]

hi @LimeHat ,

Given the tests TestPushAndVerifyInterfaceConfig and TestPushAndVerifyBGPConfig, they compare the configuration that is going to be send to the device with the configuration that is retrieved from the device after the update.

The current implementation of reflect.DeepEqual(val, in) doesn't take into account default values that are returned from gnmi.LookupConfig(...). Thus, the test may give a false positive by failing a comparison that should've passed if we are looking at the field changed that were pushed vs what was retrieved.

There is a gnmi function PopulateDefaults() that populates all default values, however, it also populates some unsupported leafs, making the update call fail.

My proposal is to iterate over the struct that we are going to push (want), and evaluate it against what we got after we push the changes (got) and ignore unset fields.

let me know if you want me to expand or provide more data.

Thank you! Fernando

[fmolinar]

hi @LimeHat ,

Given the tests TestPushAndVerifyInterfaceConfig and TestPushAndVerifyBGPConfig, they compare the configuration that is going to be send to the device with the configuration that is retrieved from the device after the update.

The current implementation of reflect.DeepEqual(val, in) doesn't take into account default values that are returned from gnmi.LookupConfig(...). Thus, the test may give a false positive by failing a comparison that should've passed if we are looking at the field changed that were pushed vs what was retrieved.

There is a gnmi function PopulateDefaults() that populates all default values, however, it also populates some unsupported leafs, making the update call fail.

My proposal is to iterate over the struct that we are going to push (want), and evaluate it against what we got after we push the changes (got) and ignore unset fields.

let me know if you want me to expand or provide more data.

Thank you! Fernando

On Tue, Apr 9, 2024, 11:46 AM Sergey Fomin @.***> wrote:

Could you please elaborate on the reasoning behind this change?

— Reply to this email directly, view it on GitHub https://github.com/openconfig/featureprofiles/pull/2888#issuecomment-2045859980, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACULILEQZIPLPNKFYE44OE3Y4QZQVAVCNFSM6AAAAABF655UR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBVHA2TSOJYGA . You are receiving this because you authored the thread.Message ID: @.***>