Feature/hil testing overhaul - #1926
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR replaces the legacy test workflow with HIL-focused pull-request, nightly, scheduled, and weekly workflows. It adds selectable platform and sanitizer runs, Slack reporting, required-result checks, and optional standard-test execution in the reusable child workflow. ChangesHIL testing workflow overhaul
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/hil_testing_pull_request.yml:
- Line 32: Pin every external GitHub Actions reference in the listed workflow
steps to a full immutable commit SHA instead of mutable tags, updating the
`actions/checkout`, `actions/upload-artifact`, and `actions/download-artifact`
usages in `.github/workflows/hil_testing_pull_request.yml` and
`.github/workflows/hil_testing_weekly.yml`. Keep the workflow behavior unchanged
and replace each tagged action reference at the anchor and sibling sites with
its trusted SHA-pinned equivalent.
In @.github/workflows/hil_testing_schedule_develop.yml:
- Line 19: Update the Saturday schedule’s gh workflow run command to dispatch
hil_testing_weekly.yml instead of hil_testing_main.yml, while preserving the
existing repository and develop ref arguments.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1a97d72d-28a2-4835-b676-31c12b64a40b
📒 Files selected for processing (7)
.github/workflows/hil_testing_nightly.yml.github/workflows/hil_testing_pull_request.yml.github/workflows/hil_testing_schedule_develop.yml.github/workflows/hil_testing_weekly.yml.github/workflows/main.workflow.yml.github/workflows/test.workflow.yml.github/workflows/test_child.yml
💤 Files with no reviewable changes (1)
- .github/workflows/test.workflow.yml
📜 Review details
🧰 Additional context used
🪛 actionlint (1.7.12)
.github/workflows/hil_testing_pull_request.yml
[error] 28-28: label "hil-test-v3" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2025-vs2026", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xlarge", "macos-latest-large", "macos-26-intel", "macos-26-xlarge", "macos-26-large", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xlarge", "macos-14-large", "macos-14", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
[error] 52-52: label "testbed-runner" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2025-vs2026", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xlarge", "macos-latest-large", "macos-26-intel", "macos-26-xlarge", "macos-26-large", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xlarge", "macos-14-large", "macos-14", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
[error] 87-87: label "testbed-runner" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2025-vs2026", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xlarge", "macos-latest-large", "macos-26-intel", "macos-26-xlarge", "macos-26-large", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xlarge", "macos-14-large", "macos-14", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🪛 zizmor (1.29.0)
.github/workflows/hil_testing_schedule_develop.yml
[error] 9-9: overly broad permissions (excessive-permissions): actions: write is overly broad at the workflow level
(excessive-permissions)
[warning] 9-9: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[info] 12-12: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
[warning] 3-6: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
.github/workflows/hil_testing_nightly.yml
[warning] 1-26: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
.github/workflows/hil_testing_pull_request.yml
[warning] 32-32: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 55-55: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 89-89: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 132-132: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 1-180: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 17-48: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 49-79: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 80-113: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 40-40: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 42-42: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 46-46: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 59-59: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 67-67: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 67-67: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 68-68: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 68-68: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 93-93: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 94-94: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 100-100: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 101-101: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 101-101: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 102-102: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 102-102: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 174-174: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 174-174: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 176-176: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 177-177: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 32-32: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 55-55: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 73-73: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 89-89: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 107-107: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 132-132: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 135-135: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[warning] 129-129: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
.github/workflows/hil_testing_weekly.yml
[warning] 1-224: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 25-31: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 33-46: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 48-59: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 61-72: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 74-85: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 87-101: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 103-114: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 31-31: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 129-129: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[info] 25-25: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
[info] 116-116: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
🔇 Additional comments (5)
.github/workflows/test_child.yml (1)
20-23: LGTM!Also applies to: 86-86, 114-114, 144-144, 170-170, 197-197
.github/workflows/main.workflow.yml (1)
20-20: LGTM!.github/workflows/hil_testing_weekly.yml (1)
25-114: LGTM!.github/workflows/hil_testing_nightly.yml (1)
12-25: LGTM!.github/workflows/hil_testing_pull_request.yml (1)
40-46: 🔒 Security & PrivacySensitive Data Exposure (CWE-367): Time-of-check Time-of-use (TOCTOU) Race Condition
Reachability: External
Bind the build to an immutable reviewed revision.
SOURCE_REFresolves the mutablerefs/pull/<number>/mergeref when the build starts. A pull request can change after the approval or label event and before a self-hosted runner resolves this ref. The resulting image then runs withHIL_PAT_TOKENand telemetry secrets.Pass an immutable event SHA to the build script. Verify that
scripts/hil/build_and_push.shchecks out that SHA and does not resolve the pull-request ref again.#!/bin/bash set -euo pipefail # Inspect whether the build script resolves its source argument as a mutable ref. rg -n -C 4 'SOURCE_REF|\$2|git (fetch|checkout|clone)|refs/pull' \ scripts/hil/build_and_push.sh
05118c3 to
87b7031
Compare
danilo-pejovic
left a comment
danilo-pejovic
left a comment
There was a problem hiding this comment.
Mostly lgtm, barring those nitpicks. You will probably have to create nightly/weekly workflows in main before running them else github can be bit hard to work with as it cant find them to run them.
| name: DepthAI Core HIL Testing Develop Schedule | ||
|
|
||
| on: | ||
| schedule: |
There was a problem hiding this comment.
Why do we need a separate workflow to trigger nightly runs? I think just adding this schedule directly into nightly.yml/weekly file should work well? Nightly/weekly can also be combined into a single file that runs with different OS/platform requirements based on schedule.
|
|
||
| jobs: | ||
| # Build the docker container | ||
| build_docker_container: |
There was a problem hiding this comment.
This seems like code duplication as these same functions will also be in test_child? Can this pipeline work similarly to test.workflow.yml previously where it would just call relevant jobs from test_child.yml and then we move relevant reporting there?
Nothing critical but having such behaviour in reusable scripts makes it much easier to quickly set up testing to replicate specific issues we find during testing.
2 participants
Purpose
Adds the following testing structure:
Specification
Dependencies & Potential Impact
None / not applicable
Deployment Plan
None / not applicable
Testing & Validation
None / not applicable
AI Usage
Assisted-by: Codex
Submitted code was reviewed by a human: YES
The author is taking the responsibility for the contribution: YES
Summary by CodeRabbit
New Features
Changes