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

EqualExportedValues can compare two slices #1584

Open
redachl opened this issue Apr 11, 2024 · 3 comments
Open

EqualExportedValues can compare two slices #1584

redachl opened this issue Apr 11, 2024 · 3 comments
Labels
assert.EqualValues About equality enhancement pkg-assert Change related to package testify/assert

Comments

@redachl
Copy link
Contributor

redachl commented Apr 11, 2024

Description

EqualExportedValues does not handle slices for now, although Equal and EqualValues compare each element of 2 slices.

Proposed solution

Removing the check that the provided variables are structs or pointers to structs here allows comparing two slices.

if aType.Kind() != reflect.Struct {

Happy to propose a PR if you agree with this design!

I made a PR to remove the described checks.

Use case

The implementation of EqualExportedValues can actually compare slices, if they are inside a struct.

Here is an example.

func TestSomething(t *testing.T) {
	type sliceWrapper struct {
		ExportedSlice []int
	}
	a := sliceWrapper{ExportedSlice: []int{1, 2, 3}}
	b := sliceWrapper{ExportedSlice: []int{1, 2, 3}}

	assert.Equal(t, a.ExportedSlice, b.ExportedSlice)               // OK
	assert.EqualValues(t, a.ExportedSlice, b.ExportedSlice)         // OK
	assert.EqualExportedValues(t, a.ExportedSlice, b.ExportedSlice) // Types expected to both be struct or pointer to struct

	// Possible workaround: use a struct wrapper.
	assert.Equal(t, a, b)               // OK
	assert.EqualValues(t, a, b)         // OK
	assert.EqualExportedValues(t, a, b) // OK
}	
@dolmen dolmen added pkg-assert Change related to package testify/assert assert.EqualValues About equality labels May 16, 2024
@Antonboom
Copy link

Antonboom commented May 23, 2024

The word Exported binds this function to structs, not to specific struct's field.

See the doc:

// EqualExportedValues asserts that the types of two objects are equal and their public fields are also equal.

How does this relate to slices and why is this function expected to behave the same as Equal and EqualValues?

Looks like nonsense.

@redachl
Copy link
Contributor Author

redachl commented May 29, 2024

That's one way to look at it.

But I think it's also a pity that you have to use a slice wrapper when testing a function that returns a slice.
Especially when it's a slice of proto messages, which with the latest code generation have unexported fields.

Another option is to add a new function, but I'm not sure it's the best thing to do.

Or we just accept using a slice wrapper is enough.

@brackendawson
Copy link
Collaborator

For me it's key that the doc says "objects", which is not defined in the go spec, and not "structs". I think this leaves scope for nested structs (already supported) and structs nested in other container types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert.EqualValues About equality enhancement pkg-assert Change related to package testify/assert
Projects
None yet
Development

No branches or pull requests

4 participants