Skip to content

add interfaces flag with unit test#200

Merged
sywhang merged 12 commits into
uber-go:mainfrom
kliukovkin:add_interfaces_flag
Oct 4, 2025
Merged

add interfaces flag with unit test#200
sywhang merged 12 commits into
uber-go:mainfrom
kliukovkin:add_interfaces_flag

Conversation

@kliukovkin
Copy link
Copy Markdown
Contributor

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 golang/mock#660

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Aug 28, 2024

CLA assistant check
All committers have signed the CLA.

Comment thread mockgen/mockgen.go Outdated
@kliukovkin
Copy link
Copy Markdown
Contributor Author

Hey @JacobOaks , can you please take a look?

@JacobOaks
Copy link
Copy Markdown
Contributor

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.

- 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.
@kliukovkin
Copy link
Copy Markdown
Contributor Author

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
Copy link
Copy Markdown
Contributor Author

@JacobOaks hey Jacob! Gentle ping here.

Copy link
Copy Markdown
Contributor

@r-hang r-hang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. Looks good to me with some minor comments. Let's merge this when they are addressed.

Comment thread mockgen/parse_test.go Outdated
Comment thread mockgen/parse_test.go Outdated
Comment thread mockgen/parse.go Outdated
@kliukovkin
Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor

@abhinav abhinav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sywhang This PR implements the changes you suggested in #281 (failing on missing expected interfaces).

@kliukovkin, thanks for updating. left some minor suggestions for improvements.
I see that the tests are failing, though.

Comment thread mockgen/parse.go Outdated
Comment thread mockgen/parse.go Outdated
Comment thread mockgen/parse.go
Comment thread mockgen/parse.go Outdated
kliukovkin and others added 2 commits October 3, 2025 08:55
@kliukovkin
Copy link
Copy Markdown
Contributor Author

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 ! 🚀

@sywhang
Copy link
Copy Markdown
Contributor

sywhang commented Oct 3, 2025

@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
Copy link
Copy Markdown
Contributor Author

kliukovkin commented Oct 4, 2025

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

Copy link
Copy Markdown
Contributor

@sywhang sywhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@sywhang sywhang merged commit 043dd32 into uber-go:main Oct 4, 2025
5 checks passed
pableeee pushed a commit to pableeee/implgen that referenced this pull request Mar 7, 2026
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 golang/mock#660

---------

Co-authored-by: Abhinav Gupta <mail@abhinavg.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Introduce possibility to list required interfaces so mockgen won't generate all of them in source mode

7 participants