-
Notifications
You must be signed in to change notification settings - Fork 609
return value depend on input value #34
Comments
Have you tried making that anonymous function have a return value, instead of using .Return? |
yes, it doesn't work. you can see here why |
Ah, right. Maybe you just want a hand-written fake instead? You're stretching the bounds of what gomock is really intended for. |
I can write my own interface implementation, but then II need to add empty implementation to all the method in the interface, witch it can be a lt of work, also I loss the call count and etc of the controller. |
What exactly is the intended use case of gomock? Seems like it is more than reasonable to pick up the return value of |
+1 on this. One other option would be to allow Return() to take a function, and if the function has the same parameters as the called function, the passed functor is called to generate the return value with the passed in parameters. This is basically what Invoke() does in C++ gmock. I'm new to Go and it would take me a bit to figure out how to use reflect to do this, but I think this is a major missing feature of the library. |
This package already supports what's being asked for here. Use ctrl := gomock.NewController()
v := NewFoo(ctrl)
quux := v.EXPECT().Quux(gomock.Any())
quux.Do(func(id int){
if id > 0 {
quux.Return(nil)
} else {
quux.Return(errors.New("id was <= 0"))
}
}) And, of course, I would not want the return value of the action to used as the return value of the call. |
Why not? I am very confused by this line of reasoning. |
Because the actions may have nothing to do with return values. For instance, the mock may have return values registered already, and the action registered with ctrl := gomock.NewController()
v := NewFoo(ctrl)
quux := v.EXPECT().Quux(gomock.Any())
quux.Do(func(i int){
if i > 0 {
t.Logf("unexpected non-zero value (%d)", i)
}
})
call.Return(nil) The action here gives useful information for debugging a test, and I wouldn't want its return value to be used by the mock. |
@bhcleek How is that better than just returning |
The current implementation gives us more flexibility without sacrificing the ability of the action to specify what the return values of the call should be (it just needs to call There are certainly cases where it would make sense to use the return values of the action as the return values of the call, but why force that restriction on all use cases? |
@bhcleek I am confused about how having functions work as expected in the rest of language is all of a suddenly a restriction. This subtlety not only affects usability but also adoption. I just don't get why one needs to learn "this one trick" above to get anything useful done. What does that achieve? Do you have examples of where this can do things that a regular function cannot? |
There cannot be any examples of where this can do things that a regular function cannot, because the action is a regular function. The advantage of the current implementation is that it allows the action to decorate the |
is this package support conditional return? if yes, can anyone write an example? |
@alioygur it does. see #34 (comment) |
I tried a variation of the solution above, and I got the zero-value returned. When I added .AnyTimes() and called it more than once, the first call set the return value of the second call, and so on. So the method above is apparently not valid in the version of gomock available to go get. If the above should work, is this a bug? My test: func TestTryIt(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
a := mock.NewMockAdder(ctrl)
c := a.EXPECT().Add(gomock.Any()).AnyTimes()
c.Do(func(v int) {
c.Return(v + 1)
})
tryIt(a)
} In main: type Adder interface {
Add(int) int
}
func tryIt(a Adder) {
fmt.Println(a.Add(11), a.Add(10), a.Add(9)
} My output:
|
My earlier assertion was mistaken. I thought I had seen it work once, but it doesn't work now; I'm not sure if something changed or if I misremembered. There are two options that do work, though. The first is that The second thing that works is to setup the calls that you expect instead of trying to mutate what will be returned on the next invocation. Note that this is completely valid:
|
@bhcleek But both of the options that you mentioned doesn't satisfy the requirement of returning values based on the input. May I know what the problem you are seeing with mutating values within |
I think i like the other idea proposed in PR, let Do be what it does today and expose DoAndReturn() that returns values based on mutating input. Minor issue which can be addressed is calling Return inside Do() has data race issue that it releases lock() an calls Return(), which is addressable. Now if some one can merge this pull request: we can make some forward progress of where we want to get to. #115 |
I'm a relatively new user who was also missing dynamic return values. In my experience with other frameworks, it's a great way to deal with more complex/hybrid objects that can't be handled with "pure" mocking. Further, sometimes specifying more exact return values overly constrains the way a function can run and makes tests brittle. Two thoughts:
If others agree with this sentiment, I am happy to put this in a PR. |
@josh-tepper I agree with all that you've said here, but I'm not a maintainer and have no more sway than any other user. Hopefully, though, the maintainers can weigh in with their opinions soon. |
Who would be the appropriate maintainer to @ mention on this? |
I am the project maintainer |
Nice to meet you Hesky! I actually think that we might have met when I was at Google NY. In any case, see above. There's been a discussion about ways to provide a delegate function to compute return values -- a feature that a lot of other frameworks provide. Let me know your thoughts. Was thinking of contributing a PR along the lines of my suggestions in #34 (comment) |
Hi Josh! Take a look at #126. This is my take on how to add dynamic return values (I'd love to just change Do but that will surely break someone). It doesn't have the extra checks that you mentioned. I wasn't sure how necessary they are. In my version actions just override each other. For example in This makes sense to me but let me know what you think. |
Sure thing -- I have some commitments and won't be able to take a look until the weekend. Will let you know what I think. |
See my comments on #126. This implementation is IMO better than my suggestion |
Is there way do return value depend on the input?
I want to do something like the following pseudo-code
var response string mock.EXPECT().SomeMethod(gomock.Any()).AnyTimes().Do(func(input string) { response = input + input }).Return(response)
I got deep into the code, and it look like it unsupported, I there other way to do it?
The text was updated successfully, but these errors were encountered: