Skip to content

Commit 911796e

Browse files
committed
fix(ci): improve bench workflow robustness and reliability
Address critical issues to make the workflow less flaky and more reliable. Fixes: 1. Permissions - Added issues: write and pull-requests: write to check-permission job - Allows posting comments and reactions without permission errors 2. Concurrency control - Added concurrency group: bench-${{ issue.number }} - Prevents overlapping benchmark runs on the same PR - Uses cancel-in-progress: false to queue instead of cancel 3. Ref fetching - Added targeted git fetch before validation - Fetches refs, tags, and heads explicitly - Prevents "ref not found" errors for remote branches/tags 4. Shell robustness - Added set -euo pipefail to all bash scripts - Ensures early failure on any error - Prevents silent failures and undefined variables 5. Early parse feedback - Added continue-on-error to parse step - Posts immediate error message on invalid format - Shows usage examples with all parameters - Prevents workflow from proceeding with bad params Benefits: - More reliable ref resolution - Clear error messages for invalid input - No concurrent runs causing conflicts - Proper permissions for all operations - Fail-fast behavior prevents wasted CI time
1 parent b4d7ca1 commit 911796e

File tree

1 file changed

+71
-4
lines changed

1 file changed

+71
-4
lines changed

.github/workflows/bench-command.yml

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ on:
1010
issue_comment:
1111
types: [created]
1212

13+
# Prevent concurrent benchmark runs on the same PR
14+
concurrency:
15+
group: bench-${{ github.event.issue.number }}
16+
cancel-in-progress: false
17+
1318
jobs:
1419
check-permission:
1520
name: Check Command Permission
@@ -18,6 +23,9 @@ jobs:
1823
github.event.issue.pull_request &&
1924
startsWith(github.event.comment.body, '/bench ')
2025
runs-on: ubuntu-latest
26+
permissions:
27+
issues: write
28+
pull-requests: write
2129
outputs:
2230
authorized: ${{ steps.check.outputs.authorized }}
2331
ref1: ${{ steps.parse.outputs.ref1 }}
@@ -66,7 +74,9 @@ jobs:
6674
- name: Parse benchmark command
6775
id: parse
6876
if: steps.check.outputs.authorized == 'true'
77+
continue-on-error: true
6978
run: |
79+
set -euo pipefail
7080
COMMENT="${{ github.event.comment.body }}"
7181
7282
# Parse command: /bench ref1 ref2 [iterations] [sizes]
@@ -81,7 +91,8 @@ jobs:
8191
8292
# Validate required parameters
8393
if [ -z "$REF1" ] || [ -z "$REF2" ]; then
84-
echo "❌ Invalid format. Usage: /bench <ref1> <ref2> [iterations] [sizes]"
94+
echo "error=Invalid format. Missing required parameters." >> $GITHUB_OUTPUT
95+
echo "parse_failed=true" >> $GITHUB_OUTPUT
8596
exit 1
8697
fi
8798
@@ -96,23 +107,58 @@ jobs:
96107
97108
# Validate sizes format (comma-separated numbers)
98109
if ! echo "$SIZES" | grep -qE '^[0-9]+(,[0-9]+)*$'; then
99-
echo "❌ Invalid sizes format. Use comma-separated numbers like: 1000,5000,10000"
110+
echo "error=Invalid sizes format: $SIZES" >> $GITHUB_OUTPUT
111+
echo "parse_failed=true" >> $GITHUB_OUTPUT
100112
exit 1
101113
fi
102114
103115
echo "ref1=$REF1" >> $GITHUB_OUTPUT
104116
echo "ref2=$REF2" >> $GITHUB_OUTPUT
105117
echo "iterations=$ITERATIONS" >> $GITHUB_OUTPUT
106118
echo "sizes=$SIZES" >> $GITHUB_OUTPUT
119+
echo "parse_failed=false" >> $GITHUB_OUTPUT
107120
108121
echo "Parsed parameters:"
109122
echo " ref1: $REF1"
110123
echo " ref2: $REF2"
111124
echo " iterations: $ITERATIONS"
112125
echo " sizes: $SIZES"
113126
127+
- name: Post parse error
128+
if: steps.check.outputs.authorized == 'true' && steps.parse.outcome == 'failure'
129+
uses: actions/github-script@v7
130+
with:
131+
script: |
132+
await github.rest.reactions.createForIssueComment({
133+
owner: context.repo.owner,
134+
repo: context.repo.repo,
135+
comment_id: context.payload.comment.id,
136+
content: 'confused'
137+
});
138+
139+
await github.rest.issues.createComment({
140+
owner: context.repo.owner,
141+
repo: context.repo.repo,
142+
issue_number: context.issue.number,
143+
body: `❌ **Invalid command format**
144+
145+
**Usage:** \`/bench <ref1> <ref2> [iterations] [sizes]\`
146+
147+
**Examples:**
148+
\`\`\`
149+
/bench main v0.13.0
150+
/bench abc123 def456 100 1000,5000,10000
151+
\`\`\`
152+
153+
**Parameters:**
154+
- \`ref1\` (required): Baseline git reference
155+
- \`ref2\` (required): Current git reference
156+
- \`iterations\` (optional): Number of iterations (default: 100)
157+
- \`sizes\` (optional): Comma-separated sizes (default: 1000,5000,10000)`
158+
});
159+
114160
- name: Post acknowledgment
115-
if: steps.check.outputs.authorized == 'true'
161+
if: steps.check.outputs.authorized == 'true' && steps.parse.outcome == 'success'
116162
uses: actions/github-script@v7
117163
with:
118164
script: |
@@ -141,7 +187,7 @@ Results will be posted here when complete...`
141187
run-benchmarks:
142188
name: Run Benchmark Comparison
143189
needs: check-permission
144-
if: needs.check-permission.outputs.authorized == 'true'
190+
if: needs.check-permission.outputs.authorized == 'true' && needs.check-permission.outputs.ref1 != ''
145191
runs-on: ubuntu-latest
146192
permissions:
147193
contents: read
@@ -160,9 +206,25 @@ Results will be posted here when complete...`
160206
- name: Cache Rust dependencies
161207
uses: Swatinem/rust-cache@v2
162208

209+
- name: Fetch refs from remote
210+
run: |
211+
set -euo pipefail
212+
REF1="${{ needs.check-permission.outputs.ref1 }}"
213+
REF2="${{ needs.check-permission.outputs.ref2 }}"
214+
215+
echo "Fetching ref1: $REF1"
216+
git fetch origin "$REF1" || git fetch origin "refs/tags/$REF1" || git fetch origin "refs/heads/$REF1" || true
217+
218+
echo "Fetching ref2: $REF2"
219+
git fetch origin "$REF2" || git fetch origin "refs/tags/$REF2" || git fetch origin "refs/heads/$REF2" || true
220+
221+
# Update remote refs
222+
git fetch origin --tags
223+
163224
- name: Validate refs exist
164225
id: validate
165226
run: |
227+
set -euo pipefail
166228
REF1="${{ needs.check-permission.outputs.ref1 }}"
167229
REF2="${{ needs.check-permission.outputs.ref2 }}"
168230
@@ -182,6 +244,7 @@ Results will be posted here when complete...`
182244
- name: Check benchmark tool exists in ref1
183245
id: check_ref1_tool
184246
run: |
247+
set -euo pipefail
185248
REF1="${{ needs.check-permission.outputs.ref1 }}"
186249
echo "Checking out $REF1..."
187250
git checkout "$REF1"
@@ -206,6 +269,7 @@ Results will be posted here when complete...`
206269
- name: Check benchmark tool exists in ref2
207270
id: check_ref2_tool
208271
run: |
272+
set -euo pipefail
209273
REF2="${{ needs.check-permission.outputs.ref2 }}"
210274
echo "Checking out $REF2..."
211275
git checkout "$REF2"
@@ -271,6 +335,7 @@ Results will be posted here when complete...`
271335
- name: Benchmark ref1 (baseline)
272336
if: steps.check_ref1_tool.outputs.exists == 'true' && steps.check_ref2_tool.outputs.exists == 'true'
273337
run: |
338+
set -euo pipefail
274339
REF1="${{ needs.check-permission.outputs.ref1 }}"
275340
ITERATIONS="${{ needs.check-permission.outputs.iterations }}"
276341
SIZES="${{ needs.check-permission.outputs.sizes }}"
@@ -294,6 +359,7 @@ Results will be posted here when complete...`
294359
- name: Benchmark ref2 (current)
295360
if: steps.check_ref1_tool.outputs.exists == 'true' && steps.check_ref2_tool.outputs.exists == 'true'
296361
run: |
362+
set -euo pipefail
297363
REF2="${{ needs.check-permission.outputs.ref2 }}"
298364
ITERATIONS="${{ needs.check-permission.outputs.iterations }}"
299365
SIZES="${{ needs.check-permission.outputs.sizes }}"
@@ -318,6 +384,7 @@ Results will be posted here when complete...`
318384
- name: Compare results
319385
if: steps.check_ref1_tool.outputs.exists == 'true' && steps.check_ref2_tool.outputs.exists == 'true'
320386
run: |
387+
set -euo pipefail
321388
# Use the comparison script from ref2 (current)
322389
if [ -f scripts/compare_benchmarks.py ]; then
323390
python3 scripts/compare_benchmarks.py \

0 commit comments

Comments
 (0)