stretchr / testify

A toolkit with common assertions and mocks that plays nicely with the standard library
MIT License
22.5k stars 1.56k forks source link

EqualExportedValues can compare two slices #1584

Open redachl opened 2 months ago

redachl commented 2 months ago

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. https://github.com/stretchr/testify/blob/352d2438b9da776308844714c38c0c9a16f3b5b6/assert/assertions.go#L629

~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
}   
Antonboom commented 1 month ago

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 commented 1 month ago

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.