Skip to content

Conversation

@Abhinavpv28
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings December 5, 2025 07:23
@Abhinavpv28 Abhinavpv28 requested a review from a team as a code owner December 5, 2025 07:23
Comment on lines 13 to 42
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: Run unit tests
run: sh unit_test.sh
- name: Log in to GitHub Container Registry
uses: docker/login-action@v2
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Upload test results to automatic test result management system
- 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 L1 Unit Tests inside container
run: docker exec -i native-platform /bin/bash -c "cd /mnt/L1_CONTAINER_SHARED_VOLUME/ && sh test/unit_test.sh"

- 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

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}

Copilot Autofix

AI 2 months ago

To fix this problem, we need to add an explicit permissions block to the workflow. The principle of least privilege should be applied: the workflow should only be granted permissions that it requires. Both jobs in this workflow only need to read repository contents and interact with containers, but neither needs write access to repository contents, issues, or pull requests. Thus, a sensible starting point is to add permissions: contents: read at the root level (after the name: and before on:) to ensure all jobs inherit minimal permissions. If a specific job, such as uploading results, ever needs scoped write permissions for a particular resource, add that granularity to that job's own permissions block. In the absence of evidence for the need for write access, setting contents: read globally is the best and safest option.

The required change is to edit .github/workflows/L1-Test.yml and insert the following lines:

permissions:
  contents: read

right after the name: line, before on:.

Suggested changeset 1
.github/workflows/L1-Test.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/L1-Test.yml b/.github/workflows/L1-Test.yml
--- a/.github/workflows/L1-Test.yml
+++ b/.github/workflows/L1-Test.yml
@@ -1,4 +1,6 @@
 name: L1 Unit Tests
+permissions:
+  contents: read
 
 on:
   pull_request:
EOF
@@ -1,4 +1,6 @@
name: L1 Unit Tests
permissions:
contents: read

on:
pull_request:
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Contributor

Copilot AI left a 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 refactors the GitHub Actions workflow from running simple unit tests directly in a container to executing L1 tests within a dedicated Docker test environment. The workflow is split into two jobs: one for test execution and another for uploading results to the RDK orchestration service.

Key changes:

  • Introduces containerized test execution using the native-platform Docker image
  • Splits workflow into separate test execution and result upload jobs
  • Updates actions/checkout from v3 to v4

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

needs: execute-L1-tests-on-pr
runs-on: ubuntu-latest
container:
image: ghcr.io/rdkcentral/docker-rdk-ci:latest
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Docker image tag is pinned to :latest, which can lead to inconsistent behavior across workflow runs. Consider using a specific version tag for reproducible builds.

Suggested change
image: ghcr.io/rdkcentral/docker-rdk-ci:latest
image: ghcr.io/rdkcentral/docker-rdk-ci:v1.2.3

Copilot uses AI. Check for mistakes.
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 L1 Unit Tests inside container
run: docker exec -i native-platform /bin/bash -c "cd /mnt/L1_CONTAINER_SHARED_VOLUME/ && sh test/unit_test.sh"
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test execution will not fail the workflow if the tests fail inside the container. The docker exec command needs explicit error handling to propagate test failures. Consider adding set -e in the bash command or checking the exit code.

Example:

run: docker exec -i native-platform /bin/bash -c "set -e && cd /mnt/L1_CONTAINER_SHARED_VOLUME/ && sh test/unit_test.sh"

Alternatively, capture and check the exit code:

run: |
  docker exec -i native-platform /bin/bash -c "cd /mnt/L1_CONTAINER_SHARED_VOLUME/ && sh test/unit_test.sh"
  if [ $? -ne 0 ]; then
    echo "Tests failed"
    exit 1
  fi
Suggested change
run: docker exec -i native-platform /bin/bash -c "cd /mnt/L1_CONTAINER_SHARED_VOLUME/ && sh test/unit_test.sh"
run: docker exec -i native-platform /bin/bash -c "set -e && cd /mnt/L1_CONTAINER_SHARED_VOLUME/ && sh test/unit_test.sh"

Copilot uses AI. Check for mistakes.
Comment on lines +42 to +51
upload-test-results:
name: Upload L1 test results to automatic test result management system
needs: execute-L1-tests-on-pr
runs-on: ubuntu-latest
container:
image: ghcr.io/rdkcentral/docker-rdk-ci:latest
volumes:
- /tmp/Gtest_Report:/tmp/Gtest_Report

steps:
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upload-test-results job attempts to access test results from the previous job via volume mounting, but GitHub Actions jobs run in isolated environments. The /tmp/Gtest_Report directory copied to the runner in the first job will not be available in the second job's container.

To fix this, you need to:

  1. Add an artifact upload step at the end of the execute-L1-tests-on-pr job using actions/upload-artifact
  2. Add an artifact download step at the beginning of the upload-test-results job using actions/download-artifact

Example:

# In execute-L1-tests-on-pr job, after copying results:
- name: Upload test results artifact
  uses: actions/upload-artifact@v4
  with:
    name: gtest-results
    path: /tmp/Gtest_Report

# In upload-test-results job, before uploading:
- name: Download test results artifact
  uses: actions/download-artifact@v4
  with:
    name: gtest-results
    path: /tmp/Gtest_Report
Suggested change
upload-test-results:
name: Upload L1 test results to automatic test result management system
needs: execute-L1-tests-on-pr
runs-on: ubuntu-latest
container:
image: ghcr.io/rdkcentral/docker-rdk-ci:latest
volumes:
- /tmp/Gtest_Report:/tmp/Gtest_Report
steps:
- name: Upload test results artifact
uses: actions/upload-artifact@v4
with:
name: gtest-results
path: /tmp/Gtest_Report
upload-test-results:
name: Upload L1 test results to automatic test result management system
needs: execute-L1-tests-on-pr
runs-on: ubuntu-latest
container:
image: ghcr.io/rdkcentral/docker-rdk-ci:latest
steps:
- name: Download test results artifact
uses: actions/download-artifact@v4
with:
name: gtest-results
path: /tmp/Gtest_Report

Copilot uses AI. Check for mistakes.

- 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
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test container is started but never explicitly stopped or cleaned up. If the workflow fails after this step, the container may remain running on the runner. Consider adding cleanup steps using if: always() to ensure the container is stopped and removed even if previous steps fail.

Example:

- name: Cleanup test container
  if: always()
  run: |
    docker stop native-platform || true
    docker rm native-platform || true

Copilot uses AI. Check for mistakes.
- name: Run unit tests
run: sh unit_test.sh
- name: Log in to GitHub Container Registry
uses: docker/login-action@v2
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docker/login-action is using version v2, but a newer version v3 is available that includes improvements and bug fixes. Consider updating to the latest version for better security and performance.

uses: docker/login-action@v3
Suggested change
uses: docker/login-action@v2
uses: docker/login-action@v3

Copilot uses AI. Check for mistakes.
Comment on lines +28 to +32
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
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Docker image tag is pinned to :latest, which can lead to inconsistent behavior across workflow runs as the image may change without notice. Consider using a specific version tag or commit SHA for reproducible builds.

Example:

run: docker pull ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:v1.0.0
Suggested change
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
run: docker pull ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:v1.0.0
- 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:v1.0.0

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings December 5, 2025 07:27
Copy link
Contributor

Copilot AI left a 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 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +52 to 58
- name: Upload results
if: github.repository_owner == 'rdkcentral'
run: |
echo "Contents in /tmp/Gtest_Report:"
ls -l /tmp/Gtest_Report
git config --global --add safe.directory `pwd`
gtest-json-result-push.py /tmp/Gtest_Report https://rdkeorchestrationservice.apps.cloud.comcast.net/rdke_orchestration_api/push_unit_test_results `pwd`
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upload-test-results job needs to checkout the code repository before running git config and gtest-json-result-push.py commands. The gtest-json-result-push.py script expects a git repository (as indicated by the git config --global --add safe.directory command and the pwd argument to the script).

Add a checkout step:

steps:
  - name: Checkout code
    uses: actions/checkout@v4
  
  - name: Upload results
    # ... rest of the step

Copilot uses AI. Check for mistakes.
@@ -1,29 +1,58 @@
name: L1 Unit Tests
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first line has leading whitespace before name:. YAML files should not have leading whitespace on the first line. This could cause parsing issues in some YAML parsers.

Remove the leading spaces:

name: L1 Unit Tests

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants