RFC: Generics, functions with multiple return values, and Expect(err).NotTo(HaveOccured())

[thediveo]

I've always wondered when writing test specs if there might be a (semi) elegant way to deal with functions returning multiple values and asserting there wasn't any error in a compact way. Now, this is not throwing "the dead cat" of Go error handling proposals "onto the table".

foo, err := Bar(42)
Expect(err).NotTo(HaveOccured()) // boring boilerplate
// here come the real assertions

Especially when there are several sequential steps (calls) the error handling really becomes (IMHO) distracting.

While there is unfortunatly (yet?) no way in Go Generics to deal with a variable number of return values, there's a known workaround for the most common cases for two, three, etc return values. For instance in case of functions returning one value plus an error:

func Successfully[R any](retval R, err error) R {
    GinkgoHelper()  // hehe
    Expect(err).NotTo(HaveOccured())
    return retval
}

The spec now compacts into:

foo := Successfully(Bar(42)) // look mum, no err
// here come the real assertions

There would probably be variants for Successfully2 and probably 3 return values.

Due to the current limitations of how Go plumbs multiple return values into function calls, I don't see a way to pass a message, except by providing an error message type that implements receivers named Successfully, etc.

Feedback highly appreciated, more so on the (hah!) generic question on introducing generics into the gomega godebase?

[onsi]

I've had a few discussions here and there about what generics + Gomega might look like. On the whole it hasn't looked like there are obvious places where the current generics implementation could add much value (without severely hampering flexibility)...

...but I sure do think you're on to something here. This usecase makes a ton of sense and would delete a lot of boilerplate out there. I'm a fan, and I don't think the issue around passing a message is a dealbreaker (the stack trace should be helpful enough to most users).

Gomega formally only supports the current and previous release of Go, which means generics are a fine thing to make a hard requirement. I'd be happy to pull this in if you want to start noodling on a PR.

[blgm]

For the case:

foo, err := Bar(42)
Expect(err).NotTo(HaveOccured()) // boring boilerplate
// here come the real assertions

Gomega will allow you do to this:

Expect(Bar(42)).To(Equal("the answer to life the universe and everything"))

It will check that there was no error, and also check that the value matches.

But this feature is a bit magical, hard to reason about, and maybe not that widely known. It also doesn't work for three or more values:

foo, baz, err := Bar(42)
Expect(err).NotTo(HaveOccured()))
Expect(foo).To(Equal("the answer to life the universe and everything"))
Expect(baz).To(Equal("something else"))

What I'd really like to do is:

Expect(Bar(42)).To(Equal(
  "the answer to life the universe and everything",
  "something else",
  nil,
))

[thediveo]

@blgm my RFC here is tackling a different use case, that I admittedly not clearly delineated: you are correct for the common simple cases, where you have a clear expectation of the value. Often time, you might not, but instead the foo return value might be something that you don't really know much about, but instead of feeding it next into something else.

commit := Successful(worktree.Commit("check-in", commitopts))
Expect(repo.CreateTag("v1.2.3", commit, nil)).Error().NotTo(HaveOccured())

Here, there are no specific expectations about the returned commit, as long as worktree.Commit didn't fail.

[blgm]

@thediveo, I've definitely used the pattern of defining a generic must() function in tests, and reflected that I should not need to keep writing the same function:

func must[A any](a A, err error) A {
	Expect(err).WithOffset(1).NotTo(HaveOccurred())
	return a
}

Which is essentially the same as the Successful() function. I definitely have sympathy with the idea, but I like to try and think of problems with an idea as well, as it can result in better solutions. The things I'm thinking through at the moment are:

• I think I've used this pattern in implementation code as well as test code, so maybe it should be a Go language thing. Or a library outside of Gomega? Is there a reason to restrict this only to tests?
• Part of Go's philosophy is to not have too many helper functions. For example you often have to define your own filter and sort functions. Maybe it's ok to keep writing this function? The authors of Go really want to force you to think about errors, so maybe it goes contrary to Go's philosophy to give you a way to think about them less.
• It get's really ugly with more than two return values: Successful2() and Successful3() would work, but for some (perhaps irrational) reason, I really don't like them.
• What if Go 2.0 implemented a better way of doing this, such as try-catch? How easy would it be to remove this from Gomega in favour of a better solution?

I'm genuinely conflicted as to how to move forwards with this. My main fear being that once something has been added to Gomega, it's very hard to get rid of.

[onsi]

A couple of thoughts, perhaps to allay (corroborate?) some of these concerns from my point of view:

On restricting to test code: While it's true that this pattern doesn't need to belong in tests I do think it makes a fair bit of sense for Gomega to include helpers like this out of the box because they are expressing Gomega's opinion on how to consistently handle all errors: pass them through Expect(err).NotTo(HaveOccurred()). It's pretty cool to see you've also implemented a similar must() function and seems to corroborate the value of such a method.

On Go's philosophy: lots could be said here. On the one hand - I've never been too hung up on Ginkgo and Gomega abiding by Go's philosophies. On the other, and perhaps more compelling: that link to the try-catch conversation on the Go 2.0 thread shows just how poorly some of Go's philosophy has translated in practice. Developers routinely don't think about err to the point where the Go community is thinking about rethinking aspects of the philosophy. The strength of helpers like Successfully() is that they allow you to correctly and seamlessly handle errors everywhere. The alternative one-liner that cleans up all the boiler plate is:

foo, _ := Bar(42)
// do stuff with foo

Successfully, like the support for Expect(Bar(42)).To(Equal("forty-two")), means you really have no excuse to handle the error.

On the ugliness of Successfully2/3/etc. yeah agreed :( - I poked around at it a bit last night and also couldn't find a way to do it with generics.

On Go 2.0: I suspect that the advent of Go 2.0 will be an opportunity for a rethink for much of Ginkgo/Gomega. It's all murky to me at this point, but I could imagine it necessitating a Gomega 2.0 (and possibly, , a Ginkgo 3.0). So I wouldn't be too hung up on adding things to Gomega today... the great upheaval that is Go 2.0 will likely have a substantial blast radius.

[blgm]

Perhaps the thing to do would be to start by defining Successfully() and delaying writing Successfully2() until later? The documentation could tell you how to do it yourself, so users might write their own SuccessfullyCoordinates() with names that better match their code. Or there might be a consensus that Successfully2() is the way to go.

[onsi]

Yet another time where I find myself wishing functions could have properties. Wouldn't it be so much (relatively) cleaner if we could write:

foo := Successfully(Bar(42))
fooo := Successfully.Two(Barr(42))
foooo := Successfully.Three(Barrr(42))

Sadly that's not a thing and I imagine the Successfully.One() usecase would dominate so we'd really rather just have Successfully()

@thediveo - any thoughts on where all this lands for you?

[thediveo]

foo := IfNot.Successfully(Bar(42)).Report("oh, dang!")

I envision that we keep this helper in a separate, optional, experimental "success" package that can be dot-imported only when needed, so not part of the baseline standard DSL parts import.

[onsi]

😂 lol

sounds good :)

[thediveo]

Over on the Go issue tracker our beloved topic of better error handling washed another nugget to our shores: a module called "must" that warns its users that it should be used with high caution and for most benefit in unit tests and (that's plain "testing") and code where writing speed is of much more importance than extensive error handling. The latter I often jokingly refer to as "write-once code".

Alas, basically same multi-return value handling using Generics, with the well-known limitations. Its design appears to me to be similar to Ginkgo's and Gomega's in throwing specific panics and then catching them elsewhere and understanding what they mean.

[blgm]

@onsi, in response to this comment, it is possible for functions to have properties in Go as long as they are named types, and it's done in the Go standard library. I'm not saying that this approach should be used, but it's interesting to know it can be done.

[thediveo]

I think there's a caveat at least regarding onsi's example: Successful needs to be a variable and thus means it looses its variablity.