Extend assert.Same to support pointer-like objects #1777
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Today, assert.Same and assert.NotSame support comparing only literal pointers. This PR generalizes both functions so they also support comparing maps and channels by type and pointer equality. Why these two types? Because the [Go language reference][0] states: > A map or channel value is a reference to the implementation-specific > data structure of the map or channel. That is, it states maps and channels are de-facto pointers. So, my belief is that it would minimize surprise if we allow Same/NotSame to compare any two values that consist of just a reference to some underlying value, regardless if that underlying value is a user-defined object vs some implementation-specific data structure. By this logic, I decided against modifying Same/NotSame to support comparing things like slices, which are not represented via a single reference. [0]: https://go.dev/ref/spec#Representation_of_values
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Modify assert.Same and assert.NotSame to support comparing maps and channels by pointer equality.
Changes
samePointerstosameReferencesand generalize it so it will try comparing objects that are represented using a single pointer/reference, as opposed to comparing only literal pointers. We maintain the invariant that the two args must have the same Kind and Type.Test_samePointerstoTest_sameReferencesTest_sameReferences,TestSame, andTestNotSame.Motivation
Today, assert.Same and assert.NotSame support comparing only literal pointers. This PR generalizes both functions so they also support comparing maps and channels by type and pointer equality.
Why these two types? Because the Go language reference states:
That is, all three types are represented by a single reference to some underlying value.
Given this, my belief is it would minimize surprise for users if we handle all three cases in the same way in Same/NotSame: we compare the objects by pointer equality, regardless if the underlying value is a user-defined object vs some implementation-specific data structure.
By this logic, I decided against modifying Same/NotSame to support comparing things like slices, which are not represented via a single reference.
Related issues
There is a related discussion #1732, where (iiuc) the claim was made that we should not support Same/NotSame for maps because we cannot compare map keys and values for sameness in all cases.
However, I found this a somewhat surprising argument: if Same is given a pointer to some user-defined struct, it currently does not attempt to introspect and compare the contents of that struct. So, I don't see why it's necessary or expected to do the same for maps.
Hence, this PR.