frankban / quicktest

Quick helpers for testing Go applications
MIT License
529 stars 27 forks source link

proposal: add Try and Recover #138

Open dsnet opened 2 years ago

dsnet commented 2 years ago

Error handling is tedious, and when writing tests we are prioritizing getting the test written quickly rather than necessarily making it the most robust piece of code we ever wrote.

I propose adding the following:

func (c *C) Recover()
func Try(error)
func Try1[A any](A, error) A
func Try2[A, B any](A, B, error) (A, B)
func Try3[A, B, C any](A, B, C, error) (A, B, C)
func Try4[A, B, C, D any](A, B, C, D, error) (A, B, C, D)

where the Try functions panic if err is non-nil. Otherwise, it peels off the error and returns the other arguments as is. The TryN functions are defined such that N is the number of return arguments. Generics in Go do not have variadic parametric types, so we cannot express a generic function with a generic number of arguments. Try4 should be sufficient since no standard library function has more than 4 return arguments.

Example usage:

func Test(t *testing.T) {
    c := qt.New(t)
    defer c.Recover() // catch errors panicked by calls to `Try` below

    zr := qt.Try1(gzip.NewReader(...))
    b := qt.Try1(io.ReadAll(zr))
    ... = b
}
dsnet commented 2 years ago

\cc @josharian @rogpeppe @mvdan

mvdan commented 2 years ago

FYI, the generics API experiment is at https://github.com/go-quicktest/qt, so I imagine it would go there as another set of generic APIs. I don't think we want to inconsistently add generics to this module. I'll leave it to Francesco to decide if the issue should be moved, though :)

josharian commented 2 years ago

This is very appealing, but I'd prefer:

func (c *C) Must(error)
func (c *C) Must1[A any](A, error) A
func (c *C) Must2[A, B any](A, B, error) (A, B)
func (c *C) Must3[A, B, C any](A, B, C, error) (A, B, C)
func (c *C) Must4[A, B, C, D any](A, B, C, D, error) (A, B, C, D)

Then you don't need Recover. And Must is already in wide use for "handle the error by failing immediately".

dsnet commented 2 years ago

That would be preferred, but unfortunately, generic methods cannot currently be defined on C (golang/go#49085).

josharian commented 2 years ago

Oh yeah. Sadness.

Still, do you really need the Recover? Can you always set up the recover in qt.New and then bubble up all irrelevant panics?

josharian commented 2 years ago

Now that you planted the seed in my brain I wanted to actually use it, so I hacked together something locally. Quick and dirty but it works, albeit using a recover. Improvements quite welcome. :)

Usage:

func TestFoo(t *testing.T) {
        defer recoverMust(t)

        must(x.Y())
        z := must1(x.W())
        // ...
}

Implementation:

type failedMust struct {
        error
}

func must(err error) {
        if err != nil {
                panic(failedMust{err})
        }
}

func must1[T any](t T, err error) T {
        if err != nil {
                panic(failedMust{err})
        }
        return t
}

func recoverMust(t *testing.T) {
        err := recover()
        if err == nil {
                return
        }
        _, ok := err.(failedMust)
        if !ok {
                // Some other panic; bubble it up.
                panic(err)
        }
        pc := make([]uintptr, 1)
        n := runtime.Callers(4, pc)
        pc = pc[:n]
        frames := runtime.CallersFrames(pc)
        f, _ := frames.Next()
        if f.File != "" {
                t.Fatalf("%v:%v: %v", f.File, f.Line, err)
        } else {
                // f is a zero frame; something went wrong above.
                // Don't annotate with file/line info.
                t.Fatal(err)
        }
}
dsnet commented 2 years ago

I've been playing around with https://pkg.go.dev/github.com/dsnet/golib/try in quick throwaway programs and I actually like how it improves my development velocity.

Since a panic in a Test function causes the test to fail anyways, we could just use external logic like my try package apart form the quicktest package.

dsnet commented 2 years ago

Given the existence of https://pkg.go.dev/github.com/dsnet/try, which works decently alongside quicktest, I think we can put this proposal on hold.

The only way that I see quicktest can have better ergonomics is if we can have generic methods (https://github.com/golang/go/issues/49085). Then we can have:

func (c *C) E(error)
func (c *C) E1[A any](A, error) A
func (c *C) E2[A, B any](A, B, error) (A, B)
func (c *C) E3[A, B, C any](A, B, C, error) (A, B, C)
func (c *C) E4[A, B, C, D any](A, B, C, D, error) (A, B, C, D)

Then usage would be really nice as:

func Test(t *testing.T) {
    c := qt.New(t)
    b := c.E1(os.ReadFile(...))
    ...
}

We wouldn't even need a defer since the E methods can just call t.Fatal internally.

Thus, we can resurrect this proposal if/when generic methods become available.