assert: Remove the struct type constraint on 'TestEqualExportedValues' to support pointer comparisons

[HaraldNordgren]

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 (#1309 (comment)), which made sense then but now I think the time is right to remove it.

[HaraldNordgren]

Ping @boyan-soubachov @mchlp @feidtmb

[HaraldNordgren]

Ping @boyan-soubachov

This will improve the user experience for TestEqualExportedValues.

[HaraldNordgren]

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]

I am also getting rid of the duplicated ObjectsExportedFieldsAreEqual

While this looks sane, this is not that easy. We have to preserve the API surface. Please keep the function and mark it as // Deprecated:.

[HaraldNordgren]

@dolmen Done, I have update the code! 🤗

[jufemaiz]

👀

[HaraldNordgren]

@dolmen Hi, did you have time to take another look at this?

[HaraldNordgren]

Hi @dolmen @boyan-soubachov, did you have a chance to take a look at this?

[boyan-soubachov]

LGTM, thank you

[dolmen]

Please open a separate MR to deprecate ObjectsExportedFieldsAreEqual, and move that change there.

Also cleanup commits, and rebase (do not merge master into your branch!).

[HaraldNordgren]

@dolmen Done!

[dolmen]

Ready for v1.9.0: this is a behaviour change that require a minor release.

[HaraldNordgren]

@dolmen Any update on when 1.9.0 is expected? Let's merge this in the meantime?

[HaraldNordgren]

Ping @dolmen

[brackendawson]

I think #1517 fully implemented this.