mock icon indicating copy to clipboard operation
mock copied to clipboard

add interfaces flag with unit test

Open kliukovkin opened this issue 1 year ago • 5 comments

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

kliukovkin avatar Aug 28 '24 03:08 kliukovkin

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Aug 28 '24 03:08 CLAassistant

Hey @JacobOaks , can you please take a look?

kliukovkin avatar Sep 10 '24 06:09 kliukovkin

Hey @kliukovkin! Thanks for the PR!

I am in favor of having this feature, just two questions/comments:

  • Instead of using an -interfaces flag, 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.

JacobOaks avatar Sep 11 '24 20:09 JacobOaks

Hey @kliukovkin! Thanks for the PR!

I am in favor of having this feature, just two questions/comments:

  • Instead of using an -interfaces flag, 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?

kliukovkin avatar Oct 14 '24 01:10 kliukovkin

@JacobOaks hey Jacob! Gentle ping here.

kliukovkin avatar Feb 06 '25 08:02 kliukovkin

Updated according to feedback: moved filtering inline into parseFile, removed filterInterfaces, updated tests. Thanks @r-hang @abhinav for reviewing!

kliukovkin avatar Oct 03 '25 05:10 kliukovkin

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 avatar Oct 03 '25 16:10 kliukovkin

@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 avatar Oct 03 '25 23:10 sywhang

@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.

kliukovkin avatar Oct 04 '25 00:10 kliukovkin