Skip to content

Commit

Permalink
NotSame should fail if args are not pointers #1661 (#1664)
Browse files Browse the repository at this point in the history
## 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]>
  • Loading branch information
sikehish and brackendawson authored Oct 28, 2024
1 parent a1b9c9e commit 118fb83
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 27 deletions.
30 changes: 22 additions & 8 deletions assert/assertions.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,13 @@ func Same(t TestingT, expected, actual interface{}, msgAndArgs ...interface{}) b
h.Helper()
}

if !samePointers(expected, actual) {
same, ok := samePointers(expected, actual)
if !ok {
return Fail(t, "Both arguments must be pointers", msgAndArgs...)
}

if !same {
// both are pointers but not the same type & pointing to the same address
return Fail(t, fmt.Sprintf("Not same: \n"+
"expected: %p %#v\n"+
"actual : %p %#v", expected, expected, actual, actual), msgAndArgs...)
Expand All @@ -522,29 +528,37 @@ func NotSame(t TestingT, expected, actual interface{}, msgAndArgs ...interface{}
h.Helper()
}

if samePointers(expected, actual) {
same, ok := samePointers(expected, actual)
if !ok {
//fails when the arguments are not pointers
return !(Fail(t, "Both arguments must be pointers", msgAndArgs...))
}

if same {
return Fail(t, fmt.Sprintf(
"Expected and actual point to the same object: %p %#v",
expected, expected), msgAndArgs...)
}
return true
}

// samePointers compares two generic interface objects and returns whether
// they point to the same object
func samePointers(first, second interface{}) bool {
// samePointers checks if two generic interface objects are pointers of the same
// type pointing to the same object. It returns two values: same indicating if
// they are the same type and point to the same object, and ok indicating that
// both inputs are pointers.
func samePointers(first, second interface{}) (same bool, ok bool) {
firstPtr, secondPtr := reflect.ValueOf(first), reflect.ValueOf(second)
if firstPtr.Kind() != reflect.Ptr || secondPtr.Kind() != reflect.Ptr {
return false
return false, false //not both are pointers
}

firstType, secondType := reflect.TypeOf(first), reflect.TypeOf(second)
if firstType != secondType {
return false
return false, true // both are pointers, but of different types
}

// compare pointer addresses
return first == second
return first == second, true
}

// formatUnequalValues takes two values of arbitrary types and returns string
Expand Down
58 changes: 39 additions & 19 deletions assert/assertions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -655,39 +655,59 @@ func Test_samePointers(t *testing.T) {
second interface{}
}
tests := []struct {
name string
args args
assertion BoolAssertionFunc
name string
args args
same BoolAssertionFunc
ok BoolAssertionFunc
}{
{
name: "1 != 2",
args: args{first: 1, second: 2},
assertion: False,
name: "1 != 2",
args: args{first: 1, second: 2},
same: False,
ok: False,
},
{
name: "1 != 1 (not same ptr)",
args: args{first: 1, second: 1},
same: False,
ok: False,
},
{
name: "ptr(1) == ptr(1)",
args: args{first: p, second: p},
same: True,
ok: True,
},
{
name: "1 != 1 (not same ptr)",
args: args{first: 1, second: 1},
assertion: False,
name: "int(1) != float32(1)",
args: args{first: int(1), second: float32(1)},
same: False,
ok: False,
},
{
name: "ptr(1) == ptr(1)",
args: args{first: p, second: p},
assertion: True,
name: "array != slice",
args: args{first: [2]int{1, 2}, second: []int{1, 2}},
same: False,
ok: False,
},
{
name: "int(1) != float32(1)",
args: args{first: int(1), second: float32(1)},
assertion: False,
name: "non-pointer vs pointer (1 != ptr(2))",
args: args{first: 1, second: p},
same: False,
ok: False,
},
{
name: "array != slice",
args: args{first: [2]int{1, 2}, second: []int{1, 2}},
assertion: False,
name: "pointer vs non-pointer (ptr(2) != 1)",
args: args{first: p, second: 1},
same: False,
ok: False,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.assertion(t, samePointers(tt.args.first, tt.args.second))
same, ok := samePointers(tt.args.first, tt.args.second)
tt.same(t, same)
tt.ok(t, ok)
})
}
}
Expand Down

0 comments on commit 118fb83

Please sign in to comment.