lib: Use counterfeiter to mock interfaces in tests #7375
Conversation
imsodin
force-pushed
the
counterfeiter
branch
from
4d576a9 to
de658eb
Compare
imsodin
force-pushed
the
counterfeiter
branch
from
de658eb to
054d049
Compare
imsodin
force-pushed
the
counterfeiter
branch
from
f1309d1 to
d2d022f
Compare
imsodin
force-pushed
the
counterfeiter
branch
from
c3e3130 to
cbd0e7e
Compare
|
This seems like a fair amount of complexity for something that isn't super hard to do manually, but allright. However I think we should treat it the same as protobuf stuff -- generate explicitly and check in, so that |
|
I see that. It's definitely not hard, just a bit annoying. And I liked this "mock-generator", so wanted to make use of it. Having done the change myself I am not a good judge for the trade-off between complexity/tediousness of the generation vs convenience/functionality of the mocks, as I am set to underestimate the former. I don't want to push this through, if you feel it's too much baggage or something. |
|
No I kinda like adding the mock behavior as required in the test rather than having it somewhere else in the mock, so this is fine to me. Just need to look at it again. |
| }) | ||
| } | ||
|
|
||
| func pruneInterfaceCheck(path string, size int64) error { |
There was a problem hiding this comment.
This is kind of meta, but this seems like a fragile function for several reasons. One obvious alternative would be to pass it through a filter that removes lines starting with var _ =, though of course you gain some extra juggling of a temp file and whatnot.
meta/copyright_test.go
Outdated
| @@ -66,6 +66,9 @@ func checkCopyright(path string, info os.FileInfo, err error) error { | |||
| if copyrightRe.MatchString(scanner.Text()) { | |||
| return nil | |||
| } | |||
| if scanner.Text() == "// Code generated by counterfeiter. DO NOT EDIT." { | |||
There was a problem hiding this comment.
More general: golang/go#13560 (comment)
calmh
added
the
frozen-due-to-age
I was looking for test tooling to time tests (found gotestsum) and in the process stumbled over counterfeiter: It generates functional test classes for any given interface. As I was annoyed by having to change the various
lib/api/mocked_...types to make tests pass in the past, I very much appreciate the idea of having those generated automatically. There's more boilerplate to get rid of in the fake connection in model testing. And the mocked classes provide lots of functionality (controlling what functions do, providing info on which functions were called including arguments, ...) which we could make use of (I guess a lot of the request_test.go stuff could be simplified.The mocks are created with go:generate, i.e. there's now more assets to generate. I refactored the lazy asset building to work on a list of assets, and rebuild those which are necessary. An asset specifies a list of source and destination paths, which are all walked to get the newest time (and missing root means rebuild too).
There's one ugly hack: The mocked classes always include a reference to the interface. That can lead to cyclic imports. And it seems unlikely that's going to change given the upstream history: maxbrunsfeld/counterfeiter#121. Thus I added a generation script to remove this reference from the generated code for one protocol interface that we do use in protocol tests.