diff --git a/.github/scripts/check_docs_build.py b/.github/scripts/check_docs_build.py index 0de95da4..0834fb49 100644 --- a/.github/scripts/check_docs_build.py +++ b/.github/scripts/check_docs_build.py @@ -37,13 +37,15 @@ Local use:: - python .github/scripts/check_docs_build.py # build, then check + python .github/scripts/check_docs_build.py # full build, then check python .github/scripts/check_docs_build.py build.log # parse an existing log python .github/scripts/check_docs_build.py --report # print counts, never fail """ import re import sys +import shutil import argparse +import tempfile import subprocess from collections import Counter from pathlib import Path @@ -79,15 +81,30 @@ def read_baseline(path=BASELINE_PATH): def build_docs(source_dir=SOURCE_DIR, out_dir=None): - """Build the HTML docs and return the combined build log. + """Build the HTML docs from scratch and return the combined build log. + + The build always goes into a FRESH directory, because the counts are only + meaningful for a full build. Sphinx is incremental: a second build into the + same output directory re-reads only the changed pages, so every message from + an untouched page is missing from the log. Locally that silently undercounts + -- a re-run reported 89 critical against a true 167 and invited the baseline + to be lowered to a number CI would never reproduce. CI always builds a fresh + checkout, so this keeps a local run honest and agreeing with it. Sphinx writes its messages to stderr and returns 0 with errors present, so both streams are captured and the return code is deliberately ignored. """ - out_dir = out_dir or (REPO_ROOT / "docs" / "_build" / "gate") + tmp_dir = None + if out_dir is None: + tmp_dir = tempfile.mkdtemp(prefix="aaanalysis-docs-gate-") + out_dir = tmp_dir cmd = [sys.executable, "-m", "sphinx", "-b", "html", str(source_dir), str(out_dir)] proc = subprocess.run(cmd, capture_output=True, text=True) log = proc.stdout + proc.stderr + if tmp_dir is not None: + # Always discard it: the log is the deliverable, and a kept tree would make the + # NEXT run incremental, which is the very thing this function exists to avoid. + shutil.rmtree(tmp_dir, ignore_errors=True) if not log.strip(): raise RuntimeError(f"sphinx produced no output (exit {proc.returncode})") return log @@ -142,7 +159,8 @@ def evaluate(log, baseline=None): code = 1 elif n_critical < baseline: lines.append(f"IMPROVED: {n_critical} critical < baseline {baseline} " - f"(-{baseline - n_critical}). Lower the baseline to {n_critical}.") + f"(-{baseline - n_critical}). Lower the baseline to the count a CI " + f"run reports, not to a local one.") else: lines.append(f"OK: critical at baseline ({baseline}).") return code, lines diff --git a/tests/unit/api_tests/test_check_docs_build.py b/tests/unit/api_tests/test_check_docs_build.py index ccb3da9e..7d87d4e5 100644 --- a/tests/unit/api_tests/test_check_docs_build.py +++ b/tests/unit/api_tests/test_check_docs_build.py @@ -145,7 +145,14 @@ def test_critical_above_baseline_fails(self, mod): def test_critical_below_baseline_passes_and_asks_to_lower(self, mod): code, lines = mod.evaluate(CLEAN_LOG, baseline=5) assert code == 0 - assert any("Lower the baseline to 0" in line for line in lines) + assert any("IMPROVED" in line for line in lines) + + def test_improved_message_points_at_a_ci_run_not_a_local_count(self, mod): + """A local build can undercount, so the message must not name a number to commit.""" + _, lines = mod.evaluate(CLEAN_LOG, baseline=5) + improved = next(line for line in lines if "IMPROVED" in line) + assert "CI" in improved + assert "Lower the baseline to 0" not in improved def test_error_fails_even_when_critical_is_at_baseline(self, mod): code, _ = mod.evaluate(ERROR_LOG + CRITICAL_LOG, baseline=1)