From 288ab7c4d2bc41a37142af290bd473a224c97798 Mon Sep 17 00:00:00 2001 From: Josh Smith Date: Tue, 15 Nov 2022 05:57:26 +1000 Subject: [PATCH 1/4] Extend gomock to allow passing an `source_importpath` instead of `library` when operating in source mode. This enables a use case where mocks can be included as part of the source files in a go_library. Added additional test cases and documentation. --- README.rst | 2 + docs/go/extras/extras.md | 7 ++-- extras/gomock.bzl | 27 ++++++++++---- tests/extras/gomock/README.rst | 17 +++++++++ tests/extras/gomock/reflective/BUILD.bazel | 34 +++++++++++++++++ .../extras/gomock/{ => reflective}/client.go | 0 tests/extras/gomock/reflective/client_test.go | 3 ++ tests/extras/gomock/{ => source}/BUILD.bazel | 0 tests/extras/gomock/source/client.go | 10 +++++ tests/extras/gomock/source/client_test.go | 3 ++ .../gomock/source_with_importpath/BUILD.bazel | 37 +++++++++++++++++++ .../gomock/source_with_importpath/client.go | 10 +++++ .../client_test.go | 0 13 files changed, 140 insertions(+), 10 deletions(-) create mode 100644 tests/extras/gomock/README.rst create mode 100644 tests/extras/gomock/reflective/BUILD.bazel rename tests/extras/gomock/{ => reflective}/client.go (100%) create mode 100644 tests/extras/gomock/reflective/client_test.go rename tests/extras/gomock/{ => source}/BUILD.bazel (100%) create mode 100644 tests/extras/gomock/source/client.go create mode 100644 tests/extras/gomock/source/client_test.go create mode 100644 tests/extras/gomock/source_with_importpath/BUILD.bazel create mode 100644 tests/extras/gomock/source_with_importpath/client.go rename tests/extras/gomock/{ => source_with_importpath}/client_test.go (100%) diff --git a/README.rst b/README.rst index c47d56249d..4a92eb77de 100644 --- a/README.rst +++ b/README.rst @@ -54,6 +54,7 @@ Go rules for Bazel_ .. _go_cross_binary: docs/go/core/rules.md#go_cross_binary .. _go_toolchain: go/toolchains.rst#go_toolchain .. _go_wrap_sdk: go/toolchains.rst#go_wrap_sdk +.. _gomock: docs/go/extras/extras.md#gomock .. External rules .. _git_repository: https://docs.bazel.build/versions/master/repo/git.html @@ -209,6 +210,7 @@ Documentation * `go_context`_ * `Extra rules `_ + * `gomock`_ * `nogo build-time static analysis`_ * `Build modes `_ diff --git a/docs/go/extras/extras.md b/docs/go/extras/extras.md index b66a4dcbd2..b3eb5be1df 100644 --- a/docs/go/extras/extras.md +++ b/docs/go/extras/extras.md @@ -33,8 +33,8 @@ This rule has moved. See [gazelle rule] in the Gazelle repository. ## gomock 
-gomock(name, library, out, source, interfaces, package, self_package, aux_files, mockgen_tool,
-       imports, copyright_file, mock_names, kwargs)
+gomock(name, out, library, source_importpath, source, interfaces, package, self_package, aux_files,
+       mockgen_tool, imports, copyright_file, mock_names, kwargs)
Calls [mockgen](https://github.com/golang/mock) to generates a Go file containing mocks from the given library. @@ -48,8 +48,9 @@ If `source` is given, the mocks are generated in source mode; otherwise in refle | Name | Description | Default Value | | :------------- | :------------- | :------------- | | name | the target name. | none | -| library | the Go library to took for the interfaces (reflecitve mode) or source (source mode). | none | | out | the output Go file name. | none | +| library | the Go library to look into for the interfaces (reflective mode) or source (source mode). If running in source mode, you can specify source_importpath instead of this parameter. | None | +| source_importpath | the importpath for the source file. Alternative to passing library, which can lead to circular dependencies between mock and library targets. Only valid for source mode. | "" | | source | a Go file in the given library. If this is given, gomock will call mockgen in source mode to mock all interfaces in the file. | None | | interfaces | a list of interfaces in the given library to be mocked in reflective mode. | [] | | package | the name of the package the generated mocks should be in. If not specified, uses mockgen's default. See [mockgen's -package](https://github.com/golang/mock#flags) for more information. | "" | diff --git a/extras/gomock.bzl b/extras/gomock.bzl index 78bb3c2e5e..2d1b092e77 100644 --- a/extras/gomock.bzl +++ b/extras/gomock.bzl @@ -34,10 +34,17 @@ _MOCKGEN_MODEL_LIB = Label("//extras/gomock:mockgen_model") def _gomock_source_impl(ctx): go_ctx = go_context(ctx) + # In Source mode, it's not necessary to pass through a library, as the only thing we use it for is setting up + # the relative file locations. Forcing users to pass a library makes it difficult in the case where a mock should + # be included as part of that same library, as it results in a dependency loop (GoMock -> GoLibrary -> GoMock). + # Allowing users to pass an importpath directly bypasses this issue. + # See the test case in //tests/extras/gomock/source_with_importpath for an example. + importpath = ctx.attr.source_importpath if ctx.attr.source_importpath else ctx.attr.library[GoLibrary].importmap + # create GOPATH and copy source into GOPATH go_path_prefix = "gopath" - source_relative_path = paths.join("src", ctx.attr.library[GoLibrary].importmap, ctx.file.source.basename) - source = ctx.actions.declare_file(paths.join(go_path_prefix, source_relative_path)) + source_relative_path = paths.join("src", importpath, ctx.file.source.basename) + source = ctx.actions.declare_file(paths.join("gopath", source_relative_path)) # trim the relative path of source to get GOPATH gopath = source.path[:-len(source_relative_path)] @@ -107,7 +114,11 @@ _gomock_source = rule( "library": attr.label( doc = "The target the Go library where this source file belongs", providers = [GoLibrary], - mandatory = True, + mandatory = False, + ), + "source_importpath": attr.string( + doc = "The importpath for the source file. Alternative to passing library, which can lead to circular dependencies between mock and library targets.", + mandatory = False, ), "source": attr.label( doc = "A Go source file to find all the interfaces to generate mocks for. See also the docs for library.", @@ -156,15 +167,16 @@ _gomock_source = rule( toolchains = [GO_TOOLCHAIN], ) -def gomock(name, library, out, source = None, interfaces = [], package = "", self_package = "", aux_files = {}, mockgen_tool = _MOCKGEN_TOOL, imports = {}, copyright_file = None, mock_names = {}, **kwargs): +def gomock(name, out, library = None, source_importpath = "", source = None, interfaces = [], package = "", self_package = "", aux_files = {}, mockgen_tool = _MOCKGEN_TOOL, imports = {}, copyright_file = None, mock_names = {}, **kwargs): """Calls [mockgen](https://github.com/golang/mock) to generates a Go file containing mocks from the given library. If `source` is given, the mocks are generated in source mode; otherwise in reflective mode. Args: name: the target name. - library: the Go library to took for the interfaces (reflecitve mode) or source (source mode). out: the output Go file name. + library: the Go library to look into for the interfaces (reflective mode) or source (source mode). If running in source mode, you can specify source_importpath instead of this parameter. + source_importpath: the importpath for the source file. Alternative to passing library, which can lead to circular dependencies between mock and library targets. Only valid for source mode. source: a Go file in the given `library`. If this is given, `gomock` will call mockgen in source mode to mock all interfaces in the file. interfaces: a list of interfaces in the given `library` to be mocked in reflective mode. package: the name of the package the generated mocks should be in. If not specified, uses mockgen's default. See [mockgen's -package](https://github.com/golang/mock#flags) for more information. @@ -179,8 +191,9 @@ def gomock(name, library, out, source = None, interfaces = [], package = "", sel if source: _gomock_source( name = name, - library = library, out = out, + library = library, + source_importpath = source_importpath, source = source, package = package, self_package = self_package, @@ -194,8 +207,8 @@ def gomock(name, library, out, source = None, interfaces = [], package = "", sel else: _gomock_reflect( name = name, - library = library, out = out, + library = library, interfaces = interfaces, package = package, self_package = self_package, diff --git a/tests/extras/gomock/README.rst b/tests/extras/gomock/README.rst new file mode 100644 index 0000000000..b59af8ad2e --- /dev/null +++ b/tests/extras/gomock/README.rst @@ -0,0 +1,17 @@ +gomock +===================== + +Tests that ensure the gomock rules can be called correctly under different input permutations. + +reflective +------------------------ +Checks that gomock can be run in "reflective" mode when passed a `GoLibrary` and `interfaces`. + +source +------------------------ +Checks that gomock can be run in "source" mode when passed a `GoLibrary` and `source`. + +source_with_importpath +------------------------ +Checks that gomock can be run in "source" mode when passed an `importpath` and `source`. +This test case also demonstrates the circumstance in which `importpath` is necessary to prevent a circular dependency. diff --git a/tests/extras/gomock/reflective/BUILD.bazel b/tests/extras/gomock/reflective/BUILD.bazel new file mode 100644 index 0000000000..85d90308d5 --- /dev/null +++ b/tests/extras/gomock/reflective/BUILD.bazel @@ -0,0 +1,34 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test", "gomock") + +go_library( + name = "client", + srcs = [ + "client.go", + ], + importpath = "github.com/bazelbuild/rules_go/gomock/client", + visibility = ["//visibility:public"], + deps = [ + "@go_googleapis//google/bytestream:bytestream_go_proto", + "@org_golang_google_grpc//:grpc", + ], +) + +# Build the mocks using reflective mode (i.e. without passing source) +gomock( + name = "mocks", + out = "client_mock.go", + library = ":client", + package = "client", + interfaces = ["Client"], + visibility = ["//visibility:public"], +) + +go_test( + name = "client_test", + srcs = [ + "client_mock.go", + "client_test.go", + ], + embed = [":client"], + deps = ["@com_github_golang_mock//gomock"], +) diff --git a/tests/extras/gomock/client.go b/tests/extras/gomock/reflective/client.go similarity index 100% rename from tests/extras/gomock/client.go rename to tests/extras/gomock/reflective/client.go diff --git a/tests/extras/gomock/reflective/client_test.go b/tests/extras/gomock/reflective/client_test.go new file mode 100644 index 0000000000..92180b8761 --- /dev/null +++ b/tests/extras/gomock/reflective/client_test.go @@ -0,0 +1,3 @@ +package client + +var _ Client = (*MockClient)(nil) diff --git a/tests/extras/gomock/BUILD.bazel b/tests/extras/gomock/source/BUILD.bazel similarity index 100% rename from tests/extras/gomock/BUILD.bazel rename to tests/extras/gomock/source/BUILD.bazel diff --git a/tests/extras/gomock/source/client.go b/tests/extras/gomock/source/client.go new file mode 100644 index 0000000000..b8cd7ea254 --- /dev/null +++ b/tests/extras/gomock/source/client.go @@ -0,0 +1,10 @@ +package client + +import ( + "google.golang.org/genproto/googleapis/bytestream" + "google.golang.org/grpc" +) + +type Client interface { + Connect(grpc.ClientConnInterface) *bytestream.ByteStreamClient +} diff --git a/tests/extras/gomock/source/client_test.go b/tests/extras/gomock/source/client_test.go new file mode 100644 index 0000000000..92180b8761 --- /dev/null +++ b/tests/extras/gomock/source/client_test.go @@ -0,0 +1,3 @@ +package client + +var _ Client = (*MockClient)(nil) diff --git a/tests/extras/gomock/source_with_importpath/BUILD.bazel b/tests/extras/gomock/source_with_importpath/BUILD.bazel new file mode 100644 index 0000000000..effd379325 --- /dev/null +++ b/tests/extras/gomock/source_with_importpath/BUILD.bazel @@ -0,0 +1,37 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test", "gomock") + +# For this test, the mock is included as part of the library +go_library( + name = "client", + srcs = [ + "client.go", + "client_mock.go", + ], + importpath = "github.com/bazelbuild/rules_go/gomock/client", + visibility = ["//visibility:public"], + deps = [ + "@go_googleapis//google/bytestream:bytestream_go_proto", + "@org_golang_google_grpc//:grpc", + "@com_github_golang_mock//gomock", + ], +) + +# Pass importpath instead of library to the generation step +# Passing library instead of importpath here will cause a circular dependency +gomock( + name = "mocks", + out = "client_mock.go", + source_importpath = "github.com/bazelbuild/rules_go/gomock/client", + package = "client", + source = "client.go", + visibility = ["//visibility:public"], +) + +# Don't include client_mock.go as a source file, instead use it from the library +go_test( + name = "client_test", + srcs = [ + "client_test.go", + ], + embed = [":client"], +) diff --git a/tests/extras/gomock/source_with_importpath/client.go b/tests/extras/gomock/source_with_importpath/client.go new file mode 100644 index 0000000000..b8cd7ea254 --- /dev/null +++ b/tests/extras/gomock/source_with_importpath/client.go @@ -0,0 +1,10 @@ +package client + +import ( + "google.golang.org/genproto/googleapis/bytestream" + "google.golang.org/grpc" +) + +type Client interface { + Connect(grpc.ClientConnInterface) *bytestream.ByteStreamClient +} diff --git a/tests/extras/gomock/client_test.go b/tests/extras/gomock/source_with_importpath/client_test.go similarity index 100% rename from tests/extras/gomock/client_test.go rename to tests/extras/gomock/source_with_importpath/client_test.go From 1889d266518a09475deced87362deba1a210dbd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Herv=C3=A9?= Date: Tue, 9 Jan 2024 12:56:52 +0100 Subject: [PATCH 2/4] Fix bytestream refs --- tests/extras/gomock/reflective/BUILD.bazel | 2 +- tests/extras/gomock/source_with_importpath/BUILD.bazel | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/extras/gomock/reflective/BUILD.bazel b/tests/extras/gomock/reflective/BUILD.bazel index 85d90308d5..4313cae93b 100644 --- a/tests/extras/gomock/reflective/BUILD.bazel +++ b/tests/extras/gomock/reflective/BUILD.bazel @@ -8,7 +8,7 @@ go_library( importpath = "github.com/bazelbuild/rules_go/gomock/client", visibility = ["//visibility:public"], deps = [ - "@go_googleapis//google/bytestream:bytestream_go_proto", + "@org_golang_google_genproto//googleapis/bytestream", "@org_golang_google_grpc//:grpc", ], ) diff --git a/tests/extras/gomock/source_with_importpath/BUILD.bazel b/tests/extras/gomock/source_with_importpath/BUILD.bazel index effd379325..32308586c2 100644 --- a/tests/extras/gomock/source_with_importpath/BUILD.bazel +++ b/tests/extras/gomock/source_with_importpath/BUILD.bazel @@ -10,7 +10,7 @@ go_library( importpath = "github.com/bazelbuild/rules_go/gomock/client", visibility = ["//visibility:public"], deps = [ - "@go_googleapis//google/bytestream:bytestream_go_proto", + "@org_golang_google_genproto//googleapis/bytestream", "@org_golang_google_grpc//:grpc", "@com_github_golang_mock//gomock", ], From bc1d3a5e813afb91a1acff14a257a0baca07ae7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Herv=C3=A9?= Date: Tue, 9 Jan 2024 13:05:31 +0100 Subject: [PATCH 3/4] Move wrapper file --- tests/extras/gomock/{ => source}/client_wrapper.go | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/extras/gomock/{ => source}/client_wrapper.go (100%) diff --git a/tests/extras/gomock/client_wrapper.go b/tests/extras/gomock/source/client_wrapper.go similarity index 100% rename from tests/extras/gomock/client_wrapper.go rename to tests/extras/gomock/source/client_wrapper.go From 39a1d8d98c2dec498bfc4ec10e21915b33c22af6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Herv=C3=A9?= Date: Tue, 9 Jan 2024 13:13:59 +0100 Subject: [PATCH 4/4] Remove wrapper for importpath test --- tests/extras/gomock/source_with_importpath/client_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/extras/gomock/source_with_importpath/client_test.go b/tests/extras/gomock/source_with_importpath/client_test.go index 0f76af7821..92180b8761 100644 --- a/tests/extras/gomock/source_with_importpath/client_test.go +++ b/tests/extras/gomock/source_with_importpath/client_test.go @@ -1,4 +1,3 @@ package client var _ Client = (*MockClient)(nil) -var _ ClientWrapper = (*MockClientWrapper)(nil)