-
Notifications
You must be signed in to change notification settings - Fork 2
Usblog l2 60497 backup #88
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?
Changes from all commits
749fd94
772bb94
1c2349e
fac910f
6a43107
61268e9
11fff1c
f4182d5
b8fb729
8278af6
5818f1e
d3ad9be
91a3102
7b80aa4
da2e8f1
2d4e3a6
d4a9002
ff322fb
23d0fc9
a6e03c0
85df765
2c7c5fa
9e6ac13
c419adf
b114d46
1294aef
d4fe9fd
3cbf156
256f22e
6e4755c
2dbec5f
1b5c64b
0ff7872
2cb8759
3488e8d
4bf28f5
e0c1b64
c6e358b
cb104a7
bce77d9
b9184a8
5712cdb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| Feature: USB Log Upload | ||
| This feature covers the USB log upload functionality, including error handling, archive creation, MAC address logging, temp directory cleanup, and success/failure scenarios. | ||
|
Comment on lines
+1
to
+2
|
||
|
|
||
| Scenario: USB not mounted or missing log path | ||
| Given the USB log upload binary is available | ||
| When I run usblogupload with a non-existent mount point | ||
| Then the process should fail with code 2 or 3 | ||
| And a failure message should be logged | ||
|
|
||
| Scenario: Archive creation on valid mount | ||
| Given a valid USB mount point | ||
| When I run usblogupload | ||
| Then the process should exit with code 0 or 3 | ||
| And an archive creation log may appear | ||
|
|
||
| Scenario: MAC address and file log | ||
| Given a valid USB mount point | ||
| When I run usblogupload | ||
| Then the process should exit with code 0 or 3 | ||
| And a log line with MAC address and file name may appear | ||
|
|
||
| Scenario: Temp directory cleanup | ||
| Given a valid USB mount point | ||
| When I run usblogupload | ||
| Then the process should exit with code 0 or 3 | ||
| And a cleanup log may appear | ||
|
|
||
| Scenario: Successful USB log upload | ||
| Given the USB log upload binary is available | ||
| When I run usblogupload with a valid mount point | ||
| Then the process should exit with code 0 | ||
| And a completion message should be logged | ||
|
|
||
| Scenario: Invalid usage | ||
| Given the USB log upload binary is available | ||
| When I run usblogupload with no arguments | ||
| Then the process should exit with code 4 | ||
| And a log about failed logging system initialization may appear | ||
|
|
||
| Scenario: USB not mounted | ||
| Given the USB log upload binary is available | ||
| When I run usblogupload with an unmounted path | ||
| Then the process should exit with code 2 | ||
| And a log about failed USB mount point validation may appear | ||
|
Comment on lines
+4
to
+44
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -34,14 +34,9 @@ def test_upload_cron_present(): | |||||||||||||||||||||
| sleep(20) | ||||||||||||||||||||||
| assert "urn:settings:LogUploadSettings:UploadSchedule:cron" in grep_dcmdlogs("is present setting cron jobs") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @pytest.mark.run(order=2) | ||||||||||||||||||||||
| def test_upload_script_started_onboot_false(): | ||||||||||||||||||||||
| assert "UploadOnReboot=0" in grep_dcmdlogs("Triggered uploadSTBLogs.sh with arguments") | ||||||||||||||||||||||
| assert "Called uploadLogOnReboot with false" in grep_dcmdlogs("Called uploadLogOnReboot with false") | ||||||||||||||||||||||
| sleep(420) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @pytest.mark.run(order=3) | ||||||||||||||||||||||
| def test_fw_cron_scheduled(): | ||||||||||||||||||||||
| sleep(420) | ||||||||||||||||||||||
|
Comment on lines
37
to
+39
|
||||||||||||||||||||||
| @pytest.mark.run(order=3) | |
| def test_fw_cron_scheduled(): | |
| sleep(420) | |
| @pytest.mark.run(order=2) | |
| def test_wait_before_fw_cron_check(): | |
| # Wait for FW cron scheduling window before validation | |
| sleep(420) | |
| @pytest.mark.run(order=3) | |
| def test_fw_cron_scheduled(): |
Copilot
AI
Mar 3, 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.
After the removal of the order=2 test in this file, there is now a gap from order=1 directly to order=3, skipping order=2. While pytest-ordering handles gaps gracefully, it is good practice to renumber the remaining tests sequentially to keep the intent clear and avoid confusion for future maintainers.
Copilot
AI
Mar 2, 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.
In test_log_upload_onreboot_false_case.py, the test test_upload_script_started_onboot_false (order=2) was removed, but the sleep(420) that was inside it has been moved into test_fw_cron_scheduled (order=3). This means the sleep now runs unconditionally for that test case instead of only in the reboot-false path. The sleep was previously in the order=2 function to wait before checking for the false-onboot case. The intent and correctness of this logic change should be verified — if the sleep is truly needed here and in the right test is fine, but the duplication of logic (the moved sleep) could indicate an oversight.
| sleep(420) | |
| assert "Scheduling DCM_FW_UPDATE Job handle" in grep_dcmdlogs("Scheduling DCM_FW_UPDATE Job handle") | |
| max_wait = 420 | |
| interval = 10 | |
| for _ in range(int(max_wait / interval)): | |
| if "Scheduling DCM_FW_UPDATE Job handle" in grep_dcmdlogs("Scheduling DCM_FW_UPDATE Job handle"): | |
| return | |
| sleep(interval) | |
| # Final check after waiting up to max_wait seconds | |
| assert "Scheduling DCM_FW_UPDATE Job handle" in grep_dcmdlogs("Scheduling DCM_FW_UPDATE Job handle") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,11 +28,6 @@ | |
| def test_upload_cron_present(): | ||
| assert "urn:settings:LogUploadSettings:UploadSchedule:cron" in grep_dcmdlogs("is present setting cron jobs") | ||
|
|
||
| @pytest.mark.run(order=2) | ||
| def test_upload_script_started(): | ||
| assert "UploadOnReboot=1" in grep_dcmdlogs("Triggered uploadSTBLogs.sh with arguments") | ||
| assert "Called uploadLogOnReboot with true" in grep_dcmdlogs("Called uploadLogOnReboot with true") | ||
|
|
||
| @pytest.mark.run(order=3) | ||
| def test_fw_cron_scheduled(): | ||
| sleep(540) | ||
|
|
@@ -50,7 +45,5 @@ def test_upload_cron_scheduled(): | |
|
|
||
| @pytest.mark.run(order=6) | ||
| def test_upload_started(): | ||
| assert "Start log upload Script" in grep_dcmdlogs("Start log upload Script") | ||
| assert "FLAG=0" in grep_dcmdlogs("Triggered uploadSTBLogs.sh with arguments") | ||
| assert "Called uploadDCMLogs" in grep_dcmdlogs("Called uploadDCMLogs") | ||
| assert "Start log upload via library API" in grep_dcmdlogs("Start log upload via library API") | ||
|
Comment on lines
47
to
+48
|
||
|
|
||
|
Comment on lines
45
to
49
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -34,11 +34,9 @@ | |||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| def run_uploadlogsnow(): | ||||||||||||||||||||||||||||||||||||||||
| """Execute uploadlogsnow using the specific binary command""" | ||||||||||||||||||||||||||||||||||||||||
| cmd = "/usr/local/bin/logupload uploadlogsnow" | ||||||||||||||||||||||||||||||||||||||||
| result = subprocess.run(cmd, shell=True, capture_output=True, text=True, timeout=300) | ||||||||||||||||||||||||||||||||||||||||
| result = subprocess.run("/usr/local/bin/logupload uploadlogsnow >> /opt/logs/logupload.log.0",shell=True) | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
| result = subprocess.run("/usr/local/bin/logupload uploadlogsnow >> /opt/logs/logupload.log.0",shell=True) | |
| # Run the command with captured output and a timeout to avoid hanging tests | |
| result = sp.run( | |
| ["/usr/local/bin/logupload", "uploadlogsnow"], | |
| capture_output=True, | |
| text=True, | |
| timeout=300, | |
| ) | |
| # Append stdout and stderr to the log file to mimic the original shell redirection | |
| try: | |
| with open("/opt/logs/logupload.log.0", "a", encoding="utf-8") as log_file: | |
| if result.stdout: | |
| log_file.write(result.stdout) | |
| if result.stderr: | |
| log_file.write(result.stderr) | |
| except Exception as exc: | |
| # In test code, print the error but do not fail the test solely due to logging issues | |
| print(f"Failed to write uploadlogsnow output to log file: {exc}") |
Copilot
AI
Mar 2, 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 subprocess module is imported as sp (alias), but sp is never used anywhere in this file. All calls use the bare name subprocess which is implicitly available through the from uploadstblogs_helper import * wildcard import. The unused import subprocess as sp on line 27 is dead code and should be removed to avoid confusion.
Copilot
AI
Mar 2, 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 refactored call removes capture_output=True, text=True, and timeout=300. Without capture_output=True, result.stdout and result.stderr will be None, which will cause AttributeError in any test that accesses those attributes. The missing timeout also means the test can hang indefinitely. These arguments should be restored.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,29 +54,15 @@ def test_normal_upload_initialization(self): | |
|
|
||
| # Run uploadSTBLogs | ||
| #result = run_uploadstblogs() | ||
|
|
||
| result = subprocess.run([ | ||
| "/usr/local/bin/logupload", | ||
| "", | ||
| "1", | ||
| "1", | ||
| "true", | ||
| "HTTP", | ||
| "https://mockxconf:50058/" | ||
| ]) | ||
|
|
||
|
|
||
| result = subprocess.run("/usr/local/bin/logupload '' 1 1 true HTTP https://mockxconf:50058/ >> /opt/logs/logupload.log.0",shell=True) | ||
|
||
|
|
||
| # Verify initialization | ||
| assert result.returncode == 0 or result.returncode == 1, "Upload process should complete" | ||
|
|
||
| # Check initialization logs | ||
| init_logs = grep_uploadstb_logs("Context initialization successful") | ||
| assert len(init_logs) > 0, "Context should be initialized successfully" | ||
|
|
||
| # Verify device properties loaded | ||
| logs = grep_uploadstb_logs("DEVICE_TYPE") | ||
| assert len(logs) > 0, "Device type should be loaded from properties" | ||
|
|
||
| collection_logs = grep_uploadstb_logs_regex(r"collect|archive|gather") | ||
| assert len(collection_logs) > 0, "Log collection should be attempted" | ||
|
Comment on lines
63
to
67
|
||
|
|
||
|
|
@@ -109,16 +95,7 @@ def test_large_file_collection(self): | |
| """Test: Service collects large log files within limits""" | ||
| # Create large test files (10MB each) | ||
| large_files = create_large_test_log_files(count=3, size_mb=10) | ||
|
|
||
| result = subprocess.run([ | ||
| "/usr/local/bin/logupload", | ||
| "", | ||
| "1", | ||
| "1", | ||
| "true", | ||
| "HTTP", | ||
| "https://mockxconf:50058/" | ||
| ]) | ||
| result = subprocess.run("/usr/local/bin/logupload '' 1 1 true HTTP https://mockxconf:50058/ >> /opt/logs/logupload.log.0",shell=True) | ||
|
Comment on lines
57
to
+98
|
||
|
|
||
| # Verify files were processed | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,122 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import subprocess | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import re | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import pytest | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+5
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| USBLOGUPLOAD_BIN = "/usr/local/bin/usblogupload" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LOG_FILE = "/opt/logs/logupload.log" # Adjust if needed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LOG_FILE = "/opt/logs/logupload.log" # Adjust if needed | |
| LOG_FILE = "/opt/logs/logupload.log" |
Copilot
AI
Mar 3, 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 LOG_FILE constant is set to /opt/logs/logupload.log (without the .0 suffix), while the rest of the test suite consistently uses /opt/logs/logupload.log.0 (the UPLOADSTB_LOG constant in uploadstblogs_helper.py). The binary output is redirected to logupload.log.0 in other test files (lines 57 and 98 of test_uploadstblogs_normal_upload.py). Reading from and writing to a different log file may cause tests in this file to never find the expected log entries. The comment # Adjust if needed confirms this uncertainty. This should be resolved to use the correct log file path.
| LOG_FILE = "/opt/logs/logupload.log" # Adjust if needed | |
| LOG_FILE = "/opt/logs/logupload.log.0" |
Copilot
AI
Mar 2, 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 setup_device_properties fixture is a module-level fixture (not inside the TestUSBLogUpload class), which means it applies to all tests in the module including both class-based and standalone tests. However, the tmp_path parameter is declared but never actually used by the fixture body — the backup/restore logic only uses the device_properties_path derived from os.path.dirname(__file__). The unused tmp_path parameter is misleading and should be removed.
| def setup_device_properties(tmp_path): | |
| def setup_device_properties(): |
Copilot
AI
Mar 2, 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 setup_device_properties autouse fixture writes RDK_PROFILE=TV to a device.properties file relative to the test directory (os.path.dirname(__file__)/device.properties). However, the usblogupload binary most likely reads device properties from /etc/device.properties (as established by the rest of the test suite and the setup script). This fixture will therefore not have the intended effect of configuring RDK_PROFILE=TV for the binary under test. The tmp_path parameter is also declared in the fixture signature but never used.
Copilot
AI
Mar 2, 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 fixture writes device.properties relative to the test file's directory (__file__), not under the tmp_path provided by the fixture. This means tests modify a file in the source tree rather than an isolated temporary location, which can cause side-effects between test runs. The tmp_path parameter is accepted but not used for this file. The device_properties_path should be constructed under tmp_path instead, and the binary should be pointed to that location via an environment variable or argument.
| # Path to device.properties for test | |
| device_properties_path = os.path.join(os.path.dirname(__file__), "device.properties") | |
| backup_path = device_properties_path + ".bak" | |
| # Backup original if exists | |
| if os.path.exists(device_properties_path): | |
| os.rename(device_properties_path, backup_path) | |
| # Ensure RDK_PROFILE=TV is present | |
| with open(device_properties_path, "w", encoding="utf-8") as f: | |
| f.write("RDK_PROFILE=TV\n") | |
| yield | |
| # Restore original after test | |
| if os.path.exists(backup_path): | |
| os.remove(device_properties_path) | |
| os.rename(backup_path, device_properties_path) | |
| # Path to device.properties for test, created in a per-test temporary directory | |
| device_properties_path = tmp_path / "device.properties" | |
| # Ensure RDK_PROFILE=TV is present | |
| with open(device_properties_path, "w", encoding="utf-8") as f: | |
| f.write("RDK_PROFILE=TV\n") | |
| # Point the usblogupload binary to this device.properties via environment variable | |
| env_var_name = "DEVICE_PROPERTIES_PATH" | |
| old_env_value = os.environ.get(env_var_name) | |
| os.environ[env_var_name] = str(device_properties_path) | |
| try: | |
| yield | |
| finally: | |
| # Restore previous environment and clean up temporary file | |
| if old_env_value is not None: | |
| os.environ[env_var_name] = old_env_value | |
| else: | |
| os.environ.pop(env_var_name, None) | |
| if device_properties_path.exists(): | |
| device_properties_path.unlink() |
Copilot
AI
Mar 2, 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 grep_usblogupload_logs helper uses re.escape(search) to compile the search string as a literal pattern. However, in test_usblogupload_mac_address_log (line 73), the argument ":.*File:" is intended as a regular expression pattern (using .* as a wildcard), but re.escape will escape the dot and asterisk, causing the function to search for the exact literal string ":.*File:" instead of matching log lines with MAC address and file name. The function signature or usage needs to be consistent: either remove re.escape from the helper to allow regex patterns, or change the test to pass a literal string instead of a regex pattern.
Copilot
AI
Mar 2, 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 grep_usblogupload_logs function applies re.escape() to the search string before compiling it as a regex pattern (line 13). This means that when grep_usblogupload_logs(":.*File:") is called, the metacharacters .* are escaped and treated as literal characters — the function will search for the literal string :.*File: rather than matching MAC address patterns. The search will likely never return any results. Either re.escape() should be removed to allow regex patterns, or the caller should pass a plain string without metacharacters.
Copilot
AI
Mar 2, 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 a missing blank line between test_usblogupload_temp_dir_cleanup (ending at line 98) and test_usblogupload_success (starting at line 99). All other test methods in this file have a blank line separator. This is a minor style inconsistency, but it reduces readability.
| assert result.returncode in (0, 3), "Should exit with success or write error code" | |
| assert result.returncode in (0, 3), "Should exit with success or write error code" |
Copilot
AI
Mar 2, 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.
This test hardcodes /tmp as the USB mount point. Using /tmp in a test environment is fragile and can interact with other tests or system state. The tmp_path fixture is already available and provides an isolated temporary directory — usb_mount = str(tmp_path / "usb") (with mkdir()) should be used instead, consistent with the other tests in the class.
| usb_mount = "/tmp" | |
| # Run the binary and capture output | |
| result = subprocess.run([USBLOGUPLOAD_BIN, usb_mount], capture_output=True, text=True) | |
| usb_mount = tmp_path / "usb" | |
| usb_mount.mkdir() | |
| # Run the binary and capture output | |
| result = subprocess.run([USBLOGUPLOAD_BIN, str(usb_mount)], capture_output=True, text=True) |
Copilot
AI
Mar 2, 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 test_usblogupload_success test hardcodes /tmp as the USB mount point instead of using tmp_path like all the other tests in the same class. Using /tmp as a real mount path is semantically incorrect (it is not a USB mount), and it may interact with actual system state across test runs. The parameter tmp_path is already declared in the function signature but is never used — the test should use tmp_path consistently with the rest of the test suite.
Copilot
AI
Mar 2, 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 test_usblogupload_success method declares tmp_path as a parameter (which causes pytest to inject a per-test temporary directory) but then ignores it and hardcodes usb_mount = "/tmp". This is inconsistent: either use tmp_path for the USB mount (as done in test_usblogupload_archive_creation, test_usblogupload_mac_address_log, and test_usblogupload_temp_dir_cleanup), or remove the unused tmp_path parameter.
Copilot
AI
Mar 2, 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 test_usblogupload_success test uses the hardcoded path /tmp as the USB mount point. While /tmp always exists on the test system, it is not a real USB mount point and likely does not have the structure expected by the binary (e.g., the MAC address file, available disk space, etc.). This means the test exercises a best-case path that doesn't reflect real USB mount behavior. Additionally, there's a missing blank line before test_usblogupload_success (PEP 8 requires two blank lines between method definitions in a class).
Copilot
AI
Mar 2, 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.
In test_usblogupload_success, the tmp_path fixture parameter is declared but unused. Instead, the hardcoded /tmp path is used as the USB mount point. This is inconsistent with other test methods that use tmp_path to create isolated mount directories. Using /tmp directly can cause test pollution or conflicts between test runs.
Copilot
AI
Mar 2, 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 logs variable on line 115 is assigned but never asserted or used. The comment on line 116 acknowledges this, but it leaves dead code that contributes nothing to the test. Either assert on logs or remove lines 115-116 entirely.
| logs = grep_usblogupload_logs("Failed to initialize logging system") | |
| # This log may or may not appear depending on implementation |
Copilot
AI
Mar 2, 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.
Similarly, test_usblogupload_invalid_usage and test_usblogupload_usb_not_mounted each assign a logs variable using grep_usblogupload_logs() to check for specific log output ("Failed to initialize logging system" and "Failed to validate USB mount point" respectively), but then never assert on those variables. The test descriptions imply that these log messages are important for verifying correct behavior, but without an assertion they are not actually checked.
Copilot
AI
Mar 2, 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.
Several tests in test_usb_logupload.py query the log using grep_usblogupload_logs (lines 71, 83, 96, 115, 121) but never assert on the returned logs value. The logs variable is assigned but the test does not check whether it is non-empty or contains expected content. This means the test does not actually validate the expected log output, making it a silent no-op for those checks. Each of these lines should either be removed (if the check is genuinely not needed) or followed by an assert statement.
Copilot
AI
Mar 2, 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.
Same as test_usblogupload_invalid_usage: logs is assigned on line 121 but never used or asserted. Either add an assertion or remove the unused lines.
| logs = grep_usblogupload_logs("Failed to initialize logging system") | |
| # This log may or may not appear depending on implementation | |
| def test_usblogupload_usb_not_mounted(self): | |
| result = subprocess.run([USBLOGUPLOAD_BIN, "/tmp/notmounted"], capture_output=True) | |
| assert result.returncode == 2, "Should exit with USB not mounted code 2" | |
| logs = grep_usblogupload_logs("Failed to validate USB mount point") | |
| # This log may or may not appear depending on implementation | |
| def test_usblogupload_usb_not_mounted(self): | |
| result = subprocess.run([USBLOGUPLOAD_BIN, "/tmp/notmounted"], capture_output=True) | |
| assert result.returncode == 2, "Should exit with USB not mounted code 2" |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -31,6 +31,13 @@ mkdir -p "$RESULT_DIR" | |||||
| echo "LOG.RDK.DEFAULT" >> /etc/debug.ini | ||||||
|
|
||||||
| # Ensure properties files exist | ||||||
|
|
||||||
| if grep -q '^RDK_PROFILE=' /etc/device.properties; then | ||||||
| sed -i 's/^RDK_PROFILE=.*/RDK_PROFILE=TV/' /etc/device.properties | ||||||
| else | ||||||
| echo 'RDK_PROFILE=TV' >> /etc/device.properties | ||||||
| fi | ||||||
|
Comment on lines
+35
to
+39
|
||||||
|
|
||||||
| if ! grep -q "LOG_PATH=/opt/logs/" /etc/include.properties; then | ||||||
| echo "LOG_PATH=/opt/logs/" >> /etc/include.properties | ||||||
| fi | ||||||
|
|
@@ -52,6 +59,8 @@ if ! grep -q "BUILD_TYPE=" /etc/device.properties; then | |||||
| echo "BUILD_TYPE=dev" >> /etc/device.properties | ||||||
| fi | ||||||
|
|
||||||
| echo "AA:BB:CC:dd:EE:FF" >> /tmp/.estb_mac | ||||||
|
||||||
| echo "AA:BB:CC:dd:EE:FF" >> /tmp/.estb_mac | |
| echo "AA:BB:CC:dd:EE:FF" > /tmp/.estb_mac |
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
.astylercoptions file referenced in the--options=.astylercargument does not exist in the repository. This will cause theastylecommand to fail with an error like "Cannot find options file: .astylerc", which in turn will fail the entire "Check for formatting errors" step and break the CI pipeline on every PR. The.astylercfile needs to be created in the repository root, or the reference to it should be removed (using astyle with default options or another options file that exists).