From 7a200787e3ed42c566c0fb4adf442c298c996659 Mon Sep 17 00:00:00 2001 From: Rach Pradhan <54503978+justrach@users.noreply.github.com> Date: Thu, 27 Aug 2026 08:58:04 +0800 Subject: [PATCH] fix(ci): reject regressed performance baselines --- .github/workflows/perf-regression.yml | 4 +- benchmarks/bench_regression.py | 71 +++++++++++------ tests/test_benchmark_regression_policy.py | 96 +++++++++++++++++++++++ 3 files changed, 145 insertions(+), 26 deletions(-) create mode 100644 tests/test_benchmark_regression_policy.py diff --git a/.github/workflows/perf-regression.yml b/.github/workflows/perf-regression.yml index 485cef82..f375c902 100644 --- a/.github/workflows/perf-regression.yml +++ b/.github/workflows/perf-regression.yml @@ -73,7 +73,7 @@ jobs: - name: Run regression benchmark (main - full) if: github.ref == 'refs/heads/main' - run: uv run --python 3.14t python benchmarks/bench_regression.py --save --history + run: uv run --python 3.14t python benchmarks/bench_regression.py --save --history --fail-on-regression env: BENCH_THREADS: 4 BENCH_CONNECTIONS: 100 @@ -102,7 +102,7 @@ jobs: }); - name: Commit baseline + history on main - if: github.ref == 'refs/heads/main' + if: ${{ !cancelled() && github.ref == 'refs/heads/main' }} run: | git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" diff --git a/benchmarks/bench_regression.py b/benchmarks/bench_regression.py index f2c27c5f..eaf6ebf2 100644 --- a/benchmarks/bench_regression.py +++ b/benchmarks/bench_regression.py @@ -9,6 +9,7 @@ uv run --python 3.14t python benchmarks/bench_regression.py uv run --python 3.14t python benchmarks/bench_regression.py --save # save as new baseline uv run --python 3.14t python benchmarks/bench_regression.py --ci # exit(1) on regression + uv run --python 3.14t python benchmarks/bench_regression.py --fail-on-regression uv run --python 3.14t python benchmarks/bench_regression.py --history # save history snapshot """ @@ -217,9 +218,46 @@ def generate_pr_comment(results, detailed, thresholds, avg_threshold, regression return comment +def persist_benchmark_artifacts( + results, + detailed, + *, + save_mode, + history_mode, + protect_baseline, +): + """Persist benchmark output without accepting a failed strict run as baseline.""" + if save_mode and protect_baseline: + print("\nBaseline not updated because the benchmark regressed") + elif save_mode: + with open(BASELINE_FILE, "w") as f: + json.dump(results, f, indent=2) + print(f"\nBaseline saved to {BASELINE_FILE}") + + with open(RESULTS_FILE, "w") as f: + json.dump( + { + "metadata": { + "duration": DURATION, + "threads": THREADS, + "connections": CONNECTIONS, + "workers": WORKERS, + }, + "results": results, + "detailed": detailed, + }, + f, + indent=2, + ) + + if history_mode or save_mode: + save_history(results, detailed) + + def main(): save_mode = "--save" in sys.argv ci_mode = "--ci" in sys.argv + fail_on_regression = "--fail-on-regression" in sys.argv history_mode = "--history" in sys.argv with open("/tmp/turboapi_regbench.py", "w") as f: @@ -292,29 +330,14 @@ def main(): regressions.append(("AVERAGE", avg, avg_threshold)) print(f" AVERAGE {avg:,.0f} < {avg_threshold:,} (threshold)") - if save_mode: - with open(BASELINE_FILE, "w") as f: - json.dump(results, f, indent=2) - print(f"\nBaseline saved to {BASELINE_FILE}") - - with open(RESULTS_FILE, "w") as f: - json.dump( - { - "metadata": { - "duration": DURATION, - "threads": THREADS, - "connections": CONNECTIONS, - "workers": WORKERS, - }, - "results": results, - "detailed": detailed, - }, - f, - indent=2, - ) - - if history_mode or save_mode: - save_history(results, detailed) + strict_mode = ci_mode or fail_on_regression + persist_benchmark_artifacts( + results, + detailed, + save_mode=save_mode, + history_mode=history_mode, + protect_baseline=bool(regressions) and strict_mode, + ) if ci_mode: generate_pr_comment(results, detailed, thresholds, avg_threshold, regressions) @@ -325,7 +348,7 @@ def main(): for name, actual, threshold in regressions: print(f" {name}: {actual:,.0f} < {threshold:,.0f} (threshold)") print(f"{'!' * 60}") - if ci_mode: + if strict_mode: sys.exit(1) return results diff --git a/tests/test_benchmark_regression_policy.py b/tests/test_benchmark_regression_policy.py new file mode 100644 index 00000000..da9fec89 --- /dev/null +++ b/tests/test_benchmark_regression_policy.py @@ -0,0 +1,96 @@ +import importlib.util +import json +import sys +import urllib.request +from pathlib import Path + +import pytest + +MODULE_PATH = Path(__file__).parents[1] / "benchmarks" / "bench_regression.py" +SPEC = importlib.util.spec_from_file_location("bench_regression", MODULE_PATH) +assert SPEC is not None and SPEC.loader is not None +bench_regression = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(bench_regression) + + +def configure_artifact_paths(monkeypatch, tmp_path): + history_dir = tmp_path / "history" + monkeypatch.setattr(bench_regression, "BASELINE_FILE", str(tmp_path / "baseline.json")) + monkeypatch.setattr(bench_regression, "RESULTS_FILE", str(tmp_path / "results.json")) + monkeypatch.setattr(bench_regression, "HISTORY_DIR", str(history_dir)) + return history_dir + + +def test_main_strict_regression_preserves_baseline_and_records_history(monkeypatch, tmp_path): + history_dir = configure_artifact_paths(monkeypatch, tmp_path) + baseline_path = Path(bench_regression.BASELINE_FILE) + previous_baseline = {name: 150_000.0 for name, *_ in bench_regression.BENCHMARKS} + baseline_path.write_text(json.dumps(previous_baseline)) + thresholds_path = tmp_path / "thresholds.json" + thresholds_path.write_text( + json.dumps( + { + "margin_pct": 10, + "average_threshold_rps": 0, + "endpoints": {}, + "ci": { + "average_threshold_rps": 0, + "endpoints": { + name: {"min_rps": 1} for name, *_ in bench_regression.BENCHMARKS + }, + }, + } + ) + ) + monkeypatch.setattr(bench_regression, "THRESHOLDS_FILE", str(thresholds_path)) + + class StubProcess: + def kill(self): + pass + + def wait(self): + pass + + class StubPipe: + def read(self): + return "test-commit\n" + + monkeypatch.setattr(bench_regression.subprocess, "Popen", lambda *args, **kwargs: StubProcess()) + monkeypatch.setattr(bench_regression.os, "popen", lambda *args, **kwargs: StubPipe()) + monkeypatch.setattr(bench_regression, "run_wrk", lambda *args, **kwargs: { + "requests_per_second": 100_000.0, + "latency_avg_ms": 0.1, + "latency_p99_ms": 0.2, + }) + monkeypatch.setattr(bench_regression.time, "sleep", lambda _: None) + monkeypatch.setattr(urllib.request, "urlopen", lambda *args, **kwargs: object()) + monkeypatch.setattr( + sys, + "argv", + ["bench_regression.py", "--save", "--history", "--fail-on-regression"], + ) + + with pytest.raises(SystemExit, match="1"): + bench_regression.main() + + assert json.loads(baseline_path.read_text()) == previous_baseline + snapshots = list(history_dir.glob("*.json")) + assert len(snapshots) == 1 + assert set(json.loads(snapshots[0].read_text())["results"].values()) == {100_000.0} + + +def test_passing_strict_run_updates_baseline(monkeypatch, tmp_path): + configure_artifact_paths(monkeypatch, tmp_path) + baseline_path = Path(bench_regression.BASELINE_FILE) + baseline_path.write_text(json.dumps({"GET /health": 100_000.0})) + + results = {"GET /health": 150_000.0} + bench_regression.persist_benchmark_artifacts( + results, + {}, + save_mode=True, + history_mode=False, + protect_baseline=False, + ) + + assert json.loads(baseline_path.read_text()) == results