gnolang / gno

Gno: An interpreted, stack-based Go virtual machine to build succinct and composable apps + Gno.land: a blockchain for timeless code and fair open-source.
https://gno.land/
Other
883 stars 367 forks source link

Use `jaekwon/testify` or `stretchr/testify` #777

Closed tbruyelle closed 5 months ago

tbruyelle commented 1 year ago

Codebase currently uses both of them, which is confusing because if I'm not wrong the only difference between the 2 is the expected vs actual order in methods.

moul commented 1 year ago

Regarding the .go part, I'll defer to @jaekwon.

In my opinion, I feel like I'm missing an equivalent in .gno and feel uncomfortable writing tests without it. Therefore, I propose porting the final choice to Gno and ensuring a fair approach to licensing/copyright (related to issue #679).

jaekwon commented 1 year ago

Let's go with the jaekwon one. We can just fork it over to gnolang/testify and use that. And yes, porting to gno sounds good too. Ideally lean too, if reasonable.

Please reassign if you need my input.

moul commented 1 year ago

FYI, I started experimenting in Gno with an even simpler API not relying on reflection: https://github.com/gnolang/gno/pull/928.

thehowl commented 1 year ago

I personally always found testify to be useful but often with too many features for what we generally need for tests. Here's something to prove my point:

$ rg --iglob '*_test.go' -o '(assert|require)\.[A-Za-z0-9_]*' | cut -d: -f2 | cut -d. -f2 | sort | uniq -c | sort -rn
    997 Equal
    622 NoError
    471 Nil
    318 True
    169 NotNil
    150 False
    149 EqualValues
    107 Error
     90 Panics
     63 New
     36 Contains
     34 Len
     31 NotEqual
     24 Empty
     19 NotEmpty
     17 Zero
     16 Fail
     15 NotPanics
      6 IsType
      5 NotZero
      5 NotContains
      5 Exactly
      4 Regexp
      4 EqualError
      2 JSONEq
      2 ErrorIs
      2 Errorf
      1 NoErrorf
      1 InDelta
      1 Greater
      1 ErrorContains
      1 Equalf

In another project we had made a reduced assertions package with only 18 assertion functions. I thought it was useful as having less functions means gopls is snappier and the autocompletions tend to be more around the functions that are actually used.

So if we're deviating from stretchr/testify (I understand the current fork is to swap expected/actual, which always bothered me as well), we might also have it as a reduced subset we maintain in a small package in the repo.

As for the Gno counterpart, I think a reduced subset makes sense. However, Equal/NotEqual, which are undoubtedly the most used assertions, rely on reflect.DeepEqual to work on complex types. Maybe that could be a good first function to implement in an eventual reflect package?

tbruyelle commented 1 year ago

Note that jaekwon/testify is not up to date and it's missing some methods like ErrorIs