Add V4L2 on-device test CI workflow - #369
Conversation
- Add GitHub Actions workflow that runs V4L2 pytest suite on a self-hosted Jetson runner (driver reload, sanity check, pytest, dmesg collection, artifact upload) - Add setup script for installing GitHub Actions runner on Jetson with proper labels and passwordless sudo for driver operations - Supports manual trigger with test filter and auto-trigger on push/PR to kernel/realsense or test/v4l2_test paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request adds GitHub Actions infrastructure for running V4L2 hardware tests on a self-hosted Jetson Xavier runner. The changes enable automated on-device testing of the D4XX RealSense camera driver by setting up a CI workflow that reloads the kernel module, performs sanity checks, runs pytest suites, and collects diagnostic logs.
Changes:
- Adds
.github/workflows/v4l2-test.ymlworkflow that executes V4L2 pytest tests on self-hosted Jetson hardware with configurable test filters - Adds
scripts/setup_gh_runner.shscript to automate GitHub Actions runner installation and configuration on Jetson devices with necessary sudo permissions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/v4l2-test.yml |
GitHub Actions workflow for V4L2 on-device testing with driver reload, sanity checks, pytest execution, and artifact collection |
scripts/setup_gh_runner.sh |
Bash script for automated setup of GitHub Actions self-hosted runner on Jetson with system dependencies and passwordless sudo configuration |
Comments suppressed due to low confidence (13)
.github/workflows/v4l2-test.yml:127
- The sanity check on line 127 always uses /dev/video0, but the earlier check on line 106 verifies that video devices exist (which could be /dev/video1, /dev/video2, etc.). If the camera is enumerated as a different video device number, the sanity check will fail even though video devices are present. Consider using a more flexible approach to find the first available video device, or document that /dev/video0 is required.
if v4l2-ctl -d /dev/video0 --info 2>/dev/null; then
.github/workflows/v4l2-test.yml:245
- The artifact name includes github.run_number which could lead to confusion if a workflow is re-run. The run_number remains the same for re-runs, so re-running a failed workflow will overwrite the previous artifact with the same name. Consider using github.run_id combined with github.run_attempt to create unique artifact names for each attempt, or document this behavior.
name: v4l2-test-results-${{ github.run_number }}
.github/workflows/v4l2-test.yml:87
- The driver reload step on line 87 uses grep without the '-q' flag, so it will print the matching line. However, if the d4xx module fails to load, grep will return non-zero and the step will fail without a clear error message. Consider adding '|| echo "ERROR: d4xx module failed to load"' and 'exit 1' to provide a clearer error message if the module doesn't load successfully.
lsmod | grep d4xx
.github/workflows/v4l2-test.yml:231
- The failed test extraction on line 230 searches for lines containing 'FAILED' anywhere in the line. This could match lines in stack traces or error messages that aren't actual test names. Consider using a more specific pattern that matches pytest's test failure format (e.g., lines that start with 'FAILED ' or contain ' FAILED').
for line in content.split('\n'):
if 'FAILED' in line:
print(f'- {line.strip()}')
scripts/setup_gh_runner.sh:78
- The sudoers configuration hardcodes paths as /sbin/rmmod and /sbin/modprobe. On some modern Linux distributions, these tools may be located in /usr/sbin instead of /sbin. Consider checking the actual paths on the target system or using wildcards (though this reduces security). Alternatively, document that this script is tested specifically for the Jetson Ubuntu distribution where these paths are known to be correct.
${RUNNER_USER} ALL=(ALL) NOPASSWD: /sbin/rmmod d4xx
${RUNNER_USER} ALL=(ALL) NOPASSWD: /sbin/modprobe d4xx
.github/workflows/v4l2-test.yml:34
- The workflow uses
actions/checkout@v4while all other workflows in this repository pin actions/checkout to a specific SHA (f43a0e5ff2bd294095638e18286ca9a3d1956744) for v3. For consistency with the repository's security practices, consider pinning this action to a specific commit SHA rather than using a tag.
uses: actions/checkout@v4
.github/workflows/v4l2-test.yml:37
- The workflow creates a results directory but doesn't explicitly clean it up before or after the run. On self-hosted runners, directories persist between runs. If a previous run failed or was cancelled, old results could remain and potentially interfere with the current run or cause confusion. Consider adding a cleanup step at the beginning to remove any existing results directory.
- name: Create results directory
run: mkdir -p "$RESULTS_DIR"
.github/workflows/v4l2-test.yml:172
- The PYTEST_ARGS variable is expanded without quotes on line 172, which will cause word splitting and glob expansion. If the test filter contains spaces or glob characters, the command will not execute as intended. The variable should be quoted when used in the command.
python3 -m pytest $PYTEST_ARGS 2>&1 | tee "../$RESULTS_DIR/pytest_output.log" || true
.github/workflows/v4l2-test.yml:73
- The inline Python script uses string interpolation to embed the RESULTS_DIR variable directly into the file path. While this is safe in this context since RESULTS_DIR is a workflow-controlled environment variable, for better maintainability consider using Python's os.path.join or pathlib instead of string interpolation for file paths.
with open('$RESULTS_DIR/system_info.json', 'w') as f:
json.dump(info, f, indent=2)
scripts/setup_gh_runner.sh:67
- The script uses '--break-system-packages' flag when installing pytest with pip3. This flag was introduced in pip 23.0 to allow installation into externally managed Python environments. While this works, it bypasses Python's environment protection mechanisms. Consider documenting in the comment why this approach is needed for Jetson devices, or alternatively suggest using a virtual environment for better isolation.
pip3 install pytest --break-system-packages 2>/dev/null || pip3 install pytest
scripts/setup_gh_runner.sh:81
- The sudoers configuration grants passwordless sudo for rmmod, modprobe, and dmesg commands. While these are necessary for the workflow, consider documenting in the script comments that this is a security consideration and that the runner should be in a trusted, isolated environment. The current implementation correctly restricts to specific commands rather than granting blanket sudo access.
sudo tee "$SUDOERS_FILE" > /dev/null <<SUDOERS
# Allow GitHub Actions runner to reload D4XX driver and read dmesg
${RUNNER_USER} ALL=(ALL) NOPASSWD: /sbin/rmmod d4xx
${RUNNER_USER} ALL=(ALL) NOPASSWD: /sbin/modprobe d4xx
${RUNNER_USER} ALL=(ALL) NOPASSWD: /usr/bin/dmesg
${RUNNER_USER} ALL=(ALL) NOPASSWD: /bin/dmesg
SUDOERS
scripts/setup_gh_runner.sh:104
- The '--overwrite' flag is passed to tar during extraction. If the script is re-run after a failed installation, this will overwrite existing files. However, if the runner service is already running when extraction occurs, this could cause issues. Consider stopping the service before extraction if it exists, or add a warning about stopping the service before re-running the script.
tar xzf "${TARBALL}" --overwrite
.github/workflows/v4l2-test.yml:29
- The TEST_DIR environment variable is defined but never used in the workflow. The test directory path is hardcoded as 'test' on line 160 and 'v4l2_test/' on line 162. Consider either using this variable consistently throughout the workflow or removing it if it's not needed.
TEST_DIR: test/v4l2_test
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| jobs: | ||
| v4l2-test: | ||
| runs-on: [self-hosted, jetson, xavier] | ||
| timeout-minutes: 30 |
There was a problem hiding this comment.
The workflow doesn't implement any concurrency control. If multiple workflow runs are triggered simultaneously (e.g., multiple PRs or pushes), they will all attempt to run on the same self-hosted runner and could interfere with each other by reloading the driver, resetting hardware, or accessing the same devices. Consider adding a concurrency group to ensure only one instance runs at a time.
| PYTEST_ARGS="-vs --tb=short -m d457 v4l2_test/" | ||
|
|
||
| if [ -n "${{ github.event.inputs.test_filter }}" ]; then | ||
| PYTEST_ARGS="$PYTEST_ARGS -k '${{ github.event.inputs.test_filter }}'" | ||
| fi | ||
|
|
||
| echo "Running: python3 -m pytest $PYTEST_ARGS" | ||
| echo "" | ||
|
|
||
| # Run pytest; tee output to file for artifact, allow non-zero exit | ||
| python3 -m pytest $PYTEST_ARGS 2>&1 | tee "../$RESULTS_DIR/pytest_output.log" || true |
There was a problem hiding this comment.
The test_filter input from workflow_dispatch is susceptible to shell injection. When test_filter contains shell metacharacters (e.g., semicolons, backticks, or $() constructs), they will be interpreted by the shell. The single quotes around the test filter in line 165 are added as literal string characters within the PYTEST_ARGS variable assignment (which uses double quotes), not as shell quoting. When PYTEST_ARGS is expanded unquoted on line 172, the shell will interpret any metacharacters in the test filter value. Use proper quoting by wrapping the GitHub expression in quotes when building PYTEST_ARGS, or use an intermediate variable.
| PYTEST_ARGS="-vs --tb=short -m d457 v4l2_test/" | |
| if [ -n "${{ github.event.inputs.test_filter }}" ]; then | |
| PYTEST_ARGS="$PYTEST_ARGS -k '${{ github.event.inputs.test_filter }}'" | |
| fi | |
| echo "Running: python3 -m pytest $PYTEST_ARGS" | |
| echo "" | |
| # Run pytest; tee output to file for artifact, allow non-zero exit | |
| python3 -m pytest $PYTEST_ARGS 2>&1 | tee "../$RESULTS_DIR/pytest_output.log" || true | |
| # Safely capture the test filter input as a shell string | |
| TEST_FILTER=${{ toJSON(github.event.inputs.test_filter) }} | |
| # Build pytest arguments as an array to avoid shell injection and word-splitting issues | |
| PYTEST_ARGS=(-vs --tb=short -m d457 v4l2_test/) | |
| if [ -n "$TEST_FILTER" ]; then | |
| PYTEST_ARGS+=(-k "$TEST_FILTER") | |
| fi | |
| echo "Running: python3 -m pytest ${PYTEST_ARGS[*]}" | |
| echo "" | |
| # Run pytest; tee output to file for artifact, allow non-zero exit | |
| python3 -m pytest "${PYTEST_ARGS[@]}" 2>&1 | tee "../$RESULTS_DIR/pytest_output.log" || true |
Summary
.github/workflows/v4l2-test.yml) that runs the V4L2 pytest suite on a self-hosted Jetson runnerscripts/setup_gh_runner.sh) for installing a GitHub Actions self-hosted runner on a Jetson deviceWorkflow Details
workflow_dispatchwith optional test filter), push/PR tomaster/devwhenkernel/realsense/**ortest/v4l2_test/**change[self-hosted, jetson, xavier]labeled runnerRunner Setup
Test Plan
gh workflow run v4l2-test.yml --ref v4l2-test-workflow🤖 Generated with Claude Code