RDK-60805: Adding L1 unit test cases for reportprofiles by rosemarybennyy · Pull Request #265 · rdkcentral/telemetry

rosemarybennyy · 2026-02-17T14:18:37Z

Reason for change: Adding L1 unit test cases for reportprofiles
Test Procedure: Tested and verified
Risks: Medium
Priority: P1

Copilot

Pull request overview

Adds new L1 unit tests for reportprofiles and wires them into the bulkdata gtest build to improve coverage around report profile ingestion/uninit/delete flows.

Changes:

Added a new reportprofiles gtest binary and associated test sources/mocks.
Introduced new unit tests for reportprofiles blob/msgpack processing and a few profile/reportprofiles functions.
Minor whitespace cleanup in reportprofiles.c.

Reviewed changes

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

Show a summary per file

File	Description
source/test/bulkdata/reportprofilesTest.cpp	New gtest suite for reportprofiles blob/msgpack processing paths
source/test/bulkdata/reportprofileMock.h	New mock interface for `isRbusEnabled()`
source/test/bulkdata/reportprofileMock.cpp	Linker-wrap implementation forwarding `__wrap_isRbusEnabled()` to gmock
source/test/bulkdata/profileTest.cpp	Adds/enables a few tests related to ReportProfiles/uninit/delete and RemovePreRPfromDisk
source/test/bulkdata/Makefile.am	Builds/links new `reportprofiles_gtest.bin` target
source/bulkdata/reportprofiles.c	Whitespace-only edits

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

source/test/bulkdata/reportprofilesTest.cpp

source/test/bulkdata/reportprofileMock.h

Copilot · 2026-02-17T14:24:39Z

source/test/bulkdata/profileTest.cpp

+TEST_F(ProfileTest, ReportProfiles_deleteProfileXConf) {
+    ProfileXConf profile;
+    EXPECT_CALL(*g_vectorMock, Vector_Size(_))
+        .Times(::testing::AtMost(1))
+        .WillRepeatedly(Return(0)); // Return 1 to indicate only one profile (no duplicates)
+    EXPECT_EQ(ReportProfiles_deleteProfileXConf(&profile), T2ERROR_SUCCESS);
+}


TEST_F(ProfileTest, ReportProfiles_deleteProfileXConf) appears to already exist earlier in the file (per the surrounding diff hunk), so this can cause a duplicate test symbol/name in builds where both blocks are enabled. Rename this test to a more specific unique name (e.g., ReportProfiles_deleteProfileXConf_EmptyList) or remove/merge the duplicate.

source/test/bulkdata/profileTest.cpp

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

Copilot

Pull request overview

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

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

source/test/bulkdata/reportprofileMock.cpp

Copilot · 2026-02-17T14:54:40Z

source/test/bulkdata/reportprofilesTest.cpp

+    void SetUp() override {
+	g_reportprofileMock = new reportprofileMock();    
+        g_systemMock = new SystemMock();
+        g_fileIOMock = new FileMock();
+        m_rdklogMock = new rdklogMock();
+        g_rbusMock = new rbusMock();
+        g_vectorMock = new VectorMock();
+    }


The test fixture declares g_vectorMock and g_schedulerMock as extern but doesn't initialize or clean them up in SetUp/TearDown. For consistency with profileTest.cpp (lines 60-61 and 69-70), these mocks should be initialized in SetUp and deleted in TearDown. Add g_vectorMock = new VectorMock() and g_schedulerMock = new SchedulerMock() in SetUp, and delete them (with nullptr assignment) in TearDown.

Copilot · 2026-02-17T14:54:41Z

source/test/bulkdata/reportprofilesTest.cpp

+        delete g_vectorMock;
+	g_reportprofileMock = nullptr;
+        g_systemMock = nullptr;
+        g_fileIOMock = nullptr;
+        m_rdklogMock = nullptr;
+        g_rbusMock = nullptr;
+        g_vectorMock = nullptr;


The test fixture declares g_vectorMock and g_schedulerMock as extern but doesn't delete them in TearDown. For consistency with profileTest.cpp (lines 69-70 and 76-77), add deletion of g_schedulerMock and setting it to nullptr in TearDown.

Suggested change

delete g_vectorMock;

g_reportprofileMock = nullptr;

g_systemMock = nullptr;

g_fileIOMock = nullptr;

m_rdklogMock = nullptr;

g_rbusMock = nullptr;

g_vectorMock = nullptr;

delete g_vectorMock;

delete g_schedulerMock;

g_reportprofileMock = nullptr;

g_systemMock = nullptr;

g_fileIOMock = nullptr;

m_rdklogMock = nullptr;

g_rbusMock = nullptr;

g_vectorMock = nullptr;

g_schedulerMock = nullptr;

Copilot · 2026-02-17T14:54:41Z

source/test/bulkdata/reportprofilesTest.cpp

+TEST_F(reportprofilesTestFixture, ProcessReportProfilesBlob_EmptyProfile_T2_TEMP_RP) {
+    cJSON *root = cJSON_CreateObject();
+    cJSON *profiles = cJSON_CreateArray();
+    cJSON_AddItemToObject(root, "profiles", profiles);


This test case doesn't verify any behavior or outcome after calling ReportProfiles_ProcessReportProfilesBlob with an empty profiles array and T2_TEMP_RP flag. Consider adding assertions to verify the expected behavior, such as checking that the function returns without error or that no profiles are created. Without assertions, this test only verifies that the function doesn't crash.

Suggested change

cJSON_AddItemToObject(root, "profiles", profiles);

cJSON_AddItemToObject(root, "profiles", profiles);

// For temporary report profiles with an empty array, we expect that

// no vector-based delete or cleanup operations are triggered.

EXPECT_CALL(*g_vectorMock, Vector_Size(_)).Times(0);

EXPECT_CALL(*g_vectorMock, Vector_Destroy(_, _)).Times(0);

source/test/bulkdata/profileTest.cpp

source/test/bulkdata/reportprofilesTest.cpp

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

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

source/test/bulkdata/Makefile.am

Copilot · 2026-02-18T13:50:28Z

source/test/bulkdata/reportprofilesTest.cpp

+   struct __msgpack__ *msg = (struct __msgpack__*)malloc(sizeof(struct __msgpack__));
+   msg->msgpack_blob = (char*)webConfigString;
+   msg->msgpack_blob_size = (int)decodedDataLen;
+


ProcessMsgPackBlob_Test1 allocates msg with malloc but never frees it. Since the test only needs the blob pointer/size, prefer a stack struct __msgpack__ (or free msg before returning) to avoid leaking memory across the test suite.

source/bulkdata/profile.c

Copilot · 2026-02-18T13:50:29Z

source/test/bulkdata/profileMock.h

+//#ifndef SOURCE_TEST_MOCKS_SYSTEMMOCK_H_
+//#define SOURCE_TEST_MOCKS_SYSTEMMOCK_H_
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+#include "telemetry2_0.h"


This new header does not have an active include guard / #pragma once. Because it defines a class, including it twice in the same translation unit would cause a redefinition error. Add #pragma once (or a conventional include guard) at the top, and remove the commented-out guard lines to avoid confusion.

rdkcmf-jenkins · 2026-02-18T13:51:56Z

Coverity Issue - Data race condition

Accessing "profile->grepSeekProfile->execCounter" without holding lock "plMutex". Elsewhere, "_GrepSeekProfile.execCounter" is written to with "plMutex" held 2 out of 2 times.

Medium Impact, CWE-366
MISSING_LOCK

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
source/bulkdata/profile.c:344

shibu-kv · 2026-02-18T19:29:46Z

Coverity Issue - Data race condition

Accessing "profile->grepSeekProfile->execCounter" without holding lock "plMutex". Elsewhere, "_GrepSeekProfile.execCounter" is written to with "plMutex" held 2 out of 2 times.

Medium Impact, CWE-366 MISSING_LOCK

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at: source/bulkdata/profile.c:344

This is addressed in #264

shibu-kv · 2026-02-18T19:33:36Z

L2 failures are due to a known issue. This needs the L2 test containers to be released post merge of https://github.com/rdkcentral/rdk-cert-config/pull/40/changes .
Merging the changes as the failures are due to known aetup issues and other changes and other contributions are awaiting.

Copilot AI review requested due to automatic review settings February 17, 2026 14:18

rosemarybennyy requested a review from a team as a code owner February 17, 2026 14:18

Copilot started reviewing on behalf of rosemarybennyy February 17, 2026 14:23 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

rosemarybennyy force-pushed the topic/RDK-60805 branch from f18b9ed to 844e286 Compare February 17, 2026 14:30

RDK-60805: Adding L1 unit test cases for reportprofiles

e4a079f

Reason for change: Adding L1 unit test cases for reportprofiles 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-60805 branch from 844e286 to e4a079f Compare February 17, 2026 14:45

Copilot AI review requested due to automatic review settings February 17, 2026 14:45

Copilot started reviewing on behalf of rosemarybennyy February 17, 2026 14:46 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

shibu-kv previously approved these changes Feb 17, 2026

View reviewed changes

Adding Apache banners to new files

4f58a73

shibu-kv dismissed their stale review via 4f58a73 February 17, 2026 18:10

Copilot AI review requested due to automatic review settings February 18, 2026 13:41

Copilot started reviewing on behalf of rosemarybennyy February 18, 2026 13:42 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

rosemarybennyy force-pushed the topic/RDK-60805 branch from 7da3f2b to 58536ae Compare February 18, 2026 13:57

added GTEST_ENABLE flag for bulkdata

cc0fa13

rosemarybennyy force-pushed the topic/RDK-60805 branch from 88661f0 to cc0fa13 Compare February 18, 2026 14:22

shibu-kv approved these changes Feb 18, 2026

View reviewed changes

shibu-kv merged commit 8b5682c into develop Feb 18, 2026
12 of 15 checks passed

github-actions bot locked and limited conversation to collaborators Feb 18, 2026

-    cJSON_AddItemToObject(root, "profiles", profiles);
+    cJSON_AddItemToObject(root, "profiles", profiles);
+    // For temporary report profiles with an empty array, we expect that
+    // no vector-based delete or cleanup operations are triggered.
+    EXPECT_CALL(*g_vectorMock, Vector_Size(_)).Times(0);
+    EXPECT_CALL(*g_vectorMock, Vector_Destroy(_, _)).Times(0);

Comments

Conversation

rosemarybennyy commented Feb 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

rosemarybennyy Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

rdkcmf-jenkins commented Feb 18, 2026

Coverity Issue - Data race condition

Issue location

Uh oh!

shibu-kv commented Feb 18, 2026

Coverity Issue - Data race condition

Issue location

Uh oh!

shibu-kv commented Feb 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants