Skip to content

Commit 118fb83

Browse files
NotSame should fail if args are not pointers #1661 (#1664)
## Summary Reduces the confusion assosciated with NotSame that previously would check nothing if any of the values is not a pointer. The changes made were tested using TestSame, TestNotSame, and Test_samePointers tests, and the changes did clear the tests. ## Changes 1. Modified samePointers to return another bool value(ok) that would determine if the 2 values are of pointer type or not, while the returned "same" bool value would determine if the 2 pointers are same. 2. Modified assert.NotSame to call Fail() if the 2 values are either i)pointers pointing to the same address or ii)either/both of the values are not pointers. 3. Modified assert.Same to call Fail() if the 2 values are either i)pointers not pointing to the same address or ii)either/both of the values are not pointers. 4. Modified Test_samePointers to handle the new behavior of samePointers by checking both if the inputs are pointers (ok) and if they point to the same object (same). ## Motivation Ensure NotSame accurately verifies pointer sameness by handling non-pointer inputs explicitly, improving clarity and reducing potential misuse. ## Related issues Closes #1661 --------- Co-authored-by: Bracken <[email protected]>
1 parent a1b9c9e commit 118fb83

File tree

2 files changed

+61
-27
lines changed

2 files changed

+61
-27
lines changed

assert/assertions.go

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,13 @@ func Same(t TestingT, expected, actual interface{}, msgAndArgs ...interface{}) b
502502
h.Helper()
503503
}
504504

505-
if !samePointers(expected, actual) {
505+
same, ok := samePointers(expected, actual)
506+
if !ok {
507+
return Fail(t, "Both arguments must be pointers", msgAndArgs...)
508+
}
509+
510+
if !same {
511+
// both are pointers but not the same type & pointing to the same address
506512
return Fail(t, fmt.Sprintf("Not same: \n"+
507513
"expected: %p %#v\n"+
508514
"actual : %p %#v", expected, expected, actual, actual), msgAndArgs...)
@@ -522,29 +528,37 @@ func NotSame(t TestingT, expected, actual interface{}, msgAndArgs ...interface{}
522528
h.Helper()
523529
}
524530

525-
if samePointers(expected, actual) {
531+
same, ok := samePointers(expected, actual)
532+
if !ok {
533+
//fails when the arguments are not pointers
534+
return !(Fail(t, "Both arguments must be pointers", msgAndArgs...))
535+
}
536+
537+
if same {
526538
return Fail(t, fmt.Sprintf(
527539
"Expected and actual point to the same object: %p %#v",
528540
expected, expected), msgAndArgs...)
529541
}
530542
return true
531543
}
532544

533-
// samePointers compares two generic interface objects and returns whether
534-
// they point to the same object
535-
func samePointers(first, second interface{}) bool {
545+
// samePointers checks if two generic interface objects are pointers of the same
546+
// type pointing to the same object. It returns two values: same indicating if
547+
// they are the same type and point to the same object, and ok indicating that
548+
// both inputs are pointers.
549+
func samePointers(first, second interface{}) (same bool, ok bool) {
536550
firstPtr, secondPtr := reflect.ValueOf(first), reflect.ValueOf(second)
537551
if firstPtr.Kind() != reflect.Ptr || secondPtr.Kind() != reflect.Ptr {
538-
return false
552+
return false, false //not both are pointers
539553
}
540554

541555
firstType, secondType := reflect.TypeOf(first), reflect.TypeOf(second)
542556
if firstType != secondType {
543-
return false
557+
return false, true // both are pointers, but of different types
544558
}
545559

546560
// compare pointer addresses
547-
return first == second
561+
return first == second, true
548562
}
549563

550564
// formatUnequalValues takes two values of arbitrary types and returns string

assert/assertions_test.go

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -655,39 +655,59 @@ func Test_samePointers(t *testing.T) {
655655
second interface{}
656656
}
657657
tests := []struct {
658-
name string
659-
args args
660-
assertion BoolAssertionFunc
658+
name string
659+
args args
660+
same BoolAssertionFunc
661+
ok BoolAssertionFunc
661662
}{
662663
{
663-
name: "1 != 2",
664-
args: args{first: 1, second: 2},
665-
assertion: False,
664+
name: "1 != 2",
665+
args: args{first: 1, second: 2},
666+
same: False,
667+
ok: False,
668+
},
669+
{
670+
name: "1 != 1 (not same ptr)",
671+
args: args{first: 1, second: 1},
672+
same: False,
673+
ok: False,
674+
},
675+
{
676+
name: "ptr(1) == ptr(1)",
677+
args: args{first: p, second: p},
678+
same: True,
679+
ok: True,
666680
},
667681
{
668-
name: "1 != 1 (not same ptr)",
669-
args: args{first: 1, second: 1},
670-
assertion: False,
682+
name: "int(1) != float32(1)",
683+
args: args{first: int(1), second: float32(1)},
684+
same: False,
685+
ok: False,
671686
},
672687
{
673-
name: "ptr(1) == ptr(1)",
674-
args: args{first: p, second: p},
675-
assertion: True,
688+
name: "array != slice",
689+
args: args{first: [2]int{1, 2}, second: []int{1, 2}},
690+
same: False,
691+
ok: False,
676692
},
677693
{
678-
name: "int(1) != float32(1)",
679-
args: args{first: int(1), second: float32(1)},
680-
assertion: False,
694+
name: "non-pointer vs pointer (1 != ptr(2))",
695+
args: args{first: 1, second: p},
696+
same: False,
697+
ok: False,
681698
},
682699
{
683-
name: "array != slice",
684-
args: args{first: [2]int{1, 2}, second: []int{1, 2}},
685-
assertion: False,
700+
name: "pointer vs non-pointer (ptr(2) != 1)",
701+
args: args{first: p, second: 1},
702+
same: False,
703+
ok: False,
686704
},
687705
}
688706
for _, tt := range tests {
689707
t.Run(tt.name, func(t *testing.T) {
690-
tt.assertion(t, samePointers(tt.args.first, tt.args.second))
708+
same, ok := samePointers(tt.args.first, tt.args.second)
709+
tt.same(t, same)
710+
tt.ok(t, ok)
691711
})
692712
}
693713
}

0 commit comments

Comments
 (0)