diff --git a/README.md b/README.md index 8d21184..9ff3a49 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,38 @@ More detail about installation is provided on the (`https://ncas-cms.github.io/cf-plot/installation.html`) of the documentation. +### Refreshing image-test references + +If image tests fail after an environment change, inspect the reference, +generated plot and failure diff before accepting the differences. From the +repository root, generate fresh plots using your testing environment: + +```bash +python -m pytest tests/integration/test_contour_plot_examples.py tests/integration/test_advanced_plot_examples.py +``` + +For a complete refresh, avoid `-k` or `::` selectors: targeted runs preserve +old generated images. The +[reference refresh script](scripts/refresh_image_references.py) previews +updates by default. Select examples by filename suffix (for example, +`gen_fig_16b.png` has ID `16b`): + +```bash +python scripts/refresh_image_references.py 4 5 16b +# After visually approving these generated plots: +python scripts/refresh_image_references.py 4 5 16b --write +``` + +Use `--all` instead of example IDs to preview every available generated +baseline, then `--all --write` only after reviewing all selected plots. +The script validates the selection before writing, replaces existing +references, and removes only their failure-diff images; generated plots +are retained. It does not render or automatically approve plots. + +Rerun the image tests afterward to verify the updated references and clear +pytest's recorded failures. Review and commit the reference-image changes; +do not accept a genuine plotting regression merely to make tests pass. + ### Contributing Everyone is welcome to contribute to cf-plot in the form diff --git a/scripts/refresh_image_references.py b/scripts/refresh_image_references.py new file mode 100644 index 0000000..bd2e930 --- /dev/null +++ b/scripts/refresh_image_references.py @@ -0,0 +1,155 @@ +"""Promote human-reviewed generated plots to test references.""" + +import argparse +import io +import os +import re +import sys +import tempfile +from dataclasses import dataclass +from pathlib import Path + +from PIL import Image + +REPO_ROOT = Path(__file__).resolve().parents[1] +GENERATED_DIRS = ( + "generated-example-images-cont", + "generated-example-images-adv", +) +EXAMPLE_ID = re.compile(r"[A-Za-z0-9_]+") + + +@dataclass(frozen=True) +class ReferenceUpdate: + generated: Path + reference: Path + data: bytes + changed: bool + size: tuple[int, int] + + +def plan_updates( + root: Path, example_ids: list[str], all_generated: bool = False +) -> list[ReferenceUpdate]: + """Validate the entire selection before allowing any file changes.""" + if all_generated: + example_ids = sorted( + { + path.stem.removeprefix("gen_fig_") + for directory in GENERATED_DIRS + for path in (root / directory).glob("gen_fig_*.png") + if EXAMPLE_ID.fullmatch( + path.stem.removeprefix("gen_fig_") + ) + } + ) + if not example_ids: + raise ValueError("No generated examples selected or found.") + + updates = [] + for example_id in dict.fromkeys(example_ids): + if not EXAMPLE_ID.fullmatch(example_id): + raise ValueError(f"Invalid example ID: {example_id!r}") + candidates = [ + root / directory / f"gen_fig_{example_id}.png" + for directory in GENERATED_DIRS + ] + sources = [path for path in candidates if path.exists()] + if len(sources) != 1: + raise ValueError( + f"Example {example_id}: expected one generated image, " + f"found {len(sources)}." + ) + generated = sources[0] + reference = ( + root / "tests" / "reference-example-images" + / f"ref_fig_{example_id}.png" + ) + if generated.is_symlink() or reference.is_symlink(): + raise ValueError(f"Example {example_id}: symlinks not allowed.") + if not reference.is_file(): + raise ValueError(f"Missing existing reference: {reference}") + data = generated.read_bytes() + with Image.open(io.BytesIO(data)) as image: + if image.format != "PNG": + raise ValueError(f"Not a PNG image: {generated}") + image.load() + size = image.size + updates.append( + ReferenceUpdate( + generated, reference, data, + data != reference.read_bytes(), size, + ) + ) + return updates + + +def apply_updates(updates: list[ReferenceUpdate]) -> None: + """Replace selected references and remove only their stale diff images.""" + for update in updates: + if update.changed: + temporary = None + try: + with tempfile.NamedTemporaryFile( + dir=update.reference.parent, suffix=".png", delete=False + ) as output: + temporary = Path(output.name) + output.write(update.data) + temporary.chmod(update.reference.stat().st_mode & 0o777) + os.replace(temporary, update.reference) + finally: + if temporary is not None: + temporary.unlink(missing_ok=True) + diff = update.generated.with_name( + f"{update.generated.stem}-failed-diff.png" + ) + diff.unlink(missing_ok=True) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "examples", nargs="*", help="Reviewed example IDs, e.g. 4 5 16b" + ) + parser.add_argument( + "--all", action="store_true", + help="Select all generated examples, not just current failures", + ) + parser.add_argument( + "--write", action="store_true", + help="Confirm human review and replace references; otherwise preview", + ) + args = parser.parse_args(argv) + if args.all and args.examples: + parser.error("Use either explicit example IDs or --all, not both.") + if not args.all and not args.examples: + parser.error("Select example IDs or use --all.") + try: + updates = plan_updates(REPO_ROOT, args.examples, args.all) + for update in updates: + action = "UPDATE" if update.changed else "UNCHANGED" + print( + f"{action}: {update.generated.relative_to(REPO_ROOT)} " + f"-> {update.reference.relative_to(REPO_ROOT)} " + f"({update.size[0]}x{update.size[1]})" + ) + if args.write: + apply_updates(updates) + print( + "References refreshed; selected failure diffs removed. " + "Rerun pytest to verify and clear its failure cache." + ) + else: + print( + "Preview only; no files changed. Inspect the generated " + "plots, then repeat with --write to confirm approval. " + "Writing also removes the selected failure diffs." + ) + except (OSError, ValueError) as error: + print(f"Reference refresh failed: {error}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/unit/test_refresh_image_references.py b/tests/unit/test_refresh_image_references.py new file mode 100644 index 0000000..6904902 --- /dev/null +++ b/tests/unit/test_refresh_image_references.py @@ -0,0 +1,98 @@ +"""Safety and cleanup checks for the human-approved baseline refresh.""" + +import pytest +from PIL import Image + +from scripts import refresh_image_references as refresh + + +def _example(root, example_id="4", directory=None): + directory = directory or refresh.GENERATED_DIRS[0] + generated = root / directory / f"gen_fig_{example_id}.png" + reference = ( + root / "tests" / "reference-example-images" + / f"ref_fig_{example_id}.png" + ) + generated.parent.mkdir(parents=True, exist_ok=True) + reference.parent.mkdir(parents=True, exist_ok=True) + Image.new("RGB", (10, 10), "white").save(reference) + Image.new("RGB", (10, 10), "red").save(generated) + diff = generated.with_name(f"{generated.stem}-failed-diff.png") + Image.new("RGB", (10, 10), "black").save(diff) + return generated, reference, diff + + +def test_preview_changes_nothing(tmp_path, monkeypatch, capsys): + generated, reference, diff = _example(tmp_path) + original = reference.read_bytes() + monkeypatch.setattr(refresh, "REPO_ROOT", tmp_path) + assert refresh.main(["4"]) == 0 + assert reference.read_bytes() == original + assert generated.exists() and diff.exists() + assert "Preview only" in capsys.readouterr().out + + +def test_write_updates_only_selected_and_cleans_its_diff( + tmp_path, monkeypatch +): + generated, reference, diff = _example(tmp_path) + other_generated, other_reference, other_diff = _example(tmp_path, "5") + original_other = other_reference.read_bytes() + monkeypatch.setattr(refresh, "REPO_ROOT", tmp_path) + assert refresh.main(["4", "--write"]) == 0 + assert reference.read_bytes() == generated.read_bytes() + assert not diff.exists() + assert other_reference.read_bytes() == original_other + assert other_generated.exists() and other_diff.exists() + assert not list(reference.parent.glob("tmp*.png")) + + +def test_all_selects_both_directories_but_not_diffs(tmp_path): + _example(tmp_path, "4") + _example(tmp_path, "16b", refresh.GENERATED_DIRS[1]) + updates = refresh.plan_updates(tmp_path, [], all_generated=True) + assert {update.reference.name for update in updates} == { + "ref_fig_4.png", "ref_fig_16b.png" + } + + +@pytest.mark.parametrize("problem", ["missing", "corrupt", "duplicate"]) +def test_preflight_failure_writes_nothing( + tmp_path, monkeypatch, problem +): + _, reference, diff = _example(tmp_path, "4") + generated, _, _ = _example(tmp_path, "5") + original = reference.read_bytes() + if problem == "missing": + generated.unlink() + elif problem == "corrupt": + generated.write_bytes(b"not an image") + else: + _example(tmp_path, "5", refresh.GENERATED_DIRS[1]) + monkeypatch.setattr(refresh, "REPO_ROOT", tmp_path) + assert refresh.main(["4", "5", "--write"]) == 1 + assert reference.read_bytes() == original + assert diff.exists() + + +def test_missing_reference_is_not_silently_created(tmp_path): + _, reference, _ = _example(tmp_path) + reference.unlink() + with pytest.raises(ValueError, match="Missing existing reference"): + refresh.plan_updates(tmp_path, ["4"]) + + +def test_path_traversal_is_rejected(tmp_path): + with pytest.raises(ValueError, match="Invalid example ID"): + refresh.plan_updates(tmp_path, ["../../outside"]) + + +def test_unchanged_reference_still_cleans_selected_diff(tmp_path): + generated, reference, diff = _example(tmp_path) + reference.write_bytes(generated.read_bytes()) + updates = refresh.plan_updates(tmp_path, ["4", "4"]) + assert len(updates) == 1 + assert not updates[0].changed + refresh.apply_updates(updates) + assert not diff.exists() + assert generated.exists()