RDK-60072: Adding L1 unit test cases to improve code coverage #219

rosemarybennyy · 2025-12-05T04:40:03Z

Reason for change: Adding L1 unit test cases to improve code coverage
Test Procedure: Tested and verified
Risks: Medium
Priority: P1
Signed-off-by: Rose Mary Benny RoseMary_Benny@comcast.com

Reason for change: Adding L1 unit test cases to improve code coverage Test Procedure: Tested and verified Risks: Medium Priority: P1 Signed-off-by: Rose Mary Benny <RoseMary_Benny@comcast.com>

Copilot

Pull request overview

This pull request adds L1 (Level 1) unit test cases to improve code coverage for the t2parser module. The PR introduces a new test file for testing XConf configuration parsing and extends existing tests with additional coverage for trigger condition verification, encoding settings, and protocol configuration. These tests aim to exercise error paths and edge cases to improve overall test coverage.

Key Changes

New test file t2parserxconfTest.cpp with 3 test cases covering XConf configuration processing, including missing required fields, default schedule handling, and slash-pattern schedule parsing
Extended t2parserTest.cpp with 13 new test cases for trigger condition verification, encoding/protocol settings, adding comprehensive coverage for validation logic
Updated build configuration in Makefile.am to include the new test file

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.

File	Description
source/test/t2parser/t2parserxconfTest.cpp	New test file with 3 test cases for processConfigurationXConf function covering error cases (missing fields) and valid configurations (default schedule and slash schedule patterns)
source/test/t2parser/t2parserTest.cpp	Added extern C declarations for internal functions and 13 new test cases for verifyTriggerCondition (7 tests), addTriggerCondition (1 test), encodingSet (1 test), protocolSet (2 tests), plus 2 commented-out tests for time_param_Reporting_Adjustments_valid_set; fixed minor formatting issues by removing blank lines and replacing an incorrect parameter
source/test/t2parser/Makefile.am	Updated SOURCES variable to include new test file t2parserxconfTest.cpp in the build

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

Copilot · 2025-12-05T16:46:31Z

source/test/t2parser/t2parserTest.cpp

+/* time_param_Reporting_Adjustments_valid_set: set values when generateNow is false */
+TEST(T2ParserTimeParam, ReportingAdjustmentsApplied)
+{
+    Profile p;
+    memset(&p, 0, sizeof(p));
+    p.generateNow = false;
+    p.activationTimeoutPeriod = 1000;
+    p.reportingInterval = 60;
+
+    cJSON* jReportingInterval = cJSON_CreateNumber(120);
+    cJSON* jActivationTimeout = cJSON_CreateNumber(400);
+    cJSON* jTimeReference = cJSON_CreateString("2025-01-01T00:00:00Z");
+
+    cJSON* jReportingAdjustments = cJSON_CreateObject();
+    cJSON* jReportOnUpdate = cJSON_CreateTrue();
+    cJSON_AddItemToObject(jReportingAdjustments, "ReportOnUpdate", jReportOnUpdate);
+    cJSON* jFirstReportingInterval = cJSON_CreateNumber(10);
+    cJSON_AddItemToObject(jReportingAdjustments, "FirstReportingInterval", jFirstReportingInterval);
+    cJSON* jMaxUploadLatency = cJSON_CreateNumber(2000);
+    cJSON_AddItemToObject(jReportingAdjustments, "MaxUploadLatency", jMaxUploadLatency);
+
+    // call the function (note: signature expects separate args - pass firstReportingInterval/maxUploadLatency separately)
+    time_param_Reporting_Adjustments_valid_set(&p, jReportingInterval, jActivationTimeout, jTimeReference, jReportOnUpdate, jFirstReportingInterval, jMaxUploadLatency);
+
+    // reportingInterval should be set from jReportingInterval
+    EXPECT_EQ(120, p.reportingInterval);
+    // activationTimeoutPeriod should be set from jActivationTimeout by surrounding callers; here we check timeRef was set
+    if (p.timeRef) {
+        EXPECT_STREQ("2025-01-01T00:00:00Z", p.timeRef);
+        free(p.timeRef);
+        p.timeRef = NULL;
+    }
+    // ReportOnUpdate should be true
+    EXPECT_EQ(true, p.reportOnUpdate);
+    // First reporting interval set
+    EXPECT_EQ(10, p.firstReportingInterval);
+    // MaxUploadLatency set
+    EXPECT_EQ(2000, p.maxUploadLatency);
+
+    // cleanup
+    cJSON_Delete(jReportingAdjustments);
+    cJSON_Delete(jReportingInterval);
+    cJSON_Delete(jActivationTimeout);
+    cJSON_Delete(jTimeReference);
+}
+#endif


Commented-out code block (lines 911-976) should either be removed or enabled with a clear explanation. Leaving large blocks of commented code reduces code maintainability. If this test is not ready, consider removing it entirely or documenting why it's disabled.

Suggested change

/* time_param_Reporting_Adjustments_valid_set: set values when generateNow is false */

TEST(T2ParserTimeParam, ReportingAdjustmentsApplied)

{

Profile p;

memset(&p, 0, sizeof(p));

p.generateNow = false;

p.activationTimeoutPeriod = 1000;

p.reportingInterval = 60;

cJSON* jReportingInterval = cJSON_CreateNumber(120);

cJSON* jActivationTimeout = cJSON_CreateNumber(400);

cJSON* jTimeReference = cJSON_CreateString("2025-01-01T00:00:00Z");

cJSON* jReportingAdjustments = cJSON_CreateObject();

cJSON* jReportOnUpdate = cJSON_CreateTrue();

cJSON_AddItemToObject(jReportingAdjustments, "ReportOnUpdate", jReportOnUpdate);

cJSON* jFirstReportingInterval = cJSON_CreateNumber(10);

cJSON_AddItemToObject(jReportingAdjustments, "FirstReportingInterval", jFirstReportingInterval);

cJSON* jMaxUploadLatency = cJSON_CreateNumber(2000);

cJSON_AddItemToObject(jReportingAdjustments, "MaxUploadLatency", jMaxUploadLatency);

// call the function (note: signature expects separate args - pass firstReportingInterval/maxUploadLatency separately)

time_param_Reporting_Adjustments_valid_set(&p, jReportingInterval, jActivationTimeout, jTimeReference, jReportOnUpdate, jFirstReportingInterval, jMaxUploadLatency);

// reportingInterval should be set from jReportingInterval

EXPECT_EQ(120, p.reportingInterval);

// activationTimeoutPeriod should be set from jActivationTimeout by surrounding callers; here we check timeRef was set

if (p.timeRef) {

EXPECT_STREQ("2025-01-01T00:00:00Z", p.timeRef);

free(p.timeRef);

p.timeRef = NULL;

}

// ReportOnUpdate should be true

EXPECT_EQ(true, p.reportOnUpdate);

// First reporting interval set

EXPECT_EQ(10, p.firstReportingInterval);

// MaxUploadLatency set

EXPECT_EQ(2000, p.maxUploadLatency);

// cleanup

cJSON_Delete(jReportingAdjustments);

cJSON_Delete(jReportingInterval);

cJSON_Delete(jActivationTimeout);

cJSON_Delete(jTimeReference);

}

#endif

/* The following test for time_param_Reporting_Adjustments_valid_set was removed due to lack of documentation and being commented out.

If this test is needed in the future, please restore it with a clear explanation for its status and readiness. */

Copilot · 2025-12-05T16:46:32Z

source/test/t2parser/t2parserTest.cpp

+    // clean up: free internal structures created by addTriggerCondition
+    // We don't have a public destructor here; best-effort free for the test's allocations:
+    // iterate vector if available (Vector API may provide size/get functions in the project),
+    // but to stay robust, simply free profile memory fields that addTriggerCondition could set.
+    // For safety, reset pointer to avoid double-free during test teardown.
+    // (The CI process reclaims process memory after tests.)
+    cJSON_Delete(arr);
+}


Memory leak: The triggerConditionList allocated by addTriggerCondition is not properly cleaned up. While the comment mentions "best-effort free" and "CI process reclaims", tests should properly clean up their allocations to avoid masking real memory leaks and to follow good testing practices.

Copilot · 2025-12-05T16:46:32Z

source/test/t2parser/t2parserTest.cpp

+    free(p.jsonEncoding);
+    cJSON_Delete(jEncodingType);
+    cJSON_Delete(jJSONReportFormat);
+    cJSON_Delete(jJSONReportTimestamp);
+}


Memory leak: The Profile structure members (jsonEncoding, name) are allocated but never freed. Add proper cleanup at the end of the test to prevent memory leaks.

Copilot · 2025-12-05T16:46:32Z

source/test/t2parser/t2parserTest.cpp

+    free(p.t2HTTPDest);
+    free(p.name);
+    cJSON_Delete(jProtocol);
+    cJSON_Delete(jHTTP);
+}


Memory leak: The Profile structure members (t2HTTPDest, name) are allocated but never freed. Add proper cleanup at the end of the test to prevent memory leaks.

Copilot · 2025-12-05T16:46:32Z

source/test/t2parser/t2parserTest.cpp

+    free(p.t2RBUSDest);
+    free(p.name);
+    cJSON_Delete(jProtocol);
+    cJSON_Delete(jRBUS);
+}


Memory leak: The Profile structure members (t2RBUSDest, name) are allocated but never freed. Add proper cleanup at the end of the test to prevent memory leaks.

Copilot · 2025-12-05T16:46:33Z

source/test/t2parser/t2parserxconfTest.cpp

+    // minimal cleanup (full deep-free is not attempted here)
+    // tests rely on process teardown to reclaim memory in CI runs


Memory leak: The profile structure and its internal allocations (t2HTTPDest->URL, name, etc.) allocated during processConfigurationXConf are not freed. Consider adding proper cleanup using a profile destructor/free function or at minimum free the profile and its allocated members before test completion.

Copilot · 2025-12-05T16:46:33Z

source/test/t2parser/t2parserxconfTest.cpp

+    // Ensure the profile name got set
+    EXPECT_STREQ("SlashSched", profile->name ? profile->name : "");


Memory leak: The profile structure and its internal allocations (t2HTTPDest->URL, name, etc.) allocated during processConfigurationXConf are not freed. Consider adding proper cleanup using a profile destructor/free function or at minimum free the profile and its allocated members before test completion.

Copilot · 2025-12-05T16:46:33Z

source/test/t2parser/t2parserTest.cpp

+	char h1[] = "hash1";
+        EXPECT_EQ(T2ERROR_FAILURE,  processConfiguration(&data, "hash1", h1, &profile));


Use spaces instead of tab character for indentation to maintain consistency with the codebase style.

Copilot · 2025-12-05T16:46:36Z

source/test/t2parser/t2parserTest.cpp

+    // tell C++ about the C function defined in t2parser.c
+    T2ERROR verifyTriggerCondition(cJSON *jprofileTriggerCondition);
+    T2ERROR addTriggerCondition(Profile *profile, cJSON *jprofileTriggerCondition);
+    //void time_param_Reporting_Adjustments_valid_set(Profile *profile, cJSON *jprofileReportingInterval, cJSON *jprofileActivationTimeout, cJSON *jprofileTimeReference, cJSON *jprofileReportOnUpdate, cJSON *jprofilefirstReportingInterval, cJSON *jprofilemaxUploadLatency, cJSON* jprofileReportingAdjustments);


Commented-out function declaration. If time_param_Reporting_Adjustments_valid_set is not being tested (see #if 0 block at lines 911-976), this declaration should be removed. Otherwise, uncomment and use it in the tests.

Suggested change

//void time_param_Reporting_Adjustments_valid_set(Profile *profile, cJSON *jprofileReportingInterval, cJSON *jprofileActivationTimeout, cJSON *jprofileTimeReference, cJSON *jprofileReportOnUpdate, cJSON *jprofilefirstReportingInterval, cJSON *jprofilemaxUploadLatency, cJSON* jprofileReportingAdjustments);

rosemarybennyy requested a review from a team as a code owner December 5, 2025 04:40

RDK-60072: Adding L1 unit test cases to improve code coverage

28c7881

Reason for change: Adding L1 unit test cases to improve code coverage Test Procedure: Tested and verified Risks: Medium Priority: P1 Signed-off-by: Rose Mary Benny <RoseMary_Benny@comcast.com>

rosemarybennyy force-pushed the topic/RDK-60072 branch from f751e71 to 28c7881 Compare December 5, 2025 05:43

shibu-kv requested a review from Copilot December 5, 2025 16:40

Copilot started reviewing on behalf of shibu-kv December 5, 2025 16:40 View session

Copilot finished reviewing on behalf of shibu-kv December 5, 2025 16:43

Copilot AI reviewed Dec 5, 2025

View reviewed changes

resolved review comments from github copilot

66a15d7

shibu-kv approved these changes Dec 17, 2025

View reviewed changes

Merge branch 'develop' into topic/RDK-60072

5653708

shibu-kv merged commit a272470 into develop Dec 18, 2025
15 checks passed

github-actions bot locked and limited conversation to collaborators Dec 18, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RDK-60072: Adding L1 unit test cases to improve code coverage #219

RDK-60072: Adding L1 unit test cases to improve code coverage #219

Uh oh!

rosemarybennyy commented Dec 5, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// minimal cleanup (full deep-free is not attempted here)
		// tests rely on process teardown to reclaim memory in CI runs

		// Ensure the profile name got set
		EXPECT_STREQ("SlashSched", profile->name ? profile->name : "");

		char h1[] = "hash1";
		EXPECT_EQ(T2ERROR_FAILURE, processConfiguration(&data, "hash1", h1, &profile));

RDK-60072: Adding L1 unit test cases to improve code coverage #219

RDK-60072: Adding L1 unit test cases to improve code coverage #219

Uh oh!

Conversation

rosemarybennyy commented Dec 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants