Testing: move tests into black box scope

[titpetric]

User description

The PR tries to refactor tests away from the gateway package using black box package scope. The source set has been limited to only the test files which contain StartTest, a known entrypoint for integration tests, but some post processing increased the number from 21 to 31 tests (by size, from smallest).

Black box tests only allow accessing public symbols from the package.

Limiting factor: big test files are notoriously hard to extract due to a lot of couplings, while smaller test functions could be moved.

After the change:

Blackbox tests size: 186496/1239669 (15.04% of total package)
Blackbox test functions: 140/760 (18.42% of total functions)
Blackbox test files: 31/83 (37.35% of total test files)

bash script to calculate black box tests metrics

#!/bin/bash

# List all _test.go files containing the gateway_test package
gateway_test_files=$(grep -rl '^package gateway_test' *_test.go)

# Calculate the combined size of gateway_test package files
gateway_test_size=$(du -cb $gateway_test_files | grep total | awk '{print $1}')

# Calculate the total size of all test files
all_test_size=$(du -cb *_test.go | grep total | awk '{print $1}')

# Count the number of functions in gateway_test files
gateway_test_functions=$(grep -h '^func' $gateway_test_files | wc -l)

# Count the number of functions in all test files
all_test_functions=$(grep -h '^func' *_test.go | wc -l)

# Count the number of files in gateway_test package
gateway_test_file_count=$(echo "$gateway_test_files" | wc -l)

# Count the total number of test files
all_test_file_count=$(ls *_test.go | wc -l)

# Calculate the percentage of size, functions, and file count
size_percentage=$(awk "BEGIN {printf \"%.2f\", ($gateway_test_size/$all_test_size)*100}")
func_percentage=$(awk "BEGIN {printf \"%.2f\", ($gateway_test_functions/$all_test_functions)*100}")
file_percentage=$(awk "BEGIN {printf \"%.2f\", ($gateway_test_file_count/$all_test_file_count)*100}")

# Print the results for size
echo "Blackbox tests size: $gateway_test_size/$all_test_size ($size_percentage% of total package)"

# Print the results for functions
echo "Blackbox test functions: $gateway_test_functions/$all_test_functions ($func_percentage% of total functions)"

# Print the results for file count
echo "Blackbox test files: $gateway_test_file_count/$all_test_file_count ($file_percentage% of total test files)"

---

PR Type

Tests, Enhancement

---

Description

• Introduced black box testing with new test files and utilities.
• Added utility functions in httputil for CORS headers and basic auth.
• Refactored multiple test files to use new test preparation utilities.
• Exported functions and types for better test integration and usage.
• Simplified and improved code readability and maintainability.

---

Changes walkthrough 📝

	Relevant files
Tests	3 files blackbox_test.go Introduce black box testing with symbol shims                        gateway/blackbox_test.go Added a new test file blackbox_test.go for black box testing. Introduced symbol shims from the gateway package for testing. Defined constants and global functions for testing purposes. +101/-0  testutil_test.go Add test utility functions for various test preparations  gateway/testutil_test.go Added new test utility functions for preparing basic auth, context vars middleware, and virtual endpoints. Included JSON schema preparation for validation tests. +126/-0  tracing_test.go Update tracing tests to use exported TraceHttpRequest        gateway/tracing_test.go Updated test to use TraceHttpRequest and its ToRequest method. +3/-3
Enhancement	11 files headers.go Add utility functions for CORS headers and basic auth        internal/httputil/headers.go Introduced CORSHeaders variable for common CORS headers. Added AuthHeader function for generating basic auth headers. +26/-0    listen_proxy_proto.go Introduce proxy protocol listener utility for tests            internal/httputil/listen_proxy_proto.go Added TestListenProxyProto function as a test utility for proxy protocol. +26/-0    api_definition_test.go Update method calls to use exported HandleRedisEvent          gateway/api_definition_test.go Updated method calls to use exported HandleRedisEvent. +3/-3      api_test.go Refactor test preparation and context session setting        gateway/api_test.go Updated method calls to use TestPrepareBasicAuth. Replaced ctxSetSession with ctx.SetSession. +7/-6      auth_manager_test.go Use httputil for authorization header generation                  gateway/auth_manager_test.go Imported httputil for AuthHeader usage. +2/-1      dashboard_register.go Refactor to use BuildDashboardConnStr for connection strings gateway/dashboard_register.go Changed method calls to use BuildDashboardConnStr. +4/-4      mw_basic_auth_test.go Refactor basic auth tests to use httputil                                gateway/mw_basic_auth_test.go Replaced custom auth header generation with httputil.AuthHeader. Updated test preparation to use TestPrepareBasicAuth. +36/-61  mw_context_vars_test.go Refactor context vars middleware test preparation                gateway/mw_context_vars_test.go Updated test preparation to use TestPrepareContextVarsMiddleware. +3/-23    mw_virtual_endpoint_test.go Refactor virtual endpoint tests with new utilities              gateway/mw_virtual_endpoint_test.go Updated test preparation to use TestPrepareVirtualEndpoint. Replaced cachedResponseHeader with CachedResponseHeader. +13/-67  redis_signals.go Export HandleRedisEvent and simplify pubsub delay logic    gateway/redis_signals.go Exported HandleRedisEvent for test usage. Removed addPubSubDelay function and inlined its logic. +5/-8      tracing.go Export TraceHttpRequest for external usage                              gateway/tracing.go Exported TraceHttpRequest and its method ToRequest. +4/-4

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[buger]

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

Your branch: tests/blackbox

Your PR title: Testing: move tests into black box scope

Your PR description: The PR tries to refactor tests away from the gateway package using black box package scope. The source set has been limited to only the test files which contain `StartTest`, a known entrypoint for integration tests, but some post processing increased the number from 21 to 31 tests (by size, from smallest).

Black box tests only allow accessing public symbols from the package.

Limiting factor: big test files are notoriously hard to extract due to a lot of couplings, while smaller test functions could be moved.

After the change:

Blackbox tests size: 186496/1239669 (15.04% of total package)
Blackbox test functions: 140/760 (18.42% of total functions)
Blackbox test files: 31/83 (37.35% of total test files)

bash script to calculate black box tests metrics

#!/bin/bash

# List all _test.go files containing the gateway_test package
gateway_test_files=$(grep -rl '^package gateway_test' *_test.go)

# Calculate the combined size of gateway_test package files
gateway_test_size=$(du -cb $gateway_test_files | grep total | awk '{print $1}')

# Calculate the total size of all test files
all_test_size=$(du -cb *_test.go | grep total | awk '{print $1}')

# Count the number of functions in gateway_test files
gateway_test_functions=$(grep -h '^func' $gateway_test_files | wc -l)

# Count the number of functions in all test files
all_test_functions=$(grep -h '^func' *_test.go | wc -l)

# Count the number of files in gateway_test package
gateway_test_file_count=$(echo "$gateway_test_files" | wc -l)

# Count the total number of test files
all_test_file_count=$(ls *_test.go | wc -l)

# Calculate the percentage of size, functions, and file count
size_percentage=$(awk "BEGIN {printf \"%.2f\", ($gateway_test_size/$all_test_size)*100}")
func_percentage=$(awk "BEGIN {printf \"%.2f\", ($gateway_test_functions/$all_test_functions)*100}")
file_percentage=$(awk "BEGIN {printf \"%.2f\", ($gateway_test_file_count/$all_test_file_count)*100}")

# Print the results for size
echo "Blackbox tests size: $gateway_test_size/$all_test_size ($size_percentage% of total package)"

# Print the results for functions
echo "Blackbox test functions: $gateway_test_functions/$all_test_functions ($func_percentage% of total functions)"

# Print the results for file count
echo "Blackbox test files: $gateway_test_file_count/$all_test_file_count ($file_percentage% of total test files)"

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	2024-10-22 08:10:40.502668330 +0000
+++ current.txt	2024-10-22 08:10:34.258602491 +0000
@@ -7974,6 +7974,9 @@
 )
 const BackupApiKeyBase = "node-definition-backup:"
 const BackupPolicyKeyBase = "node-policy-backup:"
+const (
+	CachedResponseHeader = "x-tyk-cached-response"
+)
 const CoProcessDefaultKeyPrefix = "coprocess-data:"
     CoProcessDefaultKeyPrefix is used as a key prefix for this CP.
 
@@ -8141,6 +8144,7 @@
 func Start()
 func TestReq(t testing.TB, method, urlStr string, body interface{}) *http.Request
 func TestReqBody(t testing.TB, body interface{}) io.Reader
+func TransformBody(r *http.Request, tmeta *TransformSpec, t *TransformMiddleware) error
 func TykGetData(CKey *C.char) *C.char
     TykGetData is a CoProcess API function for fetching data.
 
@@ -8955,6 +8959,8 @@
 
 func (gw *Gateway) BuildAndLoadAPI(apiGens ...func(spec *APISpec)) (specs []*APISpec)
 
+func (gw *Gateway) BuildDashboardConnStr(resource string) string
+
 func (gw *Gateway) CoProcessInit()
     CoProcessInit creates a new CoProcessDispatcher, it will be called when Tyk
     starts.
@@ -8985,6 +8991,9 @@
 
 func (gw *Gateway) GetStorageForApi(apiID string) (ExtendedOsinStorageInterface, int, error)
 
+func (gw *Gateway) HandleRedisEvent(v interface{}, handled func(NotificationCommand), reloaded func())
+    HandleRedisEvent is exported for test usage.
+
 func (gw *Gateway) InitHostCheckManager(ctx context.Context, store storage.Handler)
 
 func (gw *Gateway) LoadAPI(specs ...*APISpec) (out []*APISpec)
@@ -9274,7 +9283,7 @@
 
 type HeaderTransform struct {
 	BaseTykResponseHandler
-	// Has unexported fields.
+	Config HeaderTransformOptions
 }
 
 func (h *HeaderTransform) Base() *BaseTykResponseHandler
@@ -10806,6 +10815,15 @@
 	Form    map[string]string
 }
 
+type TraceHttpRequest struct {
+	Method  string      `json:"method"`
+	Path    string      `json:"path"`
+	Body    string      `json:"body"`
+	Headers http.Header `json:"headers"`
+}
+
+func (tr *TraceHttpRequest) ToRequest(ignoreCanonicalMIMEHeaderKey bool) (*http.Request, error)
+
 type TraceMiddleware struct {
 	TykMiddleware
 }

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Functionality Duplication The function AuthHeader duplicates existing functionality for basic auth header generation. Consider reusing existing libraries or centralizing this utility.

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Enhancement	Use the newly introduced TransformBody function to replace direct calls to transformBody Replace the direct call to transformBody with the new TransformBody function to maintain consistency and encapsulation. gateway/mw_transform.go [47] -if err := transformBody(r, tmeta, &transform); err != nil { +if err := TransformBody(r, tmeta, &transform); err != nil { Suggestion importance[1-10]: 9 Why: The suggestion aligns with the PR changes, which introduce the TransformBody function to encapsulate transformBody. This change enhances code consistency and encapsulation.	9
Best practice	Ensure cleanup of temporary directories to prevent resource leakage Ensure that the temporary directory created is cleaned up after its use to prevent resource leakage. gateway/testutil.go [1799] gwConf.AppPath, err = ioutil.TempDir("", "apps") +defer os.RemoveAll(gwConf.AppPath) Suggestion importance[1-10]: 9 Why: Ensuring that temporary directories are cleaned up is important for preventing resource leakage, which can lead to performance issues or system instability.	9
	Implement error handling in the HandleRedisEvent function Update the HandleRedisEvent function to handle errors explicitly and log them, or pass them up the call stack for better error management. gateway/redis_signals.go [95] -func (gw *Gateway) HandleRedisEvent(v interface{}, handled func(NotificationCommand), reloaded func()) { +func (gw *Gateway) HandleRedisEvent(v interface{}, handled func(NotificationCommand), reloaded func()) error { + // Implement error handling inside the function Suggestion importance[1-10]: 7 Why: Adding error handling to HandleRedisEvent would improve robustness and error management. However, the suggestion is somewhat generic without specific guidance on implementation.	7
	Add error handling and logging to the TransformBody function Ensure that the new TransformBody function handles errors and logs them appropriately, as it now encapsulates the functionality of transformBody. gateway/mw_transform.go [54-55] func TransformBody(r *http.Request, tmeta *TransformSpec, t *TransformMiddleware) error { + // Add error handling and logging here Suggestion importance[1-10]: 6 Why: While adding error handling and logging is a good practice, the suggestion lacks specific implementation details. It is a beneficial enhancement but not critical.	6
Possible bug	Prevent potential deadlocks by ensuring proper WaitGroup decrement Ensure that the WaitGroup is properly decremented after the HandleRedisEvent function call to avoid potential deadlocks or goroutine leaks. gateway/api_definition_test.go [872] -ts.Gw.HandleRedisEvent(&msg, handled, wg.Done) +ts.Gw.HandleRedisEvent(&msg, handled, func() { wg.Done() }) Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential issue with the WaitGroup not being decremented properly, which could lead to deadlocks or goroutine leaks. The proposed change ensures that the decrement is executed correctly, enhancing the reliability of the code.	8
	Add validation for Method and Path in TraceHttpRequest to ensure they are not empty Validate tr.Method and tr.Path before creating a new HTTP request to ensure they contain valid values. gateway/tracing.go [25] +if tr.Method == "" || tr.Path == "" { + return nil, errors.New("method or path cannot be empty") +} r, err := http.NewRequest(tr.Method, tr.Path, strings.NewReader(tr.Body)) Suggestion importance[1-10]: 7 Why: Validating Method and Path ensures that the HTTP request is constructed with valid values, preventing potential errors during request creation.	7
	Add validation for testMiddlewarePath before usage to ensure it is not empty Consider checking if testMiddlewarePath is empty or invalid before setting it as gwConfig.MiddlewarePath to prevent runtime errors. gateway/testutil.go [1157] -gwConfig.MiddlewarePath = testMiddlewarePath +if testMiddlewarePath != "" { + gwConfig.MiddlewarePath = testMiddlewarePath +} Suggestion importance[1-10]: 6 Why: Adding a check for an empty testMiddlewarePath can prevent potential runtime errors, improving the reliability of the code.	6
Possible issue	Improve error handling by replacing panic with a return statement Replace the panic with a more graceful error handling mechanism to avoid crashing the application. gateway/testutil.go [1130] if err != nil { - panic(err) + return nil, err } Suggestion importance[1-10]: 8 Why: Replacing panic with a return statement improves error handling by preventing the application from crashing, which is crucial for robustness and stability.	8
	Ensure session initialization respects cache settings Verify that the TestPrepareBasicAuth function correctly initializes sessions with the expected properties, especially when cache settings are involved. gateway/api_test.go [967] -session := ts.TestPrepareBasicAuth(false) +session := ts.TestPrepareBasicAuth(true) // Assuming true enables desired caching behavior Suggestion importance[1-10]: 5 Why: The suggestion highlights the importance of verifying cache settings during session initialization. While it is contextually relevant, the suggestion assumes a change in behavior without clear evidence from the PR, making it less impactful.	5
Maintainability	Refactor copyHeader to use httputil.CORSHeaders for managing CORS headers Refactor the copyHeader function to handle CORS headers using the new httputil.CORSHeaders variable to avoid duplication and centralize CORS header management. gateway/reverse_proxy.go [52-54] -for _, vv := range corsHeaders { +for _, vv := range httputil.CORSHeaders { Suggestion importance[1-10]: 8 Why: This suggestion is valid as it aligns with the PR's refactoring efforts to centralize CORS header management, improving maintainability and reducing duplication.	8

[sonarcloud]

Quality Gate failed

Failed conditions
68.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud