Move object equality functions to another package for clarity

[mminklet]

Summary

Move none assertion functions into a seperate package to make it clear that these do no do any assertions. There is a a lot of existing code out there with useless tests because people are assuming that assert.Foobar functions will do assertions.

Happy to take guidance on comments/naming etc.

Changes

Moved ObjectsAreEqual and ObjectsAreEqualValues and tests into a new sub package equals. Label existing functions as deprecated and call new functions instead. Call new methods anywhere in assert where the previous functions are called.

Motivation

Currently very confusing that these are in the assert package when they don't do any assertions at all.

change assert.ObjectsAreEqual to equal.ObjectsAreEqual etc

Related issues

Closes 1180

[brackendawson]

If the aim of this is to prevent people from thinking that ObjectsAreEqual is an assertion that they should use, then I think this new equal package should be internal:

testify/
|-- assert
|   `-- assertions.go
|-- internal
|   `-- equal
|       `-- equal.go
|-- mock
|   `-- mock.go

[mminklet]

If the aim of this is to prevent people from thinking that ObjectsAreEqual is an assertion that they should use, then I think this new equal package should be internal:

testify/
|-- assert
|   `-- assertions.go
|-- internal
|   `-- equal
|       `-- equal.go
|-- mock
|   `-- mock.go

I was actually thinking that people would still continue to use these methods, as they are useful, they're just not immediately obvious that the bool needs to be checked to have a working test. But perhaps another name/location outside of assert makes more sense?

[brackendawson]

If the aim of this is to prevent people from thinking that ObjectsAreEqual is an assertion that they should use, then I think this new equal package should be internal:

testify/
|-- assert
|   `-- assertions.go
|-- internal
|   `-- equal
|       `-- equal.go
|-- mock
|   `-- mock.go

I was actually thinking that people would still continue to use these methods, as they are useful, they're just not immediately obvious that the bool needs to be checked to have a working test. But perhaps another name/location outside of assert makes more sense?

While it's true that they might be useful to someone, I think people will still misuse them in exactly the same way. Testify is not a utility library, I think if people are looking for a comparison library then they should be looking at go-cmp instead.

[mminklet]

Yeah that's a good point. Tbh then I'm a bit reticent about introducing this change then, as I don't want to remove useful functionality even if it is currently being misused

Actually scratch that, I realise that people just need to be directed to use the Equal and EqualValues assertions instead. I will alter my PR now

Edit: Updated with suggested changes.

[mminklet]

Is there any progress on this? If there are suggestions that are required, please let me know and I'll gladly make them. People are still being caught by this

[dolmen]

Please rebase this branch on top of master, to fix gofmt issues.

[dolmen]

Replaced by #1431.

[dolmen]

Closing in favor of #1431.

[mminklet]

Nice! Sorry I didn’t have time to rebase, it had been so long that there were more changes that needed more work. Glad to see the change got in anyway!