Replace testify usage by gotest.tools

[julienrbrt]

There are longstanding issues with testify in particular stretchr/testify#535 which is a deal breaker for comparing protobuf values and stretchr/testify#187 (testify suites should be considered an anti-pattern because of this).

Context: #12291 (comment)

Currently, we are using both testify and gotest.tools through the codebase.
Please use gotest.tools/v3/assert wherever possible.

[aaronc]

Just a note. I don't think we need to rewrite existing tests. But new tests should likely use gotest in particular DeepEqual with protocmp.Transform when using the new protobuf API

[alexanderbez]

ACK -- Use gotest.tools V3 going forward 👍

[tac0turtle]

Just a note. I don't think we need to rewrite existing tests. But new tests should likely use gotest in particular DeepEqual with protocmp.Transform when using the new protobuf API

Going forward we should use this, but also for consistency we should try to use the same thign

[scalalang2]

Hi. I'm newbie in cosmos-sdk ecosystem.
I wanna update existing testify assertion codes into gotest.tools

[tac0turtle]

Amazing, let us know if you have any questions

[scalalang2]

I'm reading CONTRIBUTION guides.
I feel exciting to join this project. Thanks for reaction :)

[ryanchristo]

gotest.tools provides a command to automate the migration:

https://pkg.go.dev/gotest.tools@v2.2.0+incompatible/assert/cmd/gty-migrate-from-testify

[alexanderbez]

@ryanchristo do you have an example of tests I can look at or even a test suite ideally I can look at? Last time I played with gotest it was lacking in rich feature functionality of testify (e.g. suites, various methods on require such as Empty, or Nil)

[ryanchristo]

@ryanchristo do you have an example of tests I can look at or even a test suite ideally I can look at? Last time I played with gotest it was lacking in rich feature functionality of testify (e.g. suites, various methods on require such as Empty, or Nil)

We are still using testify's require throughout regen ledger and I hold a similar position and see gotest as lacking features. I was actually considering a proposal to use require throughout before an issue was proposed to stay aligned with the sdk linking to the issue here, so nothing to share at the moment but I can further investigate tradeoffs.

[alexanderbez]

Gotcha. Looks like we should stick with testify's suite and require utilization, for now at least...

[aaronc]

Gotcha. Looks like we should stick with testify's suite and require utilization, for now at least...

I don't have a super strong opinion about gotest.tools/v3/assert vs testify require. They're both good for different cases.

But, I really think we should consider testify suite an anti-pattern. It does not support parallel tests and I don't see a lot of other upsides. A very simple alternative is just doing this:

type Fixture struct {
...
}

func initFixture(t *testing.T) *Fixture {
// if cleanup is needed use t.Cleanup()
}

func TestSomething(t *testing.T) {
   t.Parallel()
   f := initFixture(t)
}

This gives you pretty much everything you get from testify suite but should be much faster.

[alexanderbez]

Yup, pretty much! I just found testify's require API, e.g. Require.NoError and similar functions very useful. But we can switch to take advantage of parallel tests.

[julienrbrt]

As per our standup, we'll keep using testify and won't refactor it away in existing touched tests.
Existing packages using gotest.tools can stay as they are.
Testify suites should still be removed, but this is separately tracked: #14561.

[robert-zaremba]

I found that

• testify provides better trace information about the path to the error (useful when we compose functions).
• gotest.tools has better "diff" output when comparing values.
  So I am using both, depending on the test.