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

Adding pointer support for HaveField #494

Closed
JoelSpeed opened this issue Jan 27, 2022 · 5 comments · Fixed by #495
Closed

Adding pointer support for HaveField #494

JoelSpeed opened this issue Jan 27, 2022 · 5 comments · Fixed by #495

Comments

@JoelSpeed
Copy link

I don't know if this was previously considered and vetoed for some reason, but I think it would be useful if HaveField would support pointers to structs and pointer substructs.

I'm happy to put in some time to look into how we can modify the implementation and tests to add this support, just wanted to check if there was any reason not to do so before I propose it.

I'm thinking it would be useful if it could handle, for example:

type MyObject struct {
  InnerObject *MyInnerObject
}

type MyInnerObject struct {
  InnerField string
}

myObj := &MyObject{
  InnerObject: &MyInnerObject{
    InnerField: "FooBar",
  },
}
Expect(myObj).To(HaveField("InnerObject.InnerField", "FooBar"))

But today this won't work as far as I can tell (we check at the beginning if the object is a struct).

If you need this in the meantime, this is what I've used in the past which has a similar usage but works with pointers:

func HaveField(field string, matcher gtypes.GomegaMatcher) gtypes.GomegaMatcher {
	// Addressing Field by <struct>.<field> can be recursed
	fields := strings.SplitN(field, ".", 2)
	if len(fields) == 2 {
		matcher = HaveField(fields[1], matcher)
	}

	return gomega.WithTransform(func(obj interface{}) interface{} {
		r := reflect.ValueOf(obj)
		f := reflect.Indirect(r).FieldByName(fields[0])
		if !f.IsValid() {
			panic(fmt.Sprintf("Object '%s' does not have a field '%s'", reflect.TypeOf(obj), fields[0]))
		}
		return f.Interface()
	}, matcher)
}
@schrej
Copy link
Contributor

schrej commented Jan 27, 2022

One option would be to do it explicitly by using * to indicate that a pointer should get dereferenced.

@blgm
Copy link
Collaborator

blgm commented Jan 27, 2022

Hi @JoelSpeed @schrej, what about the PointTo() matcher, for example:

Expect(foo).To(HaveField("bar", PointTo(baz)))

This was inspired by this section of the docs, and I haven't tested it in this particular case:
https://onsi.github.io/gomega/#putting-it-all-together-testing-complex-structures

@schrej
Copy link
Contributor

schrej commented Jan 27, 2022

Oh nice, I didn't know about that one, and also not about gstruct. Looks very useful for some cases.

I'd still like to have pointer support on HaveField. The entire matcher seems like a shortcut to reach deeply nested fields.
Writing something like

HaveField("Foo.Bar.*Something.Deep")

would be way more convenient compared to

HaveField("Foo.Bar", PointTo(HaveField("Something.Deep", ...)))

@onsi
Copy link
Owner

onsi commented Jan 27, 2022

I don't know if this was previously considered and vetoed for some reason

nope it's just an oversight. @blgm 's PointTo suggestion is a good workaround for now but I agree that HaveField should just know how to traverse pointers. I, personally, don't see a huge benefit to having to be explicit about the pointer deference after the delimiter (e.g. you don't need to do it in Go - you just Foo.Bar.Something.Deep and It Just Works™.

Will take a look at the PR @schrej submitted

@onsi onsi closed this as completed in #495 Jan 27, 2022
@onsi
Copy link
Owner

onsi commented Jan 27, 2022

Done, thanks all. I've pulled in @schrej 's PR and published Gomega 1.18.1

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

Successfully merging a pull request may close this issue.

4 participants