diff --git a/.github/workflows/benchmark-history.yml b/.github/workflows/benchmark-history.yml index 9279b23d..542441e5 100644 --- a/.github/workflows/benchmark-history.yml +++ b/.github/workflows/benchmark-history.yml @@ -60,6 +60,7 @@ env: BENCHMARK_JOB: short # One line per chart: name|history|chart|filter. An environment variable cannot hold an array, # and three steps need the same three triples, so this is the one place they are written. + # verify-charts.yml reads this value from here too, so it stays the only copy. # # The quantities filter names one operation per panel. The strings and paths filters name whole # classes instead, so they measure more than they draw: ingest stores every row and only render @@ -203,19 +204,10 @@ jobs: done <<< "$SUBJECTS" done + # The same script verify-charts.yml runs, so the charts this commits are the ones it checks. - name: Redraw the charts shell: bash - run: | - set -euo pipefail - while IFS='|' read -r subject history chart filter; do - [ -n "$subject" ] || continue - if [ ! -f "$history" ]; then - echo "No history for $subject yet; nothing to draw." - continue - fi - dotnet run scripts/benchmark-history.cs -- render \ - --subject "$subject" --history "$history" --out "$chart" &2 + exit 1 + fi + echo "Every benchmark chart matches its history." diff --git a/CLAUDE.md b/CLAUDE.md index e381cf09..35f57457 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -409,7 +409,9 @@ redraws the chart the README shows. Four things about it are not guessable from version across every package. - **The histories and the SVG files are committed output that a bot pushes.** A local `render` must be diffed before commit rather than assumed. The quantities chart in particular is a regression test: - a change to the renderer that moves a byte in it has broken something. + a change to the renderer that moves a byte in it has broken something. `verify-charts.yml` enforces + this on pull requests: it redraws every chart through `scripts/render-benchmark-charts.sh`, the step + the recorder runs, and fails on any difference. - **Nothing here touches an internal member.** The `InternalsVisibleTo` that would expose one names only the test assembly, and a benchmark built on internals could only ever measure the working copy, never a published package — which would make the release history impossible to backfill. diff --git a/scripts/render-benchmark-charts.sh b/scripts/render-benchmark-charts.sh new file mode 100755 index 00000000..e65022d4 --- /dev/null +++ b/scripts/render-benchmark-charts.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# Redraws every benchmark chart from its committed history. +# +# The one render invocation both workflows run: benchmark-history.yml after it records a point, +# and verify-charts.yml to check that the committed charts are what the committed histories draw. +# Sharing it is what keeps the guard and the recorder from drifting apart (#303). +# +# Reads the subject table from $SUBJECTS, one `name|history|chart|filter` line per subject, as +# benchmark-history.yml declares it. The filter is not used here. +set -euo pipefail + +: "${SUBJECTS:?SUBJECTS must hold the subject table from benchmark-history.yml}" + +while IFS='|' read -r subject history chart _filter; do + [[ -n "$subject" ]] || continue + if [[ ! -f "$history" ]]; then + echo "No history for $subject yet; nothing to draw." + continue + fi + # stdin from /dev/null: a process that drained the here-string would end the loop early and + # still exit zero, silently skipping the remaining subjects. + dotnet run scripts/benchmark-history.cs -- render \ + --subject "$subject" --history "$history" --out "$chart"