mock
mock copied to clipboard
add interfaces flag with unit test
This is a proposal for -interfaces flag using source mode. Using this flag it is possible to list only required interfaces to be mocked in source mode. Currently, source mode mocks all interfaces which -source file contains.
Thanks @KastenMike for raising this!
Fixes https://github.com/golang/mock/issues/660
Hey @JacobOaks , can you please take a look?
Hey @kliukovkin! Thanks for the PR!
I am in favor of having this feature, just two questions/comments:
- Instead of using an
-interfacesflag, what do you think about having source mode read positional arguments (i.e.,mockgen -source <path/to/file.go> InterfaceOne,InterfaceTwo) to align it with how reflect mode works? (If there are no positional arguments specified, we would parse all interfaces to keep backwards compatibility) - Instead of parsing and then dropping interfaces that aren't specified, can we simply not parse ones that aren't requested? This is similar to how the exclusion flag already works and would avoid some wasted computation.
Hey @kliukovkin! Thanks for the PR!
I am in favor of having this feature, just two questions/comments:
Instead of using an
-interfacesflag, what do you think about having source mode read positional arguments (i.e.,mockgen -source <path/to/file.go> InterfaceOne,InterfaceTwo) to align it with how reflect mode works? (If there are no positional arguments specified, we would parse all interfaces to keep backwards compatibility)Instead of parsing and then dropping interfaces that aren't specified, can we simply not parse ones that aren't requested? This is similar to how the exclusion flag already works and would avoid some wasted computation.
Hey @JacobOaks ! Done, can you please review and let me know if that looks good?
@JacobOaks hey Jacob! Gentle ping here.
Updated according to feedback: moved filtering inline into parseFile, removed filterInterfaces, updated tests. Thanks @r-hang @abhinav for reviewing!
Updated according to feedback.
- accepted all your suggestions
- removed the “missing interfaces” error check, because it was breaking integration tests - in practice, missing names should just be ignored
Thanks for the review @abhinav ! 🚀
@kliukovkin thanks for updating the PR - could you please check the CI test run results and update the checked in golden files for tests?
@kliukovkin thanks for updating the PR - could you please check the CI test run results and update the checked in golden files for tests?
@sywhang ran go generate ./... to address the CI failure and ran ./ci/test.sh, everything looks good now. Sorry for the earlier mismatch.