✨ Improve CI performance for pull requests #304
Conversation
Signed-off-by: Dylan <[email protected]>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds selective per-ruleset testing to the local CI: workflow fetches full history, determines changed rulesets in PRs (or uses all on push), conditionally runs Kantra tests per-path, aggregates results, and uploads artifacts only when tests executed. A new script detects changed rulesets via git diff. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Script as get-changed-rulesets.sh
participant Tests as Kantra Tests
participant Art as Artifacts
Dev->>GH: Push / PR
GH->>GH: Checkout (fetch-depth: 0)
GH->>Script: Determine rulesets to test
alt PR
Script-->>GH: set skip_tests & test_paths (changed rulesets or skip)
else Push
Script-->>GH: skip_tests=false, test_paths=./default/generated
end
alt skip_tests != 'true'
loop For each path in test_paths
GH->>Tests: Run tests for path
Tests-->>GH: exit code & results
end
GH->>GH: Aggregate results and exit status
GH->>Art: Upload matching result folders
Art-->>GH: Stored
GH->>GH: Update badges from aggregated outputs
else
GH->>GH: Emit standardized skipped outputs
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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
🧹 Nitpick comments (7)
hack/get-changed-rulesets.sh (4)
4-4: Harden script error handling (pipefail, nounset).Avoid silent failures in pipes and on unset vars.
-set -e +set -Eeuo pipefail
34-39: Also match .yml files, not just .yaml.Current filter can miss ruleset/test changes with .yml extension.
- if [[ "$file" =~ \.yaml$ ]] && [[ "$file" =~ ^default/generated/ ]]; then + if [[ "$file" =~ \.(yaml|yml)$ ]] && [[ "$file" =~ ^default/generated/ ]]; then
64-69: Normalize output paths (consistency with critical branch).Critical branch emits "./default/generated" but per-ruleset outputs omit "./". Normalize for consistency.
- echo "$ruleset" + echo "./$ruleset"Also consider aligning Line 59 accordingly or dropping "./" everywhere.
20-24: Optional: Ensure base ref exists locally (fork PRs).Guard against missing refs when comparing to origin/${{ github.base_ref }}.
echo "Comparing against: $BASE_BRANCH" >&2 + +# Ensure base ref exists locally (no-op if present) +if ! git rev-parse --verify "$BASE_BRANCH" >/dev/null 2>&1; then + echo "Base ref not found locally; fetching..." >&2 + git fetch origin "${BASE_BRANCH#origin/}:${BASE_BRANCH}" || true +fi.github/workflows/local-ci.yaml (3)
32-45: Quote and group writes to $GITHUB_OUTPUT (SC2086/SC2129).Prevents word splitting/globbing and reduces repeated redirects. actionlint: SC2086, SC2129.
- if [ -z "$CHANGED_RULESETS" ]; then - echo "No ruleset changes detected in this PR" - echo "skip_tests=true" >> $GITHUB_OUTPUT - echo "test_paths=" >> $GITHUB_OUTPUT - else - echo "Changed rulesets: $CHANGED_RULESETS" - echo "skip_tests=false" >> $GITHUB_OUTPUT - echo "test_paths=$CHANGED_RULESETS" >> $GITHUB_OUTPUT - fi + if [ -z "$CHANGED_RULESETS" ]; then + echo "No ruleset changes detected in this PR" + { + echo "skip_tests=true" + echo "test_paths=" + } >> "$GITHUB_OUTPUT" + else + echo "Changed rulesets: $CHANGED_RULESETS" + { + echo "skip_tests=false" + echo "test_paths=$CHANGED_RULESETS" + } >> "$GITHUB_OUTPUT" + fi - echo "Running in push mode - testing all rulesets" - echo "skip_tests=false" >> $GITHUB_OUTPUT - echo "test_paths=./default/generated" >> $GITHUB_OUTPUT + echo "Running in push mode - testing all rulesets" + { + echo "skip_tests=false" + echo "test_paths=./default/generated" + } >> "$GITHUB_OUTPUT"
83-94: Quote and group multi-line outputs (SC2086/SC2129).Harden writes to $GITHUB_OUTPUT and avoid multiple redirects. actionlint: SC2086, SC2129.
- echo 'msg<<EOF' >> $GITHUB_OUTPUT - echo ${msg} >> $GITHUB_OUTPUT - echo 'EOF' >> $GITHUB_OUTPUT - - echo 'tcs_pass_rate<<EOF' >> $GITHUB_OUTPUT - echo ${tcs_pass_rate} >> $GITHUB_OUTPUT - echo 'EOF' >> $GITHUB_OUTPUT - - echo 'tests_pass_rate<<EOF' >> $GITHUB_OUTPUT - echo ${tests_pass_rate} >> $GITHUB_OUTPUT - echo 'EOF' >> $GITHUB_OUTPUT + { + echo 'msg<<EOF' + echo "${msg}" + echo 'EOF' + echo 'tcs_pass_rate<<EOF' + echo "${tcs_pass_rate}" + echo 'EOF' + echo 'tests_pass_rate<<EOF' + echo "${tests_pass_rate}" + echo 'EOF' + } >> "$GITHUB_OUTPUT"
101-115: Same quoting/grouping for skip handler (SC2086/SC2129).Apply consistent, quoted grouped writes.
- echo "No ruleset changes detected in this PR - skipping tests" - echo 'msg<<EOF' >> $GITHUB_OUTPUT - echo "No ruleset changes detected - tests skipped" >> $GITHUB_OUTPUT - echo 'EOF' >> $GITHUB_OUTPUT - echo 'tests_pass_rate<<EOF' >> $GITHUB_OUTPUT - echo "N/A" >> $GITHUB_OUTPUT - echo 'EOF' >> $GITHUB_OUTPUT - echo 'tcs_pass_rate<<EOF' >> $GITHUB_OUTPUT - echo "N/A" >> $GITHUB_OUTPUT - echo 'EOF' >> $GITHUB_OUTPUT + echo "No ruleset changes detected in this PR - skipping tests" + { + echo 'msg<<EOF' + echo "No ruleset changes detected - tests skipped" + echo 'EOF' + echo 'tests_pass_rate<<EOF' + echo "N/A" + echo 'EOF' + echo 'tcs_pass_rate<<EOF' + echo "N/A" + echo 'EOF' + } >> "$GITHUB_OUTPUT"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/local-ci.yaml(5 hunks)hack/get-changed-rulesets.sh(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/local-ci.yaml
27-27: shellcheck reported issue in this script: SC2086:info:7:31: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: shellcheck reported issue in this script: SC2086:info:8:27: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: shellcheck reported issue in this script: SC2086:info:11:32: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: shellcheck reported issue in this script: SC2086:info:12:44: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: shellcheck reported issue in this script: SC2086:info:16:30: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: shellcheck reported issue in this script: SC2086:info:17:44: Double quote to prevent globbing and word splitting
(shellcheck)
104-104: shellcheck reported issue in this script: SC2129:style:2:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
104-104: shellcheck reported issue in this script: SC2086:info:2:20: Double quote to prevent globbing and word splitting
(shellcheck)
104-104: shellcheck reported issue in this script: SC2086:info:3:55: Double quote to prevent globbing and word splitting
(shellcheck)
104-104: shellcheck reported issue in this script: SC2086:info:4:15: Double quote to prevent globbing and word splitting
(shellcheck)
104-104: shellcheck reported issue in this script: SC2086:info:5:32: Double quote to prevent globbing and word splitting
(shellcheck)
104-104: shellcheck reported issue in this script: SC2086:info:6:15: Double quote to prevent globbing and word splitting
(shellcheck)
104-104: shellcheck reported issue in this script: SC2086:info:7:15: Double quote to prevent globbing and word splitting
(shellcheck)
104-104: shellcheck reported issue in this script: SC2086:info:8:30: Double quote to prevent globbing and word splitting
(shellcheck)
104-104: shellcheck reported issue in this script: SC2086:info:9:15: Double quote to prevent globbing and word splitting
(shellcheck)
104-104: shellcheck reported issue in this script: SC2086:info:10:15: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build tackle2-hub
| - name: Determine rulesets to test | ||
| id: get-rulesets | ||
| run: | | ||
| if [[ "${{ github.event_name }}" == "pull_request" ]]; then | ||
| echo "Running in PR mode - detecting changed rulesets..." | ||
| CHANGED_RULESETS=$(bash hack/get-changed-rulesets.sh origin/${{ github.base_ref }} | paste -sd ' ' -) | ||
| if [ -z "$CHANGED_RULESETS" ]; then | ||
| echo "No ruleset changes detected in this PR" | ||
| echo "skip_tests=true" >> $GITHUB_OUTPUT | ||
| echo "test_paths=" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "Changed rulesets: $CHANGED_RULESETS" | ||
| echo "skip_tests=false" >> $GITHUB_OUTPUT | ||
| echo "test_paths=$CHANGED_RULESETS" >> $GITHUB_OUTPUT | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
Verify base ref availability in PRs from forks.
origin/${{ github.base_ref }} may be missing depending on checkout. Validate before relying on it.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Remotes:"
git remote -v
echo
echo "Remote branches:"
git branch -r | sed -n '1,50p'
echo
echo "Attempting to resolve base ref:"
echo "github.base_ref=${GITHUB_BASE_REF:-unset}"
if [[ -n "${GITHUB_BASE_REF:-}" ]]; then
git rev-parse "origin/${GITHUB_BASE_REF}" || {
echo "origin/${GITHUB_BASE_REF} not found"
exit 1
}
fiLength of output: 247
Fetch base branch before computing changed rulesets
Add fetch-depth: 0 to your actions/checkout@v3 step or explicitly run git fetch origin ${{ github.base_ref }} before calling hack/get-changed-rulesets.sh so origin/${{ github.base_ref }} is always available.
🧰 Tools
🪛 actionlint (1.7.8)
27-27: shellcheck reported issue in this script: SC2086:info:7:31: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: shellcheck reported issue in this script: SC2086:info:8:27: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: shellcheck reported issue in this script: SC2086:info:11:32: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: shellcheck reported issue in this script: SC2086:info:12:44: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: shellcheck reported issue in this script: SC2086:info:16:30: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: shellcheck reported issue in this script: SC2086:info:17:44: Double quote to prevent globbing and word splitting
(shellcheck)
🤖 Prompt for AI Agents
.github/workflows/local-ci.yaml around lines 25 to 40: the workflow references
origin/${{ github.base_ref }} when computing changed rulesets but the base
branch may not be fetched by default, causing the detection script to fail;
ensure the base ref is available by either setting actions/checkout@v3 to
fetch-depth: 0 or by performing an explicit git fetch of origin ${{
github.base_ref }} before running hack/get-changed-rulesets.sh so the script can
compare against the base branch reliably.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Dylan Murray <[email protected]>
Signed-off-by: Dylan Murray <[email protected]>
Signed-off-by: Dylan Murray <[email protected]>
1 participant
Summary by CodeRabbit