Refactor: move functional test suite scaffolding code to FunctionalTe…

…stBase (#7085)

## What changed?
<!-- Describe what has changed in this PR -->
Main change is: move functional test suite scaffolding code to
`FunctionalTestBase`. Most of the scaffolding code and test helpers
(more to move) are now in `FunctionalTestBase`. Hierarchy is the
following:
```
- FunctionalTestBase - base for all functional tests
  - FunctionalTestSuite - has TaskPoller in it and should be used as base for all non-SDK based tests
    - many test suites such as TestUpdateWorkflowSuite.
    - ...
    - ...
  - FunctionalTestSdkSuite - has SdkClient and SdkWorker in it and should be used as base for all SDK based tests
    - many test suites such as UpdateWorkflowSdkSuite
    - ...
    - ...
  - some test suites which don't need neither task poller nor SDK. Such as TLSFunctionalSuite
```


Few minor refactorings:
- replace `SetDynamicConfigOverrides` method with
`WithDynamicConfigOverrides` option for `SetupTestCluster()` method.
More DC cleanups are needed there.
- Many renames for better readability. 
- Left few TODOs for myself, for later.

## Why?
<!-- Tell your future self why have you made these changes -->

## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Run it.

## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
No risks.

## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->
No.

## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
No.

common/testing/taskpoller/taskpoller.go

@@ -28,7 +28,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"testing"
 	"time"
 
 	"github.com/nexus-rpc/sdk-go/nexus"
@@ -47,8 +46,11 @@ import (
 )
 
 type (
+	Helper interface {
+		Helper()
+	}
 	TaskPoller struct {
-		t         *testing.T
+		t         Helper
 		client    workflowservice.WorkflowServiceClient
 		namespace string
 	}
@@ -95,7 +97,7 @@ var (
 )
 
 func New(
-	t *testing.T,
+	t Helper,
 	client workflowservice.WorkflowServiceClient,
 	namespace string,
 ) *TaskPoller {

tests/acquire_shard_test.go

@@ -57,7 +57,7 @@ func (s *AcquireShardSuiteBase) SetupSuite() {
 func (s *AcquireShardSuiteBase) TearDownSuite() {
 	// we need to wait for all components to start before we can safely tear down
 	time.Sleep(time.Second * 5) //nolint:forbidigo
-	s.FunctionalTestBase.TearDownSuite()
+	s.FunctionalTestBase.TearDownCluster()
 }
 
 // newLogRecorder creates a new log recorder. It records all the logs to the given channel.
@@ -126,7 +126,7 @@ func TestAcquireShard_OwnershipLostErrorSuite(t *testing.T) {
 // SetupSuite reads the shard ownership lost error fault injection config from the testdata folder.
 func (s *OwnershipLostErrorSuite) SetupSuite() {
 	s.AcquireShardSuiteBase.SetupSuite()
-	s.FunctionalTestBase.SetupSuite("testdata/acquire_shard_ownership_lost_error.yaml")
+	s.FunctionalTestBase.SetupSuiteWithCluster("testdata/acquire_shard_ownership_lost_error.yaml")
 }
 
 // TestDoesNotRetry verifies that we do not retry acquiring the shard when we get an ownership lost error.
@@ -175,7 +175,7 @@ func TestAcquireShard_DeadlineExceededErrorSuite(t *testing.T) {
 // SetupSuite reads the deadline exceeded error targeted fault injection config from the test data folder.
 func (s *DeadlineExceededErrorSuite) SetupSuite() {
 	s.AcquireShardSuiteBase.SetupSuite()
-	s.FunctionalTestBase.SetupSuite("testdata/acquire_shard_deadline_exceeded_error.yaml")
+	s.FunctionalTestBase.SetupSuiteWithCluster("testdata/acquire_shard_deadline_exceeded_error.yaml")
 }
 
 // TestDoesRetry verifies that we do retry acquiring the shard when we get a deadline exceeded error because that should
@@ -226,7 +226,7 @@ func TestAcquireShard_EventualSuccess(t *testing.T) {
 // the next call to return a successful response.
 func (s *EventualSuccessSuite) SetupSuite() {
 	s.AcquireShardSuiteBase.SetupSuite()
-	s.FunctionalTestBase.SetupSuite("testdata/acquire_shard_eventual_success.yaml")
+	s.FunctionalTestBase.SetupSuiteWithCluster("testdata/acquire_shard_eventual_success.yaml")
 }
 
 // TestEventuallySucceeds verifies that we eventually succeed in acquiring the shard when we get a deadline exceeded

tests/activity_api_pause_test.go

@@ -43,7 +43,7 @@ import (
 )
 
 type ActivityApiPauseClientTestSuite struct {
-	testcore.ClientFunctionalSuite
+	testcore.FunctionalTestSdkSuite
 	tv                     *testvars.TestVars
 	initialRetryInterval   time.Duration
 	scheduleToCloseTimeout time.Duration
@@ -53,13 +53,13 @@ type ActivityApiPauseClientTestSuite struct {
 }
 
 func (s *ActivityApiPauseClientTestSuite) SetupSuite() {
-	s.ClientFunctionalSuite.SetupSuite()
+	s.FunctionalTestSdkSuite.SetupSuite()
 	s.OverrideDynamicConfig(dynamicconfig.ActivityAPIsEnabled, true)
 	s.tv = testvars.New(s.T()).WithTaskQueue(s.TaskQueue()).WithNamespaceName(s.Namespace())
 }
 
 func (s *ActivityApiPauseClientTestSuite) SetupTest() {
-	s.ClientFunctionalSuite.SetupTest()
+	s.FunctionalTestSdkSuite.SetupTest()
 
 	s.initialRetryInterval = 1 * time.Second
 	s.scheduleToCloseTimeout = 30 * time.Minute

tests/activity_api_reset_test.go

@@ -45,7 +45,7 @@ import (
 )
 
 type ActivityApiResetClientTestSuite struct {
-	testcore.ClientFunctionalSuite
+	testcore.FunctionalTestSdkSuite
 	tv                     *testvars.TestVars
 	initialRetryInterval   time.Duration
 	scheduleToCloseTimeout time.Duration
@@ -55,13 +55,13 @@ type ActivityApiResetClientTestSuite struct {
 }
 
 func (s *ActivityApiResetClientTestSuite) SetupSuite() {
-	s.ClientFunctionalSuite.SetupSuite()
+	s.FunctionalTestSdkSuite.SetupSuite()
 	s.OverrideDynamicConfig(dynamicconfig.ActivityAPIsEnabled, true)
 	s.tv = testvars.New(s.T()).WithTaskQueue(s.TaskQueue()).WithNamespaceName(s.Namespace())
 }
 
 func (s *ActivityApiResetClientTestSuite) SetupTest() {
-	s.ClientFunctionalSuite.SetupTest()
+	s.FunctionalTestSdkSuite.SetupTest()
 
 	s.initialRetryInterval = 1 * time.Second
 	s.scheduleToCloseTimeout = 30 * time.Minute

tests/activity_api_update_test.go

@@ -48,18 +48,18 @@ import (
 )
 
 type ActivityApiUpdateClientTestSuite struct {
-	testcore.ClientFunctionalSuite
+	testcore.FunctionalTestSdkSuite
 	tv *testvars.TestVars
 }
 
 func (s *ActivityApiUpdateClientTestSuite) SetupSuite() {
-	s.ClientFunctionalSuite.SetupSuite()
+	s.FunctionalTestSdkSuite.SetupSuite()
 	s.OverrideDynamicConfig(dynamicconfig.ActivityAPIsEnabled, true)
 	s.tv = testvars.New(s.T()).WithTaskQueue(s.TaskQueue()).WithNamespaceName(s.Namespace())
 }
 
 func (s *ActivityApiUpdateClientTestSuite) SetupTest() {
-	s.ClientFunctionalSuite.SetupTest()
+	s.FunctionalTestSdkSuite.SetupTest()
 }
 
 func TestActivityApiUpdateClientTestSuite(t *testing.T) {

tests/activity_test.go

@@ -61,11 +61,11 @@ import (
 )
 
 type ActivityTestSuite struct {
-	testcore.FunctionalSuite
+	testcore.FunctionalTestSuite
 }
 
 type ActivityClientTestSuite struct {
-	testcore.ClientFunctionalSuite
+	testcore.FunctionalTestSdkSuite
 }
 
 func TestActivityTestSuite(t *testing.T) {
@@ -313,9 +313,7 @@ func (s *ActivityClientTestSuite) Test_ActivityTimeouts() {
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 	workflowRun, err := s.SdkClient().ExecuteWorkflow(ctx, workflowOptions, workflowFn)
-	if err != nil {
-		s.Logger.Fatal("Start workflow failed with err", tag.Error(err))
-	}
+	s.NoError(err)
 
 	s.NotNil(workflowRun)
 	s.True(workflowRun.GetRunID() != "")
@@ -1242,9 +1240,7 @@ func (s *ActivityClientTestSuite) TestActivityHeartbeatDetailsDuringRetry() {
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 	workflowRun, err := s.SdkClient().ExecuteWorkflow(ctx, workflowOptions, workflowFn)
-	if err != nil {
-		s.Logger.Fatal("Start workflow failed with err", tag.Error(err))
-	}
+	s.NoError(err)
 
 	s.NotNil(workflowRun)
 	s.True(workflowRun.GetRunID() != "")

tests/add_tasks_test.go

@@ -33,7 +33,6 @@ import (
 	"time"
 
 	"github.com/pborman/uuid"
-	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	sdkclient "go.temporal.io/sdk/client"
 	"go.temporal.io/sdk/worker"
@@ -58,8 +57,8 @@ import (
 type (
 	// AddTasksSuite is a separate suite because we need to override the history service's executable wrapper.
 	AddTasksSuite struct {
-		testcore.FunctionalTestBase
-		*require.Assertions
+		testcore.FunctionalTestSuite
+
 		shardController *faultyShardController
 		worker          worker.Worker
 		sdkClient       sdkclient.Client
@@ -142,39 +141,29 @@ func (e *noopExecutor) shouldExecute(task tasks.Task) bool {
 
 // SetupSuite creates the test cluster and registers the executorWrapper with the history service.
 func (s *AddTasksSuite) SetupSuite() {
-	// We do this here and in SetupTest because we need assertions in the SetupSuite method as well as the individual
-	// tests, but this is called before SetupTest, and the s.T() value will change when SetupTest is called.
-	s.Assertions = require.New(s.T())
 	// Set up the test cluster and register our executable wrapper.
-	s.FunctionalTestBase.SetupSuite("testdata/es_cluster.yaml",
-		testcore.WithFxOptionsForService(
-			primitives.HistoryService,
-			fx.Provide(
-				func() queues.ExecutorWrapper {
-					return &executorWrapper{s: s}
-				},
-			),
-			fx.Decorate(
-				func(c shard.Controller) shard.Controller {
-					s.shardController = &faultyShardController{Controller: c, s: s}
-					return s.shardController
-				},
-			),
+	s.FunctionalTestSuite.SetupSuiteWithDefaultCluster(testcore.WithFxOptionsForService(
+		primitives.HistoryService,
+		fx.Provide(
+			func() queues.ExecutorWrapper {
+				return &executorWrapper{s: s}
+			},
+		),
+		fx.Decorate(
+			func(c shard.Controller) shard.Controller {
+				s.shardController = &faultyShardController{Controller: c, s: s}
+				return s.shardController
+			},
 		),
+	),
 	)
 	// Get an SDK client so that we can call ExecuteWorkflow.
 	s.sdkClient = s.newSDKClient()
 }
 
 func (s *AddTasksSuite) TearDownSuite() {
 	s.sdkClient.Close()
-	s.FunctionalTestBase.TearDownSuite()
-}
-
-func (s *AddTasksSuite) SetupTest() {
-	s.FunctionalTestBase.SetupTest()
-
-	s.Assertions = require.New(s.T())
+	s.FunctionalTestSuite.TearDownSuite()
 }
 
 func (s *AddTasksSuite) TestAddTasks_Ok() {
@@ -284,6 +273,6 @@ func (s *AddTasksSuite) newSDKClient() sdkclient.Client {
 		HostPort:  s.FrontendGRPCAddress(),
 		Namespace: s.Namespace().String(),
 	})
-	s.NoError(err)
+	s.Require().NoError(err)
 	return client
 }

tests/admin_test.go

@@ -40,7 +40,7 @@ import (
 )
 
 type AdminTestSuite struct {
-	testcore.ClientFunctionalSuite
+	testcore.FunctionalTestSdkSuite
 }
 
 func TestAdminTestSuite(t *testing.T) {

tests/advanced_visibility_test.go

@@ -36,7 +36,6 @@ import (
 
 	"github.com/pborman/uuid"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	commandpb "go.temporal.io/api/command/v1"
 	commonpb "go.temporal.io/api/common/v1"
@@ -60,7 +59,6 @@ import (
 	esclient "go.temporal.io/server/common/persistence/visibility/store/elasticsearch/client"
 	"go.temporal.io/server/common/primitives"
 	"go.temporal.io/server/common/searchattribute"
-	"go.temporal.io/server/common/testing/historyrequire"
 	"go.temporal.io/server/common/testing/protorequire"
 	"go.temporal.io/server/common/worker_versioning"
 	"go.temporal.io/server/service/worker/scanner/build_ids"
@@ -76,12 +74,8 @@ const (
 )
 
 type AdvancedVisibilitySuite struct {
-	// override suite.Suite.Assertions with require.Assertions; this means that s.NotNil(nil) will stop the test,
-	// not merely log an error
-	*require.Assertions
-	protorequire.ProtoAssertions
-	historyrequire.HistoryRequire
-	testcore.FunctionalTestBase
+	testcore.FunctionalTestSuite
+
 	isElasticsearchEnabled bool
 
 	testSearchAttributeKey string
@@ -110,14 +104,12 @@ func (s *AdvancedVisibilitySuite) SetupSuite() {
 		dynamicconfig.RemovableBuildIdDurationSinceDefault.Key(): time.Microsecond,
 	}
 
-	s.SetDynamicConfigOverrides(dynamicConfigOverrides)
-
 	if testcore.UsingSQLAdvancedVisibility() {
-		s.FunctionalTestBase.SetupSuite("testdata/cluster.yaml")
+		s.FunctionalTestSuite.SetupSuiteWithCluster("testdata/cluster.yaml", testcore.WithDynamicConfigOverrides(dynamicConfigOverrides))
 		s.Logger.Info(fmt.Sprintf("Running advanced visibility test with %s/%s persistence", testcore.TestFlags.PersistenceType, testcore.TestFlags.PersistenceDriver))
 		s.isElasticsearchEnabled = false
 	} else {
-		s.FunctionalTestBase.SetupSuite("testdata/es_cluster.yaml")
+		s.FunctionalTestSuite.SetupSuiteWithCluster("testdata/es_cluster.yaml", testcore.WithDynamicConfigOverrides(dynamicConfigOverrides))
 		s.Logger.Info("Running advanced visibility test with Elasticsearch persistence")
 		s.isElasticsearchEnabled = true
 		// To ensure that Elasticsearch won't return more than defaultPageSize documents,
@@ -126,36 +118,28 @@ func (s *AdvancedVisibilitySuite) SetupSuite() {
 		s.updateMaxResultWindow()
 	}
 
-	sdkClient, err := sdkclient.Dial(sdkclient.Options{
+	var err error
+	s.sdkClient, err = sdkclient.Dial(sdkclient.Options{
 		HostPort:  s.FrontendGRPCAddress(),
 		Namespace: s.Namespace().String(),
 	})
-	if err != nil {
-		s.Logger.Fatal("Error when creating SDK client", tag.Error(err))
-	}
-	s.sdkClient = sdkClient
-	sysSDKClient, err := sdkclient.Dial(sdkclient.Options{
+	s.Require().NoError(err)
+
+	s.sysSDKClient, err = sdkclient.Dial(sdkclient.Options{
 		HostPort:  s.FrontendGRPCAddress(),
 		Namespace: primitives.SystemLocalNamespace,
 	})
-	if err != nil {
-		s.Logger.Fatal("Error when creating SDK client", tag.Error(err))
-	}
-	s.sysSDKClient = sysSDKClient
+	s.Require().NoError(err)
 }
 
 func (s *AdvancedVisibilitySuite) TearDownSuite() {
 	s.sdkClient.Close()
-	s.FunctionalTestBase.TearDownSuite()
+	s.FunctionalTestSuite.TearDownSuite()
 }
 
 func (s *AdvancedVisibilitySuite) SetupTest() {
-	s.FunctionalTestBase.SetupTest()
+	s.FunctionalTestSuite.SetupTest()
 
-	// Have to define our overridden assertions in the test setup. If we did it earlier, s.T() will return nil
-	s.Assertions = require.New(s.T())
-	s.ProtoAssertions = protorequire.New(s.T())
-	s.HistoryRequire = historyrequire.New(s.T())
 	s.testSearchAttributeKey = "CustomTextField"
 	s.testSearchAttributeVal = "test value"
 }
@@ -2808,7 +2792,7 @@ func (s *AdvancedVisibilitySuite) updateMaxResultWindow() {
 		}
 		time.Sleep(waitTimeInMs * time.Millisecond) //nolint:forbidigo
 	}
-	s.FailNow(fmt.Sprintf("ES max result window size hasn't reach target size within %v", (numOfRetry*waitTimeInMs)*time.Millisecond))
+	s.Require().FailNowf("", "ES max result window size hasn't reach target size within %v", numOfRetry*waitTimeInMs*time.Millisecond)
 }
 
 func (s *AdvancedVisibilitySuite) addCustomKeywordSearchAttribute(ctx context.Context, attrName string) {