From a88bf7aab8f390daf0a71952568272f6638b319e Mon Sep 17 00:00:00 2001 From: Boyan Date: Sun, 22 Sep 2019 10:20:10 +0200 Subject: [PATCH] PR comments --- assert/assertions.go | 32 ++++++++++--------------- assert/assertions_test.go | 50 +++++++++++++++------------------------ 2 files changed, 31 insertions(+), 51 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index 1dd7fe1..0e77a00 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -363,12 +363,7 @@ func Same(t TestingT, expected, actual interface{}, msgAndArgs ...interface{}) b h.Helper() } - expectedPtr, actualPtr := reflect.ValueOf(expected), reflect.ValueOf(actual) - if expectedPtr.Kind() != reflect.Ptr || actualPtr.Kind() != reflect.Ptr { - return Fail(t, "Invalid operation: both arguments must be pointers", msgAndArgs...) - } - - if !sameComparator(expected, actual) { + if !samePointers(expected, actual) { return Fail(t, fmt.Sprintf("Not same: \n"+ "expected: %p %#v\n"+ "actual : %p %#v", expected, expected, actual, actual), msgAndArgs...) @@ -388,12 +383,7 @@ func NotSame(t TestingT, expected, actual interface{}, msgAndArgs ...interface{} h.Helper() } - expectedPtr, actualPtr := reflect.ValueOf(expected), reflect.ValueOf(actual) - if expectedPtr.Kind() != reflect.Ptr || actualPtr.Kind() != reflect.Ptr { - return Fail(t, "Invalid operation: both arguments must be pointers", msgAndArgs...) - } - - if sameComparator(expected, actual) { + if samePointers(expected, actual) { return Fail(t, fmt.Sprintf( "Expected and actual point to the same object: %p %#v", expected, expected), msgAndArgs...) @@ -401,19 +391,21 @@ func NotSame(t TestingT, expected, actual interface{}, msgAndArgs ...interface{} return true } -// sameComparator compares two generic interface objects and returns whether -// they are of the same type and value -func sameComparator(first, second interface{}) bool { +// samePointers compares two generic interface objects and returns whether +// they point to the same object +func samePointers(first, second interface{}) bool { + firstPtr, secondPtr := reflect.ValueOf(first), reflect.ValueOf(second) + if firstPtr.Kind() != reflect.Ptr || secondPtr.Kind() != reflect.Ptr { + return false + } + firstType, secondType := reflect.TypeOf(first), reflect.TypeOf(second) if firstType != secondType { return false } - if first != second { - return false - } - - return true + // compare pointer addresses + return first == second } // formatUnequalValues takes two values of arbitrary types and returns string diff --git a/assert/assertions_test.go b/assert/assertions_test.go index 7053997..3023a14 100644 --- a/assert/assertions_test.go +++ b/assert/assertions_test.go @@ -217,14 +217,14 @@ func TestEqual(t *testing.T) { } } +func ptr(i int) *int { + return &i +} + func TestSame(t *testing.T) { mockT := new(testing.T) - ptr := func(i int) *int { - return &i - } - if Same(mockT, ptr(1), ptr(1)) { t.Error("Same should return false") } @@ -244,26 +244,24 @@ func TestNotSame(t *testing.T) { mockT := new(testing.T) - ptr := func(i int) *int { - return &i - } - if !NotSame(mockT, ptr(1), ptr(1)) { - t.Error("NotSame should return true") + t.Error("NotSame should return true; different pointers") } - if NotSame(mockT, 1, 1) { - t.Error("NotSame should return false; non-pointer inputs") + if !NotSame(mockT, 1, 1) { + t.Error("NotSame should return true; constant inputs") } p := ptr(2) - if NotSame(mockT, p, *p) { - t.Error("NotSame should return false; non-pointer input") + if !NotSame(mockT, p, *p) { + t.Error("NotSame should return true; mixed-type inputs") } if NotSame(mockT, p, p) { t.Error("NotSame should return false") } } -func Test_sameComparator(t *testing.T) { +func Test_samePointers(t *testing.T) { + p := ptr(2) + type args struct { first interface{} second interface{} @@ -279,8 +277,13 @@ func Test_sameComparator(t *testing.T) { assertion: False, }, { - name: "1 == 1", + name: "1 != 1 (not same ptr)", args: args{first: 1, second: 1}, + assertion: False, + }, + { + name: "ptr(1) == ptr(1)", + args: args{first: p, second: p}, assertion: True, }, { @@ -288,30 +291,15 @@ func Test_sameComparator(t *testing.T) { args: args{first: int(1), second: float32(1)}, assertion: False, }, - { - name: "true == true", - args: args{first: true, second: true}, - assertion: True, - }, - { - name: "false != true", - args: args{first: false, second: true}, - assertion: False, - }, { name: "array != slice", args: args{first: [2]int{1, 2}, second: []int{1, 2}}, assertion: False, }, - { - name: "array == array", - args: args{first: [2]int{1, 2}, second: [2]int{1, 2}}, - assertion: True, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tt.assertion(t, sameComparator(tt.args.first, tt.args.second)) + tt.assertion(t, samePointers(tt.args.first, tt.args.second)) }) } }