Add/update unit tests by RahulRengeshOfficial · Pull Request #124 · rdkcentral/xconfwebconfig

RahulRengeshOfficial · 2026-03-09T11:10:38Z

No description provided.

Copilot

Pull request overview

This PR adds a broad set of new unit tests across multiple packages (rules engine, HTTP connectors/handlers, shared types, security crypto helpers, and data API logic) to increase coverage and validate behavior across common code paths.

Changes:

Added many new test files covering shared types, rules engine primitives/evaluators, HTTP service connectors/handlers, and security AES utilities.
Expanded existing test suites with additional cases (e.g., converters, processor evaluation, simple handler).
Refactored a few test utilities/usages to modern equivalents (e.g., io.ReadAll instead of ioutil.ReadAll).

Reviewed changes

Copilot reviewed 56 out of 87 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
shared/estbfirmware/firmware_config_facade_test.go	Adds tests for firmware config facade JSON marshaling/unmarshaling and helpers
shared/estbfirmware/estb_fimware_context_test.go	Minor whitespace-only change in an existing test file
shared/estbfirmware/estb_converters_func_test.go	Adds tests for legacy free-arg detection and conversion helpers
shared/estbfirmware/config_change_logs_test.go	Adds tests for config change log and RuleInfo helpers
shared/coretypes_test.go	Adds tests for shared core types (validation, cloning, constants)
shared/change/change_test.go	Adds tests for change models and validation logic
security/aes_decrypt_test.go	Adds tests for standalone `Decrypt` behavior (including error scenarios)
security/aes_codec_test.go	Adds tests for AES codec encrypt/decrypt and padding/digest helpers
rulesengine/rule_test.go	Adds tests for Rule construction, stringification, and tree/list helpers
rulesengine/rule_processor_test.go	Adds tests for processor evaluation and evaluator selection
rulesengine/rule_processor_factory_test.go	Adds tests for factory getter and `evalRange` parsing branches
rulesengine/parser_test.go	Adds tests for parser construction and base parse behavior
rulesengine/free_arg_test.go	Adds tests for `FreeArg` setters and `String()` output
rulesengine/fixed_arg_test.go	Adds tests for `FixedArg` validity/type helpers and `Collection.MarshalJSON`
rulesengine/condition_test.go	Adds tests for Condition setters/getters and `String()`
rulesengine/base_time_evaluator_test.go	Adds tests for base time evaluator construction/interface and panic behavior
rulesengine/base_evaluator_test.go	Adds comprehensive tests for base evaluator behavior across operations/types
http/tagging_connector_test.go	Adds tests for tagging connector endpoints with httptest servers
http/simple_handler_test.go	Updates read helpers and adds direct handler tests for several endpoints
http/security_manager_test.go	Adds tests for security token manager helpers via mock infrastructure
http/sat_token_manager_test.go	Adds tests for SAT token manager and token expiry helpers via mocks
http/sat_connector_test.go	Adds tests for SAT connector getters and response structs
http/response_writer_test.go	Adds tests for custom response writer behavior
http/metrics_test.go	Adds nil-metrics safety coverage for metrics helper functions
http/main_test.go	Updates to `io.Discard` from `ioutil.Discard`
http/http_client_test.go	Adds tests for HTTP client trace/moracide header propagation helpers
http/groupsync_service_connector_test.go	Adds tests for groupsync connector and protobuf headers
http/account_service_connector_test.go	Adds tests for account service connector models and request/response handling
dataapi/featurecontrol/feature_converter_test.go	Adds tests for RFC feature conversion and whitelist list expansion
dataapi/feature_control_handler_test.go	Adds tests for precook hash matching helper
dataapi/estbfirmware/estb_comnames_test.go	Adds tests for constants/default values in estbfirmware dataapi package
dataapi/estb_firmware_handler_test.go	Adds tests for estb firmware handlers and helper behavior
dataapi/dcm/telemetry/telemetry_profile_test.go	Adds tests for telemetry profile service conversion/dedup/create/drop
dataapi/database_mock_test.go	Adds DAO-mocking tests for group service cache interactions and early-return paths
dataapi/dataapi_common_test.go	Adds tests for common request/context helpers and prefix removal
dataapi/data_service_info_test.go	Adds tests for service info refresh/statistics handlers
common/server_config_test.go	Adds tests for server config loading/from-text and origin ID behavior
common/const_var_test.go	Adds tests for constants and build-time variables presence

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-09T11:13:04Z

+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := tc.rule.String()
+			assert.Assert(t, len(result) > 0, "String should not be empty")
+			// Verify it contains key elements
+			for _, substring := range tc.contains {
+				if substring != "" {
+					// Basic validation that string representation is working
+					break
+				}
+			}
+		})
+	}


This test never verifies that the expected substrings are present in result—the inner loop breaks immediately on the first non-empty substring without asserting anything. Replace the loop with explicit strings.Contains(result, substring) assertions for each expected substring (and import strings).

Copilot · 2026-03-09T11:13:05Z

+		// Test case where freeArgValue fits in range
+		result := evalRange("AA:BB:CC:DD:EE:50", "0.0-75.0")
+		assert.Assert(t, result == true || result == false, "Should return boolean") // Actual result depends on FitsPercent implementation
+	})


Assertions like result == true || result == false are tautologies for a bool return type and don't validate behavior. Consider removing these cases or making them meaningful by asserting a deterministic expected value (e.g., by choosing inputs whose FitsPercent outcome is known/controlled, or by refactoring to allow injecting/mocking the percent-fit function).

Copilot · 2026-03-09T11:13:05Z

+	t.Run("Valid_range_format", func(t *testing.T) {
+		// This should call FitsPercent twice and return the logical result
+		result := evalRange("test", "25.0-75.0")
+		assert.Assert(t, result == true || result == false, "Should return boolean")
+	})


Assertions like result == true || result == false are tautologies for a bool return type and don't validate behavior. Consider removing these cases or making them meaningful by asserting a deterministic expected value (e.g., by choosing inputs whose FitsPercent outcome is known/controlled, or by refactoring to allow injecting/mocking the percent-fit function).

Copilot · 2026-03-09T11:13:05Z

+			if model, ok := tt.input[common.MODEL]; ok && model != "" {
+				assert.Equal(t, model, tt.input[common.MODEL])
+			}
+			if env, ok := tt.input[common.ENV]; ok && env != "" {
+				assert.Equal(t, env, tt.input[common.ENV])
+			}
+			if partnerId, ok := tt.input[common.PARTNER_ID]; ok && partnerId != "" {
+				assert.Equal(t, partnerId, tt.input[common.PARTNER_ID])
+			}


These assertions compare the map values to themselves after calling NormalizeCommonContext, so they will always pass regardless of normalization behavior. Capture the original values and assert the expected transformations (e.g., MODEL/ENV uppercased, MAC addresses normalized, PARTNER_ID uppercased) against concrete expected strings.

Suggested change

if model, ok := tt.input[common.MODEL]; ok && model != "" {

assert.Equal(t, model, tt.input[common.MODEL])

}

if env, ok := tt.input[common.ENV]; ok && env != "" {

assert.Equal(t, env, tt.input[common.ENV])

}

if partnerId, ok := tt.input[common.PARTNER_ID]; ok && partnerId != "" {

assert.Equal(t, partnerId, tt.input[common.PARTNER_ID])

}

switch tt.name {

case "Uppercase model and env":

// MODEL and ENV should be uppercased

assert.Equal(t, "MODEL123", tt.input[common.MODEL])

assert.Equal(t, "DEV", tt.input[common.ENV])

case "Normalize MAC addresses":

// MAC addresses should be normalized to a canonical uppercase, delimiter-free form

assert.Equal(t, "AABBCCDDEEFF", tt.input["estbMac"])

assert.Equal(t, "112233445566", tt.input["ecmMac"])

case "Uppercase partnerId":

// PARTNER_ID should be uppercased

assert.Equal(t, "PARTNER1", tt.input[common.PARTNER_ID])

}

Copilot · 2026-03-09T11:13:06Z

+	largeData := make([]string, 100)
+	for i := 0; i < 100; i++ {
+		largeData[i] = "item" + string(rune(i))
+	}


string(rune(i)) does not produce the decimal digits of i; for many values it produces non-printable/control characters (and can lead to confusing/duplicate test data). Use numeric formatting instead (e.g., strconv.Itoa(i) or fmt.Sprintf(\"%d\", i)) to generate stable, readable item IDs.

Copilot · 2026-03-09T11:13:06Z

+	defer func() {
+		if r := recover(); r != nil {
+			errMsg := r.(error).Error()
+			assert.Assert(t, strings.Contains(errMsg, "BaseTimeEvaluator.Evaluate() is not implemented yet"), 
+				"Should panic with not implemented message, got: %s", errMsg)
+		} else {
+			t.Fatal("Expected panic but none occurred")
+		}
+	}()


The recover() value is forcibly cast with r.(error), which will itself panic if the production code panics with a string or another type. Use a type switch (handle error, string, and fallback via fmt.Sprint(r)) before checking the message substring.

Add/update unit tests

3595f5b

Copilot AI review requested due to automatic review settings March 9, 2026 11:10

Copilot AI reviewed Mar 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add/update unit tests#124

Add/update unit tests#124
RahulRengeshOfficial wants to merge 1 commit into
PR_utmigratetomainbranchfrom
utmigratetomainbranch

RahulRengeshOfficial commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-			if model, ok := tt.input[common.MODEL]; ok && model != "" {
-				assert.Equal(t, model, tt.input[common.MODEL])
-			}
-			if env, ok := tt.input[common.ENV]; ok && env != "" {
-				assert.Equal(t, env, tt.input[common.ENV])
-			}
-			if partnerId, ok := tt.input[common.PARTNER_ID]; ok && partnerId != "" {
-				assert.Equal(t, partnerId, tt.input[common.PARTNER_ID])
-			}
+			switch tt.name {
+			case "Uppercase model and env":
+				// MODEL and ENV should be uppercased
+				assert.Equal(t, "MODEL123", tt.input[common.MODEL])
+				assert.Equal(t, "DEV", tt.input[common.ENV])
+			case "Normalize MAC addresses":
+				// MAC addresses should be normalized to a canonical uppercase, delimiter-free form
+				assert.Equal(t, "AABBCCDDEEFF", tt.input["estbMac"])
+				assert.Equal(t, "112233445566", tt.input["ecmMac"])
+			case "Uppercase partnerId":
+				// PARTNER_ID should be uppercased
+				assert.Equal(t, "PARTNER1", tt.input[common.PARTNER_ID])
+			}

Conversation

RahulRengeshOfficial commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants