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 mocks for protobuf clients? #681

Closed
1 of 5 tasks
leaanthony-sc opened this issue Aug 1, 2023 · 4 comments · Fixed by #682
Closed
1 of 5 tasks

Unable to generate mocks for protobuf clients? #681

leaanthony-sc opened this issue Aug 1, 2023 · 4 comments · Fixed by #682

Comments

@leaanthony-sc
Copy link
Contributor

Description

We have been using mockery (v2.10.6 - docker) successfully to generate mocks for protobuf clients but recently upgraded to v2.32.0 to try out the new packages configuration. It now appears to be impossible to generate protobuf client mocks. Running mockery in debug mode indicated that all source files were being skipped with the message DBG skipping file as auto-generated. Investigating further, I believe I've narrowed it down to this check in config.go. This appears to have been added as part of this commit which was intended to introduce the recursive option. The linked issue doesn't have any information with regards to now skipping auto generated code so it's not clear what the driver was for this change. There also does not appear to be a way around this limitation. I did search through the issue tracker prior to opening this but could not find anything related. Generating mocks for protobuf clients is not uncommon so I'm surprised this hasn't been raised already. Thanks for your time.

Mockery Version

v2.32.0 via docker image

Golang Version

v1.20.7

Installation Method

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

Steps to Reproduce

  1. Generate client protobuf
  2. Use mockery to try and generate a mock for the client

Expected Behavior

I would expect a mock to be generated like it did prior to the change on the 9th of April, or at least an option to allow it to be generated.

Actual Behavior

The file does not get processed due to a hard coded restriction.

@LandonTClipp
Copy link
Contributor

LandonTClipp commented Aug 3, 2023

The reason why this behavior was added is because mockery might generate mocks of its own interfaces (so-called "infinite mocking"). However a change was introduced such that mockery no longer creates interfaces in its auto-generated files, so this behavior might not be needed anymore. This had previously been a huge source of pain and my original solution may not have been the optimal one, but it worked for the time. The PR for the removal of the interfaces also includes proper integration tests to assert infinite mocking no longer happens. This is indeed the first time this issue has been posed for the packages feature, and I agree there needs to be a solution for this.

I think this could be easily fixed by simply adding a parameter called include_auto_generated: true that will disable the check you linked to. I'm also open to the idea of removing the check altogether as the original motivation is no longer a factor, however I'm not sure that it wouldn't break other use cases.

Would you or anyone have the time to implement this new feature?

@LandonTClipp
Copy link
Contributor

LandonTClipp commented Aug 3, 2023

On second thought, I think it might be better to remove the check by default and consider the current behavior to be a bug. We should preemptively also add include_auto_generated but set the default to true so if people need to revert to the "buggy" behavior, they can. What are your thoughts?

LandonTClipp added a commit to LandonTClipp/mockery that referenced this issue Aug 3, 2023
Fixes vektra#681

There are many legitimate use-cases where mockery needs to include auto-generated files
in its recursive package discovery. We add an `include-auto-generated` parameter that
is set to `true` by default, but can be set to `false` if users want to retain the
old behavior. We are considering the old behavior to be a bug due to the large number
of GitHub issues we'll likely get. The original motivation for adding this behavior
was fixed through a different mechanism, which is why we're considering this to be
a bugfix.
LandonTClipp added a commit to LandonTClipp/mockery that referenced this issue Aug 3, 2023
This adds e2e tests for vektra#681, which asserts packages containing only
auto-generated files are added to the list of importable packages
when doing recursive package discovery.
@LandonTClipp
Copy link
Contributor

@leaanthony-sc please try v2.32.3, which introduces this "bug fix" that also adds the include-auto-generated parameter to retain the buggy behavior if needed.

LandonTClipp added a commit to LandonTClipp/mockery that referenced this issue Aug 3, 2023
This adds e2e tests for vektra#681, which asserts packages containing only
auto-generated files are added to the list of importable packages
when doing recursive package discovery.
LandonTClipp added a commit that referenced this issue Aug 3, 2023
@leaanthony-sc
Copy link
Contributor Author

@LandonTClipp - Thanks for the quick turnaround on this 🙏 I can confirm that auto-generated files are now picked up by default. Thank you!

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 a pull request may close this issue.

2 participants