add interfaces flag with unit test #200

kliukovkin · 2024-08-28T03:42:14Z

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 · 2024-08-28T03:42:20Z

All committers have signed the CLA.

mockgen/mockgen.go

kliukovkin · 2024-09-10T06:45:10Z

Hey @JacobOaks , can you please take a look?

JacobOaks · 2024-09-11T20:56:32Z

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 · 2024-10-14T01:44:08Z

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 · 2025-02-06T08:10:51Z

@JacobOaks hey Jacob! Gentle ping here.

r-hang

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

r-hang · 2025-10-02T17:49:12Z

mockgen/parse_test.go

+			wantErr: true,
+		},
+		{
+			name: "missing interface (Baz not found)",


Could we also add a test case for accidentally specified duplicate requested interfaces?

r-hang · 2025-10-02T17:50:14Z

mockgen/parse_test.go

+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := filterInterfaces(tt.args.all, tt.args.requested)
+			if (err != nil) != tt.wantErr {


instead of wantErr being a bool, could we make this a string and compare the error contents?

abhinav · 2025-10-02T17:54:37Z

mockgen/parse.go

+
+	// If there are interfaces provided as positional arguments, filter them
+	if len(ifaces) > 0 {
+		if pkg.Interfaces, err = filterInterfaces(pkg.Interfaces, ifaces); err != nil {
+			log.Fatalf("Filtering interfaces failed: %v", err)
+		}
+	} else {
+		// No interfaces provided, process all interfaces for backward compatibility
+		log.Printf("No interfaces specified, processing all interfaces")
+	}


FYI, this is still doing the thing @JacobOaks's comment advised against:

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.

…terInterfaces, update tests

kliukovkin · 2025-10-03T05:45:16Z

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

abhinav

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

mockgen/parse.go

Co-authored-by: Abhinav Gupta <[email protected]>

…ccordingly

kliukovkin · 2025-10-03T16:40:35Z

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 · 2025-10-03T23:22:32Z

@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 · 2025-10-04T00:18:00Z

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

sywhang

Thank you!

kliukovkin mentioned this pull request Aug 28, 2024

Support specifying interfaces to mock when using source mode #140

Open

KastenMike reviewed Aug 28, 2024

View reviewed changes

mockgen/mockgen.go Outdated Show resolved Hide resolved

kliukovkin added 4 commits September 9, 2024 23:40

add interfaces flag with unit test

900159e

move reading to a better place

5a7af37

Moving to a parse.go

6cc01b1

remove spaces

2553835

kliukovkin force-pushed the add_interfaces_flag branch from 59bd1d7 to 2553835 Compare September 10, 2024 06:41

r-hang approved these changes Oct 2, 2025

View reviewed changes

abhinav reviewed Oct 2, 2025

View reviewed changes

abhinav mentioned this pull request Oct 2, 2025

source mode: support specifying interfaces #281

Closed

Refactor interface filtering: inline logic into parseFile, remove fil…

df7752f

…terInterfaces, update tests

abhinav approved these changes Oct 3, 2025

View reviewed changes

mockgen/parse.go Outdated Show resolved Hide resolved

mockgen/parse.go Outdated Show resolved Hide resolved

mockgen/parse.go Show resolved Hide resolved

mockgen/parse.go Outdated Show resolved Hide resolved

kliukovkin and others added 2 commits October 3, 2025 08:55

Apply suggestions from code review

89fa9e9

Co-authored-by: Abhinav Gupta <[email protected]>

Update parseFile: ignore missing requested interfaces, adjust tests a…

db487ea

…ccordingly

kliukovkin added 3 commits October 3, 2025 16:58

Update generated mocks (bugreport_mock.go)

ca7477a

regenerate mocks after merge from upstream

6c12923

test(typed): regenerate bugreport_mock.go after upstream merge

e1e3ec9

regenerate goldens: update generated mocks after upstream merge

a6a5b06

sywhang approved these changes Oct 4, 2025

View reviewed changes

sywhang merged commit 043dd32 into uber-go:main Oct 4, 2025
5 checks passed

add interfaces flag with unit test #200

add interfaces flag with unit test #200

Uh oh!

Conversation

kliukovkin commented Aug 28, 2024

Uh oh!

CLAassistant commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

kliukovkin commented Sep 10, 2024

Uh oh!

JacobOaks commented Sep 11, 2024

Uh oh!

kliukovkin commented Oct 14, 2024

Uh oh!

kliukovkin commented Feb 6, 2025

Uh oh!

r-hang left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

r-hang Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

r-hang Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

abhinav Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

kliukovkin commented Oct 3, 2025

Uh oh!

abhinav left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kliukovkin commented Oct 3, 2025

Uh oh!

sywhang commented Oct 3, 2025

Uh oh!

kliukovkin commented Oct 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sywhang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

CLAassistant commented Aug 28, 2024 •

edited

Loading

r-hang left a comment •

edited

Loading

kliukovkin commented Oct 4, 2025 •

edited

Loading