label test for more granular testing - #1855
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used🪛 zizmor (1.25.2).github/workflows/test.workflow.yml[warning] 40-40: code injection via template expansion (template-injection): may expand into attacker-controllable code (template-injection) 🔇 Additional comments (4)
📝 WalkthroughWalkthroughThe ChangesPer-platform CI gating
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
🤖 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/test.workflow.yml:
- Around line 47-49: The has_label function silently masks JSON parsing errors
from jq when $LABELS contains malformed JSON, causing the function to return
false instead of failing. Add error handling at the beginning of the has_label
function (such as set -e or adding error checks after the jq command) to ensure
that if jq encounters invalid JSON, the function exits with an error rather than
continuing silently and returning a false result.
- Line 33: The `dcl` output that is set based on the `testable_with_dcl` label
is never consumed by any downstream job in the workflow, unlike the other
outputs (`linux`, `windows`, `mac`, `replay`, `sanitizers`) which are referenced
via `needs.precheck.outputs.*` conditions. Either remove the `dcl` output
assignment entirely, or if it is intended for future use, add a clear comment
above the output definition explaining its intended purpose and when it will be
used.
- Around line 38-45: The RAW_LABELS variable is being assigned directly via
shell string interpolation of the GitHub template expression, which poses a
template injection risk. Instead, declare the labels data as an environment
variable at the step level using an env section, and then reference that
environment variable within the shell script. This ensures the JSON data is
passed safely through the environment rather than being directly interpolated
into the shell command.
🪄 Autofix (Beta)
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
Run ID: 13ecd202-4367-49ca-bb24-89dc01cbe6f7
📒 Files selected for processing (1)
.github/workflows/test.workflow.yml
📜 Review details
🧰 Additional context used
🪛 zizmor (1.25.2)
.github/workflows/test.workflow.yml
[warning] 38-38: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 39-39: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🔇 Additional comments (1)
.github/workflows/test.workflow.yml (1)
92-94: LGTM!Also applies to: 109-111, 122-124, 135-137, 148-150, 164-166
| mac: ${{ steps.check.outputs.mac }} | ||
| replay: ${{ steps.check.outputs.replay }} | ||
| sanitizers: ${{ steps.check.outputs.sanitizers }} | ||
| dcl: ${{ steps.check.outputs.dcl }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if dcl output is referenced anywhere in workflow files
# Search for dcl output usage in workflow files
rg -n 'outputs\.dcl' .github/workflows/Repository: luxonis/depthai-core
Length of output: 145
🏁 Script executed:
# Read the workflow file to understand the context around lines 33 and 82-89
cat -n .github/workflows/test.workflow.yml | sed -n '1,50p'Repository: luxonis/depthai-core
Length of output: 1596
🏁 Script executed:
# Search for references to precheck outputs in downstream jobs
rg -n 'needs\.precheck\.outputs' .github/workflows/Repository: luxonis/depthai-core
Length of output: 1234
🏁 Script executed:
# Search for any reference to dcl variable or precheck job usage
rg -n 'needs\.precheck|precheck:' .github/workflows/test.workflow.ymlRepository: luxonis/depthai-core
Length of output: 395
🏁 Script executed:
# Search for any reference to dcl in the workflow file
rg -n '\bdcl\b' .github/workflows/test.workflow.ymlRepository: luxonis/depthai-core
Length of output: 210
🏁 Script executed:
# View lines 70-100 to see the context around 82-89
cat -n .github/workflows/test.workflow.yml | sed -n '70,100p'Repository: luxonis/depthai-core
Length of output: 1440
Remove the unused dcl output or document if intended for future use.
The dcl output is computed based on the testable_with_dcl label (lines 82–89) but is never referenced by any downstream job. All other outputs (linux, windows, mac, replay, sanitizers) are consumed via needs.precheck.outputs.* conditions, while needs.precheck.outputs.dcl has no references in the workflow. Either remove the output or add a comment explaining its intended purpose.
🤖 Prompt for 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.
In @.github/workflows/test.workflow.yml at line 33, The `dcl` output that is set
based on the `testable_with_dcl` label is never consumed by any downstream job
in the workflow, unlike the other outputs (`linux`, `windows`, `mac`, `replay`,
`sanitizers`) which are referenced via `needs.precheck.outputs.*` conditions.
Either remove the `dcl` output assignment entirely, or if it is intended for
future use, add a clear comment above the output definition explaining its
intended purpose and when it will be used.
| has_label() { | ||
| echo "$LABELS" | jq -r '.[].name' | grep -qx "$1" | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
has_label silently fails if jq encounters invalid JSON.
If $LABELS contains malformed JSON, jq will error but has_label will return false (grep finds nothing), masking the parse failure. Consider adding set -e or validating JSON first.
💡 Optional: Add error handling
+ set -euo pipefail
+
has_label() {
- echo "$LABELS" | jq -r '.[].name' | grep -qx "$1"
+ echo "$LABELS" | jq -e -r '.[].name' 2>/dev/null | grep -qx "$1"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| has_label() { | |
| echo "$LABELS" | jq -r '.[].name' | grep -qx "$1" | |
| } | |
| set -euo pipefail | |
| has_label() { | |
| echo "$LABELS" | jq -e -r '.[].name' 2>/dev/null | grep -qx "$1" | |
| } |
🤖 Prompt for 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.
In @.github/workflows/test.workflow.yml around lines 47 - 49, The has_label
function silently masks JSON parsing errors from jq when $LABELS contains
malformed JSON, causing the function to return false instead of failing. Add
error handling at the beginning of the has_label function (such as set -e or
adding error checks after the jq command) to ensure that if jq encounters
invalid JSON, the function exits with an error rather than continuing silently
and returning a false result.
danilo-pejovic
added
testable
2 participants
label tests to tone down hil usage
Summary by CodeRabbit