gomega: RFC on error assertions for functions with multi-return values and trailing error value

This is a request for comment on extending Gomega to more smoothly handle (phrase) error assertions for functions which return multiple values, including a trailing error. While working on my Gomega 3rd party extension “errxpect” (https://github.com/TheDiveO/errxpect) I noticed some things and I would like to get feedback on ways to do it better or if gomega could be opened to 3rd party implementors of gomega.Assertion.

So, instead of:

    _, _, err := foo(42)
    Expect(err).To(MatchError("DOH!"))

simply spell it out as:

    Errxpect(foo(42)).To(MatchError("DOH!"))

Things I noticed and had to work around:

Gomega’s internal implementation of gomega.Assertion basically does two separate things of relevance to our discussion here:
- it implements the important logic for calling a fail handler and generating useful stack context information – which is something that any 3rd party implementation of the gomega.Assertion probably would like to tap into (“inherit”).
- specific multi-return value checking in form of a single actual parameter, and a extra… parameters. Please note that this check conflicts with my specific usecase of having a non-zero error, or a zero error with potentially non-zero other return values.
Gomega’s ExpectWithOffset pattern cannot be applied to multi-return value functions. This actually can be easily remedied with a generic WithOffset(o int) function added to the gomega.Assertion interface. In my errexpect module I return a more specially typed assertion object, which then offers WithOffset(...), so I don’t need to touch Gomega’s Assertion interface.

My questions for feedback:

At the moment I have to get Gomega’s assertion functionality for triggering fail handlers and producing stack context information by running a transient, separate assertion when checking the multiple return values for calling the user’s matcher(s). Could you imagine opening up the functionality for triggering fail handlers and stack context traces to 3rd party implementers of Assertion?
With me coming up with Errxpect I’m throwing stones in my glasshouse … but since ExpectWithOffset(...) slightly stutters and with Option receivers being all the rage, could you imagine a generic Assertion.WithOffset(o), so Gomega’s assertions/expectations can also be spelled out as Expect(...).WithOffset(1).To(...)?

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 1
Comments: 23 (10 by maintainers)

Most upvoted comments

❤️ thanks for the kind shout out!

Once you get the WithTransform PR in I’ll a cut a release.

Unrelated BTW I just pushed a new HaveField() matcher that I think will make it a bit easier to write custom matchers. It’ll be in the next release too!

onsi on Nov 5, 2021

We can provide ItAllEndsInTears() for that.

thediveo on Nov 3, 2021

hey - I just went back and reread the original issue up top and am realizing that I’ve misunderstood what you were proposing all along! I must have been reading it quickly or jumped through a bunch of unspoken assumptions on my part, but I ended up thinking the use case is “making arbitrary assertions against multiple return values within a single Expect call”.

But now I see that the use case is “there are times when I want to assert that an error has occurred (or matches a specific error) and that all the other return values are nil/zero-valued” - which… makes more sense.

Sorry for the confusion! I generally agree that v1, v2, v3, err := foo() is just clearer in the multi-return value use case.

Now that we’re on the same page (my bad!) I can see adding Expect().Error() as a simple mechanism to accomplish this and we wouldn’t need any additional chaining. The user is simply choosing between the default behavior “Assert on v1, ensure all others are niladic” and error-matching behavior “Assert on the last return value, ensure all others are niladic”. And we could just stop there and see what further patterns emerge.

Sorry again for misreading your original issue. It’s been quite the journey to get here but I think we’re on the same page now!

onsi on Nov 3, 2021

I’m imagining it looks like overhauling the implementation over at https://github.com/onsi/gomega/blob/master/internal/assertion/assertion.go to enable the chaining I outlined above. I can get to this eventually, so no worries - though i’m trying to focus my free coding time (which there isn’t too much of due to covid and in-person schooling being closed!) on Ginkgo 2.0 right now… but if you have time to give it a shot, I’d start there.

onsi on Sep 30, 2020

Hey @TheDiveO sorry for the delay.

I like pulling out .WithOffset and, more broadly, appreciate the problem you’re trying to solve around Gomega better handling multiple return values. I wonder if there’s a clean backwards compatible (i.e. can we avoid a major version bump) way to extend Expect to handle multiple return values more explicitly - and basically pull in what you’re accomplishing with Errxpect into the core.

Here’s a first sketch of what this might look like. We keep the default behavior but extend Assertion and AsyncAssertion to enable users to explicitly call out individual return values and then apply assertions. So maybe something like:

Expect(foo(42)).WithOffset(2).FirstValue().To(Equal(41))
                           .NthValue(1).To(Equal("42"))
                           .LastValue().To(MatchError("DOH!"))

I think that could be pulled off in a largely backward compatible way. Thoughts?

onsi on Sep 30, 2020