Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assert.Same() for map #1691

Open
tturbs opened this issue Jan 3, 2025 · 2 comments
Open

assert.Same() for map #1691

tturbs opened this issue Jan 3, 2025 · 2 comments

Comments

@tturbs
Copy link

tturbs commented Jan 3, 2025

Hello everyone,

I’d like to ask a quick question informally.
118fb83#diff-cfb0727d3ab618e656775b3bb99089481b38a25f8ece2b10913e2d88e492a165R531

I have some concerns regarding this commit. While the changes to the assert.Same() function seem appropriate for pointer comparisons, they are not suitable for comparing maps. In my case, it has become impossible to compare a map with its cloned copy.

If this change is considered appropriate, I believe there should be a separate function specifically for comparing pointer-like objects.

Thanks in advance.

@william-snyders-appfolio

This is also an issue for me. We have many tests that are verifying cloning of maps, and those tests now fail with this error. This feels like a breaking change.

@william-snyders-appfolio
Copy link

william-snyders-appfolio commented Jan 8, 2025

After testing things out, it seems like NotSame was not testing what we thought it was. (The below is using the previous version v1.9.0)

func TestSameMap(t *testing.T) {
	a := map[string]string{"z": "z"}
	b := map[string]string{"z": "z"}

  // succeeds
	assert.Equal(t, a, b)

	// Both of these succeed
	assert.NotSame(t, a, a)
	assert.NotSame(t, a, b)

	// Both of these fail
	assert.Same(t, a, a)
	assert.Same(t, a, b)
}

Interestingly, assert.Same(t, a, a) fails with the following error


        	Error:      Not same:
        	           	expected: 0x14000108f30 map[string]string{"z":"z"}
        	           	actual  : 0x14000108f30 map[string]string{"z":"z"}

I would hope this function would work with maps though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants