Repository: openconfig/ygot
Pull request #464 (2020-10-05 21:00:17)

Add `util.PathElemSlicesEqual` to compare slices of *gpb.PathElem.
Now we have
`util.PathElemsEqual` (same as before)
`util.PathElemSlicesEqual`

coveralls: 
[![Coverage Status](https://coveralls.io/builds/33944936/badge)](https://coveralls.io/builds/33944936)

Coverage increased (+0.007%) to 90.565% when pulling **f07727281c24675d1f38cebe9dbcd4abb77e47b5 on pathelemsequal** into **50b44fcf38c0a20a5828232e058ad58ce1204d02 on master**.

robshakir: Do we have to make this a backwards incompatible change? I guess the reason that you chose to is the naming of `Elems` vs `Elem` in the function name? It's reasonable to estimate the impact to this code, but seems like it requires other callers to change for relatively little benefit?

Patch:
diff --git a/util/gnmi.go b/util/gnmi.go
index cedc6ba..de61937 100644
--- a/util/gnmi.go
+++ b/util/gnmi.go
@@ -44,21 +44,34 @@ func PathMatchesPrefix(path *gpb.Path, prefix []string) bool {
 
 // PathElemsEqual replaces the proto.Equal() check for PathElems.
 // This significantly improves comparison speed.
-func PathElemsEqual(this, other *gpb.PathElem) bool {
+func PathElemsEqual(a, b *gpb.PathElem) bool {
 	// This check allows avoiding to deal with any null PathElems later on.
-	if this == nil || other == nil {
-		return this == nil && other == nil
+	if a == nil || b == nil {
+		return a == nil && b == nil
 	}
 
-	if this.Name != other.Name {
+	if a.Name != b.Name {
 		return false
 	}
-	if len(this.Key) != len(other.Key) {
+	if len(a.Key) != len(b.Key) {
 		return false
 	}
 
-	for k, v := range this.Key {
-		if vo, ok := other.Key[k]; !ok || vo != v {
+	for k, v := range a.Key {
+		if vo, ok := b.Key[k]; !ok || vo != v {
+			return false
+		}
+	}
+	return true
+}
+
+// PathElemSlicesEqual compares whether two PathElem slices are equal.
+func PathElemSlicesEqual(a, b []*gpb.PathElem) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if !PathElemsEqual(a[i], b[i]) {
 			return false
 		}
 	}


Test patch:
diff --git a/util/gnmi_test.go b/util/gnmi_test.go
index d570e0e..dbeb207 100644
--- a/util/gnmi_test.go
+++ b/util/gnmi_test.go
@@ -331,6 +331,105 @@ func TestPathElemsEqual(t *testing.T) {
 	}
 }
 
+func TestPathElemSlicesEqual(t *testing.T) {
+	tests := []struct {
+		desc     string
+		inElemsA []*gpb.PathElem
+		inElemsB []*gpb.PathElem
+		want     bool
+	}{{
+		desc: "equal elems with no keys",
+		inElemsA: []*gpb.PathElem{{
+			Name: "one",
+		}, {
+			Name: "two",
+		}},
+		inElemsB: []*gpb.PathElem{{
+			Name: "one",
+		}, {
+			Name: "two",
+		}},
+		want: true,
+	}, {
+		desc: "equal elems with keys",
+		inElemsA: []*gpb.PathElem{{
+			Name: "one",
+			Key:  map[string]string{"two": "three"},
+		}, {
+			Name: "four",
+		}},
+		inElemsB: []*gpb.PathElem{{
+			Name: "one",
+			Key:  map[string]string{"two": "three"},
+		}, {
+			Name: "four",
+		}},
+		want: true,
+	}, {
+		desc: "unequal elems",
+		inElemsA: []*gpb.PathElem{{
+			Name: "fourteen",
+		}, {
+			Name: "twelve",
+		}},
+		inElemsB: []*gpb.PathElem{{
+			Name: "three",
+		}},
+		want: false,
+	}, {
+		desc: "unequal elems with keys",
+		inElemsA: []*gpb.PathElem{{
+			Name: "one",
+			Key:  map[string]string{"two": "three"},
+		}, {
+			Name: "four",
+			Key:  map[string]string{"five": "six"},
+		}},
+		inElemsB: []*gpb.PathElem{{
+			Name: "one",
+			Key:  map[string]string{"two": "three"},
+		}, {
+			Name: "eight",
+			Key:  map[string]string{"five": "six"},
+		}},
+		want: false,
+	}, {
+		desc: "unequal elem length",
+		inElemsA: []*gpb.PathElem{{
+			Name: "one",
+		}, {
+			Name: "two",
+		}},
+		inElemsB: []*gpb.PathElem{{
+			Name: "one",
+		}},
+		want: false,
+	}, {
+		desc: "unequal elems due to keys",
+		inElemsA: []*gpb.PathElem{{
+			Name: "three",
+			Key:  map[string]string{"four": "five"},
+		}, {
+			Name: "six",
+		}},
+		inElemsB: []*gpb.PathElem{{
+			Name: "three",
+			Key:  map[string]string{"seven": "eight"},
+		}, {
+			Name: "six",
+		}},
+		want: false,
+	}}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			if got := PathElemSlicesEqual(tt.inElemsA, tt.inElemsB); got != tt.want {
+				t.Fatalf("did not get expected result, got: %v, want: %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestPathMatchesPathElemPrefix(t *testing.T) {
 	tests := []struct {
 		desc     string