-
Notifications
You must be signed in to change notification settings - Fork 409
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
Add support for generating mocks for generic interfaces #456
Add support for generating mocks for generic interfaces #456
Conversation
Thanks for the PR, this is looking great! Semantic Versioning DiscussionThis change could/will break anyone who is using
However I realize that in one of the installation methods listed is We do clearly outline in the Semantic Versioning section of the README that semver won't be tracking any of the code in the mockery package, and that the only thing that is subject to semver are the CLI and the generated mocks. Because of this, I think that this update warrants only a minor version bump. The reason why I'm really against a major version bump can be distilled to two reasons:
I am opening this up for discussion from the community. Please share your thoughts, let's come to a consensus before solidifying it! |
@cruickshankpg could you fix the merge conflicts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few comments on this. I have a few other thoughts that I will share in the coming days. Thanks again.
e7e706c
to
a129a32
Compare
@cruickshankpg could you resolve the merge conflicts? Also the tests are (expectedly) failing, I will modify the PR builder as anything pre 1.18 will never pass now. |
aaee69e
to
e676f7a
Compare
I'm hitting golang/go#51629 when manually running mockery agaIinst my own interfaces, I'm not entirely sure why I'm not hitting this in the unit tests. The fix hasn't been included in a release yet so I've pinned to the relevant sha of the fix. |
Thanks for the update @cruickshankpg. So I have thought about this for a while and I want to add a new release before this with an explicit deprecation notice in the logs for Sorry to delay this but I think this is the right way forward. |
If not |
The only recommended/supported methods are:
You can use go install/go get if you want, but there are two caveats here:
|
What about |
@fastcat using any golang command that compiles the source code using a local go binary isn't supported. You are certainly free to use the command if you find it useful for your purposes but I strongly recommend against it. Intrinsically, the issue is that we break semantic versioning if minor bumps might require people to upgrade their golang version. We could do major bumps any time we support a new language feature but that's a hassle and probably not the right path. Sure you can pin the mockery version but I certainly expect if we don't do it this way that people will express frustration that a simple minor bump broke their processes. So ultimately, you can obviously do what you want, but just understand the caveats listed above. Officially, we will no longer support it, semantic versioning demands we don't. Realistically, you're free to tread at your own caution. |
@cruickshankpg go 1.18.2 was released yesterday. Maybe this solves golang/go#51629 for you and you can get rid of pinning the version. Worth a try I'd say. |
@LandonTClipp ok, thanks for clarifying what the risk/concern is. It wasn't clear before, and seemed like a Pinning the version in my |
I'll go ahead and merge this in the coming days and give the version a |
8a33ed4
to
4ae58ff
Compare
yep I'm not seeing it after upgrading to 1.18.2. I've removed the upgrade to an interim x/tools version |
For anyone coming to this, I had to |
Retested and I was still seeing |
Is there an easy link where I can always download the latest release similar to how |
I am using GitLab CI with go image. How use mockery on GitLab CI then? When go image has no docker inside, and go image has no brew to install. |
Unfortunately, no longer supporting Versioning via go.mod has the benefit of automated dependency update tools like Renovate being able to automatically create MRs for tool updates. Not being able to use I don't have a solution ready, but maybe this is an argument for looking for a better solution. I'd be ok with requiring an up-to-date Go version for the newest mockery versions, I'm just afraid that generally declaring |
@mfrister Thank you for your views on the matter. I do know that many people are using Practically, what I think I'm comfortable with doing, is having a gentleman's agreement where we will do our best to make sure that People who still want to use |
- Add support for rendering union types - Add support for rendering the type arguments of named types - Add support for rendering types embedded in interfaces - Test generic generation against a single more complicated interface - Fix package namespacing on type arguments
47cac5a
to
ed55a47
Compare
@slessard we can probably modify the asset names so that you can reference the https://github.com/vektra/mockery/releases/download/v2.12.3/mockery_2.12.3_Darwin_x86_64.tar.gz But since the tar file has the version name it makes it hard to download the latest as the name will always change. So instead we could do something like: https://github.com/vektra/mockery/releases/latest/mockery_Darwin_x86_64.tar.gz How does that sound? |
8be74a7
to
35bfca3
Compare
Re-run mock generation Fix tests
206f4c8
to
4c0f6c8
Compare
That is certainly a positive change. Also an apt-get package for certain linux distributions would be nice |
This is proven false with your follow up response.
So it will work as long as the local golang binary matches at least the go.mod version. For a project whose sole purpose is to closely follow a language spec and generate code based on that language's changes, I believe using semantic versioning as a reason for abandoning a core language design ( Programming gets messy sometimes (a lot of times). For a project like this, I think it is acceptable to bump minor versions for new golang versions that contain minor breaking changes when the project is forced to in order to keep on producing correct mocks (otherwise, no go.mod change is necessary). When a major change like generics comes along, if there is any time to bump the major version this is it. While my statement is my own opinion, I believe Golang devs will understand and be willing to work with you on these aspects regarding semantic versioning since by definition you are not in complete control of how you can semantically version your project. That being said, I urge you to remove the "official deprecation" label of |
Thanks for your insight @wspowell .
I'm not sure I follow. It will break anyone who doesn't have an up-to-date Golang version installed. Yes it is corrected by updating your Golang version but intrinsically if we are requiring users to modify their environment to support updates to this project outside of major version bumps, we have broken semver. We could do major bumps as you say but I think that's a bit much. Maybe it is a personal preference. I prefer to introduce feature additions without breaking people where possible.
The stance I am taking is that officially we do not want to tell people to use
We have already publicized that
Certainly I want to avoid pushing you away :)
There is a burden placed on the users either way. I'm simply picking a burden that I feel is in the best interest of semantic and logical correctness. I do hear your concerns and I greatly appreciate your input. I'm amenable to the idea of perhaps not deprecating it entirely, but recommending people not to use it. And if folks want to use it, then I can point to our stance when they submit tickets asking why their workflows are broken. |
To me there are huge differences between:
Items (2) and (3) create very clear errors for the user. Something fails very clearly, and you either upgrade While I disagree with your stance on (not) requiring folks to update Go, I recognize it's something on which reasonable people may disagree 🙂. What I do ask is that the deprecation notice be made clear about the risks and reasons for why Absent that clarity, the current documentation reads as equivalent to "despite this being an open source project, we do not support users compiling it for themselves". That would be a very anti-open-source thing to say, and from the more detailed contents of this thread (as opposed to the one linked comment from the README), it's obviously not what you mean. So, in the interest of staying positive & being helpful, some suggested wording that I think would clarify this:
|
I tend to agree with @wspowell. The following is IMHO, and I do get that this particular ship has already sailed. It feels like all of the issues discussed in this PR thread can be distilled down to the consequences of one decision: not wanting to use semantic versioning as intended because "it's a bit much". If you're releasing software that is no longer backwards-compatible, you increase the major version. This is widely understood, even assumed, and I would posit a critical foundation of modern software engineering. I agree bumping a major version just for this support is a lot, but it's the only answer that fixes more than it breaks. While major version tagging is a bit hairy in go, I think it's safe to assume consumers (developers) now understand the tagging/versioning semantics of the go ecosystem more reliably than they will know the one-off decisions of arbitrary GitHub projects that could be direct or indirect dependencies on their work. And they would likely prefer to understand one integrated versioning and tooling mechanism than several potential mechanisms (binary, docker, etc) each with their own mockery-specific version choice caveats. Removing guarantees of support for a foundational part of go tooling is a slippery slope (looking at you, I happen to use |
Hi folks. I've thought about the above comments and based on the feedback everyone has provided, I will continue to support
The deprecation notice will be removed in the coming days and I'll make the necessary changes to documentation. Thanks for the productive discussions everyone. |
Thanks for the points and followup! |
The `mockery` project recommends against using a binary of `mockery` that has been created using `go install`. vektra/mockery#456. Developers of Tendermint wishing to generate mocks should avoid having a version of `mockery` on their path that does not match the version listed in [mockery_generate.sh](https://github.com/tendermint/tendermint/blob/10e1ac8feabd8ce7fd560ebecb11b929376fcb9c/scripts/mockery_generate.sh#L11). To make this easier for developers, the `mockery_generate.sh` script uses a containerized copy of `mockery` if `mockery` is not present on the developer's `PATH`. This containerized version of `mockery` uses the same version of mockery as our CI pipelines and allows all developers to automatically use the same version without having to manage it themselves. #### PR checklist - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed
The `mockery` project recommends against using a binary of `mockery` that has been created using `go install`. vektra/mockery#456. Developers of Tendermint wishing to generate mocks should avoid having a version of `mockery` on their path that does not match the version listed in [mockery_generate.sh](https://github.com/tendermint/tendermint/blob/10e1ac8feabd8ce7fd560ebecb11b929376fcb9c/scripts/mockery_generate.sh#L11). To make this easier for developers, the `mockery_generate.sh` script uses a containerized copy of `mockery` if `mockery` is not present on the developer's `PATH`. This containerized version of `mockery` uses the same version of mockery as our CI pipelines and allows all developers to automatically use the same version without having to manage it themselves. #### PR checklist - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed
The `mockery` project recommends against using a binary of `mockery` that has been created using `go install`. vektra/mockery#456. Developers of Tendermint wishing to generate mocks should avoid having a version of `mockery` on their path that does not match the version listed in [mockery_generate.sh](https://github.com/tendermint/tendermint/blob/10e1ac8feabd8ce7fd560ebecb11b929376fcb9c/scripts/mockery_generate.sh#L11). To make this easier for developers, the `mockery_generate.sh` script uses a containerized copy of `mockery` if `mockery` is not present on the developer's `PATH`. This containerized version of `mockery` uses the same version of mockery as our CI pipelines and allows all developers to automatically use the same version without having to manage it themselves. #### PR checklist - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed
The `mockery` project recommends against using a binary of `mockery` that has been created using `go install`. vektra/mockery#456. Developers of Tendermint wishing to generate mocks should avoid having a version of `mockery` on their path that does not match the version listed in [mockery_generate.sh](https://github.com/tendermint/tendermint/blob/10e1ac8feabd8ce7fd560ebecb11b929376fcb9c/scripts/mockery_generate.sh#L11). To make this easier for developers, the `mockery_generate.sh` script uses a containerized copy of `mockery` if `mockery` is not present on the developer's `PATH`. This containerized version of `mockery` uses the same version of mockery as our CI pipelines and allows all developers to automatically use the same version without having to manage it themselves. #### PR checklist - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed
The `mockery` project recommends against using a binary of `mockery` that has been created using `go install`. vektra/mockery#456. Developers of Tendermint wishing to generate mocks should avoid having a version of `mockery` on their path that does not match the version listed in [mockery_generate.sh](https://github.com/tendermint/tendermint/blob/10e1ac8feabd8ce7fd560ebecb11b929376fcb9c/scripts/mockery_generate.sh#L11). To make this easier for developers, the `mockery_generate.sh` script uses a containerized copy of `mockery` if `mockery` is not present on the developer's `PATH`. This containerized version of `mockery` uses the same version of mockery as our CI pipelines and allows all developers to automatically use the same version without having to manage it themselves. #### PR checklist - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed
The `mockery` project recommends against using a binary of `mockery` that has been created using `go install`. vektra/mockery#456. Developers of Tendermint wishing to generate mocks should avoid having a version of `mockery` on their path that does not match the version listed in [mockery_generate.sh](https://github.com/tendermint/tendermint/blob/10e1ac8feabd8ce7fd560ebecb11b929376fcb9c/scripts/mockery_generate.sh#L11). To make this easier for developers, the `mockery_generate.sh` script uses a containerized copy of `mockery` if `mockery` is not present on the developer's `PATH`. This containerized version of `mockery` uses the same version of mockery as our CI pipelines and allows all developers to automatically use the same version without having to manage it themselves. - [ ] Tests written/updated, or no tests needed - [ ] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [ ] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed
Description
This adds support for generating mocks for generic interfaces. It does bump the go version to 1.18 so not sure how you want to handle this.
Type of change
Version of Golang used when building/testing:
How Has This Been Tested?
I've added unit tests for different types of generic interfaces, happy to add any others you think are worthwhile.
Checklist