Skip to content

Update run_ut.sh#1

Open
Nithishkumar-T wants to merge 28 commits intodevelopfrom
feature/update_run_ut
Open

Update run_ut.sh#1
Nithishkumar-T wants to merge 28 commits intodevelopfrom
feature/update_run_ut

Conversation

@Nithishkumar-T
Copy link
Copy Markdown
Contributor

Updating run_ut.sh to be more generic.

Updating run_ut.sh to be more generic.
@Nithishkumar-T Nithishkumar-T self-assigned this Jul 15, 2025
@Nithishkumar-T Nithishkumar-T requested a review from a team as a code owner July 15, 2025 09:52
@Nithishkumar-T Nithishkumar-T added the enhancement New feature or request label Jul 15, 2025
@Nithishkumar-T Nithishkumar-T removed the request for review from GoutamD2905 July 21, 2025 11:55
@Nithishkumar-T Nithishkumar-T removed their assignment Oct 9, 2025
GoutamD2905
GoutamD2905 previously approved these changes Apr 30, 2026
Comment thread source/test/ccsp_table_helper_test.cpp Outdated
.Times(1)
.WillOnce(Return(table_data));
EXPECT_CALL(*g_netsnmpMock, netsnmp_tdata_create_row())
3EXPECT_CALL(*g_netsnmpMock, netsnmp_tdata_create_row())
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this 3EXPECT_CALL api

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3EXPECT_CALL was a typo introduced accidentally in commit 836a546 — the 3 should not have been there. It was already corrected in commit 4371660, which changed it back to EXPECT_CALL. Note that this call is inside a /* ... */ comment block, so it is intentionally disabled test code.

@GoutamD2905
Copy link
Copy Markdown

@copilot resolve the merge conflicts in this pull request

…onflict

Co-authored-by: GoutamD2905 <140494857+GoutamD2905@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 30, 2026 09:46
Copilot AI requested review from a team as code owners April 30, 2026 09:46
@Nithishkumar-T Nithishkumar-T review requested due to automatic review settings April 30, 2026 09:46
Copy link
Copy Markdown

Copilot AI commented Apr 30, 2026

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in commit 692aef5. The only conflict was in CHANGELOG.md — resolved by incorporating the full release history (1.0.0 through 2.1.0) from main.

Copilot AI review requested due to automatic review settings April 30, 2026 10:33
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to make the unit-test runner more generic by switching the UT dependency clone/fetch flow to GitHub (rdkcentral/gmock-broadband) and by building/running all gtest binaries under source/test.

Changes:

  • Updated run_ut.sh to clone/fetch RdkbGMock from GitHub and run all *.bin tests found under source/test.
  • Modified multiple gtest files to adjust mock setup/expectations and some test inputs.
  • Updated test build flags and cleaned minor formatting in CHANGELOG.md.

Reviewed changes

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

Show a summary per file
File Description
source/test/run_ut.sh Switches dependency cloning to GitHub and adds a generic “build all + run all binaries” flow plus coverage generation.
source/test/ccsp_table_helper_test.cpp Alters several tests’ setup/expectations; introduces multiple commented-out blocks and input changes.
source/test/ccsp_scalar_helper_test.cpp Alters the “LoadMibs success” test setup/expectations, largely commenting out the expected calls.
source/test/ccsp_misc_utilities_test.cpp Alters token-chain mocking and changes several “success” tests to use uIndexCount=0 / commented-out assertions.
source/test/Makefile.am Adds an include path for CCSP headers.
CHANGELOG.md Removes an extra blank line.

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

Comment on lines 2756 to 2760
EXPECT_CALL(*g_anscWrapperApiMock, AnscCloneString(_))
.Times(3)
.Times(2)
.WillOnce(Return(componentStruct->componentName))
.WillOnce(Return(componentStruct->dbusPath))
.WillOnce(Return(temp));
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue here: .Times(2) is specified but there are three chained WillOnce(...) actions. This mismatch makes the test unreliable and can hide real regressions. Adjust the expected call count and/or remove the extra WillOnce(...) to match the actual behavior.

Copilot uses AI. Check for mistakes.
.WillRepeatedly(Return(0));
EXPECT_CALL(*g_baseapiMock, CcspBaseIf_GetNextLevelInstances(_,_,_,_,_,_))
.Times(3)
.Times(1)
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expectation declares .Times(1) but provides multiple chained WillOnce(...) actions. Align the call count with the number of expected invocations (or collapse to a single WillOnce(...)) so the test behavior is deterministic.

Suggested change
.Times(1)
.Times(3)

Copilot uses AI. Check for mistakes.
Comment on lines 87 to 142
@@ -114,9 +114,9 @@ TEST_F(CcspSnmpPaTestFixture, CcspScalarHelperLoadMibsSuccess)
EXPECT_CALL(*g_safecLibMock, _sprintf_s_chk(_,_,_,_))
.WillRepeatedly(Return(0));
EXPECT_CALL(*g_anscWrapperApiMock, AnscTcAllocate(_,_))
.Times(2)
.Times(1)
.WillRepeatedly(Return(pTokenChain));
EXPECT_CALL(*g_anscWrapperApiMock, AnscTcPopToken(_))
/*EXPECT_CALL(*g_anscWrapperApiMock, AnscTcPopToken(_))
.Times(1)
.WillOnce(Return(static_cast<ANSC_HANDLE>(pStringToken)));
EXPECT_CALL(*g_safecLibMock, _memcpy_s_chk(_,_,_,_,_,_))
@@ -128,7 +128,7 @@ TEST_F(CcspSnmpPaTestFixture, CcspScalarHelperLoadMibsSuccess)
EXPECT_CALL(*g_netsnmpMock, netsnmp_cache_create(_,_,_,_,_))
.Times(1)
.WillOnce(Return(cache));
//EXPECT_CALL(*g_netsnmpMock, netsnmp_cache_handler_owns_cache(_));
EXPECT_CALL(*g_netsnmpMock, netsnmp_cache_handler_owns_cache(_));
EXPECT_CALL(*g_netsnmpMock, netsnmp_create_handler_registration(_,_,_,_,_))
.Times(1)
.WillOnce(Return(reginfo));
@@ -137,7 +137,7 @@ TEST_F(CcspSnmpPaTestFixture, CcspScalarHelperLoadMibsSuccess)
.WillOnce(Return(0));
EXPECT_CALL(*g_netsnmpMock, netsnmp_inject_handler(_, _))
.Times(1)
.WillOnce(Return(0));
.WillOnce(Return(0));*/

CcspScalarHelperLoadMibs(pThisObject, pNode, NULL);
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This “success” test now forces AnscTcAllocate to return NULL, which makes CcspUtilParseOidValueString() return FALSE immediately and causes CcspScalarHelperLoadMibs() to return FALSE early (baseOid parsing fails). The test also no longer asserts the return value, so it can pass while not exercising the success path. Set up AnscTcAllocate to return a valid token chain (and mock the token operations it uses) and assert that CcspScalarHelperLoadMibs() returns TRUE.

Copilot uses AI. Check for mistakes.
Comment on lines +224 to 236
// Create a fake token chain (dummy content, not used directly)
PANSC_TOKEN_CHAIN pTokenChain = NULL;

EXPECT_CALL(*g_anscWrapperApiMock, AnscTcAllocate(_,_))
.Times(1)
.WillOnce(Return(pTokenChain));
EXPECT_CALL(*g_anscWrapperApiMock, AnscTcPopToken(_))
.Times(1)
.WillOnce(Return(static_cast<ANSC_HANDLE>(nullptr)));


// Call the actual function
BOOL result = CcspUtilParseOidValueString(oidString, oidArray, &size);
EXPECT_EQ(result, FALSE);
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CcspUtilParseOidValueString() returns FALSE immediately when AnscTcAllocate() returns NULL (and it does not call AnscTcPopToken at all). With this setup, the AnscTcPopToken expectation will never be satisfied and the test should fail. Either remove the AnscTcPopToken expectation for this failure case, or instead return a valid token chain and mock the token-unlinking calls that the implementation actually uses (e.g. AnscTcUnlinkToken).

Copilot uses AI. Check for mistakes.
Comment on lines +2178 to +2186
/*EXPECT_CALL(*g_baseapiMock, CcspBaseIf_GetNextLevelInstances(_,_,_,_,_,_))
.Times(1)
.WillOnce(DoAll(SetArgPointee<4>(1), SetArgPointee<5>(insArray), Return(CCSP_SUCCESS)));
EXPECT_CALL(*g_baseapiMock, CcspBaseIf_AddTblRow(_,_,_,_,_,_))
.WillRepeatedly(Return(CCSP_SUCCESS));
EXPECT_CALL(*g_safecLibMock, _sprintf_s_chk(_,_,_,_))
.WillRepeatedly(Return(0));
.WillRepeatedly(Return(0));*/

CcspUtilCreateCosaEntry(pValue, value, 3);
CcspUtilCreateCosaEntry(pValue, value, 0);
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This “success” test now calls CcspUtilCreateCosaEntry(..., uIndexCount=0) and comments out the mocks that would normally be exercised. With uIndexCount == 0, the implementation’s loops do not run, so the test can pass without verifying the intended behavior (creating an entry / adding mappings). Use a realistic non-zero uIndexCount and restore/assert the expected backend calls.

Copilot uses AI. Check for mistakes.
Comment on lines +2131 to +2134
CcspUtilDeleteCosaEntry(pValue, value, 0);
// CcspUtilDeleteCosaEntry(pValue, value, 2);
// CcspUtilDeleteCosaEntry(pValue, value, 3);
// CcspUtilDeleteCosaEntry(pValue, value, 4);
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This “success” test now calls CcspUtilDeleteCosaEntry(..., uIndexCount=0), but the implementation explicitly returns FALSE when uIndexCount == 0 (see CcspUtilDeleteCosaEntry in ccsp_mib_utilities.c). As written, the test no longer validates the success case. Pass a non-zero index count and assert the function returns TRUE.

Suggested change
CcspUtilDeleteCosaEntry(pValue, value, 0);
// CcspUtilDeleteCosaEntry(pValue, value, 2);
// CcspUtilDeleteCosaEntry(pValue, value, 3);
// CcspUtilDeleteCosaEntry(pValue, value, 4);
EXPECT_TRUE(CcspUtilDeleteCosaEntry(pValue, value, 1));

Copilot uses AI. Check for mistakes.
Comment on lines +2230 to +2233
EXPECT_STREQ(CcspUtilGetDMParamName(pQueueHeader, value, 0, tempName), "parameter");
// EXPECT_STREQ(CcspUtilGetDMParamName(pQueueHeader, value, 2, tempName), "parameter");
// EXPECT_STREQ(CcspUtilGetDMParamName(pQueueHeader, value, 3, tempName), "parameter");
// EXPECT_STREQ(CcspUtilGetDMParamName(pQueueHeader, value, 4, tempName), "parameter");
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CcspUtilGetDMParamName() does not handle uIndexCount == 0 (it will return an empty string because none of the uIndexCount == 1..4 branches run). This test now passes uIndexCount=0 while still expecting "parameter", so it no longer reflects the real behavior being tested. Pass a valid non-zero index count and assert the returned DM parameter name matches the formatted namespace.

Copilot uses AI. Check for mistakes.
Comment on lines 806 to +830
@@ -817,16 +818,16 @@ TEST_F(CcspSnmpPaTestFixture, CcspTableHelperSetMibValuesModeR2AsnIntSuccess)
EXPECT_CALL(*g_anscWrapperApiMock, AnscQueueSearchEntryByIndex(_,_))
.Times(2)
.WillOnce(Return(static_cast<PSINGLE_LINK_ENTRY>(nullptr)))
.WillOnce(Return((PSINGLE_LINK_ENTRY)pIndexMapping));
.WillOnce(Return((PSINGLE_LINK_ENTRY)pIndexMapping));*/
EXPECT_CALL(*g_safecLibMock, _sprintf_s_chk(_,_,_,_))
.WillRepeatedly(Return(0));
EXPECT_CALL(*g_anscWrapperApiMock, AnscCloneString(_))
.WillRepeatedly(Return(pTemp));
EXPECT_CALL(*g_baseapiMock, CcspBaseIf_setParameterValues(_,_,_,_,_,_,_,_,_))
/*EXPECT_CALL(*g_baseapiMock, CcspBaseIf_setParameterValues(_,_,_,_,_,_,_,_,_))
.Times(1)
.WillOnce(Return(CCSP_SUCCESS));
.WillOnce(Return(CCSP_SUCCESS));*/

EXPECT_EQ(CcspTableHelperSetMibValues((ANSC_HANDLE)pThisObject, reqInfo, requests), SNMP_ERR_NOERROR);
EXPECT_EQ(CcspTableHelperSetMibValues((ANSC_HANDLE)pThisObject, reqInfo, NULL), SNMP_ERR_NOERROR);
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests were changed to call CcspTableHelperSetMibValues(..., NULL) and large parts of the mock setup are commented out. Passing requests == NULL causes the MODE_SET_RESERVE2 loop(s) to be skipped entirely in CcspTableHelperSetMibValues, so the test can pass without exercising the intended Reserve2 behavior. Keep requests non-null and restore the relevant expectations so the test validates the actual code path.

Copilot uses AI. Check for mistakes.
.WillOnce(DoAll(SetArgPointee<4>(&componentStruct), SetArgPointee<5>(1), Return(CCSP_SUCCESS)));
EXPECT_CALL(*g_anscWrapperApiMock, AnscCloneString(_))
.Times(3)
.Times(2)
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mock expectation is internally inconsistent: .Times(2) is specified but there are three chained WillOnce(...) actions. This leaves an unused action at best, and will fail if the production code actually calls AnscCloneString three times. Align the Times(...) count with the number of expected calls, and remove or adjust the extra WillOnce(...).

Suggested change
.Times(2)
.Times(3)

Copilot uses AI. Check for mistakes.
.WillRepeatedly(Return(0));
EXPECT_CALL(*g_baseapiMock, CcspBaseIf_GetNextLevelInstances(_,_,_,_,_,_))
.Times(2)
.Times(1)
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expectation is also inconsistent: .Times(1) is specified but there are two chained WillOnce(...) actions. Either reduce to a single WillOnce(...) or update the expected call count to match the code under test.

Suggested change
.Times(1)
.Times(2)

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings April 30, 2026 11:04
Copy link
Copy Markdown

Copilot AI left a 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 7 out of 7 changed files in this pull request and generated 7 comments.


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

Comment on lines 87 to 118
@@ -114,9 +114,9 @@ TEST_F(CcspSnmpPaTestFixture, CcspScalarHelperLoadMibsSuccess)
EXPECT_CALL(*g_safecLibMock, _sprintf_s_chk(_,_,_,_))
.WillRepeatedly(Return(0));
EXPECT_CALL(*g_anscWrapperApiMock, AnscTcAllocate(_,_))
.Times(2)
.Times(1)
.WillRepeatedly(Return(pTokenChain));
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This “LoadMibsSuccess” test sets pTokenChain to NULL and then mocks AnscTcAllocate to return that NULL pointer. CcspUtilParseOidValueString returns FALSE when AnscTcAllocate returns NULL, causing CcspScalarHelperLoadMibs to fail early, so the test no longer exercises the success path (and it doesn’t assert the return value). Provide a non-NULL token chain (or avoid mocking AnscTcAllocate to NULL) and assert that CcspScalarHelperLoadMibs returns TRUE for a real success case.

Copilot uses AI. Check for mistakes.
.WillRepeatedly(Return(0));
EXPECT_CALL(*g_baseapiMock, CcspBaseIf_GetNextLevelInstances(_,_,_,_,_,_))
.Times(2)
.Times(1)
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For IndexMapQueue depth 2, CcspTableHelperRefreshCache calls CcspBaseIf_GetNextLevelInstances once per index depth (2 calls total for depth==2). Reducing this expectation to .Times(1) means the test either won’t match actual behavior or will only pass if earlier logic is bypassed. Adjust the expected call count to reflect the real call pattern and ensure the test data makes the function traverse the depth-2 path.

Suggested change
.Times(1)
.Times(2)

Copilot uses AI. Check for mistakes.
Comment on lines +2113 to +2116
CcspUtilDeleteCosaEntry(pValue, value, 0);
// CcspUtilDeleteCosaEntry(pValue, value, 2);
// CcspUtilDeleteCosaEntry(pValue, value, 3);
// CcspUtilDeleteCosaEntry(pValue, value, 4);
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CcspUtilDeleteCosaEntry returns FALSE when uIndexCount == 0 (see implementation), but this test calls it with 0 and doesn’t assert the result. For a “Success” test, pass a valid uIndexCount (>=1) and add an assertion on the return value to ensure the function actually succeeds.

Copilot uses AI. Check for mistakes.
Comment on lines +2160 to 2169
/*EXPECT_CALL(*g_baseapiMock, CcspBaseIf_GetNextLevelInstances(_,_,_,_,_,_))
.Times(1)
.WillOnce(DoAll(SetArgPointee<4>(1), SetArgPointee<5>(insArray), Return(CCSP_SUCCESS)));
EXPECT_CALL(*g_baseapiMock, CcspBaseIf_AddTblRow(_,_,_,_,_,_))
.WillRepeatedly(Return(CCSP_SUCCESS));
EXPECT_CALL(*g_safecLibMock, _sprintf_s_chk(_,_,_,_))
.WillRepeatedly(Return(0));
.WillRepeatedly(Return(0));*/

CcspUtilCreateCosaEntry(pValue, value, 3);
CcspUtilCreateCosaEntry(pValue, value, 0);

Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This “CcspUtilCreateCosaEntrySuccess” test now calls CcspUtilCreateCosaEntry(..., uIndexCount=0) and has the key backend expectations commented out, so it no longer validates table-entry creation behavior. Use a realistic uIndexCount and restore/assert the expected interactions (e.g., instance enumeration / add row) so the test covers the intended success path.

Copilot uses AI. Check for mistakes.
EXPECT_STREQ(CcspUtilGetDMParamName(pQueueHeader, value, 2, tempName), "parameter");
EXPECT_STREQ(CcspUtilGetDMParamName(pQueueHeader, value, 3, tempName), "parameter");
EXPECT_STREQ(CcspUtilGetDMParamName(pQueueHeader, value, 4, tempName), "parameter");
EXPECT_STREQ(CcspUtilGetDMParamName(pQueueHeader, value, 0, tempName), "parameter");
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CcspUtilGetDMParamName doesn’t have a branch for uIndexCount == 0 and will format an empty string; calling it with 0 here makes the test rely on the AnscCloneString mock rather than validating the formatted DM name. Pass a valid uIndexCount (1-4) and assert on the real formatted output.

Copilot uses AI. Check for mistakes.
Comment thread source/test/run_ut.sh
Comment on lines +35 to +37
log "INFO" "Cloning RdkbGMock repository from GitHub..."
# Use token for authentication if provided
if git clone -b "develop" "https://github.com/rdkcentral/gmock-broadband.git" RdkbGMock; then
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says “Use token for authentication if provided”, but the clone URL is always unauthenticated HTTPS and no token env var is referenced. Either remove the comment or implement token-based auth (e.g., via GITHUB_TOKEN/GH_TOKEN) to avoid confusion.

Copilot uses AI. Check for mistakes.
Comment on lines 802 to 831
EXPECT_CALL(*g_netsnmpMock, netsnmp_tdata_extract_entry(_))
.WillRepeatedly(Return(static_cast<void *>(pEntry)));
EXPECT_CALL(*g_netsnmpMock, netsnmp_extract_table_info(_))
.WillRepeatedly(Return(table_info));
EXPECT_CALL(*g_netsnmpMock, netsnmp_tdata_extract_table(_))
/*EXPECT_CALL(*g_netsnmpMock, netsnmp_tdata_extract_table(_))
.Times(1)
.WillOnce(Return(table_data));
EXPECT_CALL(*g_netsnmpMock, netsnmp_tdata_create_row())
.Times(1)
.WillOnce(Return(row));
EXPECT_CALL(*g_netsnmpMock, snmp_varlist_add_variable(_,_,_,_,_,_))
.Times(1)
.WillOnce(Return(static_cast<variable_list*>(nullptr)));
EXPECT_CALL(*g_netsnmpMock, netsnmp_tdata_add_row(_,_))
.Times(1)
.WillOnce(Return(NULL));
EXPECT_CALL(*g_anscWrapperApiMock, AnscQueueSearchEntryByIndex(_,_))
.Times(2)
.WillOnce(Return(static_cast<PSINGLE_LINK_ENTRY>(nullptr)))
.WillOnce(Return((PSINGLE_LINK_ENTRY)pIndexMapping));
.WillOnce(Return((PSINGLE_LINK_ENTRY)pIndexMapping));*/
EXPECT_CALL(*g_safecLibMock, _sprintf_s_chk(_,_,_,_))
.WillRepeatedly(Return(0));
EXPECT_CALL(*g_anscWrapperApiMock, AnscCloneString(_))
.WillRepeatedly(Return(pTemp));
EXPECT_CALL(*g_baseapiMock, CcspBaseIf_setParameterValues(_,_,_,_,_,_,_,_,_))
/*EXPECT_CALL(*g_baseapiMock, CcspBaseIf_setParameterValues(_,_,_,_,_,_,_,_,_))
.Times(1)
.WillOnce(Return(CCSP_SUCCESS));
.WillOnce(Return(CCSP_SUCCESS));*/

EXPECT_EQ(CcspTableHelperSetMibValues((ANSC_HANDLE)pThisObject, reqInfo, requests), SNMP_ERR_NOERROR);
EXPECT_EQ(CcspTableHelperSetMibValues((ANSC_HANDLE)pThisObject, reqInfo, NULL), SNMP_ERR_NOERROR);

Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this “Success” test, most of the core expectations are commented out and CcspTableHelperSetMibValues is called with requests = NULL. In the implementation, passing NULL causes the request-processing loops to be skipped and the function returns SNMP_ERR_NOERROR trivially, so this no longer validates the MODE_SET_RESERVE2 behavior. Pass the real requests object and restore the relevant mocks/expectations so the code path under test is actually exercised.

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings April 30, 2026 11:52
Copy link
Copy Markdown

Copilot AI left a 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 7 out of 7 changed files in this pull request and generated 7 comments.


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

.WillRepeatedly(Return(0));
EXPECT_CALL(*g_baseapiMock, CcspBaseIf_GetNextLevelInstances(_,_,_,_,_,_))
.Times(3)
.Times(1)
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.Times(1) is specified but three .WillOnce(...) actions are chained for CcspBaseIf_GetNextLevelInstances. This either leaves dead actions or will fail if the function is called multiple times (as the three different return values suggest). Adjust the call count and actions to the intended behavior being tested.

Suggested change
.Times(1)
.Times(3)

Copilot uses AI. Check for mistakes.
Comment on lines +248 to 255
PANSC_TOKEN_CHAIN pTokenChain = NULL;

EXPECT_CALL(*g_anscWrapperApiMock, AnscTcAllocate(_,_))
.Times(1)
.WillOnce(Return(pTokenChain));
EXPECT_CALL(*g_anscWrapperApiMock, AnscTcPopToken(_))
.Times(1)
.WillOnce(Return(static_cast<ANSC_HANDLE>(nullptr)));
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CcspUtilParseOidValueString returns immediately when AnscTcAllocate returns NULL, so the subsequent expectation that AnscTcPopToken is called will never be satisfied. Either return a non-NULL token chain from AnscTcAllocate and drive the failure via a NULL token, or drop the AnscTcPopToken expectation when simulating allocation failure.

Copilot uses AI. Check for mistakes.
Comment thread source/test/run_ut.sh
Comment on lines +35 to +40
log "INFO" "Cloning RdkbGMock repository from GitHub..."
# Use token for authentication if provided
if git clone -b "develop" "https://github.com/rdkcentral/gmock-broadband.git" RdkbGMock; then
cd RdkbGMock
else
log "ERROR" "Failed to clone RdkbGMock repository."
log "ERROR" "Failed to clone repository with branch: develop"
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says “Use token for authentication if provided”, but the clone/fetch URLs don’t actually incorporate a token, and the branch is hard-coded to "develop". To make this script truly generic, accept the repo URL/branch via env vars (with sensible defaults) and either remove the token comment or implement token-based auth (e.g., via an injected header/credential helper).

Copilot uses AI. Check for mistakes.
Comment thread source/test/run_ut.sh
Comment on lines +137 to +141
for bin_file in "${bin_files[@]}"; do
if [[ -x "$bin_file" ]]; then
log "INFO" "Running $(basename "$bin_file")"
"$bin_file"
log "INFO" "Completed Test Execution for $(basename "$bin_file")"
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_all_gtests executes each *.bin but does not check the binary’s exit status; with set +e default, failing gtests can be ignored and the script can still exit 0. Capture the return code after each binary run and exit 1 (or accumulate failures) when any test fails.

Copilot uses AI. Check for mistakes.
Comment on lines +72 to 77
//extern ANSC_HANDLE g_pMyChildNode;
ANSC_HANDLE g_pMyChildNode = NULL;
MyCreateFunction();

if(g_pMyChildNode != NULL)
{
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test shadows the global g_pMyChildNode by declaring a new local variable initialized to NULL. As a result, MyCreateFunction() populates the global but the test checks the local (still NULL), so the test body never runs and the global is leaked for the rest of the test suite. Remove the local declaration and keep the extern so the test exercises the intended path and cleans up the created node.

Copilot uses AI. Check for mistakes.
Comment on lines +78 to 81
PCCSP_INDEX_MAPPING pIndexMapping = NULL;
memset(pIndexMapping, 0, sizeof(CCSP_INDEX_MAPPING));
pIndexMapping->MibInfo.uType = 1;

Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pIndexMapping is set to NULL and then immediately passed to memset and dereferenced. If the surrounding if (g_pMyChildNode != NULL) block executes (as intended for this “Success” test), this will segfault. Allocate pIndexMapping (and check it) before zeroing/using it, or remove the block if the test is no longer meant to exercise CcspTableHelperLoadMibs.

Copilot uses AI. Check for mistakes.
.WillOnce(DoAll(SetArgPointee<4>(&componentStruct), SetArgPointee<5>(1), Return(CCSP_SUCCESS)));
EXPECT_CALL(*g_anscWrapperApiMock, AnscCloneString(_))
.Times(3)
.Times(2)
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue here: .Times(2) but three .WillOnce(...) actions are chained for AnscCloneString. Please make the expected call count match the number of distinct returns needed (or remove the extra action) so the test reflects the real call sequence.

Suggested change
.Times(2)
.Times(3)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants