-
Notifications
You must be signed in to change notification settings - Fork 20
RDK-60610: Adding L1 unit test cases for commonlib,reportgen #254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
c471ddb to
60cab50
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds additional L1 unit tests for reportgen and commonlib behavior, including error/edge branches that were previously untested.
Changes:
- Added
reportgenunit tests covering realloc-failure handling inprepareHttpUrl, additional trim/regex branches inencodeParamResultInJSON, and a positive regex-match path forapplyRegexToValuevia callback. - Added
commonlibunit tests fort2_event_*cache-enabled/disabled paths and additionalgetParamValueboolean handling. - Introduced
#ifdef GTEST_ENABLEhelper hooks intelemetry_busmessage_sender.cto expose internal state and a callback to the internal event-marker population function for testing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| source/test/reportgen/reportgenTest.cpp | Adds new reportgen unit tests, including realloc-failure injection and additional regex/trim coverage. |
| source/test/commonlib/TelemetryBusMsgSender.cpp | Adds commonlib unit tests for event sending paths and boolean parameter retrieval, plus use of new test hooks. |
| source/commonlib/telemetry_busmessage_sender.c | Adds GTEST-only exported helpers to access internal flags and the internal marker-list function for unit tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reason for change: Adding L1 unit test cases for reportgenand commonlib Test Procedure: Tested and verified Risks: Medium Priority: P1 Signed-off-by: Rose Mary Benny <RoseMary_Benny@comcast.com>
60cab50 to
f7a59fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| CURL* curl = (CURL*) 0xffee; | ||
| char* profile = strdup("RDK_Profile"); | ||
|
|
||
| // Prepare the expectations for Curl and our mock functions | ||
| EXPECT_CALL(*m_reportgenMock, curl_easy_init()) | ||
| .Times(1) | ||
| .WillOnce(Return(curl)); | ||
| EXPECT_CALL(*m_reportgenMock, curl_easy_escape(_,_,_)) | ||
| .Times(1) | ||
| .WillOnce(Return(profile)); | ||
| EXPECT_CALL(*m_reportgenMock, curl_free(profile)) | ||
| .Times(1); | ||
| EXPECT_CALL(*m_reportgenMock, curl_easy_cleanup(curl)) | ||
| .Times(1); |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In prepareHttpUrl_ReallocFails, profile is allocated via strdup but never freed. Since curl_free is mocked, it won’t actually free this memory unless you add an action (e.g., invoke free) or explicitly free(profile) after the call. This keeps the test leak-free under sanitizers/valgrind.
Reason for change: Adding L1 unit test cases for reportgenand commonlib
Test Procedure: Tested and verified
Risks: Medium
Priority: P1