Implement Matchers for "loose" variadic argument matching: All, Len, Contains#189
Implement Matchers for "loose" variadic argument matching: All, Len, Contains#189jinnovation wants to merge 1 commit intogolang:masterfrom
Conversation
poy
left a comment
There was a problem hiding this comment.
Thanks for doing this and sorry for the horrible delay! I would love to get this in if we could get these changes in.
| } | ||
|
|
||
| // Len returns a Matcher that matches on length. The returned Matcher returns | ||
| // false if its input is not a slice. |
There was a problem hiding this comment.
Could we also add support for chan and map?
|
|
||
| func (m lenMatcher) Matches(x interface{}) bool { | ||
| return reflect.TypeOf(x).Kind() == reflect.Slice && | ||
| m.n == reflect.ValueOf(x).Cap() |
There was a problem hiding this comment.
Should we be using .Len() instead of .Cap()?
| return assignableToTypeOfMatcher{reflect.TypeOf(x)} | ||
| } | ||
|
|
||
| type compositeMatcher struct { |
There was a problem hiding this comment.
nit: Can we rename this to allMatcher to better match the existing convention?
| return containsElementsMatcher{vs} | ||
| } | ||
|
|
||
| type containsElementsMatcher struct { |
There was a problem hiding this comment.
nit: Can we rename this to containsMatcher to match existing convention?
| Elements: | ||
| for i := 0; i < len(m.Elements) && eq; i++ { | ||
| e := m.Elements[i] | ||
| for j := 0; j < xv.Cap(); j++ { |
There was a problem hiding this comment.
Should this be using Len() instead of Cap()?
| xv := reflect.ValueOf(x) | ||
|
|
||
| eq := true | ||
| Elements: |
There was a problem hiding this comment.
nit: for readability, WDYTA putting the inner loop in a helper function and removing the label?
| continue Elements | ||
| } | ||
| } | ||
| eq = false |
There was a problem hiding this comment.
nit: for readability, return false here and remove the condition above?
|
@jinnovation are you still interested in this PR? |
|
Awesome, thanks for your PR! |
|
@jinnovation Still interested? |
|
@codyoss: Apologies for leading your team on. Despite expectations, I unfortunately haven't had a chance to follow up on this PR. Feel free to close this out; I will re-open if I come around to it. |
|
I might pick this up in that case as I think these features would be nice to have. |
|
I am going to close this for now. But I will probably open up individual PRs for these style of matchers in the future. Thank you for your work on this though, we will try to get a similar feature set in! |
This matcher is to be used to combine multiple matches into one statement. Implmentation inspired by #189.
This matcher will check the length with any type that allows this behavior via reflection. Implementation inspired by #189.
This PR provides a middle-ground approach to recording variadic argument conditions, between simply using
gomock.Any()(accepting any list of argument values) or matching against exact argument lists. To that effect, we provide two Matchers to allow for "looser" matching of variadic parameters, including:Contains, which allows you to associate expected mock behavior with variadic argument lists simply containing one or more values; andLen, which likewise allows you to associate with lists of certain length.This PR also provides an
Allcompound Matcher, which, for example, can be used—in conjunction with bothContainsandLen—to match against unordered values of a variadic argument slice.