Repository: openconfig/ygot
Pull request #377 (2020-04-17 17:24:13)

Use new protobuf library for testcmp and testutils and update gnmi & goyang libraries

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

Coverage increased (+0.007%) to 90.875% when pulling **0518f4a77b15a2ac880b682a96b4df886707d396 on wenbli-dev2** into **9150fb31747ca6d9970e182a08e149cd7b01f851 on master**.


Patch:
diff --git a/go.mod b/go.mod
index 13f5bb2..aa4c0e0 100644
--- a/go.mod
+++ b/go.mod
@@ -4,13 +4,14 @@ go 1.14
 
 require (
 	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
-	github.com/golang/protobuf v1.4.0-rc.4
+	github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0
 	github.com/google/go-cmp v0.4.0
 	github.com/kylelemons/godebug v1.1.0
-	github.com/openconfig/gnmi v0.0.0-20200307010808-e7106f7f5493
-	github.com/openconfig/goyang v0.0.0-20200309174518-a00bece872fc
+	github.com/openconfig/gnmi v0.0.0-20200414194230-1597cc0f2600
+	github.com/openconfig/goyang v0.0.0-20200328051049-f3d50fd25b33
+	github.com/pborman/getopt v0.0.0-20190409184431-ee0cd42419d3 // indirect
 	github.com/pmezard/go-difflib v1.0.0
 	google.golang.org/genproto v0.0.0-20200319113533-08878b785e9c
 	google.golang.org/grpc v1.28.0
-	google.golang.org/protobuf v1.20.1
+	google.golang.org/protobuf v1.21.0
 )
diff --git a/go.sum b/go.sum
index b5fdbdb..cdb473e 100644
--- a/go.sum
+++ b/go.sum
@@ -20,19 +20,28 @@ github.com/golang/protobuf v1.4.0-rc.1.0.20200221234624-67d41d38c208/go.mod h1:x
 github.com/golang/protobuf v1.4.0-rc.2/go.mod h1:LlEzMj4AhA7rCAGe4KMBDvJI+AwstrUpVNzEA03Pprs=
 github.com/golang/protobuf v1.4.0-rc.4 h1:+EOh4OY6tjM6ZueeUKinl1f0U2820HzQOuf1iqMnsks=
 github.com/golang/protobuf v1.4.0-rc.4/go.mod h1:WU3c8KckQ9AFe+yFwt9sWVRKCVIyN9cPHBJSNnbL67w=
+github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0 h1:aRz0NBceriICVtjhCgKkDvl+RudKu1CT6h0ZvUTrNfE=
+github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0/go.mod h1:WU3c8KckQ9AFe+yFwt9sWVRKCVIyN9cPHBJSNnbL67w=
 github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/protobuf v3.11.4+incompatible/go.mod h1:lUQ9D1ePzbH2PrIS7ob/bjm9HXyH5WHB0Akwh7URreM=
 github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
 github.com/openconfig/gnmi v0.0.0-20200307010808-e7106f7f5493 h1:e/znXbq+Yiws97a4lJYlUeRw9OGxT2q27L4aMUb0GuM=
 github.com/openconfig/gnmi v0.0.0-20200307010808-e7106f7f5493/go.mod h1:jMSUQIR4z9WTtM58/QBHbElXAwbUnomFdty1aund1uY=
+github.com/openconfig/gnmi v0.0.0-20200414194230-1597cc0f2600 h1:EO0ADDEFlykRo0dVFbFglXO16iDoD60IxPJyttU49Ko=
+github.com/openconfig/gnmi v0.0.0-20200414194230-1597cc0f2600/go.mod h1:M/EcuapNQgvzxo1DDXHK4tx3QpYM/uG4l591v33jG2A=
 github.com/openconfig/goyang v0.0.0-20200115183954-d0a48929f0ea/go.mod h1:dhXaV0JgHJzdrHi2l+w0fZrwArtXL7jEFoiqLEdmkvU=
 github.com/openconfig/goyang v0.0.0-20200309174518-a00bece872fc h1:W6XYKuH3mxF5WFhsSQOPPN9DRDba1xz9lbUbQR3uHkg=
 github.com/openconfig/goyang v0.0.0-20200309174518-a00bece872fc/go.mod h1:dhXaV0JgHJzdrHi2l+w0fZrwArtXL7jEFoiqLEdmkvU=
+github.com/openconfig/goyang v0.0.0-20200328051049-f3d50fd25b33 h1:giwoPW7W3APrb2JUksjek+rAiSryYHMv+bzvh5UZRm8=
+github.com/openconfig/goyang v0.0.0-20200328051049-f3d50fd25b33/go.mod h1:dhXaV0JgHJzdrHi2l+w0fZrwArtXL7jEFoiqLEdmkvU=
 github.com/openconfig/ygot v0.6.0/go.mod h1:o30svNf7O0xK+R35tlx95odkDmZWS9JyWWQSmIhqwAs=
+github.com/pborman/getopt v0.0.0-20190409184431-ee0cd42419d3 h1:YtFkrqsMEj7YqpIhRteVxJxCeC3jJBieuLr0d4C4rSA=
+github.com/pborman/getopt v0.0.0-20190409184431-ee0cd42419d3/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
@@ -84,5 +93,7 @@ google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQ
 google.golang.org/protobuf v1.20.1-0.20200309200217-e05f789c0967/go.mod h1:A+miEFZTKqfCUM6K7xSMQL9OKL/b6hQv+e19PK+JZNE=
 google.golang.org/protobuf v1.20.1 h1:ESRXHgpUBG5D2I5mmsQIyYxB/tQIZfSZ8wLyFDf/N/U=
 google.golang.org/protobuf v1.20.1/go.mod h1:KqelGeouBkcbcuB3HCk4/YH2tmNLk6YSWA5LIWeI/lY=
+google.golang.org/protobuf v1.21.0 h1:qdOKuR/EIArgaWNjetjgTzgVTAZ+S/WXVrq9HW9zimw=
+google.golang.org/protobuf v1.21.0/go.mod h1:47Nbq4nVaFHyn7ilMalzfO3qCViNmqZ2kzikPIcrTAo=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
diff --git a/util/gnmi.go b/util/gnmi.go
index 2123db8..b3ef5ed 100644
--- a/util/gnmi.go
+++ b/util/gnmi.go
@@ -90,9 +90,9 @@ func TrimGNMIPathPrefix(path *gpb.Path, prefix []string) *gpb.Path {
 	if !PathMatchesPrefix(path, prefix) {
 		return path
 	}
-	out := *path
+	out := proto.Clone(path).(*gpb.Path)
 	out.Elem = out.GetElem()[len(prefix):]
-	return &out
+	return out
 }
 
 // TrimGNMIPathElemPrefix returns the path with the prefix trimmed. It returns
diff --git a/ygot/render.go b/ygot/render.go
index 0d649c3..ad8a3cb 100644
--- a/ygot/render.go
+++ b/ygot/render.go
@@ -446,8 +446,8 @@ func mapValuePath(key, value reflect.Value, parentPath *gnmiPath) (*gnmiPath, er
 	}
 
 	for _, e := range parentPath.pathElemPath {
-		n := *e
-		childPath.pathElemPath = append(childPath.pathElemPath, &n)
+		n := proto.Clone(e).(*gnmipb.PathElem)
+		childPath.pathElemPath = append(childPath.pathElemPath, n)
 	}
 
 	return appendgNMIPathElemKey(value, childPath)
@@ -478,7 +478,7 @@ func appendgNMIPathElemKey(v reflect.Value, p *gnmiPath) (*gnmiPath, error) {
 	if err != nil {
 		return nil, err
 	}
-	newElem := *e
+	newElem := proto.Clone(e).(*gnmipb.PathElem)
 
 	if !v.IsValid() || v.IsNil() {
 		return nil, fmt.Errorf("nil value received for element %v", p)
@@ -490,7 +490,7 @@ func appendgNMIPathElemKey(v reflect.Value, p *gnmiPath) (*gnmiPath, error) {
 	}
 	newElem.Key = k
 
-	if err := np.SetIndex(np.Len()-1, &newElem); err != nil {
+	if err := np.SetIndex(np.Len()-1, newElem); err != nil {
 		return nil, err
 	}
 	return np, nil
diff --git a/ytypes/util_schema.go b/ytypes/util_schema.go
index 7c9e034..1145fe8 100644
--- a/ytypes/util_schema.go
+++ b/ytypes/util_schema.go
@@ -102,7 +102,7 @@ func validateListAttr(schema *yang.Entry, value interface{}) util.Errors {
 	// leaf-list. Check that the data tree falls within the required size
 	// bounds.
 	if v := schema.ListAttr.MinElements; v != nil {
-		if minN, err := yang.ParseNumber(v.Name); err != nil {
+		if minN, err := yang.ParseInt(v.Name); err != nil {
 			errors = util.AppendErr(errors, err)
 		} else if min, err := minN.Int(); err != nil {
 			errors = util.AppendErr(errors, err)
@@ -113,7 +113,7 @@ func validateListAttr(schema *yang.Entry, value interface{}) util.Errors {
 		}
 	}
 	if v := schema.ListAttr.MaxElements; v != nil {
-		if maxN, err := yang.ParseNumber(v.Name); err != nil {
+		if maxN, err := yang.ParseInt(v.Name); err != nil {
 			errors = util.AppendErr(errors, err)
 		} else if max, err := maxN.Int(); err != nil {
 			errors = util.AppendErr(errors, err)


Test patch:
diff --git a/testcmp/cmp.go b/testcmp/cmp.go
index 8fc073c..8559449 100644
--- a/testcmp/cmp.go
+++ b/testcmp/cmp.go
@@ -23,11 +23,13 @@ import (
 	"reflect"
 
 	log "github.com/golang/glog"
+	"github.com/golang/protobuf/proto"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/openconfig/ygot/testutil"
 	"github.com/openconfig/ygot/ygot"
 	"github.com/openconfig/ygot/ytypes"
+	"google.golang.org/protobuf/testing/protocmp"
 
 	gnmipb "github.com/openconfig/gnmi/proto/gnmi"
 )
@@ -43,13 +45,18 @@ func UpdateComparer(schemaFunc func() (*ytypes.Schema, error)) (testutil.CustomC
 	}
 
 	return testutil.CustomComparer{
-		reflect.TypeOf(&gnmipb.Update{}): cmp.Comparer(func(a, b *gnmipb.Update) bool {
-			_, equal, err := GNMIUpdateComparer(a, b, schema)
-			if err != nil {
-				log.Errorf("cannot compare updates for Notifications, got err: %v", err)
-			}
-			return equal
-		}),
+		reflect.TypeOf(&gnmipb.Update{}): protocmp.FilterMessage(&gnmipb.Update{},
+			cmp.Comparer(func(ma, mb protocmp.Message) bool {
+				a := new(gnmipb.Update)
+				proto.Merge(a, ma)
+				b := new(gnmipb.Update)
+				proto.Merge(b, mb)
+				_, equal, err := GNMIUpdateComparer(a, b, schema)
+				if err != nil {
+					log.Errorf("cannot compare updates for Notifications, got err: %v", err)
+				}
+				return equal
+			})),
 	}, nil
 }
 
@@ -82,7 +89,7 @@ func GNMIUpdateComparer(a, b *gnmipb.Update, jsonSpec *ytypes.Schema) (*gnmipb.N
 	if !aOK || !bOK {
 		// One or both of the updates doesn't contain a JSON IETF typed value
 		// so revert to using cmp.Equal to test their equality.
-		return nil, cmp.Equal(a, b, cmpopts.EquateEmpty()), nil
+		return nil, cmp.Equal(a, b, cmpopts.EquateEmpty(), protocmp.Transform()), nil
 	}
 
 	// Create a new root, since GetOrCreateNode can modify the root even during
diff --git a/testcmp/cmp_test.go b/testcmp/cmp_test.go
index a30b3a4..ddbd782 100644
--- a/testcmp/cmp_test.go
+++ b/testcmp/cmp_test.go
@@ -17,13 +17,14 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/kylelemons/godebug/pretty"
+	"github.com/google/go-cmp/cmp"
 	"github.com/openconfig/gnmi/errdiff"
 	"github.com/openconfig/ygot/exampleoc"
 	"github.com/openconfig/ygot/testutil"
 	"github.com/openconfig/ygot/uexampleoc"
 	"github.com/openconfig/ygot/ygot"
 	"github.com/openconfig/ygot/ytypes"
+	"google.golang.org/protobuf/testing/protocmp"
 
 	gnmipb "github.com/openconfig/gnmi/proto/gnmi"
 )
@@ -262,7 +263,7 @@ func TestGNMIUpdateComparer(t *testing.T) {
 				t.Errorf("did not get expected equal status, got: %v, want: %v", gotEqual, tt.wantEqual)
 			}
 
-			if diff := pretty.Compare(gotDiff, tt.wantDiff); diff != "" {
+			if diff := cmp.Diff(gotDiff, tt.wantDiff, protocmp.Transform()); diff != "" {
 				t.Errorf("did not get expected diff, diff(-got,+want):\n%s", diff)
 			}
 		})
diff --git a/testutil/testutil.go b/testutil/testutil.go
index 10d9856..d5edb60 100644
--- a/testutil/testutil.go
+++ b/testutil/testutil.go
@@ -26,6 +26,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/openconfig/gnmi/value"
+	"google.golang.org/protobuf/testing/protocmp"
 
 	gnmipb "github.com/openconfig/gnmi/proto/gnmi"
 )
@@ -58,6 +59,9 @@ func hasIgnoreTimestamp(opts []ComparerOpt) bool {
 // compare a specific field of a gNMI message. It is a map, keyed by
 // a reflect.Type of the message field, with a value of a cmp.Option
 // produced by cmp.Comparer().
+// NOTE: Proto comparison is done through protocmp -- this means any custom
+// comparison of a proto field can only be done by supplying a protocmp-created
+// cmp.Option.
 type CustomComparer map[reflect.Type]cmp.Option
 
 // IsComparerOpt marks CustomComparer as a ComparerOpt.
@@ -68,7 +72,13 @@ func (CustomComparer) IsComparerOpt() {}
 // with those in any CustomComparer that is found within the opts slice.
 func comparers(opts []ComparerOpt) []cmp.Option {
 	cmps := map[reflect.Type]cmp.Option{
-		reflect.TypeOf(&gnmipb.TypedValue_JsonIetfVal{}): cmp.Comparer(JSONIETFComparer),
+		reflect.TypeOf(&gnmipb.TypedValue{}): protocmp.FilterMessage(&gnmipb.TypedValue{},
+			cmp.Comparer(func(a, b protocmp.Message) bool {
+				if _, ok := a["json_ietf_val"]; ok {
+					return JSONIETFComparer(a["json_ietf_val"].([]byte), b["json_ietf_val"].([]byte))
+				}
+				return cmp.Equal(a, b)
+			})),
 	}
 
 	for _, o := range opts {
@@ -131,7 +141,7 @@ func SubscribeResponseSetEqual(a, b []*gnmipb.SubscribeResponse) bool {
 func NotificationSetEqual(a, b []*gnmipb.Notification, opts ...ComparerOpt) bool {
 	ignoreTS := hasIgnoreTimestamp(opts)
 	cmps := comparers(opts)
-	cmps = append(cmps, []cmp.Option{cmpopts.SortSlices(UpdateLess), cmpopts.EquateEmpty()}...)
+	cmps = append(cmps, []cmp.Option{cmpopts.SortSlices(UpdateLess), cmpopts.EquateEmpty(), protocmp.Transform()}...)
 
 	for _, an := range a {
 		var matched bool
@@ -142,7 +152,7 @@ func NotificationSetEqual(a, b []*gnmipb.Notification, opts ...ComparerOpt) bool
 				update: cmp.Equal(an.GetUpdate(), bn.GetUpdate(),
 					cmps...,
 				),
-				delete: cmp.Equal(an.GetDelete(), bn.GetDelete(), cmpopts.SortSlices(PathLess), cmpopts.EquateEmpty()),
+				delete: cmp.Equal(an.GetDelete(), bn.GetDelete(), cmpopts.SortSlices(PathLess), cmpopts.EquateEmpty(), protocmp.Transform()),
 			}
 
 			if !ignoreTS {
@@ -164,13 +174,13 @@ func NotificationSetEqual(a, b []*gnmipb.Notification, opts ...ComparerOpt) bool
 // JSONIETFComparer compares the two provided JSON IETF TypedValues to
 // determine whether their contents are the same. If either value is
 // invalid JSON, the function returns false.
-func JSONIETFComparer(a, b *gnmipb.TypedValue_JsonIetfVal) bool {
+func JSONIETFComparer(a, b []byte) bool {
 	aj, bj := map[string]interface{}{}, map[string]interface{}{}
-	if err := json.Unmarshal(a.JsonIetfVal, &aj); err != nil {
+	if err := json.Unmarshal(a, &aj); err != nil {
 		return false
 	}
 
-	if err := json.Unmarshal(b.JsonIetfVal, &bj); err != nil {
+	if err := json.Unmarshal(b, &bj); err != nil {
 		return false
 	}
 
@@ -194,7 +204,7 @@ func (n *notificationMatch) matched() bool {
 // UpdateSetEqual compares the contents of a and b and returns true if they are
 // equal. Order of the slices is ignored.
 func UpdateSetEqual(a, b []*gnmipb.Update) bool {
-	return cmp.Equal(a, b, cmpopts.SortSlices(UpdateLess), cmpopts.EquateEmpty())
+	return cmp.Equal(a, b, cmpopts.SortSlices(UpdateLess), cmpopts.EquateEmpty(), protocmp.Transform())
 }
 
 // updateSet is an alias for a slice of gNMI Update messages.
@@ -245,7 +255,7 @@ func NotificationLess(a, b *gnmipb.Notification) bool {
 		return PathLess(a.Prefix, b.Prefix)
 	}
 
-	if !cmp.Equal(a.Update, b.Update, cmpopts.SortSlices(UpdateLess), cmpopts.EquateEmpty()) {
+	if !cmp.Equal(a.Update, b.Update, cmpopts.SortSlices(UpdateLess), cmpopts.EquateEmpty(), protocmp.Transform()) {
 		if len(a.Update) < len(b.Update) {
 			return true
 		}
@@ -267,7 +277,7 @@ func NotificationLess(a, b *gnmipb.Notification) bool {
 		}
 	}
 
-	if !cmp.Equal(a.Delete, b.Delete, cmpopts.SortSlices(PathLess), cmpopts.EquateEmpty()) {
+	if !cmp.Equal(a.Delete, b.Delete, cmpopts.SortSlices(PathLess), cmpopts.EquateEmpty(), protocmp.Transform()) {
 		if len(a.Delete) < len(b.Delete) {
 			return true
 		}
diff --git a/testutil/testutil_test.go b/testutil/testutil_test.go
index f8f1157..0c04104 100644
--- a/testutil/testutil_test.go
+++ b/testutil/testutil_test.go
@@ -20,10 +20,10 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/golang/protobuf/proto"
 	"github.com/google/go-cmp/cmp"
 	gnmipb "github.com/openconfig/gnmi/proto/gnmi"
 	"github.com/openconfig/ygot/ygot"
+	"google.golang.org/protobuf/testing/protocmp"
 )
 
 func mustPath(s string) *gnmipb.Path {
@@ -96,18 +96,19 @@ func TestGetResponseEqual(t *testing.T) {
 		},
 		inOpts: []ComparerOpt{
 			CustomComparer{
-				reflect.TypeOf(&gnmipb.Path{}): cmp.Comparer(func(a, b *gnmipb.Path) bool {
-					for _, p := range []*gnmipb.Path{a, b} {
-						for _, e := range p.Elem {
-							// Remove anything before a ":"
-							if pp := strings.Split(e.Name, ":"); len(pp) == 2 {
-								e.Name = pp[1]
+				reflect.TypeOf(&gnmipb.Path{}): protocmp.FilterMessage(&gnmipb.Path{},
+					cmp.Comparer(func(a, b protocmp.Message) bool {
+						for _, p := range []protocmp.Message{a, b} {
+							for _, e := range p["elem"].([]protocmp.Message) {
+								// Remove anything before a ":"
+								if pp := strings.Split(e["name"].(string), ":"); len(pp) == 2 {
+									e["name"] = pp[1]
+								}
 							}
 						}
-					}
 
-					return proto.Equal(a, b)
-				}),
+						return cmp.Equal(a, b)
+					})),
 			},
 		},
 		want: true,
diff --git a/ygot/render_test.go b/ygot/render_test.go
index 329e4bf..3dad030 100644
--- a/ygot/render_test.go
+++ b/ygot/render_test.go
@@ -26,6 +26,7 @@ import (
 	"github.com/kylelemons/godebug/pretty"
 	"github.com/openconfig/gnmi/errdiff"
 	"github.com/openconfig/ygot/testutil"
+	"google.golang.org/protobuf/testing/protocmp"
 
 	gnmipb "github.com/openconfig/gnmi/proto/gnmi"
 )
@@ -169,7 +170,7 @@ func TestGNMIPathCopy(t *testing.T) {
 	}}
 
 	for _, tt := range tests {
-		if got := tt.inPath.Copy(); !cmp.Equal(got, tt.inPath, cmp.AllowUnexported(gnmiPath{})) {
+		if got := tt.inPath.Copy(); !cmp.Equal(got, tt.inPath, cmp.AllowUnexported(gnmiPath{}), protocmp.Transform()) {
 			t.Errorf("%s: (gnmiPath).Copy(): did not get expected result, got: %v, want: %v", tt.name, got, tt.inPath)
 		}
 	}