Enforcing go vet -copylocks and fixing current violations (cadence-wo…

…rkflow#5967)

Thankfully this all appears to be test-related and not any real risks,
but I realized while writing some other code that `go test` doesn't check this.

I'm really not sure what scenarios it might be wrong in, but it catches loads
of incorrect concurrent code in my experience.  So now it's required.

Makefile

@@ -360,6 +360,8 @@ $(BUILD)/gomod-lint: go.mod internal/tools/go.mod common/archiver/gcloud/go.mod
 # it's a coarse "you probably don't need to re-lint" filter, nothing more.
 $(BUILD)/code-lint: $(LINT_SRC) $(BIN)/revive | $(BUILD)
 	$Q echo "lint..."
+	$Q # non-optional vet checks.  unfortunately these are not currently included in `go test`'s default behavior.
+	$Q go vet -copylocks ./... ./common/archiver/gcloud/...
 	$Q $(BIN)/revive -config revive.toml -exclude './vendor/...' -exclude './.gen/...' -formatter stylish ./...
 	$Q # look for go files with "//comments", and ignore "//go:build"-style directives ("grep -n" shows "file:line: //go:build" so the regex is a bit complex)
 	$Q bad="$$(find . -type f -name '*.go' -not -path './idls/*' | xargs grep -n -E '^\s*//\S' | grep -E -v '^[^:]+:[^:]+:\s*//[a-z]+:[a-z]+' || true)"; \

common/domain/handler_MasterCluster_test.go

@@ -990,7 +990,7 @@ func (s *domainHandlerGlobalDomainEnabledPrimaryClusterSuite) setupGlobalDomain(
 	return setupGlobalDomain(s.Suite, s.handler, s.ClusterMetadata, domainName)
 }
 
-func setupGlobalDomain(s suite.Suite, handler *handlerImpl, clusterMetadata cluster.Metadata, domainName string) *types.DescribeDomainResponse {
+func setupGlobalDomain(s *suite.Suite, handler *handlerImpl, clusterMetadata cluster.Metadata, domainName string) *types.DescribeDomainResponse {
 	description := "some random description"
 	email := "some random email"
 	retention := int32(7)
@@ -1030,7 +1030,7 @@ func setupGlobalDomain(s suite.Suite, handler *handlerImpl, clusterMetadata clus
 	return getResp
 }
 
-func setupLocalDomain(s suite.Suite, handler *handlerImpl, clusterMetadata cluster.Metadata, domainName string) *types.DescribeDomainResponse {
+func setupLocalDomain(s *suite.Suite, handler *handlerImpl, clusterMetadata cluster.Metadata, domainName string) *types.DescribeDomainResponse {
 	description := "some random description"
 	email := "some random email"
 	retention := int32(7)
@@ -1060,8 +1060,8 @@ func setupLocalDomain(s suite.Suite, handler *handlerImpl, clusterMetadata clust
 	return getResp
 }
 
-func assertDomainEqual(s suite.Suite, autual, expected *types.DescribeDomainResponse) {
-	s.NotEmpty(autual.DomainInfo.GetUUID())
-	expected.DomainInfo.UUID = autual.DomainInfo.GetUUID()
-	s.Equal(expected, autual)
+func assertDomainEqual(s *suite.Suite, actual, expected *types.DescribeDomainResponse) {
+	s.NotEmpty(actual.DomainInfo.GetUUID())
+	expected.DomainInfo.UUID = actual.DomainInfo.GetUUID()
+	s.Equal(expected, actual)
 }

common/domain/handler_NotMasterCluster_test.go

@@ -767,7 +767,7 @@ func (s *domainHandlerGlobalDomainEnabledNotPrimaryClusterSuite) setupGlobalDoma
 	return setupGlobalDomainWithMetadataManager(s.Suite, s.handler, s.ClusterMetadata, s.DomainManager, domainName)
 }
 
-func setupGlobalDomainWithMetadataManager(s suite.Suite, handler *handlerImpl, clusterMetadata cluster.Metadata, domainManager persistence.DomainManager, domainName string) *types.DescribeDomainResponse {
+func setupGlobalDomainWithMetadataManager(s *suite.Suite, handler *handlerImpl, clusterMetadata cluster.Metadata, domainManager persistence.DomainManager, domainName string) *types.DescribeDomainResponse {
 	description := "some random description"
 	email := "some random email"
 	retention := int32(7)

common/persistence/persistence-tests/historyV2PersistenceTest.go

@@ -101,7 +101,7 @@ func (s *HistoryV2PersistenceSuite) TearDownSuite() {
 // TestGenUUIDs testing  uuid.New() can generate unique UUID
 func (s *HistoryV2PersistenceSuite) TestGenUUIDs() {
 	wg := sync.WaitGroup{}
-	m := sync.Map{}
+	m := &sync.Map{}
 	concurrency := 1000
 	for i := 0; i < concurrency; i++ {
 		wg.Add(1)
@@ -377,7 +377,7 @@ func (s *HistoryV2PersistenceSuite) TestConcurrentlyCreateAndAppendBranches() {
 	treeID := uuid.New()
 	wg := sync.WaitGroup{}
 	concurrency := 20
-	m := sync.Map{}
+	m := &sync.Map{}
 
 	// test create new branch along with appending new nodes
 	for i := 0; i < concurrency; i++ {
@@ -538,8 +538,8 @@ func (s *HistoryV2PersistenceSuite) TestConcurrentlyForkAndAppendBranches() {
 	s.Nil(err)
 	s.Equal((concurrency)+1, len(events))
 
-	level1ID := sync.Map{}
-	level1Br := sync.Map{}
+	level1ID := &sync.Map{}
+	level1Br := &sync.Map{}
 	// test forking from master branch and append nodes
 	for i := 0; i < concurrency; i++ {
 		wg.Add(1)
@@ -588,7 +588,7 @@ func (s *HistoryV2PersistenceSuite) TestConcurrentlyForkAndAppendBranches() {
 	branches = s.descTreeByToken(ctx, masterBr)
 	s.Equal(concurrency, len(branches))
 	forkOnLevel1 := int32(0)
-	level2Br := sync.Map{}
+	level2Br := &sync.Map{}
 	wg = sync.WaitGroup{}
 
 	// test forking for second level of branch
@@ -687,14 +687,14 @@ func (s *HistoryV2PersistenceSuite) TestConcurrentlyForkAndAppendBranches() {
 
 }
 
-func (s *HistoryV2PersistenceSuite) getBranchByKey(m sync.Map, k int) []byte {
+func (s *HistoryV2PersistenceSuite) getBranchByKey(m *sync.Map, k int) []byte {
 	v, ok := m.Load(k)
 	s.Equal(true, ok)
 	br := v.([]byte)
 	return br
 }
 
-func (s *HistoryV2PersistenceSuite) getIDByKey(m sync.Map, k int) int64 {
+func (s *HistoryV2PersistenceSuite) getIDByKey(m *sync.Map, k int) int64 {
 	v, ok := m.Load(k)
 	s.Equal(true, ok)
 	id := v.(int64)

common/persistence/persistence-tests/persistenceTestBase.go

@@ -74,7 +74,7 @@ type (
 
 	// TestBase wraps the base setup needed to create workflows over persistence layer.
 	TestBase struct {
-		suite.Suite
+		*suite.Suite
 		ShardMgr                  persistence.ShardManager
 		ExecutionMgrFactory       client.Factory
 		ExecutionManager          persistence.ExecutionManager
@@ -114,6 +114,7 @@ const (
 // NewTestBaseFromParams returns a customized test base from given input
 func NewTestBaseFromParams(t *testing.T, params TestBaseParams) *TestBase {
 	res := &TestBase{
+		Suite:                 &suite.Suite{},
 		DefaultTestCluster:    params.DefaultTestCluster,
 		VisibilityTestCluster: params.VisibilityTestCluster,
 		ClusterMetadata:       params.ClusterMetadata,

host/pinotutils/pinotClient.go

@@ -29,7 +29,7 @@ import (
 	pnt "github.com/uber/cadence/common/pinot"
 )
 
-func CreatePinotClient(s suite.Suite, pinotConfig *config.PinotVisibilityConfig, logger log.Logger) pnt.GenericClient {
+func CreatePinotClient(s *suite.Suite, pinotConfig *config.PinotVisibilityConfig, logger log.Logger) pnt.GenericClient {
 	pinotRawClient, err := pinot.NewFromBrokerList([]string{pinotConfig.Broker})
 	s.Require().NoError(err)
 	pinotClient := pnt.NewPinotClient(pinotRawClient, logger, pinotConfig)