Repository navigation
mock does not enforce arity if parameter is Anything #587
Description
package main_test
import (
"testing"
"github.com/stretchr/testify/mock"
)
type Foo interface {
Bar()
}
type MockFoo struct {
mock.Mock
}
// Bar provides a mock function with given fields:
func (_m *MockFoo) Bar() {
_m.Called()
}
func TestFoo(t *testing.T) {
foo := new(MockFoo)
foo.On("Bar", mock.Anything).Return()
foo.Bar() // expected error for unmocked call
}The logic in Arguments.Diff does not check whether a given argument is required by the function signature before checking if the value is Anything. Phrased another way, there is no explicit arity check on the mocked method.
Activity
devdinu commentedon Apr 6, 2018
We could also think about failing the setup of expectation, when the
On(expectation)andBar(actual)arity mismatches. we should also consider variardic in it. This could be a separate PR or enhancement.dnathe4th commentedon Apr 6, 2018
I do think that is a better solution than what I ended up with. I implemented the check in
Diffsolely because that was the only part of the library I was familiar with. Let me see what kind of type checking we have inOn(expectation)I can extend for this purpose.dnathe4th commentedon Apr 6, 2018
So my
reflect-fu may be weak, but I don't believe your proposal is possible. SinceOn(expectation)is defined on*mock.Mockit has no access to the actual struct embeddingmock.Mock. I don't believe there is any way to get the actual underlying implementation onmethodNameto check argument count against. Am I missing something?devdinu commentedon Apr 11, 2018
@dnathe4th Yep, agreed its not possible in current
Onmethod, since we've reference to*mock.Mocktype and the other methods isn't available via reflection.i've also tried whether we could get the information with reflection, but i couldn't find any.
But for the above proposal, i am thinking of
This would enable us to have a strict mocking and argument verification, and helps in case of renaming function etc.
ernesto-jimenez commentedon Jun 9, 2018
@devdinu how would that work?
Requiring people to use the
reflectpackage on their tests sounds like would lead to messy tests.