From f56368575a2d93d120f68c550720afd5fd39c8c0 Mon Sep 17 00:00:00 2001
From: Michael Stapelberg <stapelberg@golang.org>
Date: Thu, 7 Mar 2024 15:50:23 +0100
Subject: [PATCH] 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>
---
 proto/encode_test.go            | 35 +++++++++++++++++++--------------
 types/dynamicpb/dynamic_test.go | 23 ++++++++++++++--------
 2 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/proto/encode_test.go b/proto/encode_test.go
index 67ffce4f..0f76004a 100644
--- a/proto/encode_test.go
+++ b/proto/encode_test.go
@@ -157,6 +157,9 @@ func TestEncodeOneofNilWrapper(t *testing.T) {
 }
 
 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 @@ func TestEncodeLarge(t *testing.T) {
 // 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 99d77d0a..15d23a65 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 @@ func TestConformance(t *testing.T) {
 		(*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 @@ func TestDynamicExtensions(t *testing.T) {
 	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 @@ func TestDynamicEnums(t *testing.T) {
 		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)
+		})
 	}
 }