From 89d098cb6f22cf9166ea83e44fe475c9cfbae7f0 Mon Sep 17 00:00:00 2001 From: propcgamer20-png <276521524+propcgamer20-png@users.noreply.github.com> Date: Thu, 22 Oct 2026 19:54:15 +0530 Subject: [PATCH] Test check_broken_links and freeze(), add an explainer structure check Tests for scripts/check_broken_links.py (96% coverage, #1109) and the whole freeze() flow in scripts/freeze_paper_results.py against a temporary tree with git stubbed (99%, #1111). New scripts/check_explainer_structure.py enforces the CONTRIBUTING.md section list, a python block, two Further Reading entries and the footer, with the existing gaps in a shrink-only KNOWN_EXCEPTIONS list; wired into make lint and pre-commit (#1118). --- .pre-commit-config.yaml | 7 + CHANGELOG.md | 2 + CONTRIBUTING.md | 1 + Makefile | 3 +- scripts/check_explainer_structure.py | 163 ++++++++++++++++++++ tests/test_check_broken_links.py | 169 ++++++++++++++++++++ tests/test_check_explainer_structure.py | 150 ++++++++++++++++++ tests/test_freeze_paper_results.py | 195 +++++++++++++++++++++++- 8 files changed, 685 insertions(+), 5 deletions(-) create mode 100644 scripts/check_explainer_structure.py create mode 100644 tests/test_check_broken_links.py create mode 100644 tests/test_check_explainer_structure.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ad0ff31..66c0587 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -48,6 +48,13 @@ repos: pass_filenames: false files: '^(explainers/.*\.md|README\.md|CONTRIBUTORS\.md|METRICS\.md|ROADMAP\.md)$' + - id: check-explainer-structure + name: check explainers have the required sections + entry: python3 scripts/check_explainer_structure.py + language: system + pass_filenames: false + files: '^(explainers/.*\.md|scripts/check_explainer_structure\.py)$' + - id: pytest name: pytest (fast suite, skips slow benchmark tests) entry: python3 -m pytest tests/ -q -m "not slow" diff --git a/CHANGELOG.md b/CHANGELOG.md index 791cac6..8d7abb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ All notable changes to Fair Code are documented here, newest first. ### Added +- `scripts/check_explainer_structure.py` (#1118), run by `make lint` and pre-commit: every explainer must have the sections CONTRIBUTING.md lists (accepting the usual heading variants, in order), a `python` block, two Further Reading entries and the footer. Ten pages with existing gaps are listed in `KNOWN_EXCEPTIONS`, and an exception that is no longer needed fails the check so the list only shrinks. +- Tests for `scripts/check_broken_links.py` (#1109, 96% coverage) and for the whole `freeze()` flow in `scripts/freeze_paper_results.py` (#1111, 99% coverage), run against temporary directories. - `scripts/check_traction.py` (#1097), run by `make lint`: fails, naming the files and values, when the stars, forks, contributors, watching, social reach, countries or audits figures differ between the README, ROADMAP and METRICS, or when the README's "Show N completed items" differs from its `- [x]` lines. It compares the files with each other, so it works offline. - `scripts/diff_results.py` marks rows measured on different test sets (#1095): each row has a `sample` field (`same`, `different` or `unknown`, with `sample_sizes` when different), different rows are marked `~` in the text output and counted in `--summary`. - CONTRIBUTING.md lists the prerequisites for `make check` and a "Checks you may meet" table saying what each failing check means and how to fix it (#989). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e86889c..8561f5f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -87,6 +87,7 @@ Prerequisites for the full `make check`: Python 3.10 or newer, Node 18 or newer | `check_broken_links.py` | A relative link in the docs points at a missing file or heading | Fix the path or heading | | `check_issue_refs.py` | A `#NNN` in ROADMAP, CHANGELOG, CONTRIBUTORS or METRICS is not a real issue or PR | Correct the number | | `check_explainer_count.py` | An "N explainers" figure, or an explainer listing in the README or `llms.txt`, is stale | `make fix-explainer-count` (a missing listing has to be added by hand) | +| `check_explainer_structure.py` | An explainer is missing a required section (definition, why it matters, concrete example, detection code, limitations, related concepts, further reading), has no `python` block, has fewer than two Further Reading entries, or lacks the footer line | Add the section (the check accepts the usual heading variants, e.g. "Why This Matters"); pages with a known gap are listed in `KNOWN_EXCEPTIONS` in the script, and an exception is removed when its page is fixed | | `check_traction.py` | Stars, forks, contributors, watching, social reach, countries or audits differ between README, ROADMAP and METRICS, or the README's "Show N completed items" differs from its `- [x]` lines | Edit the figure in the file(s) the message names so they agree; the README's "External Contributors" is one below the total | | `key-fold-check` | The JavaScript case-folding table `KEY_FOLD` no longer matches Python | `make key-fold`, then commit `assets/profiler-engine.js` | | A test that runs a README command | Every `faircode ...` line in the README, CONTRIBUTING and SPEC code blocks is checked against the CLI's argument parser; one of them uses a flag or value that no longer exists | Fix the example or the CLI | diff --git a/Makefile b/Makefile index 7484f8f..f5cb550 100644 --- a/Makefile +++ b/Makefile @@ -45,11 +45,12 @@ favicons: ## Regenerate favicon.ico/PNGs and apple-touch-icon.png from logo.svg fix-explainer-count: ## Correct stale "N explainers" mentions in README/CONTRIBUTORS/METRICS/ROADMAP $(PY) scripts/check_explainer_count.py --fix -lint: ## Enforce the em-dash-free rule + check for broken doc links + issue numbers in docs (needs gh, skipped offline) + explainer counts and listings + traction figures agreeing across README/ROADMAP/METRICS + ruff (mirrors the lint workflow) +lint: ## Enforce the em-dash-free rule + check for broken doc links + issue numbers in docs (needs gh, skipped offline) + explainer counts and listings + explainer structure + traction figures agreeing across README/ROADMAP/METRICS + ruff (mirrors the lint workflow) $(PY) scripts/check_em_dash.py $(PY) scripts/check_broken_links.py $(PY) scripts/check_issue_refs.py $(PY) scripts/check_explainer_count.py + $(PY) scripts/check_explainer_structure.py $(PY) scripts/check_traction.py ruff check faircode scripts tests diff --git a/scripts/check_explainer_structure.py b/scripts/check_explainer_structure.py new file mode 100644 index 0000000..17753f3 --- /dev/null +++ b/scripts/check_explainer_structure.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 +"""Checks that every explainers/*.md has the structure CONTRIBUTING.md section 8 asks for (#1118). + +A good explainer has, in this order: a definition, why it matters, a concrete example, detection +code, limitations, related concepts and further reading. It also has at least one `python` fenced +block, at least two Further Reading entries, and the project footer line. Headings are worded +differently from page to page ("Why It Matters" / "Why This Matters", "Limitations" / +"Limitations and Trade-offs"), so each required section lists the heading patterns it accepts. + + python3 scripts/check_explainer_structure.py + +Exit code: 0 = every explainer passes, 1 = at least one problem (each is printed with the file and +what is missing). + +KNOWN_EXCEPTIONS lists the gaps that already exist, per page, so the check can land before the +pages are fixed. An exception is removed by fixing the page; the check also fails when an exception +is no longer needed, so the list can only shrink. +""" +from __future__ import annotations + +import re +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +EXPLAINERS_DIR = ROOT / "explainers" + +# (name, regex matched against the lower-cased "## " heading text), in the required order. +SECTIONS = [ + ("definition", r"definition|core concept"), + ("why it matters", r"why (it|this) matters"), + ("concrete example", r"concrete example|real-world (proof|case)|real implementation|tabular proof|the benchmark"), + ("detection code", r"detect|how to measure"), + ("limitations", r"limitations"), + ("related concepts", r"related (concepts|projects)"), + ("further reading", r"further reading"), +] +SECTION_NAMES = [name for name, _ in SECTIONS] + +MIN_FURTHER_READING_ENTRIES = 2 +FOOTER_PREFIX = "*Part of [The Fair Code Project]" + +# slug -> what that page is allowed to lack for now. "order" excuses a section order that differs +# from CONTRIBUTING.md. Remove an entry when the page is fixed (the check fails on a stale one). +KNOWN_EXCEPTIONS: dict[str, set[str]] = { + "automation-bias": {"limitations"}, + "neural-networks": {"limitations", "detection code"}, + "proxy-variables": {"limitations"}, + "sampling-bias": {"limitations", "related concepts"}, + "differential-privacy": {"concrete example"}, + "disparate-impact": {"detection code"}, + "fairness-metric-conflicts": {"detection code"}, + "shap-values": {"detection code"}, + "how-ai-detects-patterns": {"order"}, + "predictive-parity": {"order"}, +} + +FENCE_RE = re.compile(r"^\s*(```|~~~)") +H2_RE = re.compile(r"^##\s+(.*?)\s*#*\s*$") + + +def _headings_and_flags(text: str): + """The (line_index, lower-cased text) of every top-level "## " heading outside code fences, + plus whether a python fence exists.""" + headings, in_fence, has_python = [], False, False + for i, line in enumerate(text.splitlines()): + fence = FENCE_RE.match(line) + if fence: + if not in_fence and line.strip().lower().startswith(("```python", "```py", "~~~python")): + has_python = True + in_fence = not in_fence + continue + if in_fence: + continue + m = H2_RE.match(line) + if m: + headings.append((i, m.group(1).strip().lower())) + return headings, has_python + + +def _further_reading_entries(text: str) -> int: + """List items under the last "Further Reading" heading, up to the next heading or rule.""" + lines = text.splitlines() + start = None + for i, line in enumerate(lines): + m = H2_RE.match(line) + if m and "further reading" in m.group(1).lower(): + start = i + if start is None: + return 0 + count = 0 + for line in lines[start + 1:]: + if H2_RE.match(line): + break + if re.match(r"^\s*(?:[-*]|\d+\.)\s+\S", line): + count += 1 + return count + + +def problems_in(text: str) -> set[str]: + """What is wrong with one explainer, as a set of short names (a section name, "order", + "python block", "further reading entries" or "footer").""" + headings, has_python = _headings_and_flags(text) + found: dict[str, int] = {} + for name, pattern in SECTIONS: + for line_no, heading in headings: + if re.search(pattern, heading): + found[name] = line_no + break + problems = {name for name in SECTION_NAMES if name not in found} + in_order = [found[name] for name in SECTION_NAMES if name in found] + if in_order != sorted(in_order): + problems.add("order") + if not has_python: + problems.add("python block") + if "further reading" in found and _further_reading_entries(text) < MIN_FURTHER_READING_ENTRIES: + problems.add("further reading entries") + last = next((line for line in reversed(text.splitlines()) if line.strip()), "") + if not last.startswith(FOOTER_PREFIX): + problems.add("footer") + return problems + + +def check(directory: Path | None = None, exceptions: dict[str, set[str]] | None = None) -> list[str]: + """One message per problem across every `*.md` in `directory` (empty when all is well).""" + directory = EXPLAINERS_DIR if directory is None else directory + exceptions = KNOWN_EXCEPTIONS if exceptions is None else exceptions + messages = [] + seen = set() + for path in sorted(Path(directory).glob("*.md")): + slug = path.stem + seen.add(slug) + found = problems_in(path.read_text(encoding="utf-8")) + allowed = exceptions.get(slug, set()) + new = found - allowed + if new: + messages.append(f"{path.name}: missing or wrong: {', '.join(sorted(new))}") + stale = allowed - found + if stale: + messages.append( + f"{path.name}: no longer needs the exception for {', '.join(sorted(stale))}; " + "remove it from KNOWN_EXCEPTIONS in scripts/check_explainer_structure.py" + ) + for slug in sorted(set(exceptions) - seen): + messages.append(f"KNOWN_EXCEPTIONS names {slug!r}, but explainers/{slug}.md does not exist") + return messages + + +def main() -> int: + messages = check() + if messages: + print("Explainer structure problems (see CONTRIBUTING.md, 'A good explainer should include'):", + file=sys.stderr) + for message in messages: + print(f" - {message}", file=sys.stderr) + return 1 + count = len(list(EXPLAINERS_DIR.glob("*.md"))) + print(f"OK: {count} explainers have the required sections, a python block, further reading and the footer.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_check_broken_links.py b/tests/test_check_broken_links.py new file mode 100644 index 0000000..afc4c26 --- /dev/null +++ b/tests/test_check_broken_links.py @@ -0,0 +1,169 @@ +"""scripts/check_broken_links.py catches broken in-page anchors and relative links (#1109).""" + +import importlib + +import pytest + +cbl = importlib.import_module("scripts.check_broken_links") + + +@pytest.fixture +def repo(tmp_path, monkeypatch): + """A temporary repo: write(rel_path, text) creates a file that git 'tracks'.""" + tracked = set() + + def write(rel, text=""): + path = tmp_path / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + tracked.add(rel) + return path + + monkeypatch.setattr(cbl, "ROOT", tmp_path) + monkeypatch.setattr(cbl, "_tracked_paths", lambda: set(tracked)) + monkeypatch.setattr( + cbl, "_tracked_markdown_files", + lambda: [tmp_path / rel for rel in sorted(tracked) if rel.endswith(".md")], + ) + write.root = tmp_path + return write + + +def _run(capsys): + code = cbl.main() + return code, capsys.readouterr().out + + +def test_clean_repo_passes_and_reports_the_counts(repo, capsys): + repo("README.md", "# Title\n\nSee [guide](docs/guide.md) and [top](#title).\n") + repo("docs/guide.md", "# Guide\n") + code, out = _run(capsys) + assert code == 0 + assert out.startswith("OK: 2 internal links checked across 2 markdown files") + + +def test_a_relative_link_to_a_missing_file_fails_with_file_line_and_target(repo, capsys): + repo("README.md", "# Title\n\nSee [guide](docs/missing.md).\n") + code, out = _run(capsys) + assert code == 1 + assert "Broken internal links found:" in out + assert "README.md:3: [docs/missing.md]" in out + assert "does not exist" in out + + +def test_an_in_page_anchor_must_match_a_heading(repo, capsys): + repo("README.md", "# Title\n\n[ok](#title) and [bad](#nope)\n") + code, out = _run(capsys) + assert code == 1 + assert "[#nope] - no matching heading on this page" in out + assert "[#title]" not in out + + +def test_an_anchor_in_another_file_must_match_a_heading_there(repo, capsys): + repo("README.md", "[ok](docs/guide.md#setup) [bad](docs/guide.md#teardown)\n") + repo("docs/guide.md", "# Guide\n\n## Setup\n") + code, out = _run(capsys) + assert code == 1 + assert "no heading 'teardown' in docs/guide.md" in out + assert "#setup" not in out + + +def test_a_link_to_a_directory_is_fine(repo, capsys): + repo("README.md", "[folder](docs/)\n") + repo("docs/guide.md", "# Guide\n") + code, _ = _run(capsys) + assert code == 0 + + +def test_external_links_are_ignored(repo, capsys): + repo("README.md", "[a](https://example.invalid/x) [b](mailto:a@b.c) [c](ftp://h/f)\n") + code, _ = _run(capsys) + assert code == 0 + + +def test_images_are_not_checked_as_links(repo, capsys): + repo("README.md", "![logo](assets/missing.png)\n") + code, _ = _run(capsys) + assert code == 0 + + +def test_links_inside_fenced_code_and_inline_code_are_ignored(repo, capsys): + repo("README.md", "# T\n\n```md\n[bad](nope.md) [bad2](#nowhere)\n```\n\nUse `[x](gone.md)` here.\n") + code, _ = _run(capsys) + assert code == 0 + + +def test_headings_inside_fences_do_not_count_as_anchors(repo, capsys): + repo("README.md", "# T\n\n```md\n# Fake\n```\n\n[x](#fake)\n") + code, out = _run(capsys) + assert code == 1 + assert "[#fake]" in out + + +def test_a_link_title_is_stripped_from_the_target(repo, capsys): + repo("README.md", '[guide](docs/guide.md "The guide")\n') + repo("docs/guide.md", "# Guide\n") + code, _ = _run(capsys) + assert code == 0 + + +def test_percent_encoded_paths_are_decoded(repo, capsys): + repo("README.md", "[a](docs/my%20file.md)\n") + repo("docs/my file.md", "# X\n") + code, _ = _run(capsys) + assert code == 0 + + +def test_blob_permalink_style_paths_are_skipped(repo, capsys): + repo(".github/TEMPLATE.md", "[x](../blob/main/README.md)\n") + code, _ = _run(capsys) + assert code == 0 + + +def test_a_file_on_disk_that_git_does_not_track_is_a_broken_link(repo, capsys): + repo("README.md", "[x](docs/guide.md)\n") + (repo.root / "docs").mkdir() + (repo.root / "docs" / "guide.md").write_text("# G\n", encoding="utf-8") # not tracked + code, out = _run(capsys) + assert code == 1 + assert "docs/guide.md" in out + + +def test_a_link_that_escapes_the_repo_falls_back_to_the_filesystem(repo, capsys): + repo("README.md", "[x](../outside.md)\n") + code, out = _run(capsys) + assert code == 1 + assert "../outside.md" in out + + +def test_every_bad_link_is_reported_not_just_the_first(repo, capsys): + repo("README.md", "[a](one.md)\n[b](two.md)\n") + code, out = _run(capsys) + assert code == 1 + assert "README.md:1: [one.md]" in out and "README.md:2: [two.md]" in out + + +@pytest.mark.parametrize("heading,slug", [ + ("Plain Title", "plain-title"), + ("COMPAS - Criminal Justice Bias", "compas--criminal-justice-bias"), + ("Auto-Detection", "auto-detection"), + ("What is `K`?", "what-is-k"), + ("**Bold** and [link](x.md)", "bold-and-link"), +]) +def test_slugify_matches_github_style_anchors(heading, slug): + assert cbl._slugify(heading) == slug + + +def test_duplicate_headings_get_numbered_anchors(repo, capsys): + repo("README.md", "# Notes\n\n# Notes\n\n[second](#notes-1)\n") + code, _ = _run(capsys) + assert code == 0 + + +def test_generated_explainer_mirror_is_excluded_by_default(): + assert cbl.ALLOW_PREFIXES == ("faircode/_explainers/",) + + +def test_the_real_repo_has_no_broken_links(capsys): + assert cbl.main() == 0 + capsys.readouterr() diff --git a/tests/test_check_explainer_structure.py b/tests/test_check_explainer_structure.py new file mode 100644 index 0000000..0d31a02 --- /dev/null +++ b/tests/test_check_explainer_structure.py @@ -0,0 +1,150 @@ +"""scripts/check_explainer_structure.py enforces the explainer structure from CONTRIBUTING.md (#1118).""" + +import importlib + +import pytest + +check = importlib.import_module("scripts.check_explainer_structure") + +GOOD = """# Explainer: Thing + +## The One-Sentence Definition + +A thing. + +## Why It Matters + +Because. + +## Concrete Example: COMPAS - Audit 01 + +An example. + +## Detection Code + +```python +print(1) +``` + +## Limitations + +Some. + +## Related Concepts + +- other + +## Further Reading + +- [One](https://example.com/1) +- [Two](https://example.com/2) + +--- + +*Part of [The Fair Code Project](https://instagram.com/thefaircodeproject) - exposing and fixing algorithmic bias with real data and open code.* +""" + + +def _replace(text, old, new=""): + assert old in text + return text.replace(old, new, 1) + + +def test_a_complete_explainer_has_no_problems(): + assert check.problems_in(GOOD) == set() + + +@pytest.mark.parametrize("old,new", [ + ("## Why It Matters", "## Why This Matters"), + ("## Limitations", "## Limitations and Trade-offs"), + ("## Limitations", "## Limitations / Trade-offs"), + ("## Detection Code", "## Detection/Implementation Code"), + ("## Detection Code", "## How to Detect It in Python"), + ("## Concrete Example: COMPAS - Audit 01", "## Real-World Proof: COMPAS Analysis"), + ("## Related Concepts", "## Related Projects in This Repo"), + ("## The One-Sentence Definition", "## The Formal Definition"), +]) +def test_accepted_heading_variants_pass(old, new): + assert check.problems_in(_replace(GOOD, old, new)) == set() + + +@pytest.mark.parametrize("heading,name", [ + ("## The One-Sentence Definition", "definition"), + ("## Why It Matters", "why it matters"), + ("## Concrete Example: COMPAS - Audit 01", "concrete example"), + ("## Detection Code", "detection code"), + ("## Limitations", "limitations"), + ("## Related Concepts", "related concepts"), + ("## Further Reading", "further reading"), +]) +def test_a_missing_section_is_named(heading, name): + problems = check.problems_in(_replace(GOOD, heading, "## Something Else")) + assert name in problems + + +def test_sections_out_of_order_are_reported(): + text = _replace(GOOD, "## Limitations\n\nSome.\n\n", "") + text = _replace(text, "## Why It Matters", "## Limitations\n\nSome.\n\n## Why It Matters") + assert "order" in check.problems_in(text) + + +def test_a_missing_python_block_is_reported(): + assert check.problems_in(_replace(GOOD, "```python", "```text")) == {"python block"} + + +def test_headings_inside_code_fences_do_not_count(): + # the "Limitations" below is a comment-style heading inside a fence, not a section + text = _replace(GOOD, "## Limitations\n\nSome.\n\n", "```text\n## Limitations\n```\n\n") + assert "limitations" in check.problems_in(text) + + +def test_too_few_further_reading_entries_are_reported(): + text = _replace(GOOD, "- [Two](https://example.com/2)\n") + assert check.problems_in(text) == {"further reading entries"} + + +def test_a_missing_footer_is_reported(): + text = _replace(GOOD, "*Part of [The Fair Code Project]", "*Made by someone else") + assert check.problems_in(text) == {"footer"} + + +def test_check_reports_file_and_problem_and_applies_exceptions(tmp_path): + (tmp_path / "ok.md").write_text(GOOD, encoding="utf-8") + (tmp_path / "bad.md").write_text(_replace(GOOD, "## Limitations", "## Nope"), encoding="utf-8") + messages = check.check(tmp_path, exceptions={}) + assert messages == ["bad.md: missing or wrong: limitations"] + assert check.check(tmp_path, exceptions={"bad": {"limitations"}}) == [] + + +def test_an_exception_that_is_no_longer_needed_fails_so_the_list_only_shrinks(tmp_path): + (tmp_path / "fixed.md").write_text(GOOD, encoding="utf-8") + messages = check.check(tmp_path, exceptions={"fixed": {"limitations"}}) + assert len(messages) == 1 + assert "fixed.md: no longer needs the exception for limitations" in messages[0] + + +def test_an_exception_for_a_page_that_does_not_exist_fails(tmp_path): + (tmp_path / "ok.md").write_text(GOOD, encoding="utf-8") + messages = check.check(tmp_path, exceptions={"ghost": {"limitations"}}) + assert messages == ["KNOWN_EXCEPTIONS names 'ghost', but explainers/ghost.md does not exist"] + + +def test_main_exits_1_and_prints_to_stderr_on_a_problem(tmp_path, monkeypatch, capsys): + (tmp_path / "bad.md").write_text(_replace(GOOD, "## Limitations", "## Nope"), encoding="utf-8") + monkeypatch.setattr(check, "EXPLAINERS_DIR", tmp_path) + monkeypatch.setattr(check, "KNOWN_EXCEPTIONS", {}) + assert check.main() == 1 + err = capsys.readouterr().err + assert "bad.md: missing or wrong: limitations" in err + + +def test_main_exits_0_when_everything_passes(tmp_path, monkeypatch, capsys): + (tmp_path / "ok.md").write_text(GOOD, encoding="utf-8") + monkeypatch.setattr(check, "EXPLAINERS_DIR", tmp_path) + monkeypatch.setattr(check, "KNOWN_EXCEPTIONS", {}) + assert check.main() == 0 + assert capsys.readouterr().out.startswith("OK: 1 explainers") + + +def test_the_real_explainers_pass(): + assert check.check() == [] diff --git a/tests/test_freeze_paper_results.py b/tests/test_freeze_paper_results.py index a6bccc6..0be60d6 100644 --- a/tests/test_freeze_paper_results.py +++ b/tests/test_freeze_paper_results.py @@ -1,10 +1,14 @@ -"""Tests for scripts/freeze_paper_results.py's mirror_for_mcp() - the small, -always-safe piece of this script (copies FROZEN_DIR -> MCP_MIRROR_DIR, never -touches results/ or paper/results-frozen/ itself). The full freeze() flow -needs a real results/ directory and git state, out of scope here. +"""Tests for scripts/freeze_paper_results.py: mirror_for_mcp() and mirror_current_for_mcp() +(never touch the directory they read from) and, in the second half of the file, the whole +freeze() flow against a temporary tree with git stubbed (#1111). """ import importlib +import shutil +import sys +import types + +import pytest def test_mirror_for_mcp_copies_the_three_frozen_csvs(tmp_path, monkeypatch): @@ -69,3 +73,186 @@ def test_package_mirrors_match_their_sources(): for name in script.MCP_MIRROR_FILES: assert (script.MCP_CURRENT_MIRROR_DIR / name).read_bytes() == (script.RESULTS_DIR / name).read_bytes() assert (script.MCP_MIRROR_DIR / name).read_bytes() == (script.FROZEN_DIR / name).read_bytes() + + +# ---- freeze() itself (#1111) ---------------------------------------------------------------- +# Everything runs inside tmp_path: the module's directory constants are monkeypatched, git is +# stubbed through _run, and the manifest discovery is replaced, so nothing outside tmp_path is read +# or written. + + +@pytest.fixture +def frozen_env(tmp_path, monkeypatch): + script = importlib.import_module("scripts.freeze_paper_results") + root = tmp_path / "repo" + results = root / "results" + (results / "figures").mkdir(parents=True) + (results / "results_fairness.csv").write_text("audit,metric\ncompas,dp\n", encoding="utf-8") + (results / "results_performance.csv").write_text("audit,metric\ncompas,acc\n", encoding="utf-8") + (results / "summary.csv").write_text("audit\ncompas\n", encoding="utf-8") + (results / "figures" / "a.png").write_bytes(b"png-bytes") + lock = root / "requirements-lock.txt" + lock.write_text("pandas==2.0.0\n", encoding="utf-8") + + monkeypatch.setattr(script, "REPO_ROOT", root) + monkeypatch.setattr(script, "RESULTS_DIR", results) + monkeypatch.setattr(script, "FROZEN_DIR", root / "paper" / "results-frozen") + monkeypatch.setattr(script, "LOCKFILE", lock) + monkeypatch.setattr(script, "MCP_MIRROR_DIR", root / "faircode" / "_results_frozen") + monkeypatch.setattr(script, "MCP_CURRENT_MIRROR_DIR", root / "faircode" / "_results_current") + monkeypatch.setattr(script, "_discover_manifests", lambda: ["COMPAS/audit.yaml", "Tenant Screening/audit.yaml"]) + monkeypatch.setattr(script, "_package_version", lambda name: f"{name}-9.9") + + commands = [] + git = {"rev-parse HEAD": "abc123", "rev-parse --abbrev-ref HEAD": "main", "status --porcelain": ""} + + def fake_run(cmd): + commands.append(cmd) + return git.get(" ".join(cmd[1:]), "") + + monkeypatch.setattr(script, "_run", fake_run) + return types.SimpleNamespace(script=script, root=root, tmp=tmp_path, git=git, commands=commands) + + +def _manifest(root): + return (root / "paper" / "results-frozen" / "MANIFEST.md").read_text(encoding="utf-8") + + +def test_freeze_copies_results_figures_and_lockfile(frozen_env): + e = frozen_env + frozen = e.script.freeze() + assert frozen == e.root / "paper" / "results-frozen" + for name in ("results_fairness.csv", "results_performance.csv", "summary.csv"): + assert (frozen / name).read_text(encoding="utf-8") == (e.root / "results" / name).read_text(encoding="utf-8") + assert (frozen / "figures" / "a.png").read_bytes() == b"png-bytes" + assert (frozen / "requirements-lock.txt").read_text(encoding="utf-8") == "pandas==2.0.0\n" + + +def test_freeze_manifest_records_commit_branch_versions_and_audits(frozen_env): + e = frozen_env + e.script.freeze() + text = _manifest(e.root) + assert "- **Git commit:** `abc123`\n" in text + assert "WORKING TREE HAD UNCOMMITTED CHANGES" not in text + assert "- **Git branch:** `main`" in text + for line in ("- **scikit-learn:** sklearn-9.9", "- **fairlearn:** fairlearn-9.9", + "- **pandas:** pandas-9.9", "- **numpy:** numpy-9.9", "- **Python:** "): + assert line in text + assert "## Audits included (2 domains)" in text + assert "- `COMPAS/audit.yaml`" in text and "- `Tenant Screening/audit.yaml`" in text + assert "git checkout abc123" in text + assert "## Tag" not in text + + +def test_freeze_flags_an_uncommitted_working_tree(frozen_env, capsys): + e = frozen_env + e.git["status --porcelain"] = " M README.md" + e.script.freeze() + assert "(WORKING TREE HAD UNCOMMITTED CHANGES AT FREEZE TIME)" in _manifest(e.root) + assert "[DIRTY]" in capsys.readouterr().out + + +def test_freeze_without_git_output_says_unknown(frozen_env): + e = frozen_env + e.git["rev-parse HEAD"] = "" + e.git["rev-parse --abbrev-ref HEAD"] = "" + e.script.freeze() + text = _manifest(e.root) + assert "`unknown (not a git checkout?)`" in text + assert "- **Git branch:** `unknown`" in text + + +def test_freeze_with_tag_prints_but_never_runs_a_git_tag_command(frozen_env, capsys): + e = frozen_env + e.script.freeze(tag="v9-paper") + text = _manifest(e.root) + assert "## Tag" in text and "git tag -a v9-paper" in text and "git push origin v9-paper" in text + out = capsys.readouterr().out + assert 'next step: git tag -a v9-paper -m "Results frozen for paper citation"' in out + # only read-only git commands ran: rev-parse twice and status + assert [c[1] for c in e.commands] == ["rev-parse", "rev-parse", "status"] + + +def test_freeze_without_tag_suggests_the_tag_flag(frozen_env, capsys): + frozen_env.script.freeze() + assert "pass --tag vX.Y-paper" in capsys.readouterr().out + + +def test_freeze_replaces_an_existing_snapshot_cleanly(frozen_env): + e = frozen_env + frozen = e.root / "paper" / "results-frozen" + (frozen / "figures").mkdir(parents=True) + (frozen / "stale.csv").write_text("old", encoding="utf-8") + (frozen / "figures" / "old.png").write_bytes(b"old") + e.script.freeze() + assert not (frozen / "stale.csv").exists() + assert not (frozen / "figures" / "old.png").exists() + assert (frozen / "figures" / "a.png").exists() + + +def test_freeze_refreshes_both_mcp_mirrors(frozen_env): + e = frozen_env + e.script.freeze() + for mirror in ("_results_frozen", "_results_current"): + for name in ("results_fairness.csv", "results_performance.csv", "summary.csv"): + assert (e.root / "faircode" / mirror / name).read_text(encoding="utf-8") == ( + e.root / "results" / name).read_text(encoding="utf-8") + + +def test_freeze_skips_missing_optional_inputs(frozen_env): + e = frozen_env + (e.root / "results" / "summary.csv").unlink() + (e.root / "results" / "figures" / "a.png").unlink() + (e.root / "results" / "figures").rmdir() + (e.root / "requirements-lock.txt").unlink() + frozen = e.script.freeze() + assert not (frozen / "summary.csv").exists() + assert not (frozen / "figures").exists() + assert not (frozen / "requirements-lock.txt").exists() + assert (frozen / "results_fairness.csv").exists() and (frozen / "MANIFEST.md").exists() + + +def test_freeze_fails_cleanly_when_results_are_missing(frozen_env, capsys): + e = frozen_env + shutil.rmtree(e.root / "results") + with pytest.raises(SystemExit) as exc: + e.script.freeze() + assert exc.value.code == 2 + assert "does not exist - run `faircode benchmark` first" in capsys.readouterr().err + assert not (e.root / "paper").exists() + + +def test_freeze_writes_nothing_outside_the_intended_directories(frozen_env): + e = frozen_env + before = set(e.tmp.rglob("*")) + e.script.freeze() + new = set(e.tmp.rglob("*")) - before + assert new, "freeze() should have written something" + allowed = (e.root / "paper", e.root / "faircode") + assert all(any(p == a or a in p.parents for a in allowed) for p in new) + + +def test_main_passes_the_tag_option_through(frozen_env, monkeypatch): + e = frozen_env + monkeypatch.setattr(sys, "argv", ["freeze_paper_results.py", "--tag", "v3-paper"]) + e.script.main() + assert "git tag -a v3-paper" in _manifest(e.root) + + +def test_package_version_reports_installed_missing_and_unversioned_modules(): + script = importlib.import_module("scripts.freeze_paper_results") + assert script._package_version("definitely_not_a_real_module_xyz") == "not installed" + assert script._package_version("keyword") == "unknown" # importable, has no __version__ + assert script._package_version("json") == __import__("json").__version__ + + +def test_run_returns_stripped_stdout(): + script = importlib.import_module("scripts.freeze_paper_results") + assert script._run([sys.executable, "-c", "print(' hi ')"]) == "hi" + + +def test_discover_manifests_lists_the_real_audits_sorted(): + script = importlib.import_module("scripts.freeze_paper_results") + found = script._discover_manifests() + assert found == sorted(found) + assert any(path.replace("\\", "/") == "COMPAS/audit.yaml" for path in found)