From 1a4e27f7122af97561fea383447fca270dcacc30 Mon Sep 17 00:00:00 2001 From: TheDiveO Date: Fri, 25 Mar 2022 15:28:10 +0100 Subject: [PATCH] Feature/525 return findings for ContainElement (#527) * allow ContainElement to optionally return matched elements * adds ContainElement documentation about findings pointer --- docs/index.md | 30 +++ matchers.go | 22 ++- matchers/contain_element_matcher.go | 120 +++++++++++- matchers/contain_element_matcher_test.go | 240 ++++++++++++++++++----- 4 files changed, 356 insertions(+), 56 deletions(-) diff --git a/docs/index.md b/docs/index.md index b72fa3751..61174afb3 100644 --- a/docs/index.md +++ b/docs/index.md @@ -886,6 +886,13 @@ succeeds if the capacity of `ACTUAL` is `INT`. `ACTUAL` must be of type `array`, Ω(ACTUAL).Should(ContainElement(ELEMENT)) ``` +or + +```go +Ω(ACTUAL).Should(ContainElement(ELEMENT, )) +``` + + succeeds if `ACTUAL` contains an element that equals `ELEMENT`. `ACTUAL` must be an `array`, `slice`, or `map` -- anything else is an error. For `map`s `ContainElement` searches through the map's values (not keys!). By default `ContainElement()` uses the `Equal()` matcher under the hood to assert equality between `ACTUAL`'s elements and `ELEMENT`. You can change this, however, by passing `ContainElement` a `GomegaMatcher`. For example, to check that a slice of strings has an element that matches a substring: @@ -894,6 +901,29 @@ By default `ContainElement()` uses the `Equal()` matcher under the hood to asser Ω([]string{"Foo", "FooBar"}).Should(ContainElement(ContainSubstring("Bar"))) ``` +In addition, there are occasions when you need to grab (all) matching contained elements, for instance, to make several assertions against the matching contained elements. To do this, you can ask the `ContainElement` matcher for the matching contained elements by passing it a pointer to a variable of the appropriate type. If multiple matching contained elements are expected, then a pointer to either a slice or a map should be passed (but not a pointer to an array), otherwise a pointer to a scalar (non-slice, non-map): + +```go +var findings []string +Ω([]string{"foo", "foobar", "bar"}).Should(ContainElement(ContainSubstring("foo"), &findings)) + +var finding string +Ω([]string{"foo", "foobar", "bar"}).Should(ContainElement("foobar", &finding)) +``` + +The `ContainElement` matcher will fail with a descriptive error message in case of multiple matches when the pointer references a scalar type. + +In case of maps, the matching contained elements will be returned with their keys in the map referenced by the pointer. + +```go +var findings map[int]string +Ω(map[int]string{ + 1: "bar", + 2: "foobar", + 3: "foo", +}).Should(ContainElement(ContainSubstring("foo"), &findings)) +``` + #### ContainElements(element ...interface{}) ```go diff --git a/matchers.go b/matchers.go index b09066899..b58dd67cb 100644 --- a/matchers.go +++ b/matchers.go @@ -256,16 +256,26 @@ func BeZero() types.GomegaMatcher { return &matchers.BeZeroMatcher{} } -//ContainElement succeeds if actual contains the passed in element. -//By default ContainElement() uses Equal() to perform the match, however a -//matcher can be passed in instead: +//ContainElement succeeds if actual contains the passed in element. By default +//ContainElement() uses Equal() to perform the match, however a matcher can be +//passed in instead: // Expect([]string{"Foo", "FooBar"}).Should(ContainElement(ContainSubstring("Bar"))) // -//Actual must be an array, slice or map. -//For maps, ContainElement searches through the map's values. -func ContainElement(element interface{}) types.GomegaMatcher { +//Actual must be an array, slice or map. For maps, ContainElement searches +//through the map's values. +// +//If you want to have a copy of the matching element(s) found you can pass a +//pointer to a variable of the appropriate type. If the variable isn't a slice +//or map, then exactly one match will be expected and returned. If the variable +//is a slice or map, then at least one match is expected and all matches will be +//stored in the variable. +// +// var findings []string +// Expect([]string{"Foo", "FooBar"}).Should(ContainElement(ContainSubString("Bar", &findings))) +func ContainElement(element interface{}, result ...interface{}) types.GomegaMatcher { return &matchers.ContainElementMatcher{ Element: element, + Result: result, } } diff --git a/matchers/contain_element_matcher.go b/matchers/contain_element_matcher.go index 8d6c44c7a..3d45c9ebc 100644 --- a/matchers/contain_element_matcher.go +++ b/matchers/contain_element_matcher.go @@ -3,6 +3,7 @@ package matchers import ( + "errors" "fmt" "reflect" @@ -11,6 +12,7 @@ import ( type ContainElementMatcher struct { Element interface{} + Result []interface{} } func (matcher *ContainElementMatcher) Match(actual interface{}) (success bool, err error) { @@ -18,6 +20,49 @@ func (matcher *ContainElementMatcher) Match(actual interface{}) (success bool, e return false, fmt.Errorf("ContainElement matcher expects an array/slice/map. Got:\n%s", format.Object(actual, 1)) } + var actualT reflect.Type + var result reflect.Value + switch l := len(matcher.Result); { + case l > 1: + return false, errors.New("ContainElement matcher expects at most a single optional pointer to store its findings at") + case l == 1: + if reflect.ValueOf(matcher.Result[0]).Kind() != reflect.Ptr { + return false, fmt.Errorf("ContainElement matcher expects a non-nil pointer to store its findings at. Got\n%s", + format.Object(matcher.Result[0], 1)) + } + actualT = reflect.TypeOf(actual) + resultReference := matcher.Result[0] + result = reflect.ValueOf(resultReference).Elem() // what ResultReference points to, to stash away our findings + switch result.Kind() { + case reflect.Array: + return false, fmt.Errorf("ContainElement cannot return findings. Need *%s, got *%s", + reflect.SliceOf(actualT.Elem()).String(), result.Type().String()) + case reflect.Slice: + if !isArrayOrSlice(actual) { + return false, fmt.Errorf("ContainElement cannot return findings. Need *%s, got *%s", + reflect.MapOf(actualT.Key(), actualT.Elem()).String(), result.Type().String()) + } + if !actualT.Elem().AssignableTo(result.Type().Elem()) { + return false, fmt.Errorf("ContainElement cannot return findings. Need *%s, got *%s", + actualT.String(), result.Type().String()) + } + case reflect.Map: + if !isMap(actual) { + return false, fmt.Errorf("ContainElement cannot return findings. Need *%s, got *%s", + actualT.String(), result.Type().String()) + } + if !actualT.AssignableTo(result.Type()) { + return false, fmt.Errorf("ContainElement cannot return findings. Need *%s, got *%s", + actualT.String(), result.Type().String()) + } + default: + if !actualT.Elem().AssignableTo(result.Type()) { + return false, fmt.Errorf("ContainElement cannot return findings. Need *%s, got *%s", + actualT.Elem().String(), result.Type().String()) + } + } + } + elemMatcher, elementIsMatcher := matcher.Element.(omegaMatcher) if !elementIsMatcher { elemMatcher = &EqualMatcher{Expected: matcher.Element} @@ -25,30 +70,99 @@ func (matcher *ContainElementMatcher) Match(actual interface{}) (success bool, e value := reflect.ValueOf(actual) var valueAt func(int) interface{} + + var getFindings func() reflect.Value + var foundAt func(int) + if isMap(actual) { keys := value.MapKeys() valueAt = func(i int) interface{} { return value.MapIndex(keys[i]).Interface() } + if result.Kind() != reflect.Invalid { + fm := reflect.MakeMap(actualT) + getFindings = func() reflect.Value { + return fm + } + foundAt = func(i int) { + fm.SetMapIndex(keys[i], value.MapIndex(keys[i])) + } + } } else { valueAt = func(i int) interface{} { return value.Index(i).Interface() } + if result.Kind() != reflect.Invalid { + var f reflect.Value + if result.Kind() == reflect.Slice { + f = reflect.MakeSlice(result.Type(), 0, 0) + } else { + f = reflect.MakeSlice(reflect.SliceOf(result.Type()), 0, 0) + } + getFindings = func() reflect.Value { + return f + } + foundAt = func(i int) { + f = reflect.Append(f, value.Index(i)) + } + } } var lastError error for i := 0; i < value.Len(); i++ { - success, err := elemMatcher.Match(valueAt(i)) + elem := valueAt(i) + success, err := elemMatcher.Match(elem) if err != nil { lastError = err continue } if success { - return true, nil + if result.Kind() == reflect.Invalid { + return true, nil + } + foundAt(i) } } - return false, lastError + // when the expectation isn't interested in the findings except for success + // or non-success, then we're done here and return the last matcher error + // seen, if any, as well as non-success. + if result.Kind() == reflect.Invalid { + return false, lastError + } + + // pick up any findings the test is interested in as it specified a non-nil + // result reference. However, the expection always is that there are at + // least one or multiple findings. So, if a result is expected, but we had + // no findings, then this is an error. + findings := getFindings() + if findings.Len() == 0 { + return false, lastError + } + + // there's just a single finding and the result is neither a slice nor a map + // (so it's a scalar): pick the one and only finding and return it in the + // place the reference points to. + if findings.Len() == 1 && !isArrayOrSlice(result.Interface()) && !isMap(result.Interface()) { + if isMap(actual) { + miter := findings.MapRange() + miter.Next() + result.Set(miter.Value()) + } else { + result.Set(findings.Index(0)) + } + return true, nil + } + + // at least one or even multiple findings and a the result references a + // slice or a map, so all we need to do is to store our findings where the + // reference points to. + if !findings.Type().AssignableTo(result.Type()) { + return false, fmt.Errorf("ContainElement cannot return multiple findings. Need *%s, got *%s", + findings.Type().String(), result.Type().String()) + } + result.Set(findings) + return true, nil } func (matcher *ContainElementMatcher) FailureMessage(actual interface{}) (message string) { diff --git a/matchers/contain_element_matcher_test.go b/matchers/contain_element_matcher_test.go index 448325ac4..700871953 100644 --- a/matchers/contain_element_matcher_test.go +++ b/matchers/contain_element_matcher_test.go @@ -7,70 +7,216 @@ import ( ) var _ = Describe("ContainElement", func() { - When("passed a supported type", func() { - Context("and expecting a non-matcher", func() { - It("should do the right thing", func() { - Expect([2]int{1, 2}).Should(ContainElement(2)) - Expect([2]int{1, 2}).ShouldNot(ContainElement(3)) - - Expect([]int{1, 2}).Should(ContainElement(2)) - Expect([]int{1, 2}).ShouldNot(ContainElement(3)) - - Expect(map[string]int{"foo": 1, "bar": 2}).Should(ContainElement(2)) - Expect(map[int]int{3: 1, 4: 2}).ShouldNot(ContainElement(3)) - - arr := make([]myCustomType, 2) - arr[0] = myCustomType{s: "foo", n: 3, f: 2.0, arr: []string{"a", "b"}} - arr[1] = myCustomType{s: "foo", n: 3, f: 2.0, arr: []string{"a", "c"}} - Expect(arr).Should(ContainElement(myCustomType{s: "foo", n: 3, f: 2.0, arr: []string{"a", "b"}})) - Expect(arr).ShouldNot(ContainElement(myCustomType{s: "foo", n: 3, f: 2.0, arr: []string{"b", "c"}})) + Describe("matching only", func() { + When("passed a supported type", func() { + Context("and expecting a non-matcher", func() { + It("should do the right thing", func() { + Expect([2]int{1, 2}).Should(ContainElement(2)) + Expect([2]int{1, 2}).ShouldNot(ContainElement(3)) + + Expect([]int{1, 2}).Should(ContainElement(2)) + Expect([]int{1, 2}).ShouldNot(ContainElement(3)) + + Expect(map[string]int{"foo": 1, "bar": 2}).Should(ContainElement(2)) + Expect(map[int]int{3: 1, 4: 2}).ShouldNot(ContainElement(3)) + + arr := make([]myCustomType, 2) + arr[0] = myCustomType{s: "foo", n: 3, f: 2.0, arr: []string{"a", "b"}} + arr[1] = myCustomType{s: "foo", n: 3, f: 2.0, arr: []string{"a", "c"}} + Expect(arr).Should(ContainElement(myCustomType{s: "foo", n: 3, f: 2.0, arr: []string{"a", "b"}})) + Expect(arr).ShouldNot(ContainElement(myCustomType{s: "foo", n: 3, f: 2.0, arr: []string{"b", "c"}})) + }) }) - }) - Context("and expecting a matcher", func() { - It("should pass each element through the matcher", func() { - Expect([]int{1, 2, 3}).Should(ContainElement(BeNumerically(">=", 3))) - Expect([]int{1, 2, 3}).ShouldNot(ContainElement(BeNumerically(">", 3))) - Expect(map[string]int{"foo": 1, "bar": 2}).Should(ContainElement(BeNumerically(">=", 2))) - Expect(map[string]int{"foo": 1, "bar": 2}).ShouldNot(ContainElement(BeNumerically(">", 2))) + Context("and expecting a matcher", func() { + It("should pass each element through the matcher", func() { + Expect([]int{1, 2, 3}).Should(ContainElement(BeNumerically(">=", 3))) + Expect([]int{1, 2, 3}).ShouldNot(ContainElement(BeNumerically(">", 3))) + Expect(map[string]int{"foo": 1, "bar": 2}).Should(ContainElement(BeNumerically(">=", 2))) + Expect(map[string]int{"foo": 1, "bar": 2}).ShouldNot(ContainElement(BeNumerically(">", 2))) + }) + + It("should power through even if the matcher ever fails", func() { + Expect([]interface{}{1, 2, "3", 4}).Should(ContainElement(BeNumerically(">=", 3))) + }) + + It("should fail if the matcher fails", func() { + actual := []interface{}{1, 2, "3", "4"} + success, err := (&ContainElementMatcher{Element: BeNumerically(">=", 3)}).Match(actual) + Expect(success).Should(BeFalse()) + Expect(err).Should(HaveOccurred()) + }) }) + }) + + When("passed a correctly typed nil", func() { + It("should operate succesfully on the passed in value", func() { + var nilSlice []int + Expect(nilSlice).ShouldNot(ContainElement(1)) - It("should power through even if the matcher ever fails", func() { - Expect([]interface{}{1, 2, "3", 4}).Should(ContainElement(BeNumerically(">=", 3))) + var nilMap map[int]string + Expect(nilMap).ShouldNot(ContainElement("foo")) }) + }) + + When("passed an unsupported type", func() { + It("should error", func() { + success, err := (&ContainElementMatcher{Element: 0}).Match(0) + Expect(success).Should(BeFalse()) + Expect(err).Should(HaveOccurred()) + + success, err = (&ContainElementMatcher{Element: 0}).Match("abc") + Expect(success).Should(BeFalse()) + Expect(err).Should(HaveOccurred()) - It("should fail if the matcher fails", func() { - actual := []interface{}{1, 2, "3", "4"} - success, err := (&ContainElementMatcher{Element: BeNumerically(">=", 3)}).Match(actual) + success, err = (&ContainElementMatcher{Element: 0}).Match(nil) Expect(success).Should(BeFalse()) Expect(err).Should(HaveOccurred()) }) }) }) - When("passed a correctly typed nil", func() { - It("should operate succesfully on the passed in value", func() { - var nilSlice []int - Expect(nilSlice).ShouldNot(ContainElement(1)) + Describe("returning findings", func() { + It("rejects a nil result reference", func() { + Expect(ContainElement("foo", nil).Match([]string{"foo"})).Error().To( + MatchError(MatchRegexp(`expects a non-nil pointer.+ Got\n +: nil`))) + }) + + Context("with match(es)", func() { + When("passed an assignable result reference", func() { + It("should assign a single finding to a scalar result reference", func() { + actual := []string{"bar", "foo"} + var stash string + Expect(actual).To(ContainElement("foo", &stash)) + Expect(stash).To(Equal("foo")) + + actualmap := map[int]string{ + 1: "bar", + 2: "foo", + } + Expect(actualmap).To(ContainElement("foo", &stash)) + Expect(stash).To(Equal("foo")) + }) + + It("should assign a single finding to a slice return reference", func() { + actual := []string{"bar", "foo", "baz"} + var stash []string + Expect(actual).To(ContainElement("foo", &stash)) + Expect(stash).To(HaveLen(1)) + Expect(stash).To(ContainElement("foo")) + }) - var nilMap map[int]string - Expect(nilMap).ShouldNot(ContainElement("foo")) + It("should assign multiple findings to a slice return reference", func() { + actual := []string{"bar", "foo", "bar", "foo"} + var stash []string + Expect(actual).To(ContainElement("foo", &stash)) + Expect(stash).To(HaveLen(2)) + Expect(stash).To(HaveEach("foo")) + }) + + It("should assign map findings to a map return reference", func() { + actual := map[string]string{ + "foo": "foo", + "bar": "bar", + "baz": "baz", + } + var stash map[string]string + Expect(actual).To(ContainElement(ContainSubstring("ba"), &stash)) + Expect(stash).To(HaveLen(2)) + Expect(stash).To(ConsistOf("bar", "baz")) + }) + }) + + When("passed a scalar return reference for multiple matches", func() { + It("should error", func() { + actual := []string{"foo", "foo"} + var stash string + Expect(ContainElement("foo", &stash).Match(actual)).Error().To( + MatchError(MatchRegexp(`cannot return multiple findings\. Need \*\[\]string, got \*string`))) + }) + }) + + When("passed an unassignable return reference for matches", func() { + It("should error for actual []T1, return reference T2", func() { + actual := []string{"bar", "foo"} + var stash int + Expect(ContainElement("foo", &stash).Match(actual)).Error().To(HaveOccurred()) + }) + It("should error for actual []T, return reference [...]T", func() { + actual := []string{"bar", "foo"} + var arrstash [2]string + Expect(ContainElement("foo", &arrstash).Match(actual)).Error().To(HaveOccurred()) + }) + It("should error for actual []interface{}, return reference T", func() { + actual := []interface{}{"foo", 42} + var stash int + Expect(ContainElement(Not(BeZero()), &stash).Match(actual)).Error().To( + MatchError(MatchRegexp(`cannot return findings\. Need \*interface.+, got \*int`))) + }) + It("should error for actual []interface{}, return reference []T", func() { + actual := []interface{}{"foo", 42} + var stash []string + Expect(ContainElement(Not(BeZero()), &stash).Match(actual)).Error().To( + MatchError(MatchRegexp(`cannot return findings\. Need \*\[\]interface.+, got \*\[\]string`))) + }) + It("should error for actual map[T]T, return reference map[T]interface{}", func() { + actual := map[string]string{ + "foo": "foo", + "bar": "bar", + "baz": "baz", + } + var stash map[string]interface{} + Expect(ContainElement(Not(BeZero()), &stash).Match(actual)).Error().To( + MatchError(MatchRegexp(`cannot return findings\. Need \*map\[string\]string, got \*map\[string\]interface`))) + }) + It("should error for actual map[T]T, return reference []T", func() { + actual := map[string]string{ + "foo": "foo", + "bar": "bar", + "baz": "baz", + } + var stash []string + Expect(ContainElement(Not(BeZero()), &stash).Match(actual)).Error().To( + MatchError(MatchRegexp(`cannot return findings\. Need \*map\[string\]string, got \*\[\]string`))) + }) + + It("should return a descriptive return reference-type error", func() { + actual := []string{"bar", "foo"} + var stash map[string]struct{} + Expect(ContainElement("foo", &stash).Match(actual)).Error().To( + MatchError(MatchRegexp(`cannot return findings\. Need \*\[\]string, got \*map`))) + }) + }) }) - }) - When("passed an unsupported type", func() { - It("should error", func() { - success, err := (&ContainElementMatcher{Element: 0}).Match(0) - Expect(success).Should(BeFalse()) - Expect(err).Should(HaveOccurred()) + Context("without any matches", func() { + When("the matcher did not error", func() { + It("should report non-match", func() { + actual := []string{"bar", "foo"} + var stash string + rem := ContainElement("baz", &stash) + m, err := rem.Match(actual) + Expect(m).To(BeFalse()) + Expect(err).NotTo(HaveOccurred()) + Expect(rem.FailureMessage(actual)).To(MatchRegexp(`Expected\n.+\nto contain element matching\n.+: baz`)) - success, err = (&ContainElementMatcher{Element: 0}).Match("abc") - Expect(success).Should(BeFalse()) - Expect(err).Should(HaveOccurred()) + var stashslice []string + rem = ContainElement("baz", &stashslice) + m, err = rem.Match(actual) + Expect(m).To(BeFalse()) + Expect(err).NotTo(HaveOccurred()) + Expect(rem.FailureMessage(actual)).To(MatchRegexp(`Expected\n.+\nto contain element matching\n.+: baz`)) + }) + }) - success, err = (&ContainElementMatcher{Element: 0}).Match(nil) - Expect(success).Should(BeFalse()) - Expect(err).Should(HaveOccurred()) + When("the matcher errors", func() { + It("should report last matcher error", func() { + actual := []interface{}{"bar", 42} + var stash []interface{} + Expect(ContainElement(HaveField("yeehaw", 42), &stash).Match(actual)).Error().To(MatchError(MatchRegexp(`HaveField encountered:\n.*: 42\nWhich is not a struct`))) + }) + }) }) }) + })