From 85c43814301a30f622f526d3765cabb2dd36071a Mon Sep 17 00:00:00 2001 From: Alex Shtin Date: Fri, 10 Feb 2023 19:33:05 -0800 Subject: [PATCH] Address #3923 feedback (#3938) --- .golangci.yml | 3 ++- service/frontend/workflow_handler.go | 6 +++++- tests/advanced_visibility_test.go | 11 +++++------ tests/namespace_delete_test.go | 25 ++++++++++++------------- tests/schedule_test.go | 9 +++++---- tests/xdc/advanced_visibility_test.go | 10 +++++----- 6 files changed, 34 insertions(+), 30 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 603db3a315c..9e72900d42d 100644 --- a/.golangci.yml +++ b/.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 diff --git a/service/frontend/workflow_handler.go b/service/frontend/workflow_handler.go index 56065b7cfeb..f614773478a 100644 --- a/service/frontend/workflow_handler.go +++ b/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 diff --git a/tests/advanced_visibility_test.go b/tests/advanced_visibility_test.go index 27719d776ad..9ad3e171488 100644 --- a/tests/advanced_visibility_test.go +++ b/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{ diff --git a/tests/namespace_delete_test.go b/tests/namespace_delete_test.go index 9879b4c8531..c31d6a9da64 100644 --- a/tests/namespace_delete_test.go +++ b/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() diff --git a/tests/schedule_test.go b/tests/schedule_test.go index 677bffe93ac..0b0b12a0a26 100644 --- a/tests/schedule_test.go +++ b/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()) diff --git a/tests/xdc/advanced_visibility_test.go b/tests/xdc/advanced_visibility_test.go index a2b3987a1bb..748d1658bef 100644 --- a/tests/xdc/advanced_visibility_test.go +++ b/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")