-
Notifications
You must be signed in to change notification settings - Fork 4
Add AccountID validation unit tests for RDKEMW-11615 #132
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
Vismalskumar0
commented
Dec 17, 2025
- Add test cases for empty AccountID rejection
- Add test cases for Unknown AccountID replacement
- Add test cases for valid AccountID acceptance
- Add case-insensitive comparison tests
- Add config value change detection tests
- Add test cases for empty AccountID rejection - Add test cases for Unknown AccountID replacement - Add test cases for valid AccountID acceptance - Add case-insensitive comparison tests - Add config value change detection tests
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
This PR adds comprehensive unit tests for AccountID validation functionality as part of RDKEMW-11615. The tests verify that the system correctly handles empty AccountID values, replaces "Unknown" AccountID values with stored valid values, accepts valid AccountID values, and performs case-insensitive comparisons. Additionally, the PR includes a minor fix to initialize the _ebuild_type member variable in the XconfHandler constructor.
Changes:
- Added unit tests for GetAccountID method with valid, unknown, and empty values
- Added unit tests for GetValidAccountId method to verify replacement of invalid values
- Added test fixture with helper validation tests for AccountID scenarios
- Initialized _ebuild_type member in XconfHandler constructor
- Added FRIEND_TEST declarations for new test cases
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/rfcTest.cpp | Adds blank lines (appears unrelated to PR purpose) |
| rfcMgr/xconf_handler.h | Initializes _ebuild_type member variable in constructor |
| rfcMgr/rfc_xconf_handler.h | Adds FRIEND_TEST declarations for new AccountID validation tests |
| rfcMgr/gtest/gtest_main.cpp | Adds AccountID validation test fixture and 5 new test cases for GetAccountID and GetValidAccountId methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FRIEND_TEST(rfcMgrTest, ValidPartnerId); | ||
| FRIEND_TEST(rfcMgrTest, Removed_PERSISTENCE_FILE); | ||
| FRIEND_TEST(rfcMgrTest, EmptyFeatures); | ||
| FRIEND_TEST(rfcMgrTest, GetAccountID_ValidValue); |
Copilot
AI
Jan 14, 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.
There is an extra leading space before the FRIEND_TEST declaration. All other FRIEND_TEST declarations in this file start without leading spaces. Please remove the extra space to maintain consistency with the rest of the file.
| FRIEND_TEST(rfcMgrTest, GetAccountID_ValidValue); | |
| FRIEND_TEST(rfcMgrTest, GetAccountID_ValidValue); |
| class AccountIDValidationTest : public ::testing::Test { | ||
| protected: | ||
| std::string currentAccountID; | ||
| std::string unknownStr; | ||
|
|
||
| void SetUp() override { | ||
| currentAccountID = "3064488088886635972"; | ||
| unknownStr = "Unknown"; | ||
| } | ||
| }; | ||
|
|
||
| // Test case: Empty AccountID should be rejected | ||
| TEST_F(AccountIDValidationTest, EmptyAccountIDRejected) | ||
| { | ||
| std::string emptyValue = ""; | ||
| EXPECT_TRUE(emptyValue.empty()); | ||
| } | ||
|
|
||
| // Test case: Unknown AccountID should be replaced with current value | ||
| TEST_F(AccountIDValidationTest, UnknownAccountIDReplaced) | ||
| { | ||
| std::string receivedAccountID = "Unknown"; | ||
| std::string replacementAccountID = currentAccountID; | ||
|
|
||
| // Check if received value is "Unknown" | ||
| bool isUnknown = StringCaseCompare(receivedAccountID, unknownStr); | ||
| EXPECT_TRUE(isUnknown); | ||
|
|
||
| // When unknown, should use current value | ||
| if (isUnknown) { | ||
| receivedAccountID = replacementAccountID; | ||
| } | ||
|
|
||
| EXPECT_EQ(receivedAccountID, "3064488088886635972"); | ||
| } | ||
|
|
||
| // Test case: Valid AccountID should be accepted | ||
| TEST_F(AccountIDValidationTest, ValidAccountIDAccepted) | ||
| { | ||
| std::string validAccountID = "3064488088886635972"; | ||
| std::string currentValue = "OldAccountID"; | ||
|
|
||
| // Check if it's not empty and not "Unknown" | ||
| bool isValid = (!validAccountID.empty() && !StringCaseCompare(validAccountID, unknownStr)); | ||
| EXPECT_TRUE(isValid); | ||
| } | ||
|
|
||
| // Test case: AccountID comparison should be case-insensitive | ||
| TEST_F(AccountIDValidationTest, UnknownCaseInsensitiveComparison) | ||
| { | ||
| std::string unknownUpper = "UNKNOWN"; | ||
| std::string unknownMixed = "UnKnOwN"; | ||
|
|
||
| EXPECT_TRUE(StringCaseCompare(unknownUpper, unknownStr)); | ||
| EXPECT_TRUE(StringCaseCompare(unknownMixed, unknownStr)); | ||
| } | ||
|
|
||
| // Test case: Config value change detection | ||
| TEST_F(AccountIDValidationTest, ConfigValueChangeDetection) | ||
| { | ||
| std::string currentValue = "OldAccountID"; | ||
| std::string newValue = "3064488088886635972"; | ||
|
|
||
| bool valueChanged = (currentValue != newValue); | ||
| EXPECT_TRUE(valueChanged); | ||
| } | ||
|
|
||
| // Test case: No change when current and new values are same | ||
| TEST_F(AccountIDValidationTest, NoConfigValueChangeWhenSame) | ||
| { | ||
| std::string currentValue = "3064488088886635972"; | ||
| std::string newValue = "3064488088886635972"; | ||
|
|
||
| bool valueChanged = (currentValue != newValue); | ||
| EXPECT_FALSE(valueChanged); | ||
| } |
Copilot
AI
Jan 14, 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.
These test cases in the AccountIDValidationTest fixture only test local variables and helper functions, but they do not actually test the RuntimeFeatureControlProcessor class methods (GetAccountID or GetValidAccountId) that are the focus of this PR. These tests validate the logic in isolation but don't verify that the production code correctly implements the AccountID validation behavior. Consider either removing these fixture-based tests since they provide limited value, or refactor them to actually test the RuntimeFeatureControlProcessor methods.
|
|
||
|
|
||
| int add(int num1,int num2) | ||
| { | ||
| return (num1+num2); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| TEST(Add, PositiveCase) | ||
| { | ||
| EXPECT_EQ(30,add(10,20)); | ||
| EXPECT_EQ(50,add(30,20)); | ||
| } | ||
|
|
||
|
|
||
|
|
||
|
|
Copilot
AI
Jan 14, 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.
These blank line additions appear to be unrelated to the PR's purpose of adding AccountID validation tests. The test/rfcTest.cpp file is unrelated to the AccountID validation functionality being tested. Consider removing these unintentional changes to keep the PR focused on the AccountID validation tests described in the PR description.
| int add(int num1,int num2) | |
| { | |
| return (num1+num2); | |
| } | |
| TEST(Add, PositiveCase) | |
| { | |
| EXPECT_EQ(30,add(10,20)); | |
| EXPECT_EQ(50,add(30,20)); | |
| } | |
| int add(int num1,int num2) | |
| { | |
| return (num1+num2); | |
| } | |
| TEST(Add, PositiveCase) | |
| { | |
| EXPECT_EQ(30,add(10,20)); | |
| EXPECT_EQ(50,add(30,20)); | |
| } |
| TEST(rfcMgrTest, GetAccountID_ValidValue) { | ||
| writeToTr181storeFile("Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.AccountInfo.AccountID", "3064488088886635972", "/opt/secure/RFC/tr181store.ini", Quoted); | ||
| RuntimeFeatureControlProcessor *rfcObj = new RuntimeFeatureControlProcessor(); | ||
| rfcObj->GetAccountID(); | ||
| EXPECT_EQ(rfcObj->_accountId, "3064488088886635972"); | ||
| EXPECT_FALSE(StringCaseCompare(rfcObj->_accountId, "Unknown")); | ||
| delete rfcObj; | ||
| } | ||
|
|
||
| TEST(rfcMgrTest, GetAccountID_UnknownValue) { | ||
| writeToTr181storeFile("Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.AccountInfo.AccountID", "Unknown", "/opt/secure/RFC/tr181store.ini", Quoted); | ||
| RuntimeFeatureControlProcessor *rfcObj = new RuntimeFeatureControlProcessor(); | ||
| rfcObj->GetAccountID(); | ||
| EXPECT_TRUE(StringCaseCompare(rfcObj->_accountId, "Unknown")); | ||
| delete rfcObj; | ||
| } | ||
|
|
||
| TEST(rfcMgrTest, GetAccountID_EmptyValue) { | ||
| writeToTr181storeFile("Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.AccountInfo.AccountID", "", "/opt/secure/RFC/tr181store.ini", Quoted); | ||
| RuntimeFeatureControlProcessor *rfcObj = new RuntimeFeatureControlProcessor(); | ||
| rfcObj->GetAccountID(); | ||
| EXPECT_TRUE(rfcObj->_accountId.empty()); | ||
| delete rfcObj; | ||
| } |
Copilot
AI
Jan 14, 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.
The naming convention for these new tests uses PascalCase (GetAccountID_ValidValue, GetAccountID_UnknownValue, GetAccountID_EmptyValue) while existing tests for the same functionality use camelCase (getAccountID, getAccountID_Unknown at line 309 and 1020). For consistency, consider using camelCase naming (e.g., getAccountID_ValidValue) to match the existing test naming pattern in this file.