-
Notifications
You must be signed in to change notification settings - Fork 2
Feature/test l2 #93
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?
Feature/test l2 #93
Changes from all commits
06200c5
abd1e84
973f969
971bcc5
aa59f95
3f81c93
746ae41
95f1512
594cf32
52ce524
b4f80b1
fd0d9d8
8bc2a76
a0ff2d9
107859e
eaf4ba1
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 |
|---|---|---|
|
|
@@ -55,10 +55,10 @@ cp include/*.h /usr/local/include | |
| sh build_inside_container.sh | ||
|
|
||
| cd ${ROOT} | ||
| git clone https://github.com/rdkcentral/common_utilities.git -b feature/upload_L2 | ||
| git clone https://github.com/rdkcentral/common_utilities.git -b feature/certupload | ||
| cd common_utilities | ||
|
Comment on lines
57
to
59
|
||
| autoreconf -i | ||
| ./configure --enable-rdkcertselector --prefix=${INSTALL_DIR} CFLAGS="-Wno-stringop-truncation -DL2_TEST_ENABLED -DRDK_LOGGER" | ||
| ./configure --enable-rdkcertselector --enable-mountutils --prefix=${INSTALL_DIR} CFLAGS="-Wno-stringop-truncation -DL2_TEST_ENABLED -DRDK_LOGGER" | ||
| cp uploadutils/*.h /usr/local/include | ||
| make | ||
| make install | ||
|
|
||
| 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. | ||
|
|
||
| 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 |
| 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) | |
| cmd = ["/usr/local/bin/logupload", "uploadlogsnow"] | |
| result = sp.run(cmd, capture_output=True, text=True, timeout=60) |
Copilot
AI
Mar 4, 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.
run_uploadlogsnow() no longer sets a timeout or captures stdout/stderr. If logupload hangs in CI, this test suite can block indefinitely, and failures become harder to diagnose. Consider restoring a bounded timeout and capturing output (or at least enabling it on failure) instead of relying on shell redirection.
Copilot
AI
Mar 4, 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 change removes the previous timeout=300 when running logupload uploadlogsnow. Without a timeout, CI can hang indefinitely if the process stalls. Consider restoring a timeout and capturing output to help diagnose failures.
| 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) | ||
|
|
||
|
Comment on lines
+57
to
+58
|
||
| # Verify initialization | ||
| assert result.returncode == 0 or result.returncode == 1, "Upload process should complete" | ||
|
Comment on lines
55
to
60
|
||
|
|
||
| # 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" | ||
|
|
||
|
|
@@ -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
95
to
99
|
||
| # 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| USBLOGUPLOAD_BIN = "/usr/local/bin/usblogupload" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LOG_FILE = "/opt/logs/logupload.log" # Adjust if needed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Helper to grep logs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def grep_usblogupload_logs(search: str): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| search_result = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| search_pattern = re.compile(re.escape(search), re.IGNORECASE) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| search_pattern = re.compile(re.escape(search), re.IGNORECASE) | |
| search_pattern = re.compile(search, re.IGNORECASE) |
Copilot
AI
Mar 5, 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 RDK_PROFILE=TV to a device.properties file in the test directory, but usblogupload reads device properties via getDevicePropertyData() (typically backed by /etc/device.properties). As written, this setup likely has no effect on the binary under test. Consider updating /etc/device.properties (with proper backup/restore) or stubbing getDevicePropertyData() for this test environment.
Copilot
AI
Mar 4, 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 fixture writes device.properties next to the test file, but the code under test reads properties via getDevicePropertyData (typically backed by /etc/device.properties). As-is, the fixture likely has no effect on usblogupload behavior and can give false confidence. Consider updating /etc/device.properties in an idempotent way (or mocking getDevicePropertyData) instead of creating a local file.
| # 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 used by getDevicePropertyData (typically /etc/device.properties) | |
| device_properties_path = "/etc/device.properties" | |
| file_existed_before = os.path.exists(device_properties_path) | |
| original_content = "" | |
| modified = False | |
| if file_existed_before: | |
| # Read existing content so we can restore it after the test, if we modify it | |
| try: | |
| with open(device_properties_path, "r", encoding="utf-8", errors="ignore") as f: | |
| original_content = f.read() | |
| except OSError: | |
| # If we cannot read the file, do not attempt to modify or restore it | |
| original_content = "" | |
| file_existed_before = False | |
| # Ensure RDK_PROFILE=TV is present during the test | |
| needs_write = True | |
| if file_existed_before and original_content: | |
| if "RDK_PROFILE=TV" in original_content: | |
| needs_write = False | |
| if needs_write: | |
| try: | |
| mode = "a" if file_existed_before else "w" | |
| with open(device_properties_path, mode, encoding="utf-8") as f: | |
| f.write("RDK_PROFILE=TV\n") | |
| modified = True | |
| except OSError: | |
| # If we cannot write, proceed with the test; it may fail appropriately | |
| modified = False | |
| yield | |
| # Restore original state after test if we modified the file | |
| if modified: | |
| try: | |
| if file_existed_before: | |
| with open(device_properties_path, "w", encoding="utf-8") as f: | |
| f.write(original_content) | |
| else: | |
| # File was created by this fixture; remove it | |
| if os.path.exists(device_properties_path): | |
| os.remove(device_properties_path) | |
| except OSError: | |
| # Best-effort restore; ignore errors during cleanup | |
| pass |
Copilot
AI
Mar 4, 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.
LOG_FILE is under /opt/logs, but the tests append to it without ensuring the directory exists. If /opt/logs is missing in the environment, open(LOG_FILE, "a") will raise and fail the test before any assertions. Consider creating the directory/file in a fixture (or using tmp_path for per-test logs).
Copilot
AI
Mar 4, 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.
All subprocess.run(...) calls here lack a timeout. If usblogupload blocks (e.g., waiting on I/O), these tests can hang the entire suite. Consider adding a reasonable timeout per invocation and reporting stdout/stderr on timeout to aid debugging.
Copilot
AI
Mar 4, 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.
grep_usblogupload_logs escapes the search string, so patterns like ":.*File:" are treated literally (not as regex). In test_usblogupload_mac_address_log this likely never matches. If a regex match is intended, either add a separate helper that does regex matching or avoid re.escape for that call site.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -42,7 +42,9 @@ pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/fil | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/log_upload_reboot_true_test.json test/functional-tests/tests/test_log_upload_onreboot_true_case.py | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/log_upload_reboot_false_test.json test/functional-tests/tests/test_log_upload_onreboot_false_case.py | ||||||||||||||||||||||||||||||||
| #pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/log_upload_reboot_false_test.json test/functional-tests/tests/test_log_upload_onreboot_false_case.py | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| #pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/log_upload_reboot_false_test.json test/functional-tests/tests/test_log_upload_onreboot_false_case.py | |
| pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/log_upload_reboot_false_test.json test/functional-tests/tests/test_log_upload_onreboot_false_case.py |
Copilot
AI
Mar 4, 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.
cat /opt/logs/dcmd.log.0 will dump the entire log into CI output, which can be very large and may contain sensitive data. Consider tailing a small number of lines, gating it behind a failure condition, or removing it entirely.
| cat /opt/logs/dcmd.log.0 | |
| tail -n 200 /opt/logs/dcmd.log.0 |
Copilot
AI
Mar 4, 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.
Unconditionally cat-ing /opt/logs/dcmd.log.0 can flood CI logs and may fail if the file doesn’t exist yet. Consider guarding it (e.g., only print on failure, or check existence and tail a limited number of lines).
| cat /opt/logs/dcmd.log.0 | |
| if [ -f /opt/logs/dcmd.log.0 ]; then | |
| echo "Last 200 lines of /opt/logs/dcmd.log.0:" | |
| tail -n 200 /opt/logs/dcmd.log.0 | |
| fi |
Copilot
AI
Mar 5, 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.
Commenting out the log_upload_onreboot_false_case pytest invocation reduces L2 coverage, and cat /opt/logs/dcmd.log.0 can produce very large CI logs (and is noisy even on success). If this is temporary debugging, consider reverting it or gating the cat behind a failure condition / debug flag.
| #pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/log_upload_reboot_false_test.json test/functional-tests/tests/test_log_upload_onreboot_false_case.py | |
| cat /opt/logs/dcmd.log.0 | |
| pytest -v --json-report --json-report-summary --json-report-file $RESULT_DIR/log_upload_reboot_false_test.json test/functional-tests/tests/test_log_upload_onreboot_false_case.py | |
| if [ "${DCM_SHOW_LOGS:-0}" -ne 0 ] && [ -f /opt/logs/dcmd.log.0 ]; then | |
| cat /opt/logs/dcmd.log.0 | |
| fi |
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 workflow no longer runs
test/run_l2.sh, so the DCM/L2 suites executed there (boot sequence, cron cases, etc.) won’t run in CI anymore. If the intent is to narrow coverage to uploadstblogs/usblogupload only, consider either adding those checks intorun_uploadstblogs_l2.shor running both scripts to avoid silently dropping coverage.