From d39fb62771bfc4896ca1e5f3591a61ee7ef04b80 Mon Sep 17 00:00:00 2001 From: thediveo Date: Sat, 20 Nov 2021 23:18:17 +0000 Subject: [PATCH 1/4] Add HaveValue matcher --- matchers/have_value.go | 71 +++++++++++++++++++++++++++++++++++++ matchers/have_value_test.go | 51 ++++++++++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 matchers/have_value.go create mode 100644 matchers/have_value_test.go diff --git a/matchers/have_value.go b/matchers/have_value.go new file mode 100644 index 000000000..257b51fad --- /dev/null +++ b/matchers/have_value.go @@ -0,0 +1,71 @@ +package matchers + +import ( + "reflect" + + "github.com/onsi/gomega/format" + "github.com/onsi/gomega/types" +) + +// HavePoint applies the given matcher to the resulting value after optionally +// resolving actual to the value it points to or its interface value, repeatedly +// as necessary. It fails if a pointer or interface is nil. In contrast to +// gstruct.PointTo, HaveValue does not expect actual to be a pointer but instead +// also accepts non-pointer and interface values. +// +// actual := 42 +// Expect(actual).To(HaveValue(42)) +// Expect(&actual).To(HaveValue(42)) +func HaveValue(matcher types.GomegaMatcher) types.GomegaMatcher { + return &HaveValueMatcher{ + Matcher: matcher, + } +} + +type HaveValueMatcher struct { + Matcher types.GomegaMatcher // given matcher to apply to resolved value. + failure string // failure message, if any. +} + +func (m *HaveValueMatcher) Match(actual interface{}) (bool, error) { + val := reflect.ValueOf(actual) + for allowedIndirs := 32; allowedIndirs > 0; allowedIndirs-- { + // return an error if value isn't valid. + if !val.IsValid() { + m.failure = format.Message( + actual, "not to be ") + return false, nil + } + switch val.Kind() { + case reflect.Ptr, reflect.Interface: + // resolve pointers and interfaces to their values, then rinse and + // repeat. + if val.IsNil() { + m.failure = format.Message( + actual, "not to be ") + return false, nil + } + val = val.Elem() + continue + default: + // forward the final value to the specified matcher. + elem := val.Interface() + match, err := m.Matcher.Match(elem) + if !match { + m.failure = m.Matcher.FailureMessage(elem) + } + return match, err + } + } + // too many indirections: extreme star gazing, indeed...? + m.failure = format.Message(actual, "indirecting too many times") + return false, nil +} + +func (m *HaveValueMatcher) FailureMessage(_ interface{}) (message string) { + return m.failure +} + +func (m *HaveValueMatcher) NegatedFailureMessage(actual interface{}) (message string) { + return m.Matcher.NegatedFailureMessage(actual) +} diff --git a/matchers/have_value_test.go b/matchers/have_value_test.go new file mode 100644 index 000000000..be2486660 --- /dev/null +++ b/matchers/have_value_test.go @@ -0,0 +1,51 @@ +package matchers_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/matchers" +) + +type I interface { + M() +} + +type S struct { + V int +} + +func (s S) M() {} + +var _ = Describe("HaveValue", func() { + + It("should fail when passed nil", func() { + var p *struct{} + m := HaveValue(BeNil()) + Expect(m.Match(p)).To(BeFalse()) + Expect(m.FailureMessage(p)).To(MatchRegexp("not to be $")) + }) + + It("should fail when passed nil indirectly", func() { + var p *struct{} + m := HaveValue(BeNil()) + Expect(m.Match(&p)).To(BeFalse()) + Expect(m.FailureMessage(&p)).To(MatchRegexp("not to be $")) + }) + + It("should unwrap the value pointed to", func() { + i := 1 + Expect(&i).To(HaveValue(Equal(1))) + Expect(&i).NotTo(HaveValue(Equal(2))) + + p := &i + Expect(&p).To(HaveValue(Equal(1))) + Expect(&p).NotTo(HaveValue(Equal(2))) + }) + + It("should unwrap the value inside an interface", func() { + var i I = &S{V: 42} + Expect(i).To(HaveValue(Equal(S{V: 42}))) + Expect(i).NotTo(HaveValue(Equal(S{}))) + }) + +}) From 8814ef4aa7ab22c716869c05d8e151c724547e06 Mon Sep 17 00:00:00 2001 From: thediveo Date: Tue, 23 Nov 2021 21:02:05 +0000 Subject: [PATCH 2/4] improves HaveValue matcher documentation and tests --- matchers/have_value.go | 21 ++++++++++++++------- matchers/have_value_test.go | 13 ++++++++----- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/matchers/have_value.go b/matchers/have_value.go index 257b51fad..ba2149b73 100644 --- a/matchers/have_value.go +++ b/matchers/have_value.go @@ -7,11 +7,16 @@ import ( "github.com/onsi/gomega/types" ) -// HavePoint applies the given matcher to the resulting value after optionally -// resolving actual to the value it points to or its interface value, repeatedly -// as necessary. It fails if a pointer or interface is nil. In contrast to -// gstruct.PointTo, HaveValue does not expect actual to be a pointer but instead -// also accepts non-pointer and interface values. +// HaveValue applies the given matcher to the value of actual, optionally and +// repeatedly dereferencing pointers or taking the concrete value of interfaces. +// Thus, the matcher will always be applied to non-pointer and non-interface +// values only. HaveValue will fail with an error if a pointer or interface is +// nil. It will also fail for more than 31 pointer or interface dereferences to +// guard against mistakenly applying it to arbitrarily deep linked pointers. +// +// HaveValue differs from gstruct.PointTo in that it does not expect actual to +// be a pointer (as gstruct.PointTo does) but instead also accepts non-pointer +// and even interface values. // // actual := 42 // Expect(actual).To(HaveValue(42)) @@ -23,14 +28,16 @@ func HaveValue(matcher types.GomegaMatcher) types.GomegaMatcher { } type HaveValueMatcher struct { - Matcher types.GomegaMatcher // given matcher to apply to resolved value. + Matcher types.GomegaMatcher // the matcher to apply to the "resolved" actual value. failure string // failure message, if any. } func (m *HaveValueMatcher) Match(actual interface{}) (bool, error) { val := reflect.ValueOf(actual) for allowedIndirs := 32; allowedIndirs > 0; allowedIndirs-- { - // return an error if value isn't valid. + // return an error if value isn't valid. Please note that we cannot + // check for nil here, as we might not deal with a pointer or interface + // at this point. if !val.IsValid() { m.failure = format.Message( actual, "not to be ") diff --git a/matchers/have_value_test.go b/matchers/have_value_test.go index be2486660..34c55560f 100644 --- a/matchers/have_value_test.go +++ b/matchers/have_value_test.go @@ -32,17 +32,20 @@ var _ = Describe("HaveValue", func() { Expect(m.FailureMessage(&p)).To(MatchRegexp("not to be $")) }) - It("should unwrap the value pointed to", func() { + It("should unwrap the value pointed to, even repeatedly", func() { i := 1 Expect(&i).To(HaveValue(Equal(1))) Expect(&i).NotTo(HaveValue(Equal(2))) - p := &i - Expect(&p).To(HaveValue(Equal(1))) - Expect(&p).NotTo(HaveValue(Equal(2))) + pi := &i + Expect(pi).To(HaveValue(Equal(1))) + Expect(pi).NotTo(HaveValue(Equal(2))) + + Expect(&pi).To(HaveValue(Equal(1))) + Expect(&pi).NotTo(HaveValue(Equal(2))) }) - It("should unwrap the value inside an interface", func() { + It("should unwrap the value of an interface", func() { var i I = &S{V: 42} Expect(i).To(HaveValue(Equal(S{V: 42}))) Expect(i).NotTo(HaveValue(Equal(S{}))) From 74567df8fc7b539a874591348d2ec0a8acf83555 Mon Sep 17 00:00:00 2001 From: thediveo Date: Fri, 3 Dec 2021 21:04:30 +0000 Subject: [PATCH 3/4] use resolved value in failure msgs, make nils and too many indirections errors instead of failures --- matchers/have_value.go | 36 ++++++++++++++++-------------------- matchers/have_value_test.go | 22 ++++++++++++++++++---- 2 files changed, 34 insertions(+), 24 deletions(-) diff --git a/matchers/have_value.go b/matchers/have_value.go index ba2149b73..5a9730b76 100644 --- a/matchers/have_value.go +++ b/matchers/have_value.go @@ -1,12 +1,15 @@ package matchers import ( + "errors" "reflect" "github.com/onsi/gomega/format" "github.com/onsi/gomega/types" ) +const maxIndirections = 31 + // HaveValue applies the given matcher to the value of actual, optionally and // repeatedly dereferencing pointers or taking the concrete value of interfaces. // Thus, the matcher will always be applied to non-pointer and non-interface @@ -28,51 +31,44 @@ func HaveValue(matcher types.GomegaMatcher) types.GomegaMatcher { } type HaveValueMatcher struct { - Matcher types.GomegaMatcher // the matcher to apply to the "resolved" actual value. - failure string // failure message, if any. + Matcher types.GomegaMatcher // the matcher to apply to the "resolved" actual value. + resolvedActual interface{} // the ("resolved") value. } func (m *HaveValueMatcher) Match(actual interface{}) (bool, error) { val := reflect.ValueOf(actual) - for allowedIndirs := 32; allowedIndirs > 0; allowedIndirs-- { + for allowedIndirs := maxIndirections; allowedIndirs > 0; allowedIndirs-- { // return an error if value isn't valid. Please note that we cannot // check for nil here, as we might not deal with a pointer or interface // at this point. if !val.IsValid() { - m.failure = format.Message( - actual, "not to be ") - return false, nil + return false, errors.New(format.Message( + actual, "not to be ")) } switch val.Kind() { case reflect.Ptr, reflect.Interface: // resolve pointers and interfaces to their values, then rinse and // repeat. if val.IsNil() { - m.failure = format.Message( - actual, "not to be ") - return false, nil + return false, errors.New(format.Message( + actual, "not to be ")) } val = val.Elem() continue default: // forward the final value to the specified matcher. - elem := val.Interface() - match, err := m.Matcher.Match(elem) - if !match { - m.failure = m.Matcher.FailureMessage(elem) - } - return match, err + m.resolvedActual = val.Interface() + return m.Matcher.Match(m.resolvedActual) } } // too many indirections: extreme star gazing, indeed...? - m.failure = format.Message(actual, "indirecting too many times") - return false, nil + return false, errors.New(format.Message(actual, "too many indirections")) } func (m *HaveValueMatcher) FailureMessage(_ interface{}) (message string) { - return m.failure + return m.Matcher.NegatedFailureMessage(m.resolvedActual) } -func (m *HaveValueMatcher) NegatedFailureMessage(actual interface{}) (message string) { - return m.Matcher.NegatedFailureMessage(actual) +func (m *HaveValueMatcher) NegatedFailureMessage(_ interface{}) (message string) { + return m.Matcher.NegatedFailureMessage(m.resolvedActual) } diff --git a/matchers/have_value_test.go b/matchers/have_value_test.go index 34c55560f..e3fd7363e 100644 --- a/matchers/have_value_test.go +++ b/matchers/have_value_test.go @@ -1,6 +1,8 @@ package matchers_test import ( + "reflect" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/matchers" @@ -21,15 +23,13 @@ var _ = Describe("HaveValue", func() { It("should fail when passed nil", func() { var p *struct{} m := HaveValue(BeNil()) - Expect(m.Match(p)).To(BeFalse()) - Expect(m.FailureMessage(p)).To(MatchRegexp("not to be $")) + Expect(m.Match(p)).Error().To(MatchError(MatchRegexp("not to be $"))) }) It("should fail when passed nil indirectly", func() { var p *struct{} m := HaveValue(BeNil()) - Expect(m.Match(&p)).To(BeFalse()) - Expect(m.FailureMessage(&p)).To(MatchRegexp("not to be $")) + Expect(m.Match(&p)).Error().To(MatchError(MatchRegexp("not to be $"))) }) It("should unwrap the value pointed to, even repeatedly", func() { @@ -45,6 +45,20 @@ var _ = Describe("HaveValue", func() { Expect(&pi).NotTo(HaveValue(Equal(2))) }) + It("shouldn't endlessly star-gaze", func() { + dave := "It's full of stars!" + stargazer := reflect.ValueOf(dave) + for stars := 1; stars <= 31; stars++ { + p := reflect.New(stargazer.Type()) + p.Elem().Set(stargazer) + stargazer = p + } + m := HaveValue(Equal(dave)) + Expect(m.Match(stargazer.Interface())).Error().To( + MatchError(MatchRegexp(`too many indirections`))) + Expect(m.Match(stargazer.Elem().Interface())).To(BeTrue()) + }) + It("should unwrap the value of an interface", func() { var i I = &S{V: 42} Expect(i).To(HaveValue(Equal(S{V: 42}))) From 14d98b267b1a70985d93f00fe03105c5c3a3822a Mon Sep 17 00:00:00 2001 From: thediveo Date: Mon, 6 Dec 2021 20:08:58 +0000 Subject: [PATCH 4/4] fix inverted failure message; add corresponding tests --- matchers/have_value.go | 2 +- matchers/have_value_test.go | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/matchers/have_value.go b/matchers/have_value.go index 5a9730b76..58f4404db 100644 --- a/matchers/have_value.go +++ b/matchers/have_value.go @@ -66,7 +66,7 @@ func (m *HaveValueMatcher) Match(actual interface{}) (bool, error) { } func (m *HaveValueMatcher) FailureMessage(_ interface{}) (message string) { - return m.Matcher.NegatedFailureMessage(m.resolvedActual) + return m.Matcher.FailureMessage(m.resolvedActual) } func (m *HaveValueMatcher) NegatedFailureMessage(_ interface{}) (message string) { diff --git a/matchers/have_value_test.go b/matchers/have_value_test.go index e3fd7363e..32c8ade29 100644 --- a/matchers/have_value_test.go +++ b/matchers/have_value_test.go @@ -32,6 +32,13 @@ var _ = Describe("HaveValue", func() { Expect(m.Match(&p)).Error().To(MatchError(MatchRegexp("not to be $"))) }) + It("should use the matcher's failure message", func() { + m := HaveValue(Equal(42)) + Expect(m.Match(666)).To(BeFalse()) + Expect(m.FailureMessage(nil)).To(Equal("Expected\n : 666\nto equal\n : 42")) + Expect(m.NegatedFailureMessage(nil)).To(Equal("Expected\n : 666\nnot to equal\n : 42")) + }) + It("should unwrap the value pointed to, even repeatedly", func() { i := 1 Expect(&i).To(HaveValue(Equal(1)))