pelletier / go-toml

Go library for the TOML file format
https://github.com/pelletier/go-toml
Other
1.67k stars 208 forks source link

Remove testify #872

Open pelletier opened 1 year ago

pelletier commented 1 year ago

The only dependency of go-toml v2 is stretchr/testify, which itself imports a bunch of stuff. I'd like this library to be dependence-free. The test suite only uses a few functions from testify, so it should be reasonably straightforward to implement them inside go-toml and replace the import statements.

Maybe they can be stored in /internal/require and /internal/assert to preserve the same layout.

ramisback commented 1 year ago

Hi, I would like to take this issue

pelletier commented 1 year ago

I'd be happy to review a pull request :)

sparr commented 1 year ago

@ramisback Are you still planning to work on this?

jidicula commented 9 months ago

Hey @pelletier 👋

I can see a couple of approaches - either (manually) vendorize assert and require methods used in go-toml v2 as well as their dependencies into internal, or reimplement just their behaviour in internal/assert and internal/require. Do you have a preference on which might be better?

pelletier commented 9 months ago

Hi! I think it would be better to re-implement the minimal subset needed instead of vendoring (which would also bring the dependencies).

Looking at the current use of testify in this project:

~/s/g/p/go-toml v2$ ag -o --nofilename --nobreak 'require\.([a-zA-Z]+)'|sort|uniq -c
  83 require.Equal
  57 require.Error
   3 require.IsType
   1 require.LessOrEqual
   1 require.Nil
 129 require.NoError
   2 require.Panics
~/s/g/p/go-toml v2$ ag -o --nofilename --nobreak 'assert\.([a-zA-Z]+)'|sort|uniq -c
   1 assert.Contains
   7 assert.Empty
  55 assert.Equal
   9 assert.Error
   2 assert.InDelta
   3 assert.NoError
   2 assert.NotEmpty
   3 assert.True

I'd follow something like this.

First, pick one of assert or require and map one onto the other (for example, replace all require.* to assert.*). That should be reasonably easy with a search+replace. I don't have a strong preference between the two behaviors. Maybe assert is a bit nicer since more tests can continue until something panics. This should result in approximately:

~/s/g/p/go-toml v2$ ag -o --nofilename --nobreak '(require|assert)\.([a-zA-Z]+)'|sed s/require/assert/|sort|uniq -c|sort --reverse
 138 assert.Equal
 132 assert.NoError
  66 assert.Error
   7 assert.Empty
   3 assert.True
   3 assert.IsType
   2 assert.Panics
   2 assert.NotEmpty
   2 assert.InDelta
   1 assert.Nil
   1 assert.LessOrEqual
   1 assert.Contains

Which is only 12 functions, most of them straightforward to write, or lightly adapted from testify. Then once all the testify calls are all mapped, create an internal/testing or internal/assert package, and implement those functions, and finally remove the imports. It's probably worth looking into not having dedicated functions for those that are only used < 10 times!

I believe the hard part will be printing the diff when two values differ. Though I think a simple fmt.Printf("%+v") of the expected and the actual values is probably good enough to get started. But feel free to do something more fancy with reflect :)