Introduces a type which can be used to mock calls #118
Conversation
This type is designed to easily and dynamically mock out calls to methods/functions so that tests can declare expected arguments and their return values without having to define custom functions for each test. In addition, calls and return-values are logged so that when tests fail it's easy to pinpoint why.
There was a problem hiding this comment.
This looks really useful. It should make a lot of tests more concise, and easier to write and read.
One concern I have is matching more complex call arguments. For example, what if a function takes a pointer to some type which might not be accessible at mock setup time.
I guess you can always have special handling the fake type's method in these cases. I don't think this corner case should prevent you from landing this as it is now.
| } | ||
|
|
||
| func Example() { | ||
| var logger loggo.Logger |
There was a problem hiding this comment.
Please add a comment here along the lines of:
// Set up mock
| mock.Call("Div", 1, 1).Returns(1, nil) | ||
| mock.Call("Div", 1, 0).Returns(0, fmt.Errorf("cannot divide by zero")) | ||
|
|
||
| example := ExampleTypeWhichUsesInterface{calculator: mock} |
There was a problem hiding this comment.
And another here:
// Use the mock to test ExampleTypeWhichUsesInterface
| m.timesInvoked.value++ | ||
| } | ||
|
|
||
| func (m *callMockReturner) numTimesInvoked() int { |
There was a problem hiding this comment.
Although this seems useful, it's not exposed or used
There was a problem hiding this comment.
It's returned from Returns so that you can check that functions are called. Example here.
| // returned. It returns the results previously specified by the Call | ||
| // function. If no results were specified, the returned slice will be | ||
| // nil. | ||
| func (m *CallMocker) MethodCall(receiver interface{}, fnName string, args ...interface{}) []interface{} { |
There was a problem hiding this comment.
What about naming this RecordCall? That would be closer to what it does.
| type CallMocker struct { | ||
| Stub | ||
|
|
||
| logger loggo.Logger |
There was a problem hiding this comment.
What if this took a gc.C and used the Log method on that?
There was a problem hiding this comment.
It seems like it would unnecessarily couple this type to gocheck. I wish gc.C satisified some kind of logger interface so that we could just pass it in.
There was a problem hiding this comment.
Yeah, that would be better... Not coupling this to gc.C makes sense I guess, but the way it is ties this to loggo which isn't much better. Maybe at least make this a loggo style interface?
| r.logCall() | ||
| return r.retVals | ||
| } | ||
| return nil |
There was a problem hiding this comment.
I wonder if this should be an error, that is, shouldn't the test author be required to specify each possible call up front? Otherwise tests could end up passing unintentionally.
There was a problem hiding this comment.
I thought about panicing here; what happens in practice is that when people try to resolve an index on this result, they get a panic then indicating that they haven't specified a necessary call/result. Even if the function/method only returns one result, they'll have to do results[0].(int) (or whatever), so there is no risk that tests will pass unintentionally.
What do you think?
| m.timesInvoked.value++ | ||
| } | ||
|
|
||
| func (m *callMockReturner) numTimesInvoked() int { |
|
|
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju-testing |
This type is designed to easily and dynamically mock out calls to methods/functions so that tests can declare expected arguments and their return values without having to define custom functions for each test. In addition, calls and return-values are logged so that when tests fail it's easy to pinpoint why.