From 4a4dfead350f5d334a22e6e09d2a6bb49de4358e Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Tue, 1 Feb 2022 12:15:36 -0500 Subject: [PATCH] bazel,kvclient/rangefeed,util/log: stop using source mockgen generation The source mode for mockgen is maddening because it requires the entire go tree to be copied out. This is excedingly slow. We fix that by not using that mode. To remedy that we have to export the interfaces we're mocking. I can eat that. Ideally we'd put the log one into a build-tagged away file, but github.com/jmhodges/bazel_gomock doesn't provide a way for us to plumb tags into its generator. That would be easy to fix, but not right now. This commit also removes some references to the `--source` flag in go:generate directives where they weren't doing anything. This commit also removes some previously checked in generated mock files. I wonder if that's going to cause pain? Release note: None --- Makefile | 12 +++++++++--- build/bazelutil/check.sh | 10 +++++----- pkg/cmd/roachtest/prometheus/prometheus.go | 2 +- pkg/cmd/roachtest/tests/drt.go | 2 +- pkg/kv/kvclient/rangefeed/BUILD.bazel | 3 +-- pkg/kv/kvclient/rangefeed/db_adapter.go | 10 +++++----- pkg/kv/kvclient/rangefeed/helpers_test.go | 4 ++-- pkg/kv/kvclient/rangefeed/rangefeed.go | 12 ++++++------ pkg/kv/kvclient/rangefeed/rangefeed_mock_test.go | 4 ++-- pkg/security/certmgr/cert.go | 2 +- pkg/util/log/BUILD.bazel | 12 +++++++----- .../{mock_generated.go => mocks_generated_test.go} | 12 ++++++------ pkg/util/log/sinks.go | 10 +++++++++- 13 files changed, 55 insertions(+), 40 deletions(-) rename pkg/util/log/{mock_generated.go => mocks_generated_test.go} (85%) diff --git a/Makefile b/Makefile index baf1f01d9da0..c1b9e50fbb15 100644 --- a/Makefile +++ b/Makefile @@ -801,7 +801,11 @@ GENERATED_TARGETS = \ pkg/kv/kvclient/rangefeed/mocks_generated.go \ pkg/roachprod/vm/aws/embedded.go \ pkg/security/securitytest/embedded.go \ - pkg/security/certmgr/mocks_generated.go + pkg/security/certmgr/mocks_generated.go \ + pkg/kv/kvclient/kvcoord/mocks_generated.go \ + pkg/kv/kvclient/rangecache/mocks_generated.go \ + pkg/roachpb/mocks_generated.go \ + pkg/util/log/mock_generated.go EXECGEN_TARGETS = \ pkg/col/coldata/vec.eg.go \ @@ -972,6 +976,8 @@ BUILD_TAGGED_RELEASE = ## Override for .buildinfo/tag BUILDINFO_TAG := +$(GENERATED_TARGETS): $(PROTOBUF_TARGETS) $(OPGEN_TARGETS) $(EXECGEN_TARGETS) $(SQLPARSER_TARGETS) + $(go-targets): bin/.bootstrap $(BUILDINFO) $(CGO_FLAGS_FILES) $(PROTOBUF_TARGETS) $(LIBPROJ) $(GENERATED_TARGETS) $(CLEANUP_TARGETS) $(go-targets): $(LOG_TARGETS) $(SQLPARSER_TARGETS) $(OPTGEN_TARGETS) $(go-targets): override LINKFLAGS += \ @@ -1452,10 +1458,10 @@ ui-maintainer-clean: ## Like clean, but also remove installed dependencies ui-maintainer-clean: ui-clean rm -rf pkg/ui/node_modules pkg/ui/workspaces/db-console/node_modules pkg/ui/yarn.installed pkg/ui/workspaces/cluster-ui/node_modules -pkg/cmd/roachtest/prometheus/mock_generated.go: bin/.bootstrap pkg/cmd/roachtest/prometheus/prometheus.go +pkg/cmd/roachtest/prometheus/mock_generated.go: bin/.bootstrap pkg/cmd/roachtest/prometheus/prometheus.go pkg/roachprod/vm/aws/embedded.go pkg/security/securitytest/embedded.go $(OPTGEN_TARGETS) (cd pkg/cmd/roachtest/prometheus && $(GO) generate) -pkg/cmd/roachtest/tests/drt_generated.go: bin/.bootstrap pkg/cmd/roachtest/tests/drt.go +pkg/cmd/roachtest/tests/drt_generated.go: bin/.bootstrap pkg/cmd/roachtest/tests/drt.go pkg/roachprod/vm/aws/embedded.go $(OPTGEN_TARGETS) pkg/security/securitytest/embedded.go (cd pkg/cmd/roachtest/tests && $(GO) generate) pkg/kv/kvclient/rangefeed/mocks_generated.go: bin/.bootstrap pkg/kv/kvclient/rangefeed/rangefeed.go diff --git a/build/bazelutil/check.sh b/build/bazelutil/check.sh index f5602ba93098..7f11d7626bcc 100755 --- a/build/bazelutil/check.sh +++ b/build/bazelutil/check.sh @@ -10,16 +10,16 @@ pkg/roachprod/vm/aws/config.go://go:generate go-bindata -mode 0600 -modtime 1400 pkg/roachprod/vm/aws/config.go://go:generate gofmt -s -w embedded.go pkg/roachprod/vm/aws/config.go://go:generate goimports -w embedded.go pkg/roachprod/vm/aws/config.go://go:generate terraformgen -o terraform/main.tf -pkg/cmd/roachtest/prometheus/prometheus.go://go:generate mockgen -package=prometheus -destination=mock_generated.go -source=prometheus.go . Cluster -pkg/cmd/roachtest/tests/drt.go://go:generate mockgen -source drt.go -package tests -destination drt_generated.go +pkg/cmd/roachtest/prometheus/prometheus.go://go:generate mockgen -package=prometheus -destination=mock_generated.go . Cluster +pkg/cmd/roachtest/tests/drt.go://go:generate mockgen -package tests -destination drt_generated.go . PromClient pkg/kv/kvclient/kvcoord/transport.go://go:generate mockgen -package=kvcoord -destination=mocks_generated.go . Transport pkg/kv/kvclient/rangecache/range_cache.go://go:generate mockgen -package=rangecache -destination=mocks_generated.go . RangeDescriptorDB -pkg/kv/kvclient/rangefeed/rangefeed.go://go:generate mockgen -package=rangefeed -source rangefeed.go -destination=mocks_generated.go . +pkg/kv/kvclient/rangefeed/rangefeed.go://go:generate mockgen -destination=mocks_generated.go --package=rangefeed . DB pkg/kv/kvserver/concurrency/lock_table.go://go:generate ../../../util/interval/generic/gen.sh *lockState concurrency pkg/kv/kvserver/spanlatch/manager.go://go:generate ../../../util/interval/generic/gen.sh *latch spanlatch pkg/roachpb/api.go://go:generate mockgen -package=roachpb -destination=mocks_generated.go . InternalClient,Internal_RangeFeedClient pkg/roachpb/batch.go://go:generate go run -tags gen-batch gen/main.go -pkg/security/certmgr/cert.go://go:generate mockgen -package=certmgr -destination=mocks_generated.go -source=cert.go . Cert +pkg/security/certmgr/cert.go://go:generate mockgen -package=certmgr -destination=mocks_generated.go . Cert pkg/security/securitytest/securitytest.go://go:generate go-bindata -mode 0600 -modtime 1400000000 -pkg securitytest -o embedded.go -ignore README.md -ignore regenerate.sh test_certs pkg/security/securitytest/securitytest.go://go:generate gofmt -s -w embedded.go pkg/security/securitytest/securitytest.go://go:generate goimports -w embedded.go @@ -39,7 +39,7 @@ pkg/util/log/channels.go://go:generate go run gen/main.go logpb/log.proto channe pkg/util/log/channels.go://go:generate go run gen/main.go logpb/log.proto log_channels.go log_channels_generated.go pkg/util/log/channels.go://go:generate go run gen/main.go logpb/log.proto logging.md ../../../docs/generated/logging.md pkg/util/log/channels.go://go:generate go run gen/main.go logpb/log.proto severity.go severity/severity_generated.go -pkg/util/log/sinks.go://go:generate mockgen -package=log -source=sinks.go -destination=mock_generated.go -mock_names=logSink=MockLogSink logSink +pkg/util/log/sinks.go://go:generate mockgen -package=log -destination=mocks_generated_test.go --mock_names=TestingLogSink=MockLogSink . TestingLogSink pkg/util/timeutil/zoneinfo.go://go:generate go run gen/main.go " diff --git a/pkg/cmd/roachtest/prometheus/prometheus.go b/pkg/cmd/roachtest/prometheus/prometheus.go index 7f66d806698d..284d6ac7bdc8 100644 --- a/pkg/cmd/roachtest/prometheus/prometheus.go +++ b/pkg/cmd/roachtest/prometheus/prometheus.go @@ -8,7 +8,7 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -//go:generate mockgen -package=prometheus -destination=mock_generated.go -source=prometheus.go . Cluster +//go:generate mockgen -package=prometheus -destination=mock_generated.go . Cluster package prometheus diff --git a/pkg/cmd/roachtest/tests/drt.go b/pkg/cmd/roachtest/tests/drt.go index 37c1812bfb74..a47bf59af0f6 100644 --- a/pkg/cmd/roachtest/tests/drt.go +++ b/pkg/cmd/roachtest/tests/drt.go @@ -10,7 +10,7 @@ package tests -//go:generate mockgen -source drt.go -package tests -destination drt_generated.go +//go:generate mockgen -package tests -destination drt_generated.go . PromClient import ( "context" diff --git a/pkg/kv/kvclient/rangefeed/BUILD.bazel b/pkg/kv/kvclient/rangefeed/BUILD.bazel index c2a4a95d80aa..1e67090112b4 100644 --- a/pkg/kv/kvclient/rangefeed/BUILD.bazel +++ b/pkg/kv/kvclient/rangefeed/BUILD.bazel @@ -48,11 +48,10 @@ go_library( gomock( name = "mock_rangefeed", out = "mocks_generated.go", - interfaces = [""], # work-around for https://github.com/jmhodges/bazel_gomock/issues/58 + interfaces = ["DB"], library = ":rangefeed", package = "rangefeed", self_package = "github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed", - source = "rangefeed.go", ) go_library( diff --git a/pkg/kv/kvclient/rangefeed/db_adapter.go b/pkg/kv/kvclient/rangefeed/db_adapter.go index b0884b093f82..237993e4ed8c 100644 --- a/pkg/kv/kvclient/rangefeed/db_adapter.go +++ b/pkg/kv/kvclient/rangefeed/db_adapter.go @@ -29,14 +29,14 @@ import ( "github.com/cockroachdb/errors" ) -// dbAdapter is an implementation of the kvDB interface using a real *kv.DB. +// dbAdapter is an implementation of the DB interface using a real *kv.DB. type dbAdapter struct { db *kv.DB st *cluster.Settings distSender *kvcoord.DistSender } -var _ kvDB = (*dbAdapter)(nil) +var _ DB = (*dbAdapter)(nil) var maxScanParallelism = settings.RegisterIntSetting( settings.TenantWritable, @@ -45,7 +45,7 @@ var maxScanParallelism = settings.RegisterIntSetting( 64, ) -// newDBAdapter construct a kvDB using a *kv.DB. +// newDBAdapter construct a DB using a *kv.DB. func newDBAdapter(db *kv.DB, st *cluster.Settings) (*dbAdapter, error) { var distSender *kvcoord.DistSender { @@ -67,7 +67,7 @@ func newDBAdapter(db *kv.DB, st *cluster.Settings) (*dbAdapter, error) { }, nil } -// RangeFeed is part of the kvDB interface. +// RangeFeed is part of the DB interface. func (dbc *dbAdapter) RangeFeed( ctx context.Context, spans []roachpb.Span, @@ -96,7 +96,7 @@ func (ba *concurrentBoundAccount) Shrink(ctx context.Context, x int64) { ba.BoundAccount.Shrink(ctx, x) } -// Scan is part of the kvDB interface. +// Scan is part of the DB interface. func (dbc *dbAdapter) Scan( ctx context.Context, spans []roachpb.Span, diff --git a/pkg/kv/kvclient/rangefeed/helpers_test.go b/pkg/kv/kvclient/rangefeed/helpers_test.go index ed905bbbbcb3..8f3ef171ac0c 100644 --- a/pkg/kv/kvclient/rangefeed/helpers_test.go +++ b/pkg/kv/kvclient/rangefeed/helpers_test.go @@ -23,8 +23,8 @@ var NewDBAdapter = newDBAdapter // NewFactoryWithDB allows tests to construct a factory with an injected db. var NewFactoryWithDB = newFactory -// KVDB forwards the definition of kvDB to tests. -type KVDB = kvDB +// KVDB forwards the definition of DB to tests. +type KVDB = DB // ScanConfig forwards the definition of scanConfig to tests. type ScanConfig = scanConfig diff --git a/pkg/kv/kvclient/rangefeed/rangefeed.go b/pkg/kv/kvclient/rangefeed/rangefeed.go index 53fde038e7b8..cad1b4c3988a 100644 --- a/pkg/kv/kvclient/rangefeed/rangefeed.go +++ b/pkg/kv/kvclient/rangefeed/rangefeed.go @@ -33,7 +33,7 @@ import ( "github.com/cockroachdb/logtags" ) -//go:generate mockgen -package=rangefeed -source rangefeed.go -destination=mocks_generated.go . +//go:generate mockgen -destination=mocks_generated.go --package=rangefeed . DB // TODO(ajwerner): Expose hooks for metrics. // TODO(ajwerner): Expose access to checkpoints and the frontier. @@ -42,8 +42,8 @@ import ( // TODO(yevgeniy): Instead of rolling our own logic to parallelize scans, we should // use streamer API instead (https://github.com/cockroachdb/cockroach/pull/68430) -// kvDB is an adapter to the underlying KV store. -type kvDB interface { +// DB is an adapter to the underlying KV store. +type DB interface { // RangeFeed runs a rangefeed on a given span with the given arguments. // It encapsulates the RangeFeed method on roachpb.Internal. @@ -71,7 +71,7 @@ type kvDB interface { // Factory is used to construct RangeFeeds. type Factory struct { stopper *stop.Stopper - client kvDB + client DB knobs *TestingKnobs } @@ -98,7 +98,7 @@ func NewFactory( return newFactory(stopper, kvDB, knobs), nil } -func newFactory(stopper *stop.Stopper, client kvDB, knobs *TestingKnobs) *Factory { +func newFactory(stopper *stop.Stopper, client DB, knobs *TestingKnobs) *Factory { return &Factory{ stopper: stopper, client: client, @@ -154,7 +154,7 @@ type OnValue func(ctx context.Context, value *roachpb.RangeFeedValue) type RangeFeed struct { config name string - client kvDB + client DB stopper *stop.Stopper knobs *TestingKnobs diff --git a/pkg/kv/kvclient/rangefeed/rangefeed_mock_test.go b/pkg/kv/kvclient/rangefeed/rangefeed_mock_test.go index 9d6c102e7aea..d104c05834e8 100644 --- a/pkg/kv/kvclient/rangefeed/rangefeed_mock_test.go +++ b/pkg/kv/kvclient/rangefeed/rangefeed_mock_test.go @@ -69,7 +69,7 @@ func (m *mockClient) Scan( var _ rangefeed.KVDB = (*mockClient)(nil) -// TestRangefeedMock utilizes the kvDB interface to test the behavior of the +// TestRangefeedMock utilizes the DB interface to test the behavior of the // RangeFeed. func TestRangeFeedMock(t *testing.T) { defer leaktest.AfterTest(t)() @@ -347,7 +347,7 @@ func TestBackoffOnRangefeedFailure(t *testing.T) { defer stopper.Stop(ctx) ctrl := gomock.NewController(t) - db := rangefeed.NewMockkvDB(ctrl) + db := rangefeed.NewMockDB(ctrl) // Make sure scan failure gets retried. db.EXPECT().Scan(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("scan failed")) diff --git a/pkg/security/certmgr/cert.go b/pkg/security/certmgr/cert.go index 8baa3e394e0a..7ee44fdb2775 100644 --- a/pkg/security/certmgr/cert.go +++ b/pkg/security/certmgr/cert.go @@ -15,7 +15,7 @@ import ( "crypto/tls" ) -//go:generate mockgen -package=certmgr -destination=mocks_generated.go -source=cert.go . Cert +//go:generate mockgen -package=certmgr -destination=mocks_generated.go . Cert // Cert is a generic presentation on managed certificate that can be reloaded. // A managed certificate allows the certificates to be rotated by a cert manager. diff --git a/pkg/util/log/BUILD.bazel b/pkg/util/log/BUILD.bazel index 808e522aeae0..065298b4e77f 100644 --- a/pkg/util/log/BUILD.bazel +++ b/pkg/util/log/BUILD.bazel @@ -1,7 +1,7 @@ load("@bazel_gomock//:gomock.bzl", "gomock") load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") -# gazelle:exclude mock_generated.go +# gazelle:exclude mocks_generated_test.go go_library( name = "log", @@ -206,10 +206,12 @@ genrule( gomock( name = "mock_logsink", - out = "mock_generated.go", - interfaces = ["logSink"], + out = "mocks_generated_test.go", + interfaces = [ + "TestingLogSink", + ], library = ":log", - mock_names = {"logSink": "MockLogSink"}, + mock_names = {"TestingLogSink": "MockLogSink"}, package = "log", - source = "sinks.go", + self_package = "github.com/cockroachdb/cockroach/pkg/util/log", ) diff --git a/pkg/util/log/mock_generated.go b/pkg/util/log/mocks_generated_test.go similarity index 85% rename from pkg/util/log/mock_generated.go rename to pkg/util/log/mocks_generated_test.go index 8696d918b5c8..33b9d10ebda1 100644 --- a/pkg/util/log/mock_generated.go +++ b/pkg/util/log/mocks_generated_test.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: sinks.go +// Source: github.com/cockroachdb/cockroach/pkg/util/log (interfaces: TestingLogSink) // Package log is a generated GoMock package. package log @@ -11,7 +11,7 @@ import ( gomock "github.com/golang/mock/gomock" ) -// MockLogSink is a mock of logSink interface. +// MockLogSink is a mock of TestingLogSink interface. type MockLogSink struct { ctrl *gomock.Controller recorder *MockLogSinkMockRecorder @@ -77,15 +77,15 @@ func (mr *MockLogSinkMockRecorder) exitCode() *gomock.Call { } // output mocks base method. -func (m *MockLogSink) output(b []byte, opts sinkOutputOptions) error { +func (m *MockLogSink) output(arg0 []byte, arg1 sinkOutputOptions) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "output", b, opts) + ret := m.ctrl.Call(m, "output", arg0, arg1) ret0, _ := ret[0].(error) return ret0 } // output indicates an expected call of output. -func (mr *MockLogSinkMockRecorder) output(b, opts interface{}) *gomock.Call { +func (mr *MockLogSinkMockRecorder) output(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "output", reflect.TypeOf((*MockLogSink)(nil).output), b, opts) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "output", reflect.TypeOf((*MockLogSink)(nil).output), arg0, arg1) } diff --git a/pkg/util/log/sinks.go b/pkg/util/log/sinks.go index 270896cce18d..f265e8ee3aaa 100644 --- a/pkg/util/log/sinks.go +++ b/pkg/util/log/sinks.go @@ -12,7 +12,15 @@ package log import "github.com/cockroachdb/cockroach/pkg/cli/exit" -//go:generate mockgen -package=log -source=sinks.go -destination=mock_generated.go -mock_names=logSink=MockLogSink logSink +//go:generate mockgen -package=log -destination=mocks_generated_test.go --mock_names=TestingLogSink=MockLogSink . TestingLogSink + +// TestingLogSink is exported for mock generation. +// This is painful, but it seems to be the only way, for the moment, to +// generate this mock. +// +// The reason is that there's no way to inject build tags into the current +// bazel rules for gomock. +type TestingLogSink = logSink // sinkOutputOptions provides various options for a logSink.output call. type sinkOutputOptions struct {