RDK-60236 : Remote Debugger Supports UUID Information in Debug Report File#201
Conversation
Code Coverage Summary |
There was a problem hiding this comment.
Pull request overview
Adds functional test coverage for remote debugger static profile report handling when issue types include suffixes, including an overlength suffix negative case.
Changes:
- Adds a positive suffix functional test for
Device.Info_ab1bghjh. - Adds a negative overlength suffix functional test for
Device.Info_ab1bghjhfhk. - Both tests validate log output, report generation, upload flow, and cleanup.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py |
Adds positive functional coverage for static profile report upload with an accepted issue suffix. |
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix_negative_case.py |
Adds functional coverage for overlength suffix discard behavior and upload flow. |
Comments suppressed due to low confidence (6)
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py:107
- This stop-service expectation has the same service-name mismatch: the implementation logs remote_debugger_, not remote_debugger_. The expected substring has a space immediately after Device.Info, so it will not be found in the actual log.
SERVICE_STOP = f"Stopping remote_debugger_{ISSUE_STRING} service"
assert SERVICE_STOP in grep_rrdlogs(SERVICE_STOP)
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py:131
- These constants are assigned to the opposite messages: SCRIPT_SUCCESS contains the failure log and SCRIPT_FAILURE contains the success log. This makes the test output report failures as successes and successes as failures.
SCRIPT_SUCCESS = "Debug Information Report upload Failed"
SCRIPT_FAILURE = "Debug Information Report upload Success"
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix_negative_case.py:107
- This stop-service expectation has the same service-name mismatch: the implementation logs remote_debugger_, not remote_debugger_. The expected substring has a space immediately after Device.Info, so it will not be found in the actual log.
SERVICE_STOP = f"Stopping remote_debugger_{ISSUE_STRING} service"
assert SERVICE_STOP in grep_rrdlogs(SERVICE_STOP)
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix_negative_case.py:134
- These constants are assigned to the opposite messages: SCRIPT_SUCCESS contains the failure log and SCRIPT_FAILURE contains the success log. This makes the test output report failures as successes and successes as failures.
SCRIPT_SUCCESS = "Debug Information Report upload Failed"
SCRIPT_FAILURE = "Debug Information Report upload Success"
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix_negative_case.py:130
- This test never asserts that an upload status log was present. If neither success nor failure is logged, it only prints a message and still passes, so a broken or skipped upload report path will not be caught.
if UPLOAD_SUCCESS in grep_rrdlogs(UPLOAD_SUCCESS):
print("Upload success")
elif UPLOAD_FAILURE in grep_rrdlogs(UPLOAD_FAILURE):
print("Upload failed")
else:
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py:127
- This test never asserts that an upload status log was present. If neither success nor failure is logged, it only prints a message and still passes, so a broken or skipped upload report path will not be caught.
if UPLOAD_SUCCESS in grep_rrdlogs(UPLOAD_SUCCESS):
print("Upload success")
elif UPLOAD_FAILURE in grep_rrdlogs(UPLOAD_FAILURE):
print("Upload failed")
else:
Code Coverage Summary |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (8)
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py:115
- The result from
check_output_dir()is only printed, so a missing or emptydebug_outputs.txtreturns an error string but the test still passes. Assert that the helper returned success, otherwise this test will not catch failures to generate usable debug output.
result = check_output_dir()
print(result)
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix_negative_case.py:118
- The result from
check_output_dir()is only printed, so a missing or emptydebug_outputs.txtreturns an error string but the test still passes. Assert that the helper returned success, otherwise this test will not catch failures to generate usable debug output.
result = check_output_dir()
print(result)
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py:127
- This upload-status check never asserts on the outcome: both an upload failure and a missing status log only print messages, so
test_remotedebugger_upload_reportcan pass even when the report was not uploaded. Make the expected success/failure behavior explicit with assertions for this scenario.
if UPLOAD_SUCCESS in grep_rrdlogs(UPLOAD_SUCCESS):
print("Upload success")
elif UPLOAD_FAILURE in grep_rrdlogs(UPLOAD_FAILURE):
print("Upload failed")
else:
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix_negative_case.py:130
- This upload-status check never asserts on the outcome: both an upload failure and a missing status log only print messages, so
test_remotedebugger_upload_reportcan pass even when the report was not uploaded. Make the expected success/failure behavior explicit with assertions for this scenario.
if UPLOAD_SUCCESS in grep_rrdlogs(UPLOAD_SUCCESS):
print("Upload success")
elif UPLOAD_FAILURE in grep_rrdlogs(UPLOAD_FAILURE):
print("Upload failed")
else:
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py:134
- The success/failure labels are inverted:
SCRIPT_SUCCESSsearches for the "Failed" log andSCRIPT_FAILUREsearches for the "Success" log, which makes the diagnostic output report the opposite result.
SCRIPT_SUCCESS = "Debug Information Report upload Failed"
SCRIPT_FAILURE = "Debug Information Report upload Success"
if SCRIPT_SUCCESS in grep_rrdlogs(SCRIPT_SUCCESS):
print("Script execution success")
elif SCRIPT_FAILURE in grep_rrdlogs(SCRIPT_FAILURE):
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix_negative_case.py:137
- The success/failure labels are inverted:
SCRIPT_SUCCESSsearches for the "Failed" log andSCRIPT_FAILUREsearches for the "Success" log, which makes the diagnostic output report the opposite result.
SCRIPT_SUCCESS = "Debug Information Report upload Failed"
SCRIPT_FAILURE = "Debug Information Report upload Success"
if SCRIPT_SUCCESS in grep_rrdlogs(SCRIPT_SUCCESS):
print("Script execution success")
elif SCRIPT_FAILURE in grep_rrdlogs(SCRIPT_FAILURE):
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py:85
ISSUE_FOUNDis constructed but never checked, so this intended validation of the parsed issue node has no effect. Either assert it against the logs or remove the unused code to avoid giving the test a false sense of coverage.
ISSUE_FOUND = f"Issue Data Node: {ISSUE_NODE} and Sub-Node: {ISSUE_SUBNODE} found in Static JSON File /etc/rrd/remote_debugger.json"
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix_negative_case.py:85
ISSUE_FOUNDis constructed but never checked, so this intended validation of the parsed issue node has no effect. Either assert it against the logs or remove the unused code to avoid giving the test a false sense of coverage.
ISSUE_FOUND = f"Issue Data Node: {ISSUE_NODE} and Sub-Node: {ISSUE_SUBNODE} found in Static JSON File /etc/rrd/remote_debugger.json"
Code Coverage Summary |
... Updated in 8b4805c. I changed the service log assertions to match the timestamped service name format by asserting stable prefixes ( Screenshot: N/A (no UI changes). |
Code Coverage Summary |
Code Coverage Summary |
3 similar comments
Code Coverage Summary |
Code Coverage Summary |
Code Coverage Summary |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (8)
test/functional-tests/tests/test_rrd_dynamic_subcategory_report.py:139
- This assertion now only checks the journalctl service-name prefix. The implementation logs the same prefix for both journalctl success and failure, so this test can pass even when journalctl failed.
JOURNALCTL_SUCCESS = "journalctl remote_debugger_Test.TestRun1"
assert JOURNALCTL_SUCCESS in grep_rrdlogs(JOURNALCTL_SUCCESS)
test/functional-tests/tests/test_rrd_c_api_upload.py:100
- This assertion now only checks the journalctl service-name prefix. The failure log contains the same prefix, so the test can pass even if journalctl fails.
JOURNAL_START = f"journalctl remote_debugger_{ISSUE_STRING}"
assert JOURNAL_START in grep_rrdlogs(JOURNAL_START)
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py:100
- The journalctl log includes the timestamped service name, so this exact substring without the timestamp will not match the current log format when the test is run.
JOURNAL_START = f"journalctl remote_debugger_{ISSUE_STRING} service success"
assert JOURNAL_START in grep_rrdlogs(JOURNAL_START)
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py:127
- This upload-status check never asserts a required outcome; upload failure or missing status only prints a message and the test still passes, so the report-upload behavior is not actually validated.
if UPLOAD_SUCCESS in grep_rrdlogs(UPLOAD_SUCCESS):
print("Upload success")
elif UPLOAD_FAILURE in grep_rrdlogs(UPLOAD_FAILURE):
print("Upload failed")
else:
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix_negative_case.py:130
- This upload-status check never asserts a required outcome; upload failure or missing status only prints a message and the test still passes, so the report-upload behavior is not actually validated.
if UPLOAD_SUCCESS in grep_rrdlogs(UPLOAD_SUCCESS):
print("Upload success")
elif UPLOAD_FAILURE in grep_rrdlogs(UPLOAD_FAILURE):
print("Upload failed")
else:
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py:107
- The stop-service log uses the timestamped runtime service name, so this exact substring without the timestamp will not match the current log format when the test is run.
SERVICE_STOP = f"Stopping remote_debugger_{ISSUE_STRING} service"
assert SERVICE_STOP in grep_rrdlogs(SERVICE_STOP)
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix.py:115
- check_output_dir() returns an error string when debug_outputs.txt is missing or empty, but this test only prints the result, so it can pass without verifying the generated debug output exists.
result = check_output_dir()
print(result)
test/functional-tests/tests/test_rrd_static_profile_report_with_suffix_negative_case.py:118
- check_output_dir() returns an error string when debug_outputs.txt is missing or empty, but this test only prints the result, so it can pass without verifying the generated debug output exists.
result = check_output_dir()
print(result)
Code Coverage Summary |
Code Coverage Summary |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 13 comments.
Comments suppressed due to low confidence (3)
test/functional-tests/tests/test_rrd_static_profile_category_report.py:122
- This assertion no longer verifies that journalctl collection succeeded. The implementation logs both
journalctl <service> service success...andjournalctl <service> service failed!!!, so checking only the prefix will also pass on failure.
JOURNAL_START = f"journalctl remote_debugger_{issue_string}"
assert JOURNAL_START in grep_rrdlogs(JOURNAL_START)
test/functional-tests/tests/test_rrd_dynamic_profile_report.py:169
- This assertion no longer verifies that journalctl collection succeeded. The implementation logs both
journalctl <service> service success...andjournalctl <service> service failed!!!, so checking only the prefix will also pass on failure.
JOURNAL_START = f"journalctl remote_debugger_{testrun2_string}"
assert JOURNAL_START in grep_rrdlogs(JOURNAL_START)
test/functional-tests/tests/test_rrd_deepsleep_static_report.py:122
- This assertion no longer verifies that journalctl collection succeeded. The implementation logs both
journalctl <service> service success...andjournalctl <service> service failed!!!, so checking only the prefix will also pass on failure.
JOURNAL_START = f"journalctl remote_debugger_Video.Video"
assert JOURNAL_START in grep_rrdlogs(JOURNAL_START)
Code Coverage Summary |
Code Coverage Summary |
Code Coverage Summary |
Create test_rrd_static_profile_report_with_suffix_negative_case.py Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> RRD 1.3.4 release changelog updates Fix dynamic subcategory service log assertions for timestamped names Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/f0806557-f409-45cc-8993-700be01b59f2 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Remove accidental pycache artifact from test changes Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/f0806557-f409-45cc-8993-700be01b59f2 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Update test_rrd_background_cmd_static_profile_report.py Update test_rrd_c_api_upload.py Update test_rrd_debug_report_upload.py Update test_rrd_deepsleep_static_report.py Update test_rrd_dynamic_profile_report.py Update test_rrd_static_profile_category_report.py Update test_rrd_static_profile_report.py Update test_rrd_static_profile_report_with_suffix.py Update test_rrd_static_profile_report_with_suffix_negative_case.py Update test_rrd_background_cmd_static_profile_report.py Update test_rrd_dynamic_profile_missing_report.py Strengthen service-start assertions for timestamped runtime names Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/ec85a8af-7e00-4410-8ca2-4e20cd06cfd4 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Add helper docstring for service-start success assertion Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/ec85a8af-7e00-4410-8ca2-4e20cd06cfd4 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Improve helper docstring with Args and Raises Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/ec85a8af-7e00-4410-8ca2-4e20cd06cfd4 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Refine service-start helper naming and assertion messages Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/ec85a8af-7e00-4410-8ca2-4e20cd06cfd4 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Update test_rrd_background_cmd_static_profile_report.py Update test_rrd_dynamic_profile_missing_report.py Add feature files for static suffix report scenarios Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/59dfaf33-25ef-48a2-9549-dd1f571a3b55 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> L2 Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Update test_rrd_static_profile_report_with_suffix.py Update test_rrd_static_profile_report_with_suffix_negative_case.py Update test_rrd_static_profile_report_with_suffix_negative_case.py Fix service check in negative test case Update test_rrd_static_profile_report_with_suffix_negative_case.py Update test_rrd_dynamic_profile_missing_report.py Update test_rrd_dynamic_profile_missing_report.py Update test_rrd_dynamic_profile_missing_report.py Update test_rrd_dynamic_profile_missing_report.py Update test_rrd_dynamic_profile_missing_report.py Stabilize remotedebugger startup in missing-profile test Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/eafe5a9f-2bbc-448a-966f-61c3067b7239 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Remove accidental pycache artifact from PR Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/eafe5a9f-2bbc-448a-966f-61c3067b7239 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Replace startup retry magic numbers with constants Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/eafe5a9f-2bbc-448a-966f-61c3067b7239 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Remove accidental pycache artifact again Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/eafe5a9f-2bbc-448a-966f-61c3067b7239 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Refine remotedebugger PID retry logic in missing-profile test Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/eafe5a9f-2bbc-448a-966f-61c3067b7239 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com> Simplify startup PID retry loop for missing-profile test Agent-Logs-Url: https://github.com/rdkcentral/remote_debugger/sessions/eafe5a9f-2bbc-448a-966f-61c3067b7239 Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com>
ceb9e16 to
fd6a13f
Compare
Code Coverage Summary |
Code Coverage Summary |
Reason For Change : FIx , Add L2 for the feature to support UUID Information