Consider API redesign with the current testing package
mvdan opened this issue · 9 comments
I realise that this module is a v1 and we cannot break backwards compatibility, which is a good thing. However, the testing package has evolved quite a bit in the past few years - especially with methods like TempDir and Cleanup, which came directly from this module :)
I'm starting to wonder if designing a v2 would be a good idea. The simplest changes we could make are removing deprecated APIs now available in the upstream package, such as Mkdir, Defer, and Done. I imagine there might also be some other tech debt that we could fix when given the chance to break backwards compatibility.
I also wonder if we could remove the New constructor entirely, and replace methods like C.Check with qt.Check. For example, instead of:
func TestFoo(t *testing.T) {
c := qt.New(t)
numbers, err := somepackage.Numbers()
c.Assert(numbers, qt.DeepEquals, []int{42, 47})
c.Assert(err, qt.ErrorMatches, "bad wolf")
}
We could just do:
func TestFoo(t *testing.T) {
numbers, err := somepackage.Numbers()
qt.Assert(t, numbers, qt.DeepEquals, []int{42, 47})
qt.Assert(t, err, qt.ErrorMatches, "bad wolf")
}
Presumably, part of the reason a state was needed was for Defer/Done, which should no longer apply. It seems like the only other reason to keep state around is C.SetFormat. I hope that we have alternative options there, such as a global per-package format, or deriving the format from the test name, or globally registering a format for a specific *testing.T like qt.SetFormat(t, ...).
/cc @rogpeppe who I think has been thinking about a possible v2 as well
I think it's worth keeping the current API around as we don't really want people to have to change all their code to move to the new API and the old idiom involves less typing. But it's not exclusive. We could do something like this:
package qt
func Assert(t testing.TB, etc)
func Check(t testing.TB, etc)
type C struct {
testing.TB
}
func New(t testing.TB) *C {
return &C{t}
}
func (c *C) Assert(etc) {
return Assert(c.TB, ...)
}
func (c *C) Check(etc) {
return Check(c.TB, ...)
}
This is interesting, thanks for the input!
I think it's still worth keeping Mkdir, Defer and Done for now, as they are useful when having to test a code base against multiple versions of Go, like in the library use case, or when having to migrate code to Go >= 1.15 before being able to leverage the new features of the testing package. What do you think?
I agree with Roger that we can have both top level Assert, Check, Patch, Setenv, Unsetenv functions and a C wrapper for the old idiom. Could you please expand a bit on the reason this new idiom would be more compelling in your experience? I guess one reason is that, when you just need a quick assertion on your test, calling a function is quicker than getting a *C instance and then assert in two separate lines.
Keeping deprecated APIs around for a few extra Go releases makes sense. I intended such breaking changes to be aimed towards 2021 or so :) Roger's idea to simply improve v1 for now also makes sense.
when you just need a quick assertion on your test, calling a function is quicker than getting a *C instance and then assert in two separate lines.
Indeed - for short/small tests, avoiding the extra line and variable feels better. It also feels like you're just using a library as opposed to a testing framework, as you're not wrapping nor replacing t in any way.
On the other hand, for larger or more complex tests, I see how C can be useful. In a way, this choice between simplicity and scalability reminds me of testing.T.Run for subtests; it doesn't make sense for small tests, but for larger stuff, it really does help. But both options (normal tests and many sub-tests) are options when writing test functions.
If we do a v2, then I think we could happily drop support for old Go releases and streamline the API a lot.
Ok this is now released as v1.11.0.
Hmm, why was this closed? This is about a v2 redesign, so by definition it's still a research project.
Closed by the merged pull request that implements the new API. But it's fair that this can be used for further v2 API discussion.
v2 for quicktest is essentially https://github.com/go-quicktest/qt
That includes an API redesign that takes advantage of compile time checks thanks to Go generics.
As no backward compatible constraints are in place, that also drops the functionality (Defer, Done, Cleanup, MkDir) that over time has been included in the stdlib.
I'd close this in favor of go-quicktest/qt#7 for any discussion around the new API.