Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uncompressed ordered map tests in mostly ygot package. #872

Merged
merged 40 commits into from
Jun 13, 2023
Merged

Conversation

wenovus
Copy link
Collaborator

@wenovus wenovus commented Jun 9, 2023

  • Simplified UnmarshalSetRequest to always unmarshal w/r to the root node. This allows atomic Notifications to be deleted correctly.

wenovus added 19 commits June 6, 2023 19:15
When comparing two GoStructs where a YANG `ordered-by user` list is
involved, it is not possible using a single Notification to represent
the diff such that the Notification can do the following:
1. update a gNMI cache correctly with the atomic notification.
2. unmarshal the notification to the `orig` to get back the `modified`
   GoStruct.

This function achieves this functionality, sharing the underlying
implementation with Diff, where a disclaimer has been added to the doc
comment.

Other helpers have been created as well to be shared with
`TogNMINotifications`.

Other changes:
- `TogNMINotifications` has been changed to follow the same convention of
  putting the non-atomic Notification as the first message instead of
  the last. The reason is that current code MAY be using `notifs[0]` and
  I don't want to break that, and this also seems slightly better from a
  usability perspective.
* Simplified `UnmarshalSetRequest` to always unmarshal w/r to the root
  node. This allows atomic Notifications to be deleted correctly.
@wenovus wenovus requested a review from DanG100 June 9, 2023 19:58
@coveralls
Copy link

coveralls commented Jun 9, 2023

Coverage Status

coverage: 89.633% (-0.02%) from 89.65% when pulling 1073e6f on uom-2 into 39c3eaa on master.

@wenovus wenovus changed the title Uncompressed GoStruct tests in mostly ygot package. Uncompressed ordered map tests in mostly ygot package. Jun 9, 2023
@@ -50,3 +50,52 @@ func GetDeviceWithOrderedMap(t *testing.T) *Device {
},
}
}

func GetOrderedMap2(t *testing.T) *Ctestschema_OrderedLists_OrderedList_OrderedMap {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

godoc
nit: i don't like "2" suffix, it's unclear what makes this func different from other func without looking at the code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, I've added a godoc. I think in this case it's ok to leave it like this rather than GetOrderedMapWeeWoo as the readers are mostly ygot maintainers and the code is fairly easy to understand. The "2" as a indicator for a different ordered map I think is sufficient. Let me know if you have other thoughts on this.

ygot/gostruct_exported_test.go Outdated Show resolved Hide resolved
Base automatically changed from uom-1 to master June 13, 2023 17:01
@wenovus wenovus merged commit 7c99f87 into master Jun 13, 2023
@wenovus wenovus deleted the uom-2 branch June 13, 2023 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants