Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: new BeInMapKeys matcher #589

Closed
thediveo opened this issue Sep 19, 2022 · 4 comments
Closed

RFC: new BeInMapKeys matcher #589

thediveo opened this issue Sep 19, 2022 · 4 comments

Comments

@thediveo
Copy link
Collaborator

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
Copy link
Owner

onsi commented Sep 19, 2022

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
Copy link
Collaborator Author

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
Copy link
Owner

onsi commented Sep 19, 2022

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
Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants