Skip to content

Commit 14b621e

Browse files
FrancescAltedclaude
andcommitted
Fix the bench job's venv discovery and separate "no data" from "regression"
All six bench cells failed at "Create both environments" while every build step, Windows included, succeeded. The cause was PY=$(ls venv-$v/bin/python venv-$v/Scripts/python.exe 2>/dev/null | head -1) GitHub runs `shell: bash` as `bash --noprofile --norc -eo pipefail`, so `set -e` is active, and an assignment takes the exit status of its command substitution. One of those two paths is always absent -- that is the whole point of listing both -- so ls exits 2 and kills the step. Replaced with an explicit -x test at both call sites. Also stop conflating outcomes in compare.py. It exited 1 both when a benchmark regressed and when no artifacts existed at all, so this run reported a performance regression when what actually happened was that no benchmark ever ran. It now always exits 0 and publishes status=ok|regressed|nodata through GITHUB_OUTPUT, with the workflow failing on each for its own reason and its own message. The report file is written unconditionally so the PR-comment step has something to post either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 943969e commit 14b621e

2 files changed

Lines changed: 53 additions & 23 deletions

File tree

.github/bench-abi3/compare.py

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,28 @@
2828
NOISE_FLOOR_MS = 5.0
2929

3030

31+
def emit(report, *, status, failures):
32+
"""Publish the report everywhere, and report `status` rather than exiting.
33+
34+
"no data" and "a real regression" are different outcomes and the workflow
35+
needs to tell them apart, so this always exits 0 and hands the verdict to
36+
the caller through GITHUB_OUTPUT.
37+
"""
38+
print(report)
39+
summary = os.environ.get("GITHUB_STEP_SUMMARY")
40+
if summary:
41+
with open(summary, "a") as fh:
42+
fh.write(report + "\n")
43+
out = os.environ.get("GITHUB_OUTPUT")
44+
if out:
45+
with open(out, "a") as fh:
46+
fh.write(f"status={status}\n")
47+
fh.write(f"failures={failures}\n")
48+
# Always written, so the PR-comment step has a file even when there is no data.
49+
with open("abi3-bench-report.md", "w") as fh:
50+
fh.write(report + "\n")
51+
52+
3153
def load_cell(cell_dir):
3254
out = {}
3355
for build in ("base", "abi3"):
@@ -47,8 +69,10 @@ def best(runs, name):
4769
def main(root):
4870
cells = sorted(d for d in glob.glob(os.path.join(root, "bench-*")) if os.path.isdir(d))
4971
if not cells:
50-
print(f"no result directories under {root}", file=sys.stderr)
51-
return 1
72+
msg = f"No benchmark results were produced under `{root}` (the bench jobs did not upload artifacts)."
73+
print(msg, file=sys.stderr)
74+
emit(msg + "\n", status="nodata", failures=0)
75+
return 0
5276

5377
lines = ["## abi3 vs. version-specific build\n"]
5478
lines.append(
@@ -107,22 +131,12 @@ def main(root):
107131
else:
108132
lines.append("\nNo regression past threshold on any platform. ✅\n")
109133

110-
report = "\n".join(lines)
111-
print(report)
112-
113-
summary = os.environ.get("GITHUB_STEP_SUMMARY")
114-
if summary:
115-
with open(summary, "a") as fh:
116-
fh.write(report + "\n")
117-
out = os.environ.get("GITHUB_OUTPUT")
118-
if out:
119-
with open(out, "a") as fh:
120-
fh.write(f"failures={len(failures)}\n")
121-
122-
with open("abi3-bench-report.md", "w") as fh:
123-
fh.write(report + "\n")
124-
125-
return 1 if failures else 0
134+
emit(
135+
"\n".join(lines),
136+
status="regressed" if failures else "ok",
137+
failures=len(failures),
138+
)
139+
return 0
126140

127141

128142
if __name__ == "__main__":

.github/workflows/abi3-bench.yml

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,14 @@ jobs:
8080
run: |
8181
for v in abi3 base; do
8282
python -m venv "venv-$v"
83-
PY=$(ls venv-$v/bin/python venv-$v/Scripts/python.exe 2>/dev/null | head -1)
83+
# Not `ls a b | head -1`: GitHub runs bash with `set -eo pipefail`,
84+
# and ls exits non-zero when one of the two paths is absent,
85+
# which is always the case -- it is one layout or the other.
86+
if [ -x "venv-$v/bin/python" ]; then
87+
PY="venv-$v/bin/python"
88+
else
89+
PY="venv-$v/Scripts/python.exe"
90+
fi
8491
"$PY" -m pip install --quiet --upgrade pip
8592
"$PY" -m pip install --quiet wheels-$v/*.whl
8693
echo "-- $v --"
@@ -100,7 +107,11 @@ jobs:
100107
OUT=results/bench-${{ matrix.os }}-${{ matrix.python-version }}
101108
for round in 1 2 3; do
102109
for v in abi3 base; do
103-
PY=$(ls venv-$v/bin/python venv-$v/Scripts/python.exe 2>/dev/null | head -1)
110+
if [ -x "venv-$v/bin/python" ]; then
111+
PY="venv-$v/bin/python"
112+
else
113+
PY="venv-$v/Scripts/python.exe"
114+
fi
104115
echo "== round $round / $v =="
105116
"$PY" .github/bench-abi3/bench.py > "$OUT/$v-$round.json"
106117
done
@@ -133,7 +144,6 @@ jobs:
133144

134145
- name: Build the report
135146
id: report
136-
continue-on-error: true
137147
run: python .github/bench-abi3/compare.py results
138148

139149
# The step summary is not reliably readable through the public API, so
@@ -152,7 +162,13 @@ jobs:
152162
fi
153163
154164
- name: Fail if a benchmark regressed past threshold
155-
if: steps.report.outcome == 'failure'
165+
if: steps.report.outputs.status == 'regressed'
166+
run: |
167+
echo "${{ steps.report.outputs.failures }} benchmark(s) past threshold; see the report above."
168+
exit 1
169+
170+
- name: Fail if no benchmark data was produced
171+
if: steps.report.outputs.status == 'nodata'
156172
run: |
157-
echo "See the report above: at least one benchmark regressed past threshold."
173+
echo "The bench jobs uploaded no artifacts -- this is a workflow failure, not a regression."
158174
exit 1

0 commit comments

Comments
 (0)