vmware-archive / octant

Highly extensible platform for developers to better understand the complexity of Kubernetes clusters.
https://octant.dev
Apache License 2.0
6.28k stars 489 forks source link

Consider checking-in code generated by mockgen #427

Closed antoninbas closed 5 years ago

antoninbas commented 5 years ago

Description of the problem

Currently that code (e.g. github.com/vmware-tanzu/octant/pkg/store/fake package) is not checked-in. This creates issues for other projects which use Go module support and use github.com/vmware-tanzu/octant as a dependency, as is the case when creating plugin executables. In particular, the go mod tidy command needs to record test dependencies in the go.mod file, which cannot succeed for octant because of the missing auto-generated fake packages. At best (with Go 1.12) this leads to an unstable go mod tidy, at worst go mod tidy (with Go 1.13) simply fails.

How to reproduce

I created a sample plugin repo (by copying the sample code under cmd/octant-sample-plugin). You can clone it with git clone https://github.com/antoninbas/octant-plugin-test.

git clone https://github.com/antoninbas/octant-plugin-test
cd octant-plugin-test
docker run -w /octant -v `pwd`:/octant -ti golang:1.13.0 bash
root@f61868dea0d5:/octant# go mod tidy
# packages are downloaded and extracted
github.com/antoninbas/octant-plugin-test/cmd/octant-plugin-test imports
    github.com/vmware-tanzu/octant/pkg/navigation tested by
    github.com/vmware-tanzu/octant/pkg/navigation.test imports
    github.com/vmware-tanzu/octant/pkg/store/fake: module github.com/vmware-tanzu/octant@latest (v0.9.1) found, but does not contain package github.com/vmware-tanzu/octant/pkg/store/fake
github.com/antoninbas/octant-plugin-test/cmd/octant-plugin-test imports
    github.com/vmware-tanzu/octant/pkg/plugin tested by
    github.com/vmware-tanzu/octant/pkg/plugin.test imports
    github.com/vmware-tanzu/octant/pkg/plugin/fake: module github.com/vmware-tanzu/octant@latest (v0.9.1) found, but does not contain package github.com/vmware-tanzu/octant/pkg/plugin/fake
github.com/antoninbas/octant-plugin-test/cmd/octant-plugin-test imports
    github.com/vmware-tanzu/octant/pkg/plugin/service tested by
    github.com/vmware-tanzu/octant/pkg/plugin/service.test imports
    github.com/vmware-tanzu/octant/pkg/plugin/service/fake: module github.com/vmware-tanzu/octant@latest (v0.9.1) found, but does not contain package github.com/vmware-tanzu/octant/pkg/plugin/service/fake
github.com/antoninbas/octant-plugin-test/cmd/octant-plugin-test imports
    github.com/vmware-tanzu/octant/pkg/plugin imports
    github.com/vmware-tanzu/octant/internal/module tested by
    github.com/vmware-tanzu/octant/internal/module.test imports
    github.com/vmware-tanzu/octant/internal/cluster/fake: module github.com/vmware-tanzu/octant@latest (v0.9.1) found, but does not contain package github.com/vmware-tanzu/octant/internal/cluster/fake
github.com/antoninbas/octant-plugin-test/cmd/octant-plugin-test imports
    github.com/vmware-tanzu/octant/pkg/plugin imports
    github.com/vmware-tanzu/octant/internal/module tested by
    github.com/vmware-tanzu/octant/internal/module.test imports
    github.com/vmware-tanzu/octant/internal/module/fake: module github.com/vmware-tanzu/octant@latest (v0.9.1) found, but does not contain package github.com/vmware-tanzu/octant/internal/module/fake
github.com/antoninbas/octant-plugin-test/cmd/octant-plugin-test imports
    github.com/vmware-tanzu/octant/pkg/plugin imports
    github.com/vmware-tanzu/octant/internal/octant tested by
    github.com/vmware-tanzu/octant/internal/octant.test imports
    github.com/vmware-tanzu/octant/pkg/action/fake: module github.com/vmware-tanzu/octant@latest (v0.9.1) found, but does not contain package github.com/vmware-tanzu/octant/pkg/action/fake
github.com/antoninbas/octant-plugin-test/cmd/octant-plugin-test imports
    github.com/vmware-tanzu/octant/pkg/plugin imports
    github.com/vmware-tanzu/octant/pkg/plugin/api tested by
    github.com/vmware-tanzu/octant/pkg/plugin/api.test imports
    github.com/vmware-tanzu/octant/internal/portforward/fake: module github.com/vmware-tanzu/octant@latest (v0.9.1) found, but does not contain package github.com/vmware-tanzu/octant/internal/portforward/fake
github.com/antoninbas/octant-plugin-test/cmd/octant-plugin-test imports
    github.com/vmware-tanzu/octant/pkg/plugin imports
    github.com/vmware-tanzu/octant/pkg/plugin/api tested by
    github.com/vmware-tanzu/octant/pkg/plugin/api.test imports
    github.com/vmware-tanzu/octant/pkg/plugin/api/fake: module github.com/vmware-tanzu/octant@latest (v0.9.1) found, but does not contain package github.com/vmware-tanzu/octant/pkg/plugin/api/fake
git clone https://github.com/antoninbas/octant-plugin-test
cd octant-plugin-test
docker run -w /octant -v `pwd`:/octant -ti golang:1.12.0 bash
root@e1ce811cdccb:/octant# go mod tidy
# packages are downloaded and extracted
root@e1ce811cdccb:/octant# go mod tidy
go: finding github.com/vmware-tanzu/octant/internal/cluster/fake latest
go: finding github.com/vmware-tanzu/octant/pkg/plugin/service/fake latest
go: finding github.com/vmware-tanzu/octant/internal/module/fake latest
go: finding github.com/vmware-tanzu/octant/pkg/plugin/fake latest
go: finding github.com/vmware-tanzu/octant/pkg/action/fake latest
go: finding github.com/vmware-tanzu/octant/pkg/plugin/api/fake latest
go: finding github.com/vmware-tanzu/octant/internal/portforward/fake latest
go: finding github.com/vmware-tanzu/octant/pkg/store/fake latest
go: finding github.com/vmware-tanzu/octant/internal/cluster latest
go: finding github.com/vmware-tanzu/octant/internal latest
go: finding github.com/vmware-tanzu/octant/pkg/plugin/service latest
go: finding github.com/vmware-tanzu/octant/internal/module latest
go: finding github.com/vmware-tanzu/octant/pkg/plugin latest
go: finding github.com/vmware-tanzu/octant/pkg/action latest
go: finding github.com/vmware-tanzu/octant/pkg/plugin/api latest
go: finding github.com/vmware-tanzu/octant/internal/portforward latest
go: finding github.com/vmware-tanzu/octant/pkg/store latest
go: finding github.com/vmware-tanzu/octant/pkg latest

Notice how the subsequent invocations of go mod tidy still try to look for the missing auto-generated code.

Additional context I am not a veteran Go programmer but I believe it is considered good practice to check-in the code generated by go generate (see https://blog.golang.org/generate):

Just keep in mind that it is for package authors, not clients, if only for the reason that the program it invokes might not be available on the target machine. Also, if the containing package is intended for import by go get, once the file is generated (and tested!) it must be checked into the source code repository to be available to clients.

I did not find anything in the Go release notes for 1.13 that explains the difference in behavior between the 2 Go versions.

I am also happy to submit a patch to fix this if there is agreement that the generated code should be fixed-in.

wwitzel3 commented 5 years ago

We talked about this and I think we have decided, yes. This is something we want to do. @GuessWhoSamFoo @bryanl unless you object, we'll have @antoninbas draft up a PR.

antoninbas commented 5 years ago

Thanks a lot for the quick reply @wwitzel3. I will try to draft a PR over the weekend.

bryanl commented 5 years ago

My original concern was git conflict hell. Breaking go mod is a bigger deal, so we might as well check them in.

antoninbas commented 5 years ago

@bryanl I'll add a CI check to make sure that the generated files were generated with the "correct" mockgen version and are up-to-date

antoninbas commented 5 years ago

Opened the PR for this. I confirmed that it solved the go mod tidy issue.