RDK-60610: Adding L1 unit test cases for commonlib,reportgen #254
Conversation
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>
rosemarybennyy
force-pushed
the
topic/RDK-60610
branch
from
c471ddb to
60cab50
Compare
There was a problem hiding this comment.
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.
| void* (*real_realloc_fptr)(void*, size_t) = nullptr; | ||
|
|
||
| void* realloc(void* ptr, size_t size) { | ||
| if (g_test_realloc_fail_counter > 0) { | ||
| --g_test_realloc_fail_counter; | ||
| if (g_test_realloc_fail_counter == 0) | ||
| return nullptr; | ||
| } | ||
| if (!real_realloc_fptr) { | ||
| real_realloc_fptr = (void*(*)(void*, size_t))dlsym(RTLD_NEXT, "realloc"); | ||
| } | ||
| return real_realloc_fptr(ptr, size); |
There was a problem hiding this comment.
real_realloc_fptr is defined with external linkage in this test TU. Since this file also defines an interposing realloc, exporting additional global symbols increases the risk of symbol collisions across the test binary. Make real_realloc_fptr (and any related state) static (or otherwise give it internal linkage) so it can’t conflict with other objects at link time.
| 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); |
There was a problem hiding this comment.
This test allocates profile via strdup and expects curl_free(profile) to be called, but curl_free is a mocked method here and does not actually free memory. That leaves a leak in the test (visible under ASan/LSan). Either return a non-owning buffer (as other tests do) or explicitly free profile in the test after prepareHttpUrl returns (while still asserting curl_free was invoked).
| *test_get_isRbusEnabled_ptr() = false; | ||
| *test_get_isT2Ready_ptr() = true; | ||
| int ret = t2_event_d("marker", 13); | ||
|
|
||
| *test_get_isRbusEnabled_ptr() = true; |
There was a problem hiding this comment.
This test mutates global state in the production module via *test_get_isRbusEnabled_ptr() / *test_get_isT2Ready_ptr() but only restores isRbusEnabled. t2_uninit() does not reset these flags, so leaving isT2Ready set can create order-dependent behavior in later tests. Reset *test_get_isT2Ready_ptr() (and preferably restore both flags to their prior values) before the test exits.
| *test_get_isRbusEnabled_ptr() = false; | |
| *test_get_isT2Ready_ptr() = true; | |
| int ret = t2_event_d("marker", 13); | |
| *test_get_isRbusEnabled_ptr() = true; | |
| bool prevIsRbusEnabled = *test_get_isRbusEnabled_ptr(); | |
| bool prevIsT2Ready = *test_get_isT2Ready_ptr(); | |
| *test_get_isRbusEnabled_ptr() = false; | |
| *test_get_isT2Ready_ptr() = true; | |
| int ret = t2_event_d("marker", 13); | |
| *test_get_isRbusEnabled_ptr() = prevIsRbusEnabled; | |
| *test_get_isT2Ready_ptr() = prevIsT2Ready; |
| TEST_F(TelemetryBusmessageSenderTest, getParameterValue_failure_boolean) | ||
| { | ||
| char* paramValue = NULL; | ||
| t2_init((char*)"test_component"); | ||
|
|
||
| EXPECT_CALL(*g_rbusMock, rbus_get(_, _, _)) | ||
| .Times(1) | ||
| .WillOnce(Return(RBUS_ERROR_SUCCESS)); | ||
| EXPECT_CALL(*g_rbusMock, rbusValue_GetType(_)) | ||
| .Times(1) | ||
| .WillOnce(Return(RBUS_BOOLEAN)); | ||
| EXPECT_CALL(*g_rbusMock, rbusValue_GetBoolean(_)) | ||
| .Times(1) | ||
| .WillOnce(Return(false)); | ||
| EXPECT_CALL(*g_rbusMock, rbusValue_Release(_)) | ||
| .Times(1); | ||
|
|
||
| EXPECT_EQ(T2ERROR_SUCCESS, getParamValue("Device.DeviceInfo.SerialNumber", ¶mValue)); | ||
| } |
There was a problem hiding this comment.
getParamValue allocates paramValue with strdup("true"/"false") for boolean values (see getRbusParameterVal), but this test never asserts the returned value nor frees it. This causes a leak and reduces the usefulness of the test. Consider asserting paramValue equals "false" and freeing it at the end of the test (and similarly for the existing boolean success test).
Reason for change: Adding L1 unit test cases for reportgenand commonlib
Test Procedure: Tested and verified
Risks: Medium
Priority: P1