Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to generate interface type from another interface #787

Open
1 of 5 tasks
FournyP opened this issue Jun 10, 2024 · 16 comments
Open
1 of 5 tasks

Unable to generate interface type from another interface #787

FournyP opened this issue Jun 10, 2024 · 16 comments

Comments

@FournyP
Copy link

FournyP commented Jun 10, 2024

Description

Hi, I wanted to write some mock for my interface :

type ResponseLessUseCase[M any] interface {
	Execute(message *M) *errors.Error
}

type SendEmailVerificationUseCaseInterface ResponseLessUseCase[usermessages.SendEmailVerificationMessage]

But only the mock for ResponseLessUseCase was created, maybe I miss something ?

Mockery Version

v2.43.2

Golang Version

v1.22.3

Installation Method

  • Binary Distribution
  • Docker
  • brew
  • go install
  • Other: [specify]

Steps to Reproduce

  1. Install go & mockery last versions
  2. Write a golang file that include a generic interface and another interface that is typed as your generic interface
  3. Run mockery

Expected Behavior

A mock for the interface that inherit the generic one

Actual Behavior

Only the mock of the generic interface

@FournyP
Copy link
Author

FournyP commented Jun 10, 2024

Here is my .mockery.yaml config :

with-expecter: true
all: true
filename: "mock-{{ .InterfaceName | kebabcase }}.go"
outpkg: mock{{.PackageName}}
mockname: "{{.InterfaceName}}"
dir: tests/mocks/{{trimPrefix .InterfaceDirRelative "pkg/"}}
packages:
  packageA:
    config:
      recursive: true

@LandonTClipp
Copy link
Collaborator

Hmm that's an interesting use case I have not thought of (generic interface with an instantiated type assigned to a different type name). It should work but I'm not sure what mockery thinks of this. Can you run with mockery --log-level debug | grep SendEmailVerificationUseCaseInterface and see what mockery does with it?

@FournyP
Copy link
Author

FournyP commented Jun 11, 2024

Hi @LandonTClipp,

I got logs for ResponseLessUseCase but not at all for SendEmailVerificationUseCaseInterface

@LandonTClipp
Copy link
Collaborator

If mockery did not print anything then something has gone wrong during its discovery phase of top-level interface definitions. I'll see if I can tinker with it.

@LandonTClipp
Copy link
Collaborator

LandonTClipp commented Jun 24, 2024

Well this is hilarious. The Go AST considers this to be an index expression: *ast.IndexExpr. Mockery does not see SendEmailVerificationUseCaseInterface as an interface type because of this. I can successfully induce mockery to generate a mock for it by telling it to include *ast.IndexExpr the list of types it generates mocks for (currently only interfaces and functions), but I'm not entirely sure on the implications of this. I need to think about it.

There's three ways we could resolve this:

  1. Provide an override in config to force mockery to generate the mock for unusual types like this. This is the least risky but also generates more burden on users who define their types in this way.
  2. Always generate mocks for *ast.IndexExpr. This is highly risky as I'm unsure the ways in which this might break.
  3. Submit a PR to the Go developers asking them to make type declarations that use generic instantiation to appear in the AST as interfaces, not index expressions. This is likely the right long-term solution, but it could take quite a while to gain any traction, if at all.

PR: #790

@FournyP
Copy link
Author

FournyP commented Jul 29, 2024

Hi @LandonTClipp

Do you have any news about this issue ?

I found related issues on other mock project if it can help you :

It seems to work with golang/mock but it's no longer maintained 🫤

@LandonTClipp
Copy link
Collaborator

No news about the issue... I don't know the best way to proceed other than bugging the Go devs to change the AST. How big of an issue is this for you? If it's preventing you from using this project, I can hack together a new parameter that will force generation in cases like these, but I'm miffed at the idea of adding yet another workaround.

@FournyP
Copy link
Author

FournyP commented Jul 31, 2024

Hi @LandonTClipp,

Yeah it prevent me to use this project but I see that you do a PR #790.
If your not sure about it I can just simply fork the project with this PR and I'll deal with it 😉

@LandonTClipp
Copy link
Collaborator

Give me a few days and I will modify that PR to add a config parameter to turn on generation of these generic interfaces. The PR itself has compatibility issues and can't be merged as-is.

@LandonTClipp
Copy link
Collaborator

LandonTClipp commented Aug 2, 2024

So on second thought, I think this may indeed be simpler than what I was thinking. I was worried that including *ast.IndexExpr as an allowed mock target would incorrectly generate mocks in some scenarios. For example, if we define this at the top-level:

var array = []int{1, 2, 3}
var arrayElem = array[0]

I was worried that arrayElem might be generated, but these are *ast.ValueSpec nodes, not *ast.TypeSpec, so they obviously wouldn't be considered. I also cannot think of any other instance in which a type spec would have an index expression that is not related to generics. So, I think the PR you mentioned is pretty much good to go. I have added a few finishing touches on it, but I'll merge that.

References:

LandonTClipp added a commit to LandonTClipp/mockery that referenced this issue Aug 2, 2024
Fixes issue vektra#787. Allow `*ast.IndexExpr` in a `*ast.TypeSpec` to be a mock
target.
@LandonTClipp
Copy link
Collaborator

@FournyP give https://github.com/vektra/mockery/releases/tag/v2.44.1 a try and let me know if that works for you.

@mfaizanse
Copy link

I have the same issue, I am trying to create the mock for Controller interface (reference).

Mockery version: v2.44.2

19 Aug 24 12:01 CEST INF Walking dry-run=false version=v2.44.2
19 Aug 24 12:01 CEST ERR Unable to find 'Controller' in any go files under this path dry-run=false version=v2.44.2
unable to find interface
exit status 1

@cesnietor
Copy link

cesnietor commented Sep 18, 2024

Hi, we had a similar issue when trying to mock an inteface (from protoc-gen-go-grpc) that was migrated to use a generic one .
grpc/grpc-go#7594 they did mention their intention behind that change.
I tried running with mockery and as other folks commented only mocks one interface and everytime is a different one.

e.g. of an interface that we are trying to mock:

type ServerStreamingServer[Res any] interface {
	Send(*Res) error
	ServerStream
}

@LandonTClipp
Copy link
Collaborator

I have the same issue, I am trying to create the mock for Controller interface (reference).

This is a type alias of an instantiated generic interface. Type aliases are difficult to get right, although there is a fix in the works: #808

e.g. of an interface that we are trying to mock:

This should work? Can you share more details? Maybe post results of debug log? Can you give a minimal reproducer?

@cesnietor
Copy link

@LandonTClipp sure, let me get something minimal to reproduce this issue.

@cesnietor
Copy link

Hi so while writing the test steps I was able to make it work.
Basically protoc-gen-go-grpc uses a type that selects the interface like:

type SomeAdminRpcs_Client = grpc.ServerStreamingClient[CustomResponse]

And previously I was using mockery to mock the SomeAdminRpcs_Client which used to be an interface.
But since now the interface is ServerStreamingClient I just updated mockery to mock that and I'm able to mock it just fine in the tests.
Thanks.

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

No branches or pull requests

4 participants