diff --git a/scripts/check_generated_files_current.py b/scripts/check_generated_files_current.py index 604e1b89..d7713d40 100644 --- a/scripts/check_generated_files_current.py +++ b/scripts/check_generated_files_current.py @@ -51,11 +51,11 @@ script checks the same thing for what's actually committed. This script is self-sufficient: it builds a fresh copy of the site itself -(via `git worktree`, with every currently tracked file's working-tree -content copied on top of the HEAD checkout, so uncommitted edits to an -already-tracked source file are what gets built) and compares the real -working tree against that regeneration - not against what's committed at -HEAD. Editing an explainer's .md without running `make build-explainers` +(via `git worktree`, with every tracked file and every untracked, +non-ignored file's working-tree content copied on top of the HEAD checkout, +so both edits to existing files and brand new files are what gets built) and +compares the real working tree against that regeneration - not against what's +committed at HEAD. Editing an explainer's .md without running `make build-explainers` afterward is exactly the case this now catches; it no longer needs to run after CI's own build_explainers.py/generate_og_images.py steps to be meaningful, though CI still runs it there too. @@ -126,15 +126,11 @@ def _fresh_build_dir(stack): actually on disk right now - not from HEAD. `git worktree add` gives a checkout backed by the same object database/history as ROOT (so git-log-derived dates in build_explainers.py resolve identically to a - regeneration done in ROOT itself), then every currently tracked file's - *working-tree* content is copied on top of that HEAD checkout, so - uncommitted edits to an already-tracked source file are what gets - built - exactly the scenario in the bug report this fixes. A brand new, - never-`git add`-ed file isn't covered by that overlay (there's nothing - to `git ls-files` yet), which matches this script's pre-existing - handling of that case below (falls through to "not produced by a fresh - regeneration", since a file with no source in the fresh build can't - have been generated there). + regeneration done in ROOT itself), then every tracked file's + *working-tree* content and every untracked, non-ignored file are copied + on top of that HEAD checkout, so edits and new explainer files/assets + are all included in the build. Ignored files stay out of the fresh + checkout. `stack` is an ExitStack the caller uses to guarantee the worktree is cleaned up even if a later step raises. @@ -152,7 +148,14 @@ def _fresh_build_dir(stack): tracked = subprocess.run( ["git", "ls-files"], cwd=ROOT, capture_output=True, text=True, check=True ).stdout.splitlines() - for rel in tracked: + untracked = subprocess.run( + ["git", "ls-files", "--others", "--exclude-standard"], + cwd=ROOT, + capture_output=True, + text=True, + check=True, + ).stdout.splitlines() + for rel in tracked + untracked: if not rel.strip(): continue src, dst = ROOT / rel, tmp / rel @@ -167,7 +170,12 @@ def _fresh_build_dir(stack): "from scripts.freeze_paper_results import mirror_for_mcp, mirror_current_for_mcp; " "mirror_for_mcp(); mirror_current_for_mcp()"], ): - subprocess.run(cmd, cwd=tmp, check=True, capture_output=True, text=True) + try: + subprocess.run(cmd, cwd=tmp, check=True, capture_output=True, text=True) + except subprocess.CalledProcessError as exc: + if exc.stderr: + print(exc.stderr, file=sys.stderr, end="") + raise return tmp diff --git a/tests/test_check_generated_files_current.py b/tests/test_check_generated_files_current.py index 690ba11e..bad8d33d 100644 --- a/tests/test_check_generated_files_current.py +++ b/tests/test_check_generated_files_current.py @@ -1,14 +1,17 @@ """Tests for scripts/check_generated_files_current.py. -main() operates on the real repo ROOT/TEXT_GLOBS/DATA_JSON by design (it's a -CI gate, not a library function), so these tests monkeypatch those module -globals to point at an isolated tmp_path tree and fake out the `git` -subprocess calls, rather than standing up a real git repository - the -behavior under test is the comparison/flagging logic itself, not git. +Most tests point main() at an isolated tmp_path tree and fake out the `git` +subprocess calls. A focused temporary Git repository also exercises the +fresh-build copy and builder failure path end to end. """ import importlib import json +import os +import subprocess +from contextlib import ExitStack + +import pytest from PIL import Image @@ -51,6 +54,92 @@ def _empty_repo(tmp_path, monkeypatch, script): monkeypatch.setattr(script, "TEXT_GLOBS", []) +def _git_repo(root, *, failing_builder=False, require_new_assets=False): + root.mkdir() + (root / ".gitignore").write_text("ignored.txt\n", encoding="utf-8") + (root / "tracked.txt").write_text("committed content\n", encoding="utf-8") + scripts = root / "scripts" + scripts.mkdir() + if failing_builder: + builder = "import sys\nprint('builder-specific failure', file=sys.stderr)\nsys.exit(7)\n" + elif require_new_assets: + builder = ( + "from pathlib import Path\n" + "assert Path('explainers/new-explainer.md').is_file()\n" + "assert Path('assets/og/new-explainer.png').is_file()\n" + "assert Path('assets/og-light/new-explainer.png').is_file()\n" + ) + else: + builder = "pass\n" + (scripts / "build_explainers.py").write_text(builder, encoding="utf-8") + (scripts / "generate_og_images.py").write_text("pass\n", encoding="utf-8") + (scripts / "freeze_paper_results.py").write_text( + "def mirror_for_mcp(): pass\ndef mirror_current_for_mcp(): pass\n", + encoding="utf-8", + ) + subprocess.run(["git", "init", "--quiet"], cwd=root, check=True) + subprocess.run(["git", "add", "."], cwd=root, check=True) + env = os.environ | { + "GIT_AUTHOR_NAME": "Fair-Code tests", + "GIT_AUTHOR_EMAIL": "tests@example.invalid", + "GIT_COMMITTER_NAME": "Fair-Code tests", + "GIT_COMMITTER_EMAIL": "tests@example.invalid", + } + subprocess.run( + ["git", "commit", "--quiet", "-m", "test fixture"], + cwd=root, + env=env, + check=True, + ) + return root + + +def test_fresh_build_copies_new_files_but_not_ignored_files_and_keeps_tracked_edits( + tmp_path, monkeypatch +): + script = _script() + root = _git_repo(tmp_path / "repo", require_new_assets=True) + (root / "tracked.txt").write_text("unstaged edit\n", encoding="utf-8") + explainer = root / "explainers" / "new-explainer.md" + explainer.parent.mkdir() + explainer.write_text("# New explainer\n", encoding="utf-8") + for theme in ("og", "og-light"): + image = root / "assets" / theme / "new-explainer.png" + image.parent.mkdir(parents=True) + image.write_bytes(b"new image asset") + (root / "ignored.txt").write_text("must not be copied\n", encoding="utf-8") + monkeypatch.setattr(script, "ROOT", root) + + with ExitStack() as stack: + fresh = script._fresh_build_dir(stack) + + assert (fresh / "tracked.txt").read_text(encoding="utf-8") == "unstaged edit\n" + assert (fresh / "explainers/new-explainer.md").read_text(encoding="utf-8") == ( + "# New explainer\n" + ) + assert ( + fresh / "assets/og/new-explainer.png" + ).read_bytes() == b"new image asset" + assert ( + fresh / "assets/og-light/new-explainer.png" + ).read_bytes() == b"new image asset" + assert not (fresh / "ignored.txt").exists() + + +def test_fresh_build_prints_builder_stderr_and_preserves_failure_status( + tmp_path, monkeypatch, capsys +): + script = _script() + root = _git_repo(tmp_path / "repo", failing_builder=True) + monkeypatch.setattr(script, "ROOT", root) + + with ExitStack() as stack, pytest.raises(subprocess.CalledProcessError) as error: + script._fresh_build_dir(stack) + + assert error.value.returncode == 7 + assert "builder-specific failure" in capsys.readouterr().err + + def test_text_globs_covers_the_results_frozen_mcp_mirror(): # faircode/_results_frozen/*.csv (closes #394) - without this, nothing # ever notices if paper/results-frozen/*.csv is edited directly and the