From d727131d60464993072937e388afea84b17922b3 Mon Sep 17 00:00:00 2001
From: Ilya Ozherelyev <ilya.ozherelyev@uber.com>
Date: Fri, 8 Nov 2024 11:41:48 +0100
Subject: [PATCH] address review comments

---
 internal/common/convert_test.go     |  2 +-
 internal/common/thrift_util.go      |  6 ++
 internal/common/thrift_util_test.go | 94 +++++++++++++++++++----------
 3 files changed, 68 insertions(+), 34 deletions(-)

diff --git a/internal/common/convert_test.go b/internal/common/convert_test.go
index 9569a2154..f09c90191 100644
--- a/internal/common/convert_test.go
+++ b/internal/common/convert_test.go
@@ -40,7 +40,7 @@ func TestPtrOf(t *testing.T) {
 func TestPtrHelpers(t *testing.T) {
 	assert.Equal(t, int32(1), *Int32Ptr(1))
 	assert.Equal(t, int64(1), *Int64Ptr(1))
-	assert.Equal(t, float64(1.1), *Float64Ptr(1.1))
+	assert.Equal(t, 1.1, *Float64Ptr(1.1))
 	assert.Equal(t, true, *BoolPtr(true))
 	assert.Equal(t, "a", *StringPtr("a"))
 	assert.Equal(t, s.TaskList{Name: PtrOf("a")}, *TaskListPtr(s.TaskList{Name: PtrOf("a")}))
diff --git a/internal/common/thrift_util.go b/internal/common/thrift_util.go
index 15c454e03..730127e62 100644
--- a/internal/common/thrift_util.go
+++ b/internal/common/thrift_util.go
@@ -71,6 +71,9 @@ func TListDeserialize(ts []thrift.TStruct, b []byte) (err error) {
 
 // IsUseThriftEncoding checks if the objects passed in are all encoded using thrift.
 func IsUseThriftEncoding(objs []interface{}) bool {
+	if len(objs) == 0 {
+		return false
+	}
 	// NOTE: our criteria to use which encoder is simple if all the types are serializable using thrift then we use
 	// thrift encoder. For everything else we default to gob.
 	for _, obj := range objs {
@@ -83,6 +86,9 @@ func IsUseThriftEncoding(objs []interface{}) bool {
 
 // IsUseThriftDecoding checks if the objects passed in are all de-serializable using thrift.
 func IsUseThriftDecoding(objs []interface{}) bool {
+	if len(objs) == 0 {
+		return false
+	}
 	// NOTE: our criteria to use which encoder is simple if all the types are de-serializable using thrift then we use
 	// thrift decoder. For everything else we default to gob.
 	for _, obj := range objs {
diff --git a/internal/common/thrift_util_test.go b/internal/common/thrift_util_test.go
index e6e705e43..a73647455 100644
--- a/internal/common/thrift_util_test.go
+++ b/internal/common/thrift_util_test.go
@@ -59,43 +59,71 @@ func TestTListDeserialize(t *testing.T) {
 }
 
 func TestIsUseThriftEncoding(t *testing.T) {
-	ts := []interface{}{
-		&mockThriftStruct{},
-		&mockThriftStruct{},
-	}
-
-	result := IsUseThriftEncoding(ts)
-	assert.True(t, result)
-
-	ts = []interface{}{
-		&mockThriftStruct{},
-		"string",
+	for _, tc := range []struct {
+		name     string
+		input    []interface{}
+		expected bool
+	}{
+		{
+			name:     "nil",
+			input:    nil,
+			expected: false,
+		},
+		{
+			name: "success",
+			input: []interface{}{
+				&mockThriftStruct{},
+				&mockThriftStruct{},
+			},
+			expected: true,
+		},
+		{
+			name: "fail",
+			input: []interface{}{
+				&mockThriftStruct{},
+				PtrOf("string"),
+			},
+			expected: false,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expected, IsUseThriftEncoding(tc.input))
+		})
 	}
-
-	result = IsUseThriftEncoding(ts)
-	assert.False(t, result)
 }
 
 func TestIsUseThriftDecoding(t *testing.T) {
-	t.Run("success", func(t *testing.T) {
-		str1 := &mockThriftStruct{}
-		str2 := &mockThriftStruct{}
-		ts := []interface{}{
-			&str1,
-			&str2,
-		}
-
-		assert.True(t, IsUseThriftDecoding(ts))
-	})
-
-	t.Run("fail", func(t *testing.T) {
-		ts := []interface{}{
-			&mockThriftStruct{},
-			"string",
-		}
-
-		assert.False(t, IsUseThriftDecoding(ts))
-	})
+	for _, tc := range []struct {
+		name     string
+		input    []interface{}
+		expected bool
+	}{
+		{
+			name:     "nil",
+			input:    nil,
+			expected: false,
+		},
+		{
+			name: "success",
+			input: []interface{}{
+				PtrOf(&mockThriftStruct{}),
+				PtrOf(&mockThriftStruct{}),
+			},
+			expected: true,
+		},
+		{
+			name: "fail",
+			input: []interface{}{
+				PtrOf(&mockThriftStruct{}),
+				PtrOf("string"),
+			},
+			expected: false,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expected, IsUseThriftDecoding(tc.input))
+		})
+	}
 }
 
 func TestIsThriftType(t *testing.T) {