Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
3vilhamster committed Nov 8, 2024
1 parent 5501cc5 commit d727131
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 34 deletions.
2 changes: 1 addition & 1 deletion internal/common/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")}))
Expand Down
6 changes: 6 additions & 0 deletions internal/common/thrift_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
94 changes: 61 additions & 33 deletions internal/common/thrift_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit d727131

Please sign in to comment.