all: use subtests to identify the message type This makes it a little easier to track down test failures. Also add a note to TestMarshalAppendAllocations to explain what it tests. Change-Id: Ie35f3ddd7c7d5cb300294f0fe665c6711d45d186 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/569775 Auto-Submit: Michael Stapelberg <stapelberg@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Lasse Folger <lassefolger@google.com>
diff --git a/proto/encode_test.go b/proto/encode_test.go index 67ffce4..0f76004 100644 --- a/proto/encode_test.go +++ b/proto/encode_test.go
@@ -157,6 +157,9 @@ } func TestMarshalAppendAllocations(t *testing.T) { + // This test ensures that MarshalAppend() has the same performance + // characteristics as the append() builtin, meaning that repeated calls do + // not allocate each time, but allocations are amortized. m := &test3pb.TestAllTypes{SingularInt32: 1} size := proto.Size(m) const count = 1000 @@ -257,22 +260,24 @@ // but exist to detect accidental changes in behavior. func TestEncodeEmpty(t *testing.T) { for _, m := range []proto.Message{nil, (*testpb.TestAllTypes)(nil), &testpb.TestAllTypes{}} { - isValid := m != nil && m.ProtoReflect().IsValid() + t.Run(fmt.Sprintf("%T", m), func(t *testing.T) { + isValid := m != nil && m.ProtoReflect().IsValid() - b, err := proto.Marshal(m) - if err != nil { - t.Errorf("proto.Marshal() = %v", err) - } - if isNil := b == nil; isNil == isValid { - t.Errorf("proto.Marshal() == nil: %v, want %v", isNil, !isValid) - } + b, err := proto.Marshal(m) + if err != nil { + t.Errorf("proto.Marshal() = %v", err) + } + if isNil := b == nil; isNil == isValid { + t.Errorf("proto.Marshal() == nil: %v, want %v", isNil, !isValid) + } - b, err = proto.MarshalOptions{}.Marshal(m) - if err != nil { - t.Errorf("proto.MarshalOptions{}.Marshal() = %v", err) - } - if isNil := b == nil; isNil == isValid { - t.Errorf("proto.MarshalOptions{}.Marshal() = %v, want %v", isNil, !isValid) - } + b, err = proto.MarshalOptions{}.Marshal(m) + if err != nil { + t.Errorf("proto.MarshalOptions{}.Marshal() = %v", err) + } + if isNil := b == nil; isNil == isValid { + t.Errorf("proto.MarshalOptions{}.Marshal() = %v, want %v", isNil, !isValid) + } + }) } }
diff --git a/types/dynamicpb/dynamic_test.go b/types/dynamicpb/dynamic_test.go index 99d77d0..15d23a6 100644 --- a/types/dynamicpb/dynamic_test.go +++ b/types/dynamicpb/dynamic_test.go
@@ -5,6 +5,7 @@ package dynamicpb_test import ( + "fmt" "testing" "google.golang.org/protobuf/proto" @@ -23,8 +24,10 @@ (*test3pb.TestAllTypes)(nil), (*testpb.TestAllExtensions)(nil), } { - mt := dynamicpb.NewMessageType(message.ProtoReflect().Descriptor()) - prototest.Message{}.Test(t, mt) + t.Run(fmt.Sprintf("%T", message), func(t *testing.T) { + mt := dynamicpb.NewMessageType(message.ProtoReflect().Descriptor()) + prototest.Message{}.Test(t, mt) + }) } } @@ -32,10 +35,12 @@ for _, message := range []proto.Message{ (*testpb.TestAllExtensions)(nil), } { - mt := dynamicpb.NewMessageType(message.ProtoReflect().Descriptor()) - prototest.Message{ - Resolver: extResolver{}, - }.Test(t, mt) + t.Run(fmt.Sprintf("%T", message), func(t *testing.T) { + mt := dynamicpb.NewMessageType(message.ProtoReflect().Descriptor()) + prototest.Message{ + Resolver: extResolver{}, + }.Test(t, mt) + }) } } @@ -44,8 +49,10 @@ testpb.TestAllTypes_FOO, test3pb.TestAllTypes_FOO, } { - et := dynamicpb.NewEnumType(enum.Descriptor()) - prototest.Enum{}.Test(t, et) + t.Run(fmt.Sprintf("%v", enum), func(t *testing.T) { + et := dynamicpb.NewEnumType(enum.Descriptor()) + prototest.Enum{}.Test(t, et) + }) } }