[testing] Remove dnsmock, lifecycle reorg, NewTest api

[titpetric]

PR Type

Enhancement, Tests

• pass genConf via *Test, add functional option for it
• add functional option to set the config explicitly
• default constructor should not take genConf (by usage it's 10:1)

---

Description

• Removed dnsmock dependency and related code from tests.

• Introduced NewTest API with functional options for test configuration.

• Refactored lifecycle management into a new test.go file.

• Simplified and streamlined test setup and teardown processes.

---

Changes walkthrough 📝

	Relevant files
Tests	reverse_proxy_test.go Remove `dnsmock` and update test setup                                      gateway/reverse_proxy_test.go Removed dnsmock usage and related setup/teardown calls. Updated test setup to use new flakySetupTestReverseProxyDnsCache. +0/-6
Enhancement	test.go Introduce `Test` struct and `NewTest` API                                gateway/test.go Added a new Test struct for managing test lifecycle. Introduced NewTest API with functional options for configuration. Implemented lifecycle methods like start and Close. +181/-0  testutil.go Remove `dnsmock` and refactor test utilities                          gateway/testutil.go Removed dnsmock initialization and related configuration. Deleted redundant Test struct and lifecycle methods. Simplified StartTest and removed unused fields. +0/-148

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[buger]

A JIRA Issue ID is missing from your branch name, PR title and PR description! 🦄

Your branch: testing/refactor-StartTest-scaffold

Your PR title: [testing] lifecycle reorg, NewTest api

Your PR description: null

If this is your first time contributing to this repository - welcome!

---

Please refer to jira-lint to get started.

Without the JIRA Issue ID in your branch name you would lose out on automatic updates to JIRA via SCM; some GitHub status checks might fail.

Valid sample branch names:

‣ feature/shiny-new-feature--mojo-10'
‣ 'chore/changelogUpdate_mojo-123'
‣ 'bugfix/fix-some-strange-bug_GAL-2345'

[github-actions]

API Changes

--- prev.txt	2025-02-16 16:43:23.356648986 +0000
+++ current.txt	2025-02-16 16:43:19.262599930 +0000
@@ -8102,10 +8102,6 @@
 	ErrSyncResourceNotKnown = errors.New("unknown resource to sync")
 )
 var (
-	EnableTestDNSMock = false
-	MockHandle        *test.DnsMockHandle
-)
-var (
 	VERSION = build.Version
 	Commit  = build.Commit
 )
@@ -8178,6 +8174,7 @@
 func NewPythonDispatcher(conf config.Config) (dispatcher coprocess.Dispatcher, err error)
     NewPythonDispatcher wraps all the actions needed for this CP.
 
+func NewTestConfigOption(conf TestConfig) func(*Test)
 func NormalisePath(a *analytics.AnalyticsRecord, globalConfig *config.Config)
 func ParseRSAPublicKey(data []byte) (interface{}, error)
 func ProtoMap(inputMap map[string][]string) map[string]string
@@ -10817,11 +10814,13 @@
 	Gw               *Gateway `json:"-"`
 	HttpHandler      *http.Server
 	TestServerRouter *mux.Router
-	MockHandle       *test.DnsMockHandle
 
+	Parent testing.TB
 	// Has unexported fields.
 }
 
+func NewTest(tb testing.TB, genConf func(*config.Config), opts ...TestOption) *Test
+
 func StartTest(genConf func(globalConf *config.Config), testConfig ...TestConfig) *Test
 
 func (s *Test) AddDynamicHandler(path string, handlerFunc http.HandlerFunc)
@@ -10829,6 +10828,7 @@
 func (s *Test) BundleHandleFunc(w http.ResponseWriter, r *http.Request)
 
 func (s *Test) Close()
+    Close is the shutdown lifecycle for a gateway integration test w/ storage.
 
 func (s *Test) CreatePolicy(pGen ...func(p *user.Policy)) string
 
@@ -10872,8 +10872,7 @@
 	Delay              time.Duration
 	HotReload          bool
 
-	CoprocessConfig   config.CoProcessConfig
-	EnableTestDNSMock bool
+	CoprocessConfig config.CoProcessConfig
 	// Has unexported fields.
 }
 
@@ -10887,6 +10886,8 @@
 	Form    map[string]string
 }
 
+type TestOption func(*Test)
+
 type TraceMiddleware struct {
 	TykMiddleware
 }

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The removal of pullDomains and MockHandle in flakySetupTestReverseProxyDnsCache and TestReverseProxyDnsCache may affect the functionality of DNS mocking. Ensure that the test behavior remains consistent and valid without these components. func (s *Test) flakySetupTestReverseProxyDnsCache(cfg *configTestReverseProxyDnsCache) func() { s.Gw.dnsCacheManager.InitDNSCaching( time.Duration(cfg.dnsConfig.TTL)*time.Second, time.Duration(cfg.dnsConfig.CheckInterval)*time.Second) globalConf := s.Gw.GetConfig() enableWebSockets := globalConf.HttpServerOptions.EnableWebSockets globalConf.HttpServerOptions.EnableWebSockets = true s.Gw.SetConfig(globalConf) return func() { s.Gw.dnsCacheManager.DisposeCache() globalConf.HttpServerOptions.EnableWebSockets = enableWebSockets s.Gw.SetConfig(globalConf) } } func TestReverseProxyDnsCache(t *testing.T) { test.Flaky(t) // TODO: TT-5251 const ( host = "orig-host.com." host2 = "orig-host2.com." host3 = "orig-host3.com." wsHost = "ws.orig-host.com." hostApiUrl = "http://orig-host.com/origpath" host2HttpApiUrl = "http://orig-host2.com/origpath" host2HttpsApiUrl = "https://orig-host2.com/origpath" host3ApiUrl = "https://orig-host3.com/origpath" wsHostWsApiUrl = "ws://ws.orig-host.com/connect" wsHostWssApiUrl = "wss://ws.orig-host.com/connect" cacheTTL = 5 cacheUpdateInterval = 10 ) var ( etcHostsMap = map[string][]string{ host: {"127.0.0.10", "127.0.0.20"}, host2: {"10.0.20.0", "10.0.20.1", "10.0.20.2"}, host3: {"10.0.20.15", "10.0.20.16"}, wsHost: {"127.0.0.10", "127.0.0.10"}, } ) ts := StartTest(nil) defer ts.Close() tearDown := ts.flakySetupTestReverseProxyDnsCache(&configTestReverseProxyDnsCache{t, etcHostsMap, config.DnsCacheConfig{ Enabled: true, TTL: cacheTTL, CheckInterval: cacheUpdateInterval, Lifecycle Management The new lifecycle management introduced in NewTest and start functions should be reviewed for completeness and correctness, particularly in how resources are initialized and cleaned up. func NewTest(tb testing.TB, genConf func(*config.Config), opts ...TestOption) *Test { tb.Helper() t := &Test{ Parent: tb, dynamicHandlers: make(map[string]http.HandlerFunc), } for _, optfn := range opts { optfn(t) } t.Gw = t.start(genConf) tb.Cleanup(t.Close) return t } func NewTestConfigOption(conf TestConfig) func(*Test) { return func(t *Test) { t.config = conf } } // Start is the root event point where a gateway object is created, and // can enforce lifecycle via the *Test objects, and TestOption implementation. // For example, if somebody wanted to have some default options set up, // one could set a timeout by implementing: // // - `func NewTestTimeoutOption(d time.Duration) func(*Test)` // // To use, it should be passed to NewTest as an argument. A default timeout // may be implemented in the future and set from NewTest as well. func (s *Test) start(genConf func(globalConf *config.Config)) *Gateway { // init and create gw ctx, cancel := context.WithCancel(context.Background()) log.Info("starting test") s.ctx = ctx s.cancel = func() { cancel() log.Info("Cancelling test context") } gw := s.newGateway(genConf) gw.setupPortsWhitelist() gw.startServer() gw.setupGlobals() // Set up a default org manager so we can traverse non-live paths if !gw.GetConfig().SupressDefaultOrgStore { gw.DefaultOrgStore.Init(gw.getGlobalStorageHandler("orgkey.", false)) gw.DefaultQuotaStore.Init(gw.getGlobalStorageHandler("orgkey.", false)) } s.GlobalConfig = gw.GetConfig() scheme := "http://" if s.GlobalConfig.HttpServerOptions.UseSSL { scheme = "https://" } s.URL = scheme + gw.DefaultProxyMux.getProxy(gw.GetConfig().ListenPort, gw.GetConfig()).listener.Addr().String() s.testRunner = &test.HTTPTestRunner{ RequestBuilder: func(tc *test.TestCase) (*http.Request, error) { tc.BaseURL = s.URL if tc.ControlRequest { if s.config.SeparateControlAPI { tc.BaseURL = scheme + s.controlProxy().listener.Addr().String() } else if s.GlobalConfig.ControlAPIHostname != "" { tc.Domain = s.GlobalConfig.ControlAPIHostname } } r, err := test.NewRequest(tc) if tc.AdminAuth { r = s.withAuth(r) } if s.config.Delay > 0 { tc.Delay = s.config.Delay } return r, err }, Do: test.HttpServerRunner(), } return gw } // Close is the shutdown lifecycle for a gateway integration test w/ storage. func (s *Test) Close() { defer s.cancel() for _, p := range s.Gw.DefaultProxyMux.proxies { if p.listener != nil { p.listener.Close() } } gwConfig := s.Gw.GetConfig() s.Gw.DefaultProxyMux.swap(&proxyMux{}, s.Gw) if s.config.SeparateControlAPI { gwConfig.ControlAPIPort = 0 s.Gw.SetConfig(gwConfig) } // if jsvm enabled we need to unmount to prevent high memory consumption if s.Gw.GetConfig().EnableJSVM { s.Gw.GlobalEventsJSVM.VM = nil } ctxShutDown, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() err := s.HttpHandler.Shutdown(ctxShutDown) if err != nil { log.WithError(err).Error("shutting down the http handler") } else { log.Info("server exited properly") } s.Gw.Analytics.Stop() s.Gw.ReloadTestCase.StopTicker() s.Gw.GlobalHostChecker.StopPoller() s.Gw.NewRelicApplication.Shutdown(5 * time.Second) err = s.RemoveApis() if err != nil { log.WithError(err).Error("could not remove apis") } s.Gw.cacheClose() } Removed DNS Mocking The removal of EnableTestDNSMock and MockHandle from TestConfig and Test struct may impact tests relying on DNS mocking. Verify that all dependent tests are updated accordingly. func StartTest(genConf func(globalConf *config.Config), testConfig ...TestConfig) *Test { t := &Test{ dynamicHandlers: make(map[string]http.HandlerFunc), } if len(testConfig) > 0 { t.config = testConfig[0] } t.Gw = t.start(genConf) return t } func (s *Test) AddDynamicHandler(path string, handlerFunc http.HandlerFunc) { path = strings.Trim(path, "/") s.dynamicHandlers[path] = handlerFunc } func (s *Test) newGateway(genConf func(globalConf *config.Config)) *Gateway { var gwConfig config.Config if err := config.WriteDefault("", &gwConfig); err != nil { panic(err) } l, _ := net.Listen("tcp", "127.0.0.1:0") _, port, _ := net.SplitHostPort(l.Addr().String()) l.Close() gwConfig.ListenPort, _ = strconv.Atoi(port) if s.config.SeparateControlAPI { l, _ := net.Listen("tcp", "127.0.0.1:0") _, port, _ = net.SplitHostPort(l.Addr().String()) l.Close() gwConfig.ControlAPIPort, _ = strconv.Atoi(port) } gwConfig.CoProcessOptions = s.config.CoprocessConfig gw := NewGateway(gwConfig, s.ctx) gw.setTestMode(true) gw.ConnectionWatcher = httputil.NewConnectionWatcher() var err error gwConfig.Storage.Database = mathrand.Intn(15) gwConfig.AppPath, err = ioutil.TempDir("", "tyk-test-")

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add error handling for DNS caching initialization Ensure that the flakySetupTestReverseProxyDnsCache function properly handles errors returned by InitDNSCaching to avoid potential runtime issues. gateway/reverse_proxy_test.go [143-144] -s.Gw.dnsCacheManager.InitDNSCaching( - time.Duration(cfg.dnsConfig.TTL)*time.Second, time.Duration(cfg.dnsConfig.CheckInterval)*time.Second) +if err := s.Gw.dnsCacheManager.InitDNSCaching( + time.Duration(cfg.dnsConfig.TTL)*time.Second, time.Duration(cfg.dnsConfig.CheckInterval)*time.Second); err != nil { + t.Fatalf("Failed to initialize DNS caching: %v", err) +} Suggestion importance[1-10]: 9 __ Why: Adding error handling for the InitDNSCaching function is crucial to prevent runtime issues if the initialization fails. This suggestion directly addresses a potential bug and improves the robustness of the code.	High
	Check for nil before shutting down HTTP handler Validate that HttpHandler is not nil before calling Shutdown in the Close function to prevent potential nil pointer dereferences. gateway/test.go [163-167] -err := s.HttpHandler.Shutdown(ctxShutDown) -if err != nil { - log.WithError(err).Error("shutting down the http handler") -} else { - log.Info("server exited properly") +if s.HttpHandler != nil { + err := s.HttpHandler.Shutdown(ctxShutDown) + if err != nil { + log.WithError(err).Error("shutting down the http handler") + } else { + log.Info("server exited properly") + } } Suggestion importance[1-10]: 8 __ Why: Validating that HttpHandler is not nil before calling Shutdown prevents potential nil pointer dereferences, which could cause runtime crashes. This is a practical improvement for code safety.	Medium
General	Clean up temporary directories after use Ensure that temporary directories created with ioutil.TempDir are cleaned up to avoid resource leaks. gateway/testutil.go [66] -testMiddlewarePath, _ = ioutil.TempDir("", "tyk-middleware-path") +testMiddlewarePath, err := ioutil.TempDir("", "tyk-middleware-path") +if err != nil { + log.WithError(err).Fatal("failed to create temporary middleware path") +} +defer os.RemoveAll(testMiddlewarePath) Suggestion importance[1-10]: 9 __ Why: Ensuring that temporary directories created with ioutil.TempDir are cleaned up prevents resource leaks and improves the overall resource management in the code. This is a significant enhancement for maintainability.	High
	Add timeout for server startup Add a timeout or context cancellation mechanism for startServer to prevent indefinite blocking during server startup. gateway/test.go [91] -gw.startServer() +ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) +defer cancel() +if err := gw.startServer(ctx); err != nil { + log.WithError(err).Error("failed to start server") +} Suggestion importance[1-10]: 7 __ Why: Adding a timeout for startServer ensures that the server startup process does not block indefinitely, improving the reliability of the test setup. However, the exact implementation of startServer in the PR is not shown, so the suggestion may require adaptation.	Medium

[sonarqubecloud]

Quality Gate failed

Failed conditions
11 Security Hotspots
E Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE