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

Migrate the mocking library to uber's #291

Merged

Conversation

ericpromislow
Copy link
Contributor

Related to #47742

The golang/mock repo has been archived and the world has moved to github.com/uber-go's

@ericpromislow ericpromislow requested a review from a team as a code owner October 7, 2024 22:08
@ericpromislow ericpromislow requested review from a team and removed request for a team October 7, 2024 22:08
@@ -25,12 +24,13 @@ require (
github.com/rancher/lasso v0.0.0-20240828170735-d79536cac289
github.com/rancher/norman v0.0.0-20240822182819-60ccfabc4ac5
github.com/rancher/remotedialer v0.3.2
github.com/rancher/wrangler/v3 v3.0.0
github.com/rancher/wrangler/v3 v3.0.1-rc.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this file contains more changes than needed?
Running:

git checkout main -- go.mod go.sum
go mod tidy

only produces the following diff:

diff --git a/go.mod b/go.mod
index 96274a2..0e2741e 100644
--- a/go.mod
+++ b/go.mod
@@ -12,7 +12,6 @@ replace (

 require (
        github.com/adrg/xdg v0.5.0
-       github.com/golang/mock v1.6.0
        github.com/google/gnostic-models v0.6.8
        github.com/gorilla/mux v1.8.1
        github.com/gorilla/websocket v1.5.1
@@ -30,6 +29,7 @@ require (
        github.com/stretchr/testify v1.9.0
        github.com/urfave/cli v1.22.14
        github.com/urfave/cli/v2 v2.27.4
+       go.uber.org/mock v0.4.0
        golang.org/x/sync v0.7.0
        gopkg.in/yaml.v3 v3.0.1
        k8s.io/api v0.30.1
@@ -60,6 +60,7 @@ require (
        github.com/go-openapi/jsonreference v0.20.2 // indirect
        github.com/go-openapi/swag v0.22.3 // indirect
        github.com/gogo/protobuf v1.3.2 // indirect
+       github.com/golang/mock v1.6.0 // indirect
        github.com/golang/protobuf v1.5.4 // indirect
        github.com/google/go-cmp v0.6.0 // indirect
        github.com/google/gofuzz v1.2.0 // indirect
diff --git a/go.sum b/go.sum
index c66d859..b3dfcca 100644
--- a/go.sum
+++ b/go.sum
@@ -253,6 +253,8 @@ go.opentelemetry.io/proto/otlp v1.0.0 h1:T0TX0tmXU8a3CbNXzEKGeU5mIVOdf0oykP+u2lI
 go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v80hjKIs5JXpM=
 go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
 go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
+go.uber.org/mock v0.4.0 h1:VcM4ZOtdbR4f6VXfiOpwpVJDL6lCReaZ6mw31wqh7KU=
+go.uber.org/mock v0.4.0/go.mod h1:a6FSlNadKUHUa9IP5Vyt1zh4fC7uAwxMutEAscFbkZc=
 golang.org/x/crypto v0.0.0-20190211182817-74369b46fc67/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=

@ericpromislow
Copy link
Contributor Author

If you don't use a newer version of wrangler you'll get these errors when you run bash scripts/test.sh:

?       github.com/rancher/steve/pkg/resources/virtual  [no test files]
# github.com/rancher/steve/pkg/schema/converter [github.com/rancher/steve/pkg/schema/converter.test]
pkg/schema/converter/crd_test.go:282:123: cannot use ctrl (variable of type *"go.uber.org/mock/gomock".Controller) as
*"github.com/golang/mock/gomock".Controller value in argument to
fake.NewMockNonNamespacedClientInterface[*v1.CustomResourceDefinition, *v1.CustomResourceDefinitionList]
pkg/schema/converter/k8stonorman_test.go:433:123: cannot use ctrl (variable of type
*"go.uber.org/mock/gomock".Controller) as *"github.com/golang/mock/gomock".Controller value in argument to
fake.NewMockNonNamespacedClientInterface[*v1.CustomResourceDefinition, *v1.CustomResourceDefinitionList]
FAIL    github.com/rancher/steve/pkg/schema/converter [build failed]
ok      github.com/rancher/steve/pkg/podimpersonation   0.925s
# github.com/rancher/steve/pkg/schema/definitions [github.com/rancher/steve/pkg/schema/definitions.test]
pkg/schema/definitions/handler_test.go:186:92: cannot use ctrl (variable of type *"go.uber.org/mock/gomock".Controller)
as *"github.com/golang/mock/gomock".Controller value in argument to
fake.NewMockNonNamespacedCacheInterface[*apiextv1.CustomResourceDefinition]
pkg/schema/definitions/handler_test.go:658:92: cannot use ctrl (variable of type *"go.uber.org/mock/gomock".Controller)
as *"github.com/golang/mock/gomock".Controller value in argument to
fake.NewMockNonNamespacedCacheInterface[*apiextv1.CustomResourceDefinition]
pkg/schema/definitions/schema_test.go:21:140: cannot use ctrl (variable of type *"go.uber.org/mock/gomock".Controller)
as *"github.com/golang/mock/gomock".Controller value in argument to
fake.NewMockNonNamespacedControllerInterface[*apiextv1.CustomResourceDefinition, *apiextv1.CustomResourceDefinitionList]
pkg/schema/definitions/schema_test.go:22:115: cannot use ctrl (variable of type *"go.uber.org/mock/gomock".Controller)
as *"github.com/golang/mock/gomock".Controller value in argument to
fake.NewMockNonNamespacedControllerInterface[*apiregv1.APIService, *apiregv1.APIServiceList]
FAIL    github.com/rancher/steve/pkg/schema/definitions [build failed]

@aruiz14
Copy link
Contributor

aruiz14 commented Oct 9, 2024

@ericpromislow Oh I see, because it uses both mocks generated within this repository and wrangler's generic/fake mock controllers.

We could still undo the change in the imports for the files that use "external mocks":

git checkout main -- pkg/schema/converter/crd_test.go pkg/schema/converter/k8stonorman_test.go pkg/schema/definitions/handler_test.go pkg/schema/definitions/schema_test.go
go mod tidy

Although that would bring back the deprecated gomock to the direct dependency (it's not a problem IMO).

Nonetheless, this is just a suggestion, as I thought that transitively upgrading wrangler (to an rc version) as a side effect of changing a development dependency was not desirable and could be done separately (or maybe just re-title the PR?)

Copy link
Contributor

@tomleb tomleb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The wrangle bump is necessary because the tests depends on those mocks being compatible.

Copy link
Contributor

@gehrkefc gehrkefc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ericpromislow ericpromislow merged commit 6a11ffb into rancher:main Oct 10, 2024
1 check passed
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 this pull request may close these issues.

4 participants