mockgen dependency not pinned, generated mocks not verified

openshift / cloud-credential-operator

Manage cloud provider credentials as Kubernetes CRDs

Apache License 2.0

62 stars 143 forks source link

mockgen dependency not pinned, generated mocks not verified #554

Closed stevekuznetsov closed 9 months ago

stevekuznetsov commented 1 year ago

Today the mockgen binary is used to generate mocks for the cloud-specific client packages, but the generator's not installed via make and the version of the generator is not validated to be correct when running make generate. Furthermore, the mocks are not validated to be correct in CI.

Today, the correct version is

go install github.com/golang/mock/mockgen@73266f9366fcf2ccef0b880618e5a9266e4136f4

Ideally we have:

[ ] a make target to install this tool
[ ] a make target as a dependency for make generate to validate that the version of mockgen being run is correct
[ ] a make target under make validate that ensures the mocks are not changed when re-generated, to ensure they are up to date

cc @abutcher

stevekuznetsov commented 1 year ago

cc @jstuever

2uasimojo commented 1 year ago

Ideally we would add an ensure-mockgen target to build-machinery-go.

However, installing go tools seems not to work consistently (le big sad face). But perhaps something like this would be useful.

stevekuznetsov commented 1 year ago

@2uasimojo what's wrong with go install that makes it unreliable? In previous projects I found that using a temporary go.mod file and go install (to not pollute the project's dependencies itself) was very fruitful for this sort of stuff.

2uasimojo commented 1 year ago

@stevekuznetsov I'll be the first to admit I've never had a good grasp on go's dependency or module or helper binary setups. I've never heard of

using a temporary go.mod file

but if that makes it work consistently, great! Let's do the same for ensure-imagebuilder!

stevekuznetsov commented 1 year ago

@2uasimojo sure thing! I'll update both. FWIW This is the go mod impl:

https://github.com/kcp-dev/kcp/blob/ae902beedc1f45ec37afc87881f3f9d2473bf927/hack/go-install.sh#L54-L62

openshift-bot commented 11 months ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot commented 10 months ago

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten /remove-lifecycle stale

openshift-bot commented 9 months ago

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen. Mark the issue as fresh by commenting /remove-lifecycle rotten. Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci[bot] commented 9 months ago

@openshift-bot: Closing this issue.

In response to [this](https://github.com/openshift/cloud-credential-operator/issues/554#issuecomment-1830967985): >Rotten issues close after 30d of inactivity. > >Reopen the issue by commenting `/reopen`. >Mark the issue as fresh by commenting `/remove-lifecycle rotten`. >Exclude this issue from closing again by commenting `/lifecycle frozen`. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.