Feature/rdk 60236 comma#188
Open
Abhinavpv28 wants to merge 45 commits intodevelopfrom
Open
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…dling Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/fbc52780-966b-4912-825f-3030aa43c3e9 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for issue-type “suffixes” (the substring starting at the first underscore) so that the base issue type is used for profile lookup/processing while the suffix can be preserved and appended when uploading debug output artifacts.
Changes:
- Add
data_buf::suffixand propagate initialization/cleanup in the message-buffer lifecycle. - Introduce
split_issue_type()and update issue-type processing to split base vs suffix per comma-separated issue token. - Update issue-type sanitization to retain
_and-, and adjust/add unit tests around the new behavior.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/unittest/rrdUnitTestRunner.cpp | Updates unit tests for issue-type splitting/suffix behavior and message-buffer setup. |
| src/unittest/UTJson/device.properties | Unit test data file used during GTest runs. |
| src/rrdJsonParser.h | Exposes new split_issue_type() helper. |
| src/rrdJsonParser.c | Implements split_issue_type() and uses buff->suffix when constructing upload artifact names; adds additional cleanup. |
| src/rrdInterface.c | Initializes and frees the new data_buf::suffix field. |
| src/rrdEventProcess.c | Splits each issue token into base+suffix and stores suffix per token; expands allowed characters to keep _/-. |
| src/rrdCommon.h | Extends data_buf with a suffix pointer. |
| char str[] = "a@,b,&,cd,ef"; | ||
| char **args = NULL; | ||
| int count = issueTypeSplitter(str, ',', &args); | ||
| int count = issueTypeSplitter(str, NULL, ',', &args); |
Comment on lines
1903
to
1906
| char str[] = ""; | ||
| char **args = NULL; | ||
| int count = issueTypeSplitter(str, ',', &args); | ||
| int count = issueTypeSplitter(str, NULL, ',', &args); | ||
|
|
Comment on lines
+1912
to
+1921
| TEST(IssueTypeSplitterTest, ExtractsSuffixFromIssueType) | ||
| { | ||
| char str[] = "Device.DeviceTime_Search"; | ||
| char outsuffix[128] = {0}; | ||
| char **args = NULL; | ||
| int count = issueTypeSplitter(str, outsuffix, ',', &args); | ||
|
|
||
| ASSERT_EQ(count, 1); | ||
| ASSERT_STREQ(args[0], "Device.DeviceTime"); | ||
| ASSERT_STREQ(outsuffix, "_Search"); |
Comment on lines
+1930
to
+1939
| TEST(IssueTypeSplitterTest, NoSuffixWhenNoUnderscore) | ||
| { | ||
| char str[] = "Device.Network"; | ||
| char outsuffix[128] = {0}; | ||
| char **args = NULL; | ||
| int count = issueTypeSplitter(str, outsuffix, ',', &args); | ||
|
|
||
| ASSERT_EQ(count, 1); | ||
| ASSERT_STREQ(outsuffix, ""); | ||
|
|
| char **cmdMap = NULL; | ||
| int index = 0, count = 0, dataMsgLen = 0; | ||
| data_buf *cmdBuff = NULL; | ||
| char suffix[128] = {0}; |
Comment on lines
90
to
+95
| cmdBuff->inDynamic = rbuf->inDynamic; | ||
| if(cmdBuff->inDynamic) | ||
| { | ||
| cmdBuff->jsonPath = rbuf->jsonPath; | ||
| } | ||
| cmdBuff->appendMode = rbuf->appendMode; | ||
| cmdBuff->appendMode = rbuf->appendMode; |
Comment on lines
295
to
+302
| if (sbuf->jsonPath) | ||
| { | ||
| free(sbuf->jsonPath); | ||
| } | ||
| if (sbuf->suffix) | ||
| { | ||
| free(sbuf->suffix); | ||
| } |
Comment on lines
1887
to
1890
| char str[] = "abcd"; | ||
| char **args = NULL; | ||
| int count = issueTypeSplitter(str, ',', &args); | ||
| int count = issueTypeSplitter(str, NULL, ',', &args); | ||
|
|
Comment on lines
3721
to
+3726
| data_buf rbuf; | ||
| rbuf.mtype = EVENT_MSG; | ||
| rbuf.mdata = strdup("Test"); | ||
| rbuf.inDynamic = true; | ||
| rbuf.jsonPath = nullptr; | ||
| rbuf.suffix = nullptr; |
Code Coverage Summary |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
src/unittest/rrdUnitTestRunner.cpp:1918
- These new tests expect
issueTypeSplitterto extract an underscore suffix intooutsuffix, but the productionissueTypeSplitteronly splits on the provided delimiter and has no suffix output. Consider testingsplit_issue_type()directly for suffix extraction, and keepissueTypeSplittertests focused on delimiter-splitting behavior.
/* --------------- Test processIssueTypeInDynamicProfile() from rrdEventProcess --------------- */
class ProcessIssueTypeInDynamicProfileTest : public ::testing::Test
{
protected:
issueNodeData issuestructNode;
data_buf buff;
src/unittest/rrdUnitTestRunner.cpp:1871
issueTypeSplitteris astatic int issueTypeSplitter(char*, const char, char***)inrrdEventProcess.c(3 params). These test calls pass 4 arguments, so this won’t compile when includingrrdEventProcess.cinto the runner. Update the tests to match the actual signature (or change the function signature consistently if that’s intended).
char **args = NULL;
int count = issueTypeSplitter(str, ',', &args);
src/unittest/rrdUnitTestRunner.cpp:1909
issueTypeSplitterallocatesargs[i]strings (see implementation inrrdEventProcess.c), but this test only freesargsand leaksargs[0]. Free eachargs[i](including index 0) before freeing the array, as done in the other splitter tests.
int count = issueTypeSplitter(str, ',', &args);
ASSERT_EQ(count, 1);
free(args);
Comment on lines
90
to
92
| if(cmdBuff->inDynamic) | ||
| { | ||
| cmdBuff->jsonPath = rbuf->jsonPath; |
Comment on lines
295
to
+302
| if (sbuf->jsonPath) | ||
| { | ||
| free(sbuf->jsonPath); | ||
| } | ||
| if (sbuf->suffix) | ||
| { | ||
| free(sbuf->suffix); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.