onsi / gomega

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

RFC: new `BeInMapKeys` matcher #589

Closed thediveo closed 1 year ago

thediveo commented 1 year ago

Hopefully, a BeInMapKeys might be helpful to fellow test spec writers and not only to me? BeInMapKeys checks actual to be one of the keys of the map specified to the matcher. I can supply a PR if there is interest.

Regarding the implementation of a BeInMapKeys matcher I would like to get feedback on how to check the actual, as I see two different approaches and am unsure as to which one would be the better one:

  1. comparing actual against the list of keys using the Equal() matcher;
  2. using reflection, looking up the actual key, so without using Equal() at all. This requires actual to be of the exact key type, if I'm not mistaken.

What would be the preferred method: 1. or 2.?

onsi commented 1 year ago

I think 1 would be in keeping with how similar matchers work. But I'm wondering about the use case for BeInMapKeys given we have HaveKey - or perhaps I'm missing something. Is:

Expect(key).To(BeInMapKeys(m))

intended to behave differently from

Expect(m).To(HaveKey(key))

thediveo commented 1 year ago

BeInMapKeys becomes only then useful when, for instance, you cannot rewrite the "right" side of assertions as you correctly pointed to, such as with ContainElement(..., &findings). For instance, I have several unit tests that create a bunch of test containers with varying properties as a canary workload ... and then I want to do my assertions only on these canary containers, ignoring any further workload that might be present on the system.

var names = map[string]struct {
    projectname string
}{
    "edgy_emil":              {projectname: "foobar_project"},
    "furious_freddy":         {projectname: "foobar_project"},
}

var canaries []*Container
Expect(workload).To(ContainElement(HaveField("Name", BeInMapKeys(names)), &canaries))
Expect(canaries).To(HaveLen(len(names)))

et voilà, this asserts that all canaries are (still) present in the container workload and that we get a nicely filtered list of these canaries, filtering out the other workload "noise".

onsi commented 1 year ago

ah I see. brilliant.

I also came a cross a use case for &findings while doing some Ginkgo stuff and absolutely marveled at how many lines of boilerplate plumbing I was not having to write :)

thediveo commented 1 year ago

You're lucky ... because I wrote lots of this range-test-and-append boilerplate until I finally saw that the dual-use of ContainElement might a good investment.