NotSame should fail if args are not pointers

[brackendawson]

Discussed in #1660

^{Originally posted by n4nn31355 October 16, 2024}
It's confusing and error-prone that NotSame checks nothing if any of the values is not a pointer.
Is there a reason for this, or may we consider it a bug that needs an issue?

a := struct{ byte }{}
require.NotSame(t, a, a)   // Doesn't fail
require.NotSame(t, &a, &a) // Fails
```</div>

[brackendawson]

This is a simple bug, the signature of samePointers needs to be changed to:

func samePointers(first, second interface{}) (same bool, ok bool)

[sikehish]

I'd love to take this up. Could you assign this to me?

[sikehish]

I'd love to take this up. Could you assign this to me?

Thank you. Im looking into the issue.

[sikehish]

Hi @brackendawson ,
I've raised a PR #1664 , do let me know if any changes need to be made.

[janisbiz]

Hi! What about slices? As now NotSame will not work with slices due to samePointers checking if variables are pointers which is not the case with slice.

Thus - samePointers should probably support reflect.SliceHeader for slices?

[brackendawson]

Do you have an example that shows the regression @janisbiz?

I believe Same and NotSame would statically fail and not fail when you passed slices to them, so they never actually tested anything.

If you want to open a PR or Issue to make them work for slices & maps as I think you expected them to, then I think we'd be happy to accept that.

[phughes-scwx]

https://go.dev/play/p/4kJugQJZmaG

package main

import (
	"testing"

	"github.com/stretchr/testify/assert"
)

func TestCaseA(t *testing.T) {
	s1 := []string{"a"}
	s2 := []string{"a"}
	assert.Equal(t, s1, s2)
	assert.NotSame(t, s1, s2)
}

func matchString(a, b string) (bool, error) {
	return a == b, nil
}

func main() {
	testSuite := []testing.InternalTest{
		{
			Name: "TestCaseA",
			F:    TestCaseA,
		},
	}
	testing.Main(matchString, testSuite, nil, nil)
}

This change has broken a number of tests I have that ensure an http.Header helper "clones" data, and therefore also fails for maps.

[brackendawson]

At testify v1.9.0, when passing slices to Same it will always fail, when passing slices to NotSame it will always pass. Your test in no way ensured that your helper clones the data.

playground

func TestCaseA(t *testing.T) {
	s1 := []string{"a"}
	s1b := s1
	s2 := []string{"a"}
	assert.Same(t, s1, s1b)    // false
	assert.Same(t, s1, s2)     // false
	assert.NotSame(t, s1, s1b) // true
	assert.NotSame(t, s1, s2)  // true
}

According to the docs, use of non-pointers with Same and NotSame is not valid. The change in v1.10.0 is that the invalid use is now an error.

If you want Same and NotSame to be changed to work with pointer-like types as well as pointers then that would be a new issue. I wouldn't be opposed to it being implemented.

[phughes-scwx]

Ah, seeing that this uncovers the actual behavior, and is not a change to that behavior, changes things. Thanks for your explanation.