Address #3923 feedback (#3938)

.golangci.yml

@@ -83,8 +83,9 @@ linters-settings:
         arguments:
           - "fmt.Printf"
 issues:
+  # Exclude cyclomatic and cognitive complexity rules for functional tests in the `tests` root directory.
   exclude-rules:
-    - path: tests\/.+\.go
+    - path: ^tests\/.+\.go
       text: "(cyclomatic|cognitive)"
       linters:
         - revive

service/frontend/workflow_handler.go

@@ -3071,7 +3071,9 @@ func (wh *WorkflowHandler) validateStartWorkflowArgsForSchedule(
 		return errIDReusePolicyNotAllowed
 	}
 
-	// Unalias startWorkflow search attributes only for validation. Keep aliases in request.
+	// Unalias startWorkflow search attributes only for validation.
+	// Keep aliases in the request, because the request will be
+	// sent back to frontend to start workflows, which will unalias at that point.
 	unaliasedStartWorkflowSas, err := searchattribute.UnaliasFields(wh.saMapperProvider, startWorkflow.GetSearchAttributes(), namespaceName.String())
 	if err != nil {
 		return err
@@ -3170,6 +3172,8 @@ func (wh *WorkflowHandler) DescribeSchedule(ctx context.Context, request *workfl
 			return err
 		}
 
+		// Search attributes in the Action are already in external ("aliased") form. Do not alias them here.
+
 		// for all running workflows started by the schedule, we should check that they're
 		// still running, and if not, poke the schedule to refresh
 		needRefresh := false

tests/advanced_visibility_test.go

@@ -84,12 +84,11 @@ func (s *advancedVisibilitySuite) isElasticsearchEnabled() bool {
 
 // This cluster use customized threshold for history config
 func (s *advancedVisibilitySuite) SetupSuite() {
-	if TestFlags.PersistenceDriver == mysql.PluginNameV8 ||
-		TestFlags.PersistenceDriver == postgresql.PluginNameV12 ||
-		TestFlags.PersistenceDriver == sqlite.PluginName {
+	switch TestFlags.PersistenceDriver {
+	case mysql.PluginNameV8, postgresql.PluginNameV12, sqlite.PluginName:
 		s.setupSuite("testdata/integration_test_cluster.yaml")
 		s.Logger.Info(fmt.Sprintf("Running advanced visibility test with %s/%s persistence", TestFlags.PersistenceType, TestFlags.PersistenceDriver))
-	} else {
+	default:
 		s.setupSuite("testdata/integration_test_es_cluster.yaml")
 		s.Logger.Info("Running advanced visibility test with Elasticsearch persistence")
 		s.esClient = CreateESClient(&s.Suite, s.testClusterConfig.ESConfig, s.Logger)
@@ -1782,9 +1781,9 @@ func (s *advancedVisibilitySuite) TestUpsertWorkflowExecution_InvalidKey() {
 	s.Error(err)
 	s.IsType(&serviceerror.InvalidArgument{}, err)
 	if s.isElasticsearchEnabled() {
-		s.Equal("BadSearchAttributes: search attribute INVALIDKEY is not defined", err.Error())
+		s.ErrorContains(err, "BadSearchAttributes: search attribute INVALIDKEY is not defined")
 	} else {
-		s.Equal(fmt.Sprintf("BadSearchAttributes: Namespace %s has no mapping defined for search attribute INVALIDKEY", s.namespace), err.Error())
+		s.ErrorContains(err, fmt.Sprintf("BadSearchAttributes: Namespace %s has no mapping defined for search attribute INVALIDKEY", s.namespace))
 	}
 
 	historyResponse, err := s.engine.GetWorkflowExecutionHistory(NewContext(), &workflowservice.GetWorkflowExecutionHistoryRequest{

tests/namespace_delete_test.go

@@ -83,27 +83,26 @@ func dynamicConfig() map[dynamicconfig.Key]interface{} {
 func (s *namespaceTestSuite) SetupSuite() {
 	s.logger = log.NewTestLogger()
 
-	var clusterConfig *TestClusterConfig
-	if TestFlags.PersistenceDriver == mysql.PluginNameV8 ||
-		TestFlags.PersistenceDriver == postgresql.PluginNameV12 ||
-		TestFlags.PersistenceDriver == sqlite.PluginName {
+	switch TestFlags.PersistenceDriver {
+	case mysql.PluginNameV8, postgresql.PluginNameV12, sqlite.PluginName:
 		var err error
-		clusterConfig, err = GetTestClusterConfig("testdata/integration_test_cluster.yaml")
+		s.clusterConfig, err = GetTestClusterConfig("testdata/integration_test_cluster.yaml")
 		s.Require().NoError(err)
-	} else {
+		s.logger.Info(fmt.Sprintf("Running delete namespace tests with %s/%s persistence", TestFlags.PersistenceType, TestFlags.PersistenceDriver))
+	default:
 		var err error
-		clusterConfig, err = GetTestClusterConfig("testdata/integration_test_es_cluster.yaml")
+		s.clusterConfig, err = GetTestClusterConfig("testdata/integration_test_es_cluster.yaml")
 		s.Require().NoError(err)
+		s.logger.Info("Running delete namespace tests with Elasticsearch persistence")
 		// Elasticsearch is needed to test advanced visibility code path in reclaim resources workflow.
-		s.esClient = CreateESClient(&s.Suite, clusterConfig.ESConfig, s.logger)
-		PutIndexTemplate(&s.Suite, s.esClient, fmt.Sprintf("testdata/es_%s_index_template.json", clusterConfig.ESConfig.Version), "test-visibility-template")
-		CreateIndex(&s.Suite, s.esClient, clusterConfig.ESConfig.GetVisibilityIndex())
+		s.esClient = CreateESClient(&s.Suite, s.clusterConfig.ESConfig, s.logger)
+		PutIndexTemplate(&s.Suite, s.esClient, fmt.Sprintf("testdata/es_%s_index_template.json", s.clusterConfig.ESConfig.Version), "test-visibility-template")
+		CreateIndex(&s.Suite, s.esClient, s.clusterConfig.ESConfig.GetVisibilityIndex())
 	}
 
-	s.clusterConfig = clusterConfig
-	clusterConfig.DynamicConfigOverrides = dynamicConfig()
+	s.clusterConfig.DynamicConfigOverrides = dynamicConfig()
 
-	cluster, err := NewCluster(clusterConfig, s.logger)
+	cluster, err := NewCluster(s.clusterConfig, s.logger)
 	s.Require().NoError(err)
 	s.cluster = cluster
 	s.frontendClient = s.cluster.GetFrontendClient()

tests/schedule_test.go

@@ -96,12 +96,13 @@ func (s *scheduleIntegrationSuite) SetupSuite() {
 	if TestFlags.FrontendAddr != "" {
 		s.hostPort = TestFlags.FrontendAddr
 	}
-	if TestFlags.PersistenceDriver == mysql.PluginNameV8 ||
-		TestFlags.PersistenceDriver == postgresql.PluginNameV12 ||
-		TestFlags.PersistenceDriver == sqlite.PluginName {
+	switch TestFlags.PersistenceDriver {
+	case mysql.PluginNameV8, postgresql.PluginNameV12, sqlite.PluginName:
 		s.setupSuite("testdata/integration_test_cluster.yaml")
-	} else {
+		s.Logger.Info(fmt.Sprintf("Running schedule tests with %s/%s persistence", TestFlags.PersistenceType, TestFlags.PersistenceDriver))
+	default:
 		s.setupSuite("testdata/integration_test_es_cluster.yaml")
+		s.Logger.Info("Running schedule tests with Elasticsearch persistence")
 		s.esClient = CreateESClient(&s.Suite, s.testClusterConfig.ESConfig, s.Logger)
 		PutIndexTemplate(&s.Suite, s.esClient, fmt.Sprintf("testdata/es_%s_index_template.json", s.testClusterConfig.ESConfig.Version), "test-visibility-template")
 		CreateIndex(&s.Suite, s.esClient, s.testClusterConfig.ESConfig.GetVisibilityIndex())

tests/xdc/advanced_visibility_test.go

@@ -102,14 +102,14 @@ func (s *advVisCrossDCTestSuite) isElasticsearchEnabled() bool {
 func (s *advVisCrossDCTestSuite) SetupSuite() {
 	s.logger = log.NewTestLogger()
 	var fileName string
-	isElasticsearchEnabled := false
-	if tests.TestFlags.PersistenceDriver == mysql.PluginNameV8 ||
-		tests.TestFlags.PersistenceDriver == postgresql.PluginNameV12 ||
-		tests.TestFlags.PersistenceDriver == sqlite.PluginName {
+	var isElasticsearchEnabled bool
+	switch tests.TestFlags.PersistenceDriver {
+	case mysql.PluginNameV8, postgresql.PluginNameV12, sqlite.PluginName:
 		// NOTE: can't use xdc_integration_test_clusters.yaml here because it somehow interferes with the other xDC tests.
 		fileName = "../testdata/xdc_integration_adv_vis_clusters.yaml"
+		isElasticsearchEnabled = false
 		s.logger.Info(fmt.Sprintf("Running xDC advanced visibility test with %s/%s persistence", tests.TestFlags.PersistenceType, tests.TestFlags.PersistenceDriver))
-	} else {
+	default:
 		fileName = "../testdata/xdc_integration_adv_vis_es_clusters.yaml"
 		isElasticsearchEnabled = true
 		s.logger.Info("Running xDC advanced visibility test with Elasticsearch persistence")