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

Findings while trying v3 for matryer/moq style mocks #935

Open
breml opened this issue Mar 3, 2025 · 5 comments
Open

Findings while trying v3 for matryer/moq style mocks #935

breml opened this issue Mar 3, 2025 · 5 comments

Comments

@breml
Copy link

breml commented Mar 3, 2025

Findings:

  • In the config, the accepted value for template is matryer, but the documentation does mention moq.
  • In the generated mock, it still says: If this is not the case, regenerate this file with moq., I think this should be changed to If this is not the case, regenerate this file with mockery.. Location in the template: https://github.com/vektra/mockery/blob/v3/internal/mock_matryer.templ#L27
  • With moq, if the mocked interface has used a type from an other package, that has been imported using an alias, this alias has also been used in the generated mock. This is no longer the case with mockery.
@breml
Copy link
Author

breml commented Mar 3, 2025

One more thing, I just saw, that for beta.1 the toolchain requirement is Go 1.24. Is there a technical reason for this? In corporate environments, there is often a requirement to stick with oldstable until a new Go version is released. Therefore I would appreciate, if mockery V3 would only require oldstable (Go 1.23 as of now) as a hard requirement.

@LandonTClipp
Copy link
Collaborator

Thanks for taking a look into v3, it's very helpful for me to hear from users of moq.

Regarding:

With moq, if the mocked interface has used a type from an other package, that has been imported using an alias, this alias has also been used in the generated mock. This is no longer the case with mockery.

I'm curious to know if there is a practical reason why this is an issue? This is pretty much expected, as mockery will dynamically decide whether or not to use an alias for an import, which is essentially just a function of what other names are visible in the file-global scope.

@LandonTClipp
Copy link
Collaborator

One more thing, I just saw, that for beta.1 the toolchain requirement is Go 1.24. Is there a technical reason for this? In corporate environments, there is often a requirement to stick with oldstable until a new Go version is released. Therefore I would appreciate, if mockery V3 would only require oldstable (Go 1.23 as of now) as a hard requirement.

Go 1.24 was required because of the introduction of generic type aliases, so we need an updated toolchain to support parsing this. If this existing in the toolchain is an issue, there might be a more targeted way we can go about this. We need an updated parser (from golang.org/x/tools/go/packages) and AST support. If there's a better way to do this, I'm happy to look into it.

@breml
Copy link
Author

breml commented Mar 4, 2025

Regarding:

With moq, if the mocked interface has used a type from an other package, that has been imported using an alias, this alias has also been used in the generated mock. This is no longer the case with mockery.

I'm curious to know if there is a practical reason why this is an issue? This is pretty much expected, as mockery will dynamically decide whether or not to use an alias for an import, which is essentially just a function of what other names are visible in the file-global scope.

As far as I can tell until now, I have not yet hit a case, where this became an issue, so I mainly raised this because of the additional changed lines, this caused in the diff of the PR, where I would have wished, that the changes are minimal if mockery is used as a "drop-in-replacement".
That being said, I have not tested a case, where not using the alias would cause a name clash on the imports. Imagine something like this:

import (
  someapi "github.com/some/api"
  otherapi "github.com/other/api"
)

type TheInterface interface{
  Method(some someapi.Thing, other otherapi.OtherThing)
}

In this case, omitting the aliases in the generated code would cause invalid code, since both imported packages are named api.

@breml
Copy link
Author

breml commented Mar 4, 2025

Go 1.24 was required because of the introduction of generic type aliases, so we need an updated toolchain to support parsing this. If this existing in the toolchain is an issue, there might be a more targeted way we can go about this. We need an updated parser (from golang.org/x/tools/go/packages) and AST support. If there's a better way to do this, I'm happy to look into it.

Ok, I see. I think the issue is caused by us using oldstable and preventing update of the toolchain with the env var GOTOOLCHAIN=local and then using mockery as direct dependency (mentioning it in the tools.go, such that it is listed as dependency in the go.mod file).

We do not have this issue with golangci-lint, but I assume, the reason is, that there we install the pre-compiled binary.

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

2 participants