-
Notifications
You must be signed in to change notification settings - Fork 1
Rebase #33
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: topic/RDK-57502
Are you sure you want to change the base?
Rebase #33
Conversation
Clone iarmmgrs repository and update test list
| name: Execute L1 test suite in test container environment | ||
| runs-on: ubuntu-latest | ||
| container: | ||
| image: ghcr.io/rdkcentral/docker-rdk-ci:latest | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v3 | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Log in to GitHub Container Registry | ||
| uses: docker/login-action@v2 | ||
| with: | ||
| registry: ghcr.io | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Pull test container image | ||
| run: docker pull ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:latest | ||
|
|
||
| - name: Start test container | ||
| run: | | ||
| docker run -d --name native-platform -v ${{ github.workspace }}:/mnt/L1_CONTAINER_SHARED_VOLUME ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:latest | ||
|
|
||
| - name: Run unit tests | ||
| run: sh unit_test.sh | ||
| - name: Run L1 Unit Tests inside container | ||
| run: docker exec -i native-platform /bin/bash -c "cd /mnt/L1_CONTAINER_SHARED_VOLUME/ && sh unit_test.sh" | ||
|
|
||
| - name: Upload test results to automatic test result management system | ||
| - name: Copy L1 test results to runner | ||
| run: | | ||
| docker cp native-platform:/tmp/Gtest_Report /tmp/Gtest_Report | ||
| ls -l /tmp/Gtest_Report | ||
|
|
||
| upload-test-results: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
In general, the fix is to explicitly define a permissions block either at the workflow level (root) or per job, restricting GITHUB_TOKEN to the minimal scope needed. That avoids inheriting potentially broad default repository permissions and follows the principle of least privilege.
For this workflow, the execute-L1-tests-on-pr job uses actions/checkout@v4, which requires contents: read. None of its steps write to the repo or perform other GitHub API mutations. The upload-test-results job does not interact with the GitHub API at all; it only reads local files and calls an external HTTP endpoint. Therefore:
- At minimum, we should set
permissions: contents: readfor the workflow. - To be stricter, we can define permissions per job:
execute-L1-tests-on-prgetscontents: read, andupload-test-resultsgetspermissions: {}(no permissions), since it does not needGITHUB_TOKENfor any GitHub API calls.
To minimize functional impact while being explicit, we can add a workflow-level block with contents: read, which will apply to both jobs. This ensures actions/checkout still works and restricts any write permissions. The change should be added near the top of .github/workflows/L1-Test.yml, after name: and before on:. No additional imports or dependencies are required.
-
Copy modified lines R4-R6
| @@ -1,6 +1,9 @@ | ||
|
|
||
| name: L1 Unit Tests | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [ develop, main ] |
…ts To C Implementation (#35) * Created a comprehensive L2 test suite with 6 test modules covering error handling, normal uploads, retry logic, security, resource management, and upload strategies Updated build scripts and CI/CD workflow to compile with L2_TEST_ENABLED flag and execute the new test suite This feature file contains scenarios for normal log uploads, large log file handling, and MD5 checksum verification for the uploadSTBLogs service.
…ts To C Implementation (#49) * Simplify some pieces of code over engineered by AI models. * Update run_uploadstblogs_l2.sh for L2 coverage --------- Co-authored-by: Abhinav P V <Abhinav_Valappil@comcast.com>
…ts To C Implementation (#51) * Remove logging of response URLs * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Shibu Kakkoth Vayalambron <shibu.kakkoth@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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 test infrastructure for the uploadSTBLogs component through a rebase operation. The changes include Google Test-based unit tests for multiple modules, mock implementations, build configuration, and several new source files.
Changes:
- Adds 18 comprehensive unit test files covering all major modules (verification, validation, upload engine, strategy handlers, archive manager, etc.)
- Implements mock classes for RDK utilities, RBUS, CURL, and file operations
- Adds build system configuration (Makefile.am, configure.ac)
- Includes several new source implementation files (verification.c, validation.c, uploadstblogs.c, etc.)
Reviewed changes
Copilot reviewed 86 out of 87 changed files in this pull request and generated 66 comments.
Show a summary per file
| File | Description |
|---|---|
| uploadstblogs/unittest/*_gtest.cpp | 18 unit test files with comprehensive test coverage for different modules |
| uploadstblogs/unittest/mocks/*.h/.cpp | Mock implementations for external dependencies (RDK utils, RBUS, CURL, file operations) |
| uploadstblogs/unittest/Makefile.am | Build configuration for compiling and linking test executables |
| uploadstblogs/unittest/configure.ac | Autoconf configuration for test build system |
| uploadstblogs/src/*.c | New source implementation files (verification, validation, uploadstblogs, upload_engine, strategy_handler) |
| uploadstblogs/include/md5_utils.h | Header file for MD5 utility functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the Licesnse. |
Copilot
AI
Jan 13, 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.
Corrected spelling of 'License' to 'Licesnse'
| const char* mock_curl_easy_strerror_impl(int curl_code) { | ||
| switch (curl_code) { | ||
| case 0: return "No error"; // CURLE_OK | ||
| case 7: return "Couldn't connect to server"; // CURLE_COULDNT_CONNECT |
Copilot
AI
Jan 13, 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.
Trailing whitespace at end of line 37 should be removed for code cleanliness.
| EXPECT_FALSE(is_terminal_failure(403)); | ||
| } | ||
|
|
||
| // Test is_curl_success function |
Copilot
AI
Jan 13, 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.
Trailing whitespace at end of line 144 should be removed for code cleanliness.
| result = run_uploadstblogs() | ||
|
|
||
| # Check for configuration error detection | ||
| error_logs = grep_uploadstb_logs_regex(r"ERROR|error|fail.*properties|invalid") |
Copilot
AI
Jan 13, 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.
Variable error_logs is not used.
| error_logs = grep_uploadstb_logs_regex(r"ERROR|error|fail.*properties|invalid") | |
| error_logs = grep_uploadstb_logs_regex(r"ERROR|error|fail.*properties|invalid") | |
| assert len(error_logs) > 0, f"Configuration error should be logged for corrupted properties, but found: {error_logs}" |
|
|
||
| create_test_log_files(count=1) | ||
|
|
||
| result = run_uploadstblogs() |
Copilot
AI
Jan 13, 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.
Variable result is not used.
| result = run_uploadstblogs() | |
| run_uploadstblogs() |
| import os | ||
| import time | ||
| import re | ||
| import json |
Copilot
AI
Jan 13, 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.
Import of 'json' is not used.
| try: | ||
| subprocess.run(f"echo '' > {UPLOADSTB_LOG}", shell=True) | ||
| return True | ||
| except: |
Copilot
AI
Jan 13, 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.
Except block directly handles BaseException.
| if os.path.exists(LOCK_FILE): | ||
| os.remove(LOCK_FILE) | ||
| return True | ||
| except: |
Copilot
AI
Jan 13, 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.
Except block directly handles BaseException.
| for chunk in iter(lambda: f.read(4096), b""): | ||
| md5_hash.update(chunk) | ||
| return md5_hash.hexdigest() | ||
| except: |
Copilot
AI
Jan 13, 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.
Except block directly handles BaseException.
| """Get file size in bytes""" | ||
| try: | ||
| return os.path.getsize(filepath) | ||
| except: |
Copilot
AI
Jan 13, 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.
Except block directly handles BaseException.
…ts To C Implementation (#54) * Update cov_build.sh to enable the rdkcertselector feature in the common_utilities build configuration to support the migration of operation support log upload scripts to C implementation. The change is part of resolving L2 test failures.
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
Copilot reviewed 86 out of 87 changed files in this pull request and generated 23 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result = run_uploadstblogs() | ||
|
|
||
| # Check for size limit detection | ||
| size_logs = grep_uploadstb_logs_regex(r"size.*limit|too large|exceed") |
Copilot
AI
Jan 21, 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.
Variable size_logs is not used.
| size_logs = grep_uploadstb_logs_regex(r"size.*limit|too large|exceed") | |
| size_logs = grep_uploadstb_logs_regex(r"size.*limit|too large|exceed") | |
| assert size_logs, "Expected size limit detection logs" |
| result = run_uploadstblogs() | ||
|
|
||
| # Check for telemetry markers | ||
| telemetry_logs = grep_uploadstb_logs_regex(r"telemetry|marker|warning") |
Copilot
AI
Jan 21, 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.
Variable telemetry_logs is not used.
| telemetry_logs = grep_uploadstb_logs_regex(r"telemetry|marker|warning") | |
| telemetry_logs = grep_uploadstb_logs_regex(r"telemetry|marker|warning") | |
| # Ensure telemetry warning logs are present | |
| assert telemetry_logs, "Expected telemetry warning logs to be present" |
| result = run_uploadstblogs() | ||
|
|
||
| # Check for no files detection | ||
| no_files_logs = grep_uploadstb_logs_regex(r"no.*file|empty|not found|no logs") |
Copilot
AI
Jan 21, 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.
Variable no_files_logs is not used.
| no_files_logs = grep_uploadstb_logs_regex(r"no.*file|empty|not found|no logs") | |
| _ = grep_uploadstb_logs_regex(r"no.*file|empty|not found|no logs") |
| result = run_uploadstblogs() | ||
|
|
||
| # Check for informative message | ||
| logs = grep_uploadstb_logs_regex(r"no.*file|empty|nothing to upload") |
Copilot
AI
Jan 21, 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.
Variable logs is not used.
| logs = grep_uploadstb_logs_regex(r"no.*file|empty|nothing to upload") | |
| logs = grep_uploadstb_logs_regex(r"no.*file|empty|nothing to upload") | |
| assert logs, "Expected an informative 'no files' or 'empty logs' message in uploadSTBLogs logs" |
|
|
||
| # Should not attempt upload | ||
| upload_logs = grep_uploadstb_logs_regex(r"upload.*success|uploading|HTTP") | ||
| # Might not see upload attempts |
Copilot
AI
Jan 21, 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.
Variable upload_logs is not used.
| # Might not see upload attempts | |
| assert len(upload_logs) == 0, "Should not attempt upload when no log files exist" |
| args = "'' 0 0 0 HTTP http://localhost:8080 0 0 ''" | ||
|
|
||
| result = run_uploadstblogs(args) | ||
|
|
Copilot
AI
Jan 21, 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.
Variable result is not used.
| # Ensure the upload process completes (0 or 1 are acceptable in other tests) | |
| assert result.returncode in [0, 1], "DCM scheduled upload should complete" | |
| result = run_uploadstblogs(args) | ||
|
|
||
| # Check for log collection | ||
| collection_logs = grep_uploadstb_logs_regex(r"collect|archive|DCM") |
Copilot
AI
Jan 21, 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.
Variable collection_logs is not used.
| collection_logs = grep_uploadstb_logs_regex(r"collect|archive|DCM") | |
| collection_logs = grep_uploadstb_logs_regex(r"collect|archive|DCM") | |
| assert len(collection_logs) > 0, "DCM scheduled upload should collect or archive logs" |
| result = run_uploadstblogs() | ||
|
|
||
| # Check for event publishing | ||
| event_logs = grep_uploadstb_logs_regex(r"event|publish|success") |
Copilot
AI
Jan 21, 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.
Variable event_logs is not used.
| event_logs = grep_uploadstb_logs_regex(r"event|publish|success") | |
| _ = grep_uploadstb_logs_regex(r"event|publish|success") |
|
|
||
| # Check for strategy selection | ||
| strategy_logs = grep_uploadstb_logs_regex(r"strategy|RRD|select") | ||
| assert result.returncode in [0, 1], "Should select appropriate strategy" |
Copilot
AI
Jan 21, 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.
Variable strategy_logs is not used.
| assert result.returncode in [0, 1], "Should select appropriate strategy" | |
| assert result.returncode in [0, 1], "Should select appropriate strategy" | |
| assert strategy_logs, "Expected strategy selection logs to be present" |
|
|
||
| import pytest | ||
| import time | ||
| from uploadstblogs_helper import * |
Copilot
AI
Jan 21, 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.
Import pollutes the enclosing namespace, as the imported module uploadstblogs_helper does not define 'all'.
| from uploadstblogs_helper import * | |
| from uploadstblogs_helper import ( | |
| clear_uploadstb_logs, | |
| remove_lock_file, | |
| cleanup_test_log_files, | |
| restore_device_properties, | |
| kill_uploadstblogs, | |
| create_test_log_files, | |
| run_uploadstblogs, | |
| grep_uploadstb_logs_regex, | |
| DEVICE_PROPERTIES, | |
| ) |
No description provided.