assert: Remove the struct type constraint on 'TestEqualExportedValues' to support pointer comparisons
Pointer comparisons are supported already, just as long as the are not the top-level element. Logically, if two item or sub-items have different unexported field, then they cannot have the same underlying address. With this change, the top-level will be treated no differently from the rest of the struct fields which also makes the logic simpler.
The normal use case here is when asserting any response data that has a pointer type. As the code currently stands on master, you have get the underlying values and compare those instead, otherwise it fails:
require.NotNil(t, actual)
assert.EqualExportedValues(t, *expected, *actual)
I'm adding a test case that would have failed with the old logic.
I am also getting rid of the duplicated ObjectsExportedFieldsAreEqual. Since each assert function return a boolean result, we can achieve exactly the same without the extra function. This was discussed when adding the original code (https://github.com/stretchr/testify/pull/1309#discussion_r1063031396), which made sense then but now I think the time is right to remove it.
Ping @boyan-soubachov @mchlp @feidtmb
Ping @boyan-soubachov
This will improve the user experience for TestEqualExportedValues.
Ping @boyan-soubachov
Currently the library is usable, but it gives this error when comparing Protobuf objects because you have to access the underlying pointer, otherwise the comparison is false:
call of assert.EqualExportedValues copies lock value: github.com/dietdoctor/genproto/go/my-project/v1.ListItemsResponse contains google.golang.org/protobuf/internal/impl.MessageState contains sync.Mutexcopylocks
@dolmen Done, I have update the code! 🤗
👀
@dolmen Hi, did you have time to take another look at this?
Hi @dolmen @boyan-soubachov, did you have a chance to take a look at this?
@dolmen Done!
@dolmen Any update on when 1.9.0 is expected? Let's merge this in the meantime?
Ping @dolmen
I think #1517 fully implemented this.