onsi / gomega

Ginkgo's Preferred Matcher Library
http://onsi.github.io/gomega/
MIT License
2.13k stars 282 forks source link

Storing the result of a Receive(filter)...? #749

Closed thediveo closed 2 months ago

thediveo commented 3 months ago

Time after time I end up with doing an Eventually(ch).Should(Receive(All(...))) where I'm waiting for the "golden" channel receive, ignoring the noise on this channel. Kinda like in real life :grin:

When I receive a matching element from the channel, I actually want to carry out next additional assertions that are often awkward or right impossible to put into the matching expression of the Receive(...). For this, I would to stash/store away the "golden" match.

Looking at Receive(...) it doesn't look like a good idea to overload it with an additional storage function. What do you think about a (pseudo) matcher that stores away its actual value upon its passed matcher matching? Such as

StoreMatch(receiver any, matcher types.GomegaMatcher)

receiver would need to be a non-nil pointer to something to which the actual value could be assigned upon the matcher returning true. A usage example:

var ev Event
Eventually(evch).Should(Receive(StoreMatch(&ev, All(
  HaveField("foo", BeTrue),
  HaveField("bar", ContainsSubstring("barrrz")),
))))
Expect(ev.Sprockets).NotTo(Panic())

Do you think that this would make sense to be put into the Gomega core? If so, what about naming, etc?

onsi commented 3 months ago

hey sorry for the delay - i've been out of pocket for the last couple of weeks.

this is a cool idea. it would be reusable in other collection matchers too, right? e.g.

var widget Sprocket

Expect(sprockets).To(ContainElement(StoreMatch(&widget, HaveField("id", "bob")))

It's elegant. I like it. I think it belongs in Gomega core and will save us all a lot of lines of typing and casting.

What to name it, though? StoreMatch definitely works. MatchAndStore or MatchAndAssign reads a bit better but both are overly long.

thediveo commented 3 months ago

no worries about delays; I'm just firing this and then it can take its time as needed. I want to discuss this as needed first before I drop some code.

it would be reusable in other collection matchers too, right?

I'm hesitating to say "yes" here. In particular, for the case of ContainElement we have the dedicated Ω(ACTUAL).Should(ContainElement(ELEMENT, <POINTER>)) now for some time: this handles multiple matches.

At this time, I "balk" at making StoreMatch stateful w.r.t. its <POINTER> argument, thus I'm not allowing it to aggregate matches: the rationale being that this would need an explicit contract with the "outer" matcher to invoke StoreMatch on all elements, even after the first match. We have this contract built into ContainElement itself, but not with other matchers.

This leads me to the insight that I need to clearly document what the contract is, or rather isn't: it's the ACTUAL' passed to the matcher, and thus not necessarily the ACTUAL passed to the assertion Ω(ACTUAL). It's kind of a double-edged sword, or in more modern terms, running with scissors while smoking and drinking.

onsi commented 3 months ago

Ah I see. And yes I had completely forgotten about ContainElement(ELEMENT, <POINTER>) lol.

I think if StoreMatch is only intended to be used with Receive then it’s potentially confusing to make it a separate building block. We could be explicit and have ReceiveAndStore or something, but Receive already does this if you call Receive(&foo). The issue, though is that the current implementation of receive doesn’t allow you to combine the storing feature with the matching feature. This seems like an oversight. So I suppose I’m talking myself into Receive(<matcher>, <pointer>) might be the right approach after all? (I know you had originally discounted the idea - can you say more about why?)

thediveo commented 3 months ago

So I suppose I’m talking myself into Receive(<matcher>, <pointer>) might be the right approach after all? (I know you had originally discounted the idea - can you say more about why?)

I was originally less thrilled to change Receive, but that might have been too closed-minded, so let's see how that would work:

  1. Receive()
  2. Receive(POINTER)
  3. Receive(MATCHER)
  4. 🆕 Receive(POINTER, MATCHER) ... I would reckon that the POINTER always goes before the MATCHER in order to avoid have to hunt the code for received matching value storage in case of more complex composite matcher.

Well, doesn't look bad now; if that's okay I'll work on a PR.

I notice that we need to update the documentation site, it mentions case 2, but no pseudo syntax here to quickly grasp this second meaning. And with the new combined feature, we'll need an overview of syntax variants in this part of the docs anyway. Not a big deal.

onsi commented 3 months ago

this looks good to me :)