Skip to content

Commit cf065ce

Browse files
committed
ci: fail the benchmark job when the PR run fails, not just when its YAML is missing
run_parallel_benchmarks.sh only hard-failed if the output YAML was absent; a non-zero PR SLURM exit (all cases SIGTERM'd/crashed/hung) was downgraded to a warning, so a broken PR benchmark passed green as long as a partial YAML existed (observed on MFlowCode#1588, where phoenix-bench node contention SIGTERM'd every PR case yet the check stayed green). Now a genuine PR-job failure exits 1. pr_exit is reliable: run_monitored_slurm_job.sh re-checks sacct and returns 0 when the job actually COMPLETED 0:0, so this does not red-cross on monitor flakiness. Scoped to PR only -- a master/baseline infra failure stays a warning.
1 parent 32b22ad commit cf065ce

1 file changed

Lines changed: 6 additions & 0 deletions

File tree

.github/scripts/run_parallel_benchmarks.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ bash "${SCRIPT_DIR}/run_monitored_slurm_job.sh" "$pr_job_id" "pr/${job_slug}.out
6262
if [ "$pr_exit" -ne 0 ]; then
6363
echo "PR job exited with code: $pr_exit"
6464
tail -n 50 "pr/${job_slug}.out" 2>/dev/null || echo " Could not read PR log"
65+
# The PR benchmark run genuinely failed (cases crashed/hung/SIGTERM'd, not a
66+
# monitor false-positive -- run_monitored_slurm_job.sh re-checks sacct). Fail
67+
# the job instead of falling through to the YAML-exists check, which would let
68+
# a broken PR pass green as long as a partial YAML was written. Scoped to PR
69+
# only: a master/baseline infra flake stays a warning and does not red-cross.
70+
exit 1
6571
else
6672
echo "PR job completed successfully"
6773
fi

0 commit comments

Comments
 (0)