Support dynamic return values

[gburt]

Support dynamic return arguments in a threadsafe way. (Versus setting c.ReturnValues which is shared state/not threadsafe.)

Do this by modifying Run to accept three types of functions:

1: The status quo

Eg a func(Arguments) which behaves just like today; can be combined with a call to Return() to return fixed values

2: A function which matches the signature of your mocked function itself

And determines the return values dynamically. So like a "double" but with the benefit of this library's invocation count tracking and filtering to certain input arguments. Example:

mockedGreeter.On("HelloWorld", mock.Anything).Run(func(name string) string {
  return "Hello " + name
})

3: func(Arguments) Arguments which behaves like (2) except you need to do the typecasting yourself

Example:

mockedGreeter.On("HelloWorld", mock.Anything).Run(func(args mock.Arguments) mock.Arguments {
  return mock.Arguments([]any{"Hello " + args[0].(string)})
}}

If called with (2) or (3) then calling Return() as well is not allowed and will panic. If Run() is called with an arg that is not a function then we'll panic.

[mweibel]

exactly this I'd need! any chance this gets looked at by a maintainer?

[ninthclowd]

This seems like valuable functionality. Any chance it will be reviewed?

[dolmen]

@gburt I agree with @dlwyatt review.

[gburt]

@dolmen @dlwyatt thanks for the review, I incorporated your feedback

[dlwyatt]

lgtm!

[brackendawson]

I question whether using this is actually better than just putting the behaviour in a double which does not use mock.Mock?

type myMock struct {
    mock.Mock
}

func (m *myMock) Method(i int) error {
    return m.Called(i).Error(0)
}

func TestIfy(t *testing.T) {
   clientDouble := &myMock{}
   clientDouble.Test(t)
   clientDouble.On("Method").Run(func(args mock.Arguments) mock.Arguments{
       i := args.Int(0) // panics if myMock is wrong
       if i > 10 {
           return mock.Arguments{errors.New("too big")
       }
       return mock.Arguments{error(nil)}
   }
   
   c := &myClass{
       Client: clientDouble,
   }
   actual := c.Do()
   assert.Equal(t, "expectation", actual)
}

-- vs --

type myMock struct {}

func (m *myMock) Method(i int) error {
    if i > 10 {
        return errors.New("too big")
    }
    return nil
}

func TestIfy(t *testing.T) {
   c := &myClass{
       Client: &myMock{},
   }
   actual := c.Do()
   assert.Equal(t, "expectation", actual)
}

To me, Mock is only adding a layer of complexity. Can you show an example where this functionality is an improvement to just making your own double or mock?

[brackendawson]

This should use different method. Mock is already not trivial to understand, each method should strive to do one thing and do it well.

[dlwyatt]

IMO it's a useful feature to be able to supply your own implementation to a mocked method. https://github.com/vektra/mockery accomplishes this with generated wrappers around a testify.Mock, but wouldn't have needed a lot of the generated code if Mock had already supported this out of the box

[gburt]

I question whether using this is actually better than just putting the behaviour in a double which does not use mock.Mock?

Two reasons I think:

1. for the many folks who use mockery to generate mock structs, they don't want to monkeypatch mockery's output or have to re-generate it when they decide to customize what values a given mocked method returns
2. if you use a double then you're not leveraging testify/mock's ability to assert that calls were indeed made/not made..indeed you're bypassing it entirely

[gburt]

One thing that could be nice is if the function could have the same signature as the actual method it's mocking -- and then using reflect we adapt the input Arguments into a call to it and vice versa for what it returns. (And we could panic if the signature doesn't match).

OTOH that might break the API -- since some folks might indeed be wanting to assert that the method is returning some func? (Where it's incredibly unlikely that someone wants their mocked method to return func (Args) Args )

[gburt]

Like I mentioned in the OP, we could leave Return alone and alter Run to accept func(Args) Args too (or the signature of the actual func being mocked). Or could add a new RunAndReturn that does that. I kinda like the idea of modifying Run -- before I thought it would be an API break, but it won't be if we change it to Run(fn any) and then use reflect to panic if not the right type passed. (Would be nice if Go had union types...)

[gburt]

Here's what that looks like -- pretty nice IMO:

[gburt]

I've re-implemented this using Run() instead of Return() and supporting passing in a func that matches the signature of the one being mocked, allowing for this which IMO is quite nice:

mockedGreeter.On("HelloWorld", mock.Anything).Run(func(name string) string {
  return "Hello " + name
})

[gburt]

A coworker pointed out that the "hack" of assigning call.ReturnArguments within a call.Run() callback would work -- and obviate this PR -- if we protected it with mutexes to be threadsafe, eg

m.mutex.Lock()
defer m.mutex.Unlock() // protect everything below
if call.RunFn != nil {
  call.RunFn(arguments)
}
returnArgs := call.ReturnArguments // maybe need to clone it to avoid the risk of the slice being mutated in place?
return returnArgs

That would prevent concurrently calling the RunFn; I'm not sure the ramifications of that.

[dlwyatt]

Like I mentioned in the OP, we could leave Return alone and alter Run to accept func(Args) Args too (or the signature of the actual func being mocked). Or could add a new RunAndReturn that does that. I kinda like the idea of modifying Run -- before I thought it would be an API break, but it won't be if we change it to Run(fn any) and then use reflect to panic if not the right type passed. (Would be nice if Go had union types...)

That's what mockery's "expecter" wrappers are for, strong typing, and the RunAndReturn() method on those mirrors what you're adding directly to testify.Mock here. :)

[gburt]

That's what mockery's "expecter" wrappers are for, strong typing, and the RunAndReturn() method on those mirrors what you're adding directly to testify.Mock here. :)

@dlwyatt hmm can you share how one would do the hello world example using mockery's expecter wrappers? If, as a test developer, you want to alter the implementation do you have to re-run mockery? Can different tests mock the function in different ways (eg different HelloWorld implementations)?

[brackendawson]

I still have the same two issues with this change, one can be remedied but I don't see an answer for the other.

The simple is the same comment as before: Run now has two distinct behaviours and a long docstring. The value of a paramater shouldn't change a function's function. This should be an entirely new method called ReturnFn or similar. If you fix this I will dismiss my review.

The harder one: This still wraps what would otherwise be a pure function (method) in unecessary reflection. Yes, you can hide all of the actual reflection work from the user, and it is quite pretty. But you introduce a possibility for error that the pure function does not; if the signature of the function in the Run call doesn't match then our default behaviour is to panic. For reasons including this, unless I am convinced otherwise, I won't be adding my approval to this change. I know that's not what you want to hear, sorry.

[dlwyatt]

@gburt : https://vektra.github.io/mockery/latest/features/#expecter-structs . There's a RunAndReturn example a few blocks down. No monkeypatching required.

[dlwyatt]

As for how it works, it just assigns the function itself as the "Return" value on the testify mock.Call, and the wrapper on the mockery struct checks for that type signature when it looks at the Arguments.Get(0) value. If it matches, it calls the function and returns its results.

func (_c *MockSomething_DoSomething_Call) RunAndReturn(run func(string, int) (string, error)) *MockSomething_DoSomething_Call {
	_c.Call.Return(run)
	return _c
}

// DoSomething provides a mock function with given fields: s, i
func (_m *MockSomething) DoSomething(s string, i int) (string, error) {
	ret := _m.Called(s, i)
	// snip
	var r0 string
	var r1 error
	if rf, ok := ret.Get(0).(func(string, int) (string, error)); ok {
		return rf(s, i)
	}