From 75139a55410d0a15b77274a224b305960dc1c4e1 Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Thu, 25 Jun 2026 17:33:22 +1000 Subject: [PATCH 1/4] refactor(regression): collapse path-placeholder clump into PlaceholderMap Introduce a frozen PlaceholderMap value object in climate_ref_core.output_files that owns the (token, directory) placeholder set. Config-stable tokens come from for_baseline(); the per-execution is late-bound via with_output(); sanitise()/hydrate()/as_replacements() replace the to_placeholders/from_placeholders/_placeholder_pairs free functions, which are removed. Every caller now passes a single placeholders argument instead of threading output_dir/test_data_dir/software_root_dir through ~8 signatures, and the verification side (RegressionValidator, conftest ExecutionRegression/DiagnosticValidator) derives its replacements from the same map so capture and verification can no longer declare different token sets and drift apart. Behaviour-preserving. --- .../src/climate_ref_core/output_files.py | 132 ++++++++---------- .../climate_ref_core/regression/capture.py | 48 ++----- .../src/climate_ref_core/testing.py | 21 +-- .../tests/unit/regression/test_capture.py | 40 ++++-- .../tests/unit/test_output_files.py | 78 +++++++---- .../src/climate_ref/cli/test_cases/_stages.py | 27 ++-- .../climate_ref/cli/test_cases/baselines.py | 26 ++-- .../src/climate_ref/cli/test_cases/run.py | 6 +- .../src/climate_ref/conftest_plugin.py | 16 +-- .../integration/test_native_roundtrip.py | 9 +- 10 files changed, 211 insertions(+), 192 deletions(-) diff --git a/packages/climate-ref-core/src/climate_ref_core/output_files.py b/packages/climate-ref-core/src/climate_ref_core/output_files.py index 3bbd8a8e3..18a4f42c5 100644 --- a/packages/climate-ref-core/src/climate_ref_core/output_files.py +++ b/packages/climate-ref-core/src/climate_ref_core/output_files.py @@ -15,7 +15,7 @@ Only files in the results directory are accessed by the API/public. For some tests we must sanitise paths to files as well as the contents of text files -(:func:`to_placeholders` / :func:`from_placeholders`). +(:class:`PlaceholderMap`). This ensures that the regression data is machine independent. """ @@ -25,6 +25,8 @@ from pathlib import Path from typing import TYPE_CHECKING +from attrs import frozen + from climate_ref_core.diagnostics import ensure_relative_path from climate_ref_core.logging import EXECUTION_LOG_FILENAME from climate_ref_core.pycmec.output import CMECOutput @@ -109,85 +111,75 @@ def rewrite_tree( file.write_text(rewritten, encoding="utf-8") -def _placeholder_pairs( - output_dir: Path, test_data_dir: Path, software_root_dir: Path | None -) -> list[tuple[str, Path]]: - """Return the ``(placeholder, absolute directory)`` pairs sanitised in committed artefacts. - - Shared by :func:`to_placeholders` and :func:`from_placeholders` so the placeholder set is - declared once and each picks a substitution direction. +@frozen +class PlaceholderMap: """ - pairs = [(PLACEHOLDER_OUTPUT_DIR, output_dir), (PLACEHOLDER_TEST_DATA_DIR, test_data_dir)] - if software_root_dir is not None: - pairs.append((PLACEHOLDER_SOFTWARE_ROOT_DIR, software_root_dir)) - return pairs + Bidirectional map between absolute runtime directories and portable ```` placeholders. + A committed regression bundle is made machine-independent by replacing host-specific absolute + paths with stable tokens -- ````, ```` and (optionally) + ````. The *same* map drives every direction: -def to_placeholders( - directory: Path, - *, - output_dir: Path, - test_data_dir: Path, - software_root_dir: Path | None = None, - globs: tuple[str, ...] = SANITISED_FILE_GLOBS, -) -> None: - """ - Rewrite absolute paths in committed artefacts to portable placeholders ("to"). + - :meth:`sanitise` rewrites absolute paths to tokens (capture / mint). + - :meth:`hydrate` rewrites tokens back to absolute paths (replay / rebuild). + - :meth:`as_replacements` yields the ``{absolute: token}`` mapping the bundle and series + comparators apply to a freshly regenerated artefact before diffing. - Replaces the absolute ``output_dir`` with ````, the absolute - ``test_data_dir`` with ````, and (when given) the absolute - ``software_root_dir`` with ```` in every text artefact under ``directory``. - Binary files are never touched. + The token set is declared in one place (:meth:`for_baseline` then :meth:`with_output`), so the + capture side and the verification side cannot declare different sets and drift apart. Adding a + placeholder is a one-line change here, not a new parameter threaded through every caller. - Parameters - ---------- - directory - The tree of committed artefacts to sanitise in place. - output_dir - The absolute execution output directory. - test_data_dir - The absolute provider test-data directory. - software_root_dir - The absolute shared-software root directory, if any. Substituted in provenance - command lines so the committed bundle stays machine-independent. - globs - File globs whose contents are rewritten. + The two configuration-stable tokens (```` / ````) are fixed for + a whole run; the per-execution ```` is late-bound with :meth:`with_output`. """ - pairs = _placeholder_pairs(output_dir, test_data_dir, software_root_dir) - rewrite_tree(directory, {str(abs_dir): placeholder for placeholder, abs_dir in pairs}, globs) + pairs: tuple[tuple[str, Path], ...] + """Ordered ``(token, absolute_directory)`` pairs. -def from_placeholders( - directory: Path, - *, - output_dir: Path, - test_data_dir: Path, - software_root_dir: Path | None = None, - globs: tuple[str, ...] = SANITISED_FILE_GLOBS, -) -> None: + Application order is not significant: all rewriting goes through :func:`rewrite_tree`, which + re-sorts longest-match-first so an overlapping shorter path cannot shadow a longer one. """ - Rewrite portable placeholders back to absolute paths ("from"). - - Inverse of :func:`to_placeholders`: replaces ```` with the absolute ``output_dir``, - ```` with the absolute ``test_data_dir``, and (when given) ```` - with the absolute ``software_root_dir`` in every text artefact under ``directory``. - Binary files are never touched. - Parameters - ---------- - directory - The tree of artefacts to hydrate in place. - output_dir - The absolute execution output directory to substitute in. - test_data_dir - The absolute provider test-data directory to substitute in. - software_root_dir - The absolute shared-software root directory to substitute in, if any. - globs - File globs whose contents are rewritten. - """ - pairs = _placeholder_pairs(output_dir, test_data_dir, software_root_dir) - rewrite_tree(directory, {placeholder: str(abs_dir) for placeholder, abs_dir in pairs}, globs) + @classmethod + def for_baseline(cls, *, test_data_dir: Path, software_root_dir: Path | None = None) -> PlaceholderMap: + """ + Build the configuration-stable placeholder set for a committed baseline. + + Holds every token except the per-execution output directory, which is added with + :meth:`with_output`. ``software_root_dir`` is optional: when ``None`` no + ```` substitution is applied -- a verification context that does not know + the shared-software root relies on this, and the omission is explicit and declared once. + """ + pairs: list[tuple[str, Path]] = [(PLACEHOLDER_TEST_DATA_DIR, test_data_dir)] + if software_root_dir is not None: + pairs.append((PLACEHOLDER_SOFTWARE_ROOT_DIR, software_root_dir)) + return cls(pairs=tuple(pairs)) + + def with_output(self, output_dir: Path) -> PlaceholderMap: + """Return a new map that also binds ```` to the per-execution ``output_dir``.""" + return PlaceholderMap(pairs=((PLACEHOLDER_OUTPUT_DIR, output_dir), *self.pairs)) + + def as_replacements(self) -> dict[str, str]: + """Return the ``{absolute_directory: token}`` mapping (real path -> placeholder). + + This is what the bundle and series comparators apply to a regenerated artefact before + diffing it against the committed (already-placeholdered) baseline. + """ + return {str(abs_dir): token for token, abs_dir in self.pairs} + + def sanitise(self, directory: Path, globs: tuple[str, ...] = SANITISED_FILE_GLOBS) -> None: + """Rewrite absolute paths to ```` placeholders in every text artefact under ``directory``. + + Binary files are never touched. In-place. + """ + rewrite_tree(directory, self.as_replacements(), globs) + + def hydrate(self, directory: Path, globs: tuple[str, ...] = SANITISED_FILE_GLOBS) -> None: + """Inverse of :meth:`sanitise`: rewrite ```` placeholders back to absolute paths. + + Binary files are never touched. In-place. + """ + rewrite_tree(directory, {token: str(abs_dir) for token, abs_dir in self.pairs}, globs) def copy_output_file( diff --git a/packages/climate-ref-core/src/climate_ref_core/regression/capture.py b/packages/climate-ref-core/src/climate_ref_core/regression/capture.py index 132ad47bf..17e739a18 100644 --- a/packages/climate-ref-core/src/climate_ref_core/regression/capture.py +++ b/packages/climate-ref-core/src/climate_ref_core/regression/capture.py @@ -27,7 +27,7 @@ from loguru import logger -from climate_ref_core.output_files import copy_execution_outputs, to_placeholders +from climate_ref_core.output_files import PlaceholderMap, copy_execution_outputs from climate_ref_core.paths import safe_path from ._quantise import round_floats @@ -198,16 +198,14 @@ def write_committed_bundle( source_dir: Path, regression_dir: Path, *, - output_dir: Path, - test_data_dir: Path, - software_root_dir: Path | None = None, + placeholders: PlaceholderMap, ) -> dict[str, str]: """ Write the sanitised committed CMEC bundle into ``regression_dir``. Copies each committed artefact present in ``source_dir`` into ``regression_dir``, then rewrites absolute paths to portable placeholders in place - (:func:`~climate_ref_core.output_files.to_placeholders`), + (:meth:`~climate_ref_core.output_files.PlaceholderMap.sanitise`), rounds floats (:func:`_round_committed_floats`), and redacts host/user-specific CMEC provenance fields (:func:`_redact_committed_provenance`). When a committed artefact is absent from ``source_dir``, @@ -220,13 +218,10 @@ def write_committed_bundle( results directory). regression_dir The destination ``regression/`` directory (created if needed). - output_dir - The absolute execution output directory, for path substitution. - test_data_dir - The absolute provider test-data directory, for path substitution. - software_root_dir - The absolute shared-software root directory, for path substitution in provenance - command lines. When ``None``, no software-root substitution is applied. + placeholders + The placeholder map for this execution, already bound to the output directory via + :meth:`~climate_ref_core.output_files.PlaceholderMap.with_output`. Its absolute paths are + rewritten to portable ```` placeholders in the copied bundle. Returns ------- @@ -245,12 +240,7 @@ def write_committed_bundle( # Drop a stale copy from a previous capture so it is not re-digested. dest.unlink(missing_ok=True) - to_placeholders( - regression_dir, - output_dir=output_dir, - test_data_dir=test_data_dir, - software_root_dir=software_root_dir, - ) + placeholders.sanitise(regression_dir) # Round floats in place before digesting, # so the committed bytes (and their recorded digests) are the stable, rounded ones. # Placeholder substitution only rewrites path strings, so order relative to it does not matter for floats. @@ -292,9 +282,7 @@ def capture_execution( # noqa: PLR0913 result: ExecutionResult, *, regression_dir: Path, - output_dir: Path, - test_data_dir: Path, - software_root_dir: Path | None = None, + placeholders: PlaceholderMap, # TODO: Unify the log handling include_log: bool = False, ) -> tuple[dict[str, str], dict[str, NativeEntry]]: @@ -318,13 +306,9 @@ def capture_execution( # noqa: PLR0913 The successful execution result (must carry a metric bundle filename). regression_dir The test case ``regression/`` directory for the committed bundle. - output_dir - The absolute execution output directory, for path substitution. - test_data_dir - The absolute provider test-data directory, for path substitution. - software_root_dir - The absolute shared-software root directory, for path substitution in provenance - command lines. When ``None``, no software-root substitution is applied. + placeholders + The placeholder map for this execution, already bound to the output directory via + :meth:`~climate_ref_core.output_files.PlaceholderMap.with_output`. include_log If True, the execution log is included in the persisted/native set. @@ -344,13 +328,7 @@ def capture_execution( # noqa: PLR0913 include_log=include_log, ) base_dir = results_directory / fragment - committed = write_committed_bundle( - base_dir, - regression_dir, - output_dir=output_dir, - test_data_dir=test_data_dir, - software_root_dir=software_root_dir, - ) + committed = write_committed_bundle(base_dir, regression_dir, placeholders=placeholders) native = build_native_snapshot(base_dir, relpaths) return committed, native diff --git a/packages/climate-ref-core/src/climate_ref_core/testing.py b/packages/climate-ref-core/src/climate_ref_core/testing.py index af884c9c7..1d1be5da0 100644 --- a/packages/climate-ref-core/src/climate_ref_core/testing.py +++ b/packages/climate-ref-core/src/climate_ref_core/testing.py @@ -30,7 +30,7 @@ from climate_ref_core.diagnostics import ExecutionDefinition, ExecutionResult from climate_ref_core.esgf.base import ESGFRequest from climate_ref_core.metric_values.typing import SeriesMetricValue -from climate_ref_core.output_files import from_placeholders, ordered_replacements +from climate_ref_core.output_files import PlaceholderMap, ordered_replacements from climate_ref_core.paths import safe_path from climate_ref_core.pycmec.metric import CMECMetric from climate_ref_core.pycmec.output import CMECOutput @@ -567,8 +567,10 @@ def validate_cmec_bundles(diagnostic: Diagnostic, result: ExecutionResult) -> No # structurally validated here, so a diagnostic can change the metric/output # values without any regression test failing (see issue #703 for the series # equivalent). A content comparison must sanitise the regenerated bundle first - # via `ExecutionRegression.output_replacements`, since the committed bundles - # store `` / `` placeholders. + # via `PlaceholderMap.for_baseline(...).with_output(...).as_replacements()`, since the + # committed bundles store `` / `` / `` + # placeholders. diagnostic.json provenance carries the software root, so building the + # replacements from the map (rather than a hand-rolled dict) keeps this in step with capture. assert result.successful, f"Execution failed: {result}" # Validate metric bundle @@ -701,7 +703,11 @@ def load_regression_definition(self, tmp_dir: Path) -> ExecutionDefinition: output_dir.mkdir(parents=True, exist_ok=True) shutil.copytree(regression_path, output_dir, dirs_exist_ok=True) - from_placeholders(output_dir, output_dir=output_dir, test_data_dir=self.test_data_dir) + # This verification context does not know the shared-software root, so the map omits + # ; the omission is declared once on the map rather than open-coded here. + PlaceholderMap.for_baseline(test_data_dir=self.test_data_dir).with_output(output_dir).hydrate( + output_dir + ) datasets: ExecutionDatasetCollection = load_datasets_from_yaml(catalog_path) @@ -722,10 +728,9 @@ def validate(self, definition: ExecutionDefinition) -> None: expected_path=self.paths.regression / "series.json", actual_path=definition.output_directory / "series.json", slug=self.diagnostic.slug, - replacements={ - str(definition.output_directory): "", - str(self.test_data_dir): "", - }, + replacements=PlaceholderMap.for_baseline(test_data_dir=self.test_data_dir) + .with_output(definition.output_directory) + .as_replacements(), ) diff --git a/packages/climate-ref-core/tests/unit/regression/test_capture.py b/packages/climate-ref-core/tests/unit/regression/test_capture.py index c02e544ab..9f5a95d34 100644 --- a/packages/climate-ref-core/tests/unit/regression/test_capture.py +++ b/packages/climate-ref-core/tests/unit/regression/test_capture.py @@ -10,6 +10,7 @@ PLACEHOLDER_OUTPUT_DIR, PLACEHOLDER_SOFTWARE_ROOT_DIR, PLACEHOLDER_TEST_DATA_DIR, + PlaceholderMap, ) from climate_ref_core.pycmec.output import CMECOutput from climate_ref_core.regression.capture import ( @@ -54,7 +55,9 @@ def test_write_committed_bundle_sanitises_and_digests(tmp_path): regression_dir = tmp_path / "regression" digests = write_committed_bundle( - source, regression_dir, output_dir=output_dir, test_data_dir=test_data_dir + source, + regression_dir, + placeholders=PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir), ) assert set(digests) == {"series.json", "diagnostic.json", "output.json"} @@ -102,7 +105,11 @@ def test_write_committed_bundle_rounds_floats(tmp_path): (source / "series.json").write_text(json.dumps(source_series)) regression_dir = tmp_path / "regression" - write_committed_bundle(source, regression_dir, output_dir=output_dir, test_data_dir=test_data_dir) + write_committed_bundle( + source, + regression_dir, + placeholders=PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir), + ) diag = json.loads((regression_dir / "diagnostic.json").read_text()) series = json.loads((regression_dir / "series.json").read_text()) @@ -164,7 +171,11 @@ def test_write_committed_bundle_leaves_output_json_bytes_unchanged(tmp_path): ) regression_dir = tmp_path / "regression" - write_committed_bundle(source, regression_dir, output_dir=output_dir, test_data_dir=test_data_dir) + write_committed_bundle( + source, + regression_dir, + placeholders=PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir), + ) # output.json is byte-identical to the copied source: not re-dumped, keys not reordered. assert (regression_dir / "output.json").read_bytes() == source_output_bytes @@ -209,9 +220,9 @@ def test_write_committed_bundle_redacts_and_placeholders_provenance(tmp_path): digests = write_committed_bundle( source, regression_dir, - output_dir=output_dir, - test_data_dir=test_data_dir, - software_root_dir=software_root, + placeholders=PlaceholderMap.for_baseline( + test_data_dir=test_data_dir, software_root_dir=software_root + ).with_output(output_dir), ) for filename in ("diagnostic.json", "output.json"): @@ -257,7 +268,11 @@ def test_redaction_is_noop_without_provenance_fields(tmp_path): (source / "series.json").write_text(json.dumps([])) regression_dir = tmp_path / "regression" - write_committed_bundle(source, regression_dir, output_dir=output_dir, test_data_dir=test_data_dir) + write_committed_bundle( + source, + regression_dir, + placeholders=PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir), + ) assert (regression_dir / "output.json").read_bytes() == source_output_bytes @@ -297,8 +312,7 @@ def test_capture_execution_end_to_end(tmp_path): fragment, result, regression_dir=regression_dir, - output_dir=output_dir, - test_data_dir=test_data_dir, + placeholders=PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir), ) # Committed bundle is the three CMEC artefacts. @@ -331,8 +345,7 @@ def test_capture_execution_include_log(tmp_path): fragment, result, regression_dir=tmp_path / "regression", - output_dir=output_dir, - test_data_dir=test_data_dir, + placeholders=PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir), include_log=True, ) @@ -348,8 +361,9 @@ def test_capture_execution_requires_metric_bundle(tmp_path): "frag", result, regression_dir=tmp_path / "regression", - output_dir=tmp_path / "out", - test_data_dir=tmp_path / "td", + placeholders=PlaceholderMap.for_baseline(test_data_dir=tmp_path / "td").with_output( + tmp_path / "out" + ), ) diff --git a/packages/climate-ref-core/tests/unit/test_output_files.py b/packages/climate-ref-core/tests/unit/test_output_files.py index 875d68b34..32b1af691 100644 --- a/packages/climate-ref-core/tests/unit/test_output_files.py +++ b/packages/climate-ref-core/tests/unit/test_output_files.py @@ -7,10 +7,9 @@ PLACEHOLDER_OUTPUT_DIR, PLACEHOLDER_SOFTWARE_ROOT_DIR, PLACEHOLDER_TEST_DATA_DIR, + PlaceholderMap, copy_execution_outputs, copy_output_file, - from_placeholders, - to_placeholders, ) from climate_ref_core.pycmec.output import CMECOutput @@ -21,17 +20,18 @@ def test_sanitise_and_expand_round_trip(tmp_path): directory = tmp_path / "regression" directory.mkdir() + placeholders = PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir) original = f"path={output_dir}\ndata={test_data_dir}\n" (directory / "diagnostic.json").write_text(original) - to_placeholders(directory, output_dir=output_dir, test_data_dir=test_data_dir) + placeholders.sanitise(directory) sanitised = (directory / "diagnostic.json").read_text() assert str(output_dir) not in sanitised assert str(test_data_dir) not in sanitised assert PLACEHOLDER_OUTPUT_DIR in sanitised assert PLACEHOLDER_TEST_DATA_DIR in sanitised - from_placeholders(directory, output_dir=output_dir, test_data_dir=test_data_dir) + placeholders.hydrate(directory) assert (directory / "diagnostic.json").read_text() == original @@ -42,27 +42,20 @@ def test_sanitise_software_root_round_trip(tmp_path): directory = tmp_path / "regression" directory.mkdir() + placeholders = PlaceholderMap.for_baseline( + test_data_dir=test_data_dir, software_root_dir=software_root_dir + ).with_output(output_dir) # A provenance command line referencing the software root and the output dir. original = f"cmd={software_root_dir}/conda/bin/driver.py --out {output_dir}\n" (directory / "diagnostic.json").write_text(original) - to_placeholders( - directory, - output_dir=output_dir, - test_data_dir=test_data_dir, - software_root_dir=software_root_dir, - ) + placeholders.sanitise(directory) sanitised = (directory / "diagnostic.json").read_text() assert str(software_root_dir) not in sanitised assert PLACEHOLDER_SOFTWARE_ROOT_DIR in sanitised assert PLACEHOLDER_OUTPUT_DIR in sanitised - from_placeholders( - directory, - output_dir=output_dir, - test_data_dir=test_data_dir, - software_root_dir=software_root_dir, - ) + placeholders.hydrate(directory) assert (directory / "diagnostic.json").read_text() == original @@ -74,7 +67,9 @@ def test_software_root_substitution_is_opt_in(tmp_path): original = f"cmd={software_root_dir}/conda/bin/driver.py\n" (directory / "diagnostic.json").write_text(original) - to_placeholders(directory, output_dir=tmp_path / "out", test_data_dir=tmp_path / "td") + PlaceholderMap.for_baseline(test_data_dir=tmp_path / "td").with_output(tmp_path / "out").sanitise( + directory + ) assert (directory / "diagnostic.json").read_text() == original @@ -89,7 +84,7 @@ def test_sanitise_only_touches_text_globs(tmp_path): (directory / "data.nc").write_bytes(binary) (directory / "meta.json").write_text(str(output_dir)) - to_placeholders(directory, output_dir=output_dir, test_data_dir=test_data_dir) + PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir).sanitise(directory) # Binary file is untouched; JSON is rewritten. assert (directory / "data.nc").read_bytes() == binary @@ -104,7 +99,7 @@ def test_sanitise_longest_match_first(tmp_path): directory.mkdir() (directory / "a.json").write_text(str(output_dir)) - to_placeholders(directory, output_dir=output_dir, test_data_dir=test_data_dir) + PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir).sanitise(directory) assert (directory / "a.json").read_text() == PLACEHOLDER_OUTPUT_DIR @@ -117,7 +112,7 @@ def test_sanitise_covers_all_text_globs(suffix, tmp_path): directory.mkdir() (directory / f"artefact.{suffix}").write_text(str(output_dir)) - to_placeholders(directory, output_dir=output_dir, test_data_dir=test_data_dir) + PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir).sanitise(directory) assert (directory / f"artefact.{suffix}").read_text() == PLACEHOLDER_OUTPUT_DIR @@ -130,11 +125,8 @@ def test_sanitise_respects_custom_globs(tmp_path): (directory / "config.cfg").write_text(str(output_dir)) (directory / "skip.json").write_text(str(output_dir)) - to_placeholders( - directory, - output_dir=output_dir, - test_data_dir=test_data_dir, - globs=("*.cfg",), + PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir).sanitise( + directory, globs=("*.cfg",) ) # Only the custom glob is rewritten; the default JSON glob is left untouched. @@ -154,12 +146,46 @@ def test_sanitise_leaves_unmatched_content_untouched(tmp_path): target.write_text(original) before = target.stat().st_mtime_ns - to_placeholders(directory, output_dir=output_dir, test_data_dir=test_data_dir) + PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir).sanitise(directory) assert target.read_text() == original assert target.stat().st_mtime_ns == before +def test_placeholder_map_as_replacements_maps_real_to_token(tmp_path): + output_dir = tmp_path / "out" + test_data_dir = tmp_path / "td" + software_root_dir = tmp_path / "sw" + + placeholders = PlaceholderMap.for_baseline( + test_data_dir=test_data_dir, software_root_dir=software_root_dir + ).with_output(output_dir) + + assert placeholders.as_replacements() == { + str(output_dir): PLACEHOLDER_OUTPUT_DIR, + str(test_data_dir): PLACEHOLDER_TEST_DATA_DIR, + str(software_root_dir): PLACEHOLDER_SOFTWARE_ROOT_DIR, + } + + +def test_placeholder_map_for_baseline_omits_software_root_when_absent(tmp_path): + # The optional software root is declared once on the map; an absent root is an explicit omission. + tokens = {token for token, _ in PlaceholderMap.for_baseline(test_data_dir=tmp_path / "td").pairs} + + assert PLACEHOLDER_TEST_DATA_DIR in tokens + assert PLACEHOLDER_SOFTWARE_ROOT_DIR not in tokens + assert PLACEHOLDER_OUTPUT_DIR not in tokens # only added by with_output + + +def test_placeholder_map_with_output_returns_a_new_map(tmp_path): + # Frozen value object: with_output does not mutate the base map. + base = PlaceholderMap.for_baseline(test_data_dir=tmp_path / "td") + bound = base.with_output(tmp_path / "out") + + assert PLACEHOLDER_OUTPUT_DIR not in {token for token, _ in base.pairs} + assert PLACEHOLDER_OUTPUT_DIR in {token for token, _ in bound.pairs} + + @pytest.mark.parametrize("filename", ("bundle.zip", "nested/bundle.zip")) def test_copy_output_file_returns_relpath(filename, tmp_path): scratch = (tmp_path / "scratch").resolve() diff --git a/packages/climate-ref/src/climate_ref/cli/test_cases/_stages.py b/packages/climate-ref/src/climate_ref/cli/test_cases/_stages.py index 9ffab820a..10748bf0f 100644 --- a/packages/climate-ref/src/climate_ref/cli/test_cases/_stages.py +++ b/packages/climate-ref/src/climate_ref/cli/test_cases/_stages.py @@ -29,7 +29,7 @@ from loguru import logger from climate_ref_core.diagnostics import ExecutionDefinition -from climate_ref_core.output_files import copy_execution_outputs, from_placeholders +from climate_ref_core.output_files import PlaceholderMap, copy_execution_outputs from climate_ref_core.regression import ( COMMITTED_BUNDLE_FILES, Tolerance, @@ -136,13 +136,11 @@ def stage_materialise( # noqa: PLR0913 manifest: Manifest, store: NativeStore, slot: Path, - software_root_dir: Path | None = None, + placeholders: PlaceholderMap, ) -> SourceOutputs: """Fetch the manifest's native blobs into ``slot`` and rebuild the result from them.""" materialise_native(manifest.native, store, slot) - return stage_rebuild_from_slot( - diag=diag, tc=tc, paths=paths, slot=slot, software_root_dir=software_root_dir - ) + return stage_rebuild_from_slot(diag=diag, tc=tc, paths=paths, slot=slot, placeholders=placeholders) def stage_rebuild_from_slot( @@ -151,7 +149,7 @@ def stage_rebuild_from_slot( tc: TestCase, paths: TestCasePaths, slot: Path, - software_root_dir: Path | None = None, + placeholders: PlaceholderMap, ) -> SourceOutputs: """ Rebuild the execution result from native already present in ``slot``. @@ -159,12 +157,13 @@ def stage_rebuild_from_slot( Hydrates portable placeholders to concrete paths, then re-runs ``build_execution_result`` so the rebuilt bundle is written into the slot (referencing the slot). No execution and no store access -- this is the shared core of ``replay`` (after a fetch) and ``build``. + + The slot is its own output directory, so the placeholder map is bound to it + (``placeholders.with_output(slot)``) before hydrating. """ from climate_ref_core.testing import load_datasets_from_yaml - from_placeholders( - slot, output_dir=slot, test_data_dir=paths.test_data_dir, software_root_dir=software_root_dir - ) + placeholders.with_output(slot).hydrate(slot) datasets = load_datasets_from_yaml(paths.catalog) definition = ExecutionDefinition( diagnostic=diag, @@ -181,8 +180,7 @@ def stage_build( *, slot: Path, source: SourceOutputs, - paths: TestCasePaths, - software_root_dir: Path | None = None, + placeholders: PlaceholderMap, ) -> dict[str, str]: """ Assemble the committed bundle into the slot's ``regression/`` directory. @@ -190,13 +188,14 @@ def stage_build( Returns the committed digests ``{filename: sha256}`` of the sanitised, float-quantised bytes just written -- suitable for ``Manifest.committed`` and identical to what would be promoted to the tracked baseline. + + The rebuilt bundle's text still references ``source.bundle_output_dir``, so the placeholder + map is bound to it before sanitising. """ return write_committed_bundle( slot, slot / SLOT_REGRESSION_DIRNAME, - output_dir=source.bundle_output_dir, - test_data_dir=paths.test_data_dir, - software_root_dir=software_root_dir, + placeholders=placeholders.with_output(source.bundle_output_dir), ) diff --git a/packages/climate-ref/src/climate_ref/cli/test_cases/baselines.py b/packages/climate-ref/src/climate_ref/cli/test_cases/baselines.py index 85426a65e..14db0b8b0 100644 --- a/packages/climate-ref/src/climate_ref/cli/test_cases/baselines.py +++ b/packages/climate-ref/src/climate_ref/cli/test_cases/baselines.py @@ -39,6 +39,7 @@ write_source_stamp, ) from climate_ref.config import Config +from climate_ref_core.output_files import PlaceholderMap if TYPE_CHECKING: from rich.console import Console @@ -143,6 +144,9 @@ def replay_test_case( # noqa: PLR0912, PLR0915 continue slot = prepare_slot(paths, label) + placeholders = PlaceholderMap.for_baseline( + test_data_dir=paths.test_data_dir, software_root_dir=config.paths.software + ) try: source = stage_materialise( diag=diag, @@ -151,14 +155,14 @@ def replay_test_case( # noqa: PLR0912, PLR0915 manifest=manifest, store=store, slot=slot, - software_root_dir=config.paths.software, + placeholders=placeholders, ) except Exception as exc: logger.error(f"{case_id}: failed to materialise/rebuild native: {exc}") failures.append(case_id) continue - stage_build(slot=slot, source=source, paths=paths, software_root_dir=config.paths.software) + stage_build(slot=slot, source=source, placeholders=placeholders) cmp_failures, compared = stage_compare( slot=slot, paths=paths, slug=diag.slug, expected=manifest.committed ) @@ -318,6 +322,9 @@ def mint_native( # noqa: PLR0912, PLR0913, PLR0915 continue slot = prepare_slot(paths, label) + placeholders = PlaceholderMap.for_baseline( + test_data_dir=paths.test_data_dir, software_root_dir=config.paths.software + ) # Populate the slot's native set: either re-execute the diagnostic, or (with # --from-replay) materialise the previously minted native from the store. The @@ -331,7 +338,7 @@ def mint_native( # noqa: PLR0912, PLR0913, PLR0915 manifest=previous, store=store, slot=slot, - software_root_dir=config.paths.software, + placeholders=placeholders, ) source_kind = "materialise" else: @@ -355,9 +362,7 @@ def mint_native( # noqa: PLR0912, PLR0913, PLR0915 failures.append(case_id) continue - committed = stage_build( - slot=slot, source=source, paths=paths, software_root_dir=config.paths.software - ) + committed = stage_build(slot=slot, source=source, placeholders=placeholders) if from_replay and previous is not None: # --from-replay reuses the already-minted native verbatim: stage_materialise hydrated # the slot's copy in place (placeholders -> concrete paths) while rebuilding, so a fresh @@ -475,18 +480,19 @@ def build_test_case( # noqa: PLR0912, PLR0913, PLR0915 failures.append(case_id) continue + placeholders = PlaceholderMap.for_baseline( + test_data_dir=paths.test_data_dir, software_root_dir=config.paths.software + ) try: source = stage_rebuild_from_slot( - diag=diag, tc=tc, paths=paths, slot=slot, software_root_dir=config.paths.software + diag=diag, tc=tc, paths=paths, slot=slot, placeholders=placeholders ) except Exception as exc: logger.error(f"{case_id}: failed to rebuild bundle from slot: {exc}") failures.append(case_id) continue - committed = stage_build( - slot=slot, source=source, paths=paths, software_root_dir=config.paths.software - ) + committed = stage_build(slot=slot, source=source, placeholders=placeholders) previous = Manifest.load(paths.manifest) if paths.manifest.exists() else None version = previous.test_case_version if previous else 1 diff --git a/packages/climate-ref/src/climate_ref/cli/test_cases/run.py b/packages/climate-ref/src/climate_ref/cli/test_cases/run.py index 359f55fba..ea5fdc43b 100644 --- a/packages/climate-ref/src/climate_ref/cli/test_cases/run.py +++ b/packages/climate-ref/src/climate_ref/cli/test_cases/run.py @@ -41,6 +41,7 @@ NoTestDataSpecError, TestCaseNotFoundError, ) +from climate_ref_core.output_files import PlaceholderMap if TYPE_CHECKING: from rich.console import Console @@ -209,7 +210,10 @@ def _run_single_test_case( # noqa: PLR0911, PLR0912, PLR0913, PLR0915 # Rebuild the slot's committed bundle, then decide whether to promote it to the # tracked baseline. The native block is mint-owned, so a run preserves the previous # one (or seeds an empty set) and never authors native here. - committed = stage_build(slot=slot, source=source, paths=paths, software_root_dir=config.paths.software) + placeholders = PlaceholderMap.for_baseline( + test_data_dir=paths.test_data_dir, software_root_dir=config.paths.software + ) + committed = stage_build(slot=slot, source=source, placeholders=placeholders) previous = Manifest.load(paths.manifest) if paths.manifest.exists() else None version = previous.test_case_version if previous else 1 diff --git a/packages/climate-ref/src/climate_ref/conftest_plugin.py b/packages/climate-ref/src/climate_ref/conftest_plugin.py index a213ccf92..e6a84dd6b 100644 --- a/packages/climate-ref/src/climate_ref/conftest_plugin.py +++ b/packages/climate-ref/src/climate_ref/conftest_plugin.py @@ -61,7 +61,7 @@ from climate_ref_core.diagnostics import DataRequirement, Diagnostic, ExecutionDefinition, ExecutionResult from climate_ref_core.exceptions import TestCaseError from climate_ref_core.logging import add_log_handler, remove_log_handler -from climate_ref_core.output_files import SANITISED_FILE_GLOBS, rewrite_tree +from climate_ref_core.output_files import SANITISED_FILE_GLOBS, PlaceholderMap, rewrite_tree from climate_ref_core.providers import DiagnosticProvider from climate_ref_core.testing import validate_series_regression @@ -434,7 +434,7 @@ class ExecutionRegression: diagnostic: Diagnostic regression_data_dir: Path request: pytest.FixtureRequest - replacements: dict[str, str] + placeholders: PlaceholderMap def path(self, key: str) -> Path: """Return the regression data path for the given key.""" @@ -450,7 +450,7 @@ def hydrate_output_directory(self, output_dir: Path, replacements: dict[str, str def output_replacements(self, output_directory: Path) -> dict[str, str]: """Map real paths to regression placeholders for a given output directory.""" - return {str(output_directory): "", **self.replacements} + return self.placeholders.with_output(output_directory).as_replacements() def check(self, key: str, output_directory: Path) -> None: """Check and optionally regenerate regression data.""" @@ -476,7 +476,7 @@ def _regression(diagnostic: Diagnostic) -> ExecutionRegression: diagnostic=diagnostic, regression_data_dir=regression_data_dir, request=request, - replacements={str(test_data_dir): ""}, + placeholders=PlaceholderMap.for_baseline(test_data_dir=test_data_dir), ) return _regression @@ -513,12 +513,8 @@ def get_regression_definition(self) -> ExecutionDefinition: regression_output_dir = self.execution_regression.path(definition.key) definition.output_directory.mkdir(parents=True, exist_ok=True) shutil.copytree(regression_output_dir, definition.output_directory, dirs_exist_ok=True) - self.execution_regression.replace_references( - definition.output_directory, - { - "": str(definition.output_directory), - **{value: key for key, value in self.execution_regression.replacements.items()}, - }, + self.execution_regression.placeholders.with_output(definition.output_directory).hydrate( + definition.output_directory ) return definition diff --git a/packages/climate-ref/tests/integration/test_native_roundtrip.py b/packages/climate-ref/tests/integration/test_native_roundtrip.py index 5f864ec3a..d29ae0727 100644 --- a/packages/climate-ref/tests/integration/test_native_roundtrip.py +++ b/packages/climate-ref/tests/integration/test_native_roundtrip.py @@ -41,7 +41,7 @@ ExecutionDefinition, ExecutionResult, ) -from climate_ref_core.output_files import from_placeholders +from climate_ref_core.output_files import PlaceholderMap from climate_ref_core.providers import DiagnosticProvider from climate_ref_core.pycmec.metric import CMECMetric from climate_ref_core.pycmec.output import CMECOutput, OutputCV @@ -306,8 +306,7 @@ def _capture_synthetic( fragment, result, regression_dir=regression_dir, - output_dir=output_dir, - test_data_dir=test_data_dir, + placeholders=PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(output_dir), ) @@ -392,7 +391,7 @@ def test_run_mint_sync_replay(self, tmp_path: Path) -> None: replay_dir = tmp_path / "replay_output" replay_dir.mkdir() materialise_native(reloaded.native, store, replay_dir) - from_placeholders(replay_dir, output_dir=replay_dir, test_data_dir=test_data_dir) + PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(replay_dir).hydrate(replay_dir) replay_definition = ExecutionDefinition( diagnostic=definition.diagnostic, @@ -592,7 +591,7 @@ def test_example_roundtrip(self, tmp_path: Path) -> None: # noqa: PLR0915 replay_dir = tmp_path / "example_replay" replay_dir.mkdir() materialise_native(manifest.native, store, replay_dir) - from_placeholders(replay_dir, output_dir=replay_dir, test_data_dir=test_data_dir) + PlaceholderMap.for_baseline(test_data_dir=test_data_dir).with_output(replay_dir).hydrate(replay_dir) replay_definition = ExecutionDefinition( diagnostic=diag, From 4e11267a12c836daf02ae8dd44c55266803de27a Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Thu, 25 Jun 2026 17:35:38 +1000 Subject: [PATCH 2/4] docs: add changelog fragment for #759 --- changelog/759.trivial.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/759.trivial.md diff --git a/changelog/759.trivial.md b/changelog/759.trivial.md new file mode 100644 index 000000000..37603089f --- /dev/null +++ b/changelog/759.trivial.md @@ -0,0 +1 @@ +Collapsed the regression-baseline path placeholders (`` / `` / ``) into a single `PlaceholderMap` value object shared by the capture and verification paths, replacing the directory parameters previously threaded through both. Committed baselines and their digests are unchanged. From 072642041e55180e346602ada0b78e1dd27a8dbc Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Thu, 25 Jun 2026 17:43:17 +1000 Subject: [PATCH 3/4] style(regression): semantic line breaks in PlaceholderMap docstrings --- .../src/climate_ref_core/output_files.py | 48 +++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/packages/climate-ref-core/src/climate_ref_core/output_files.py b/packages/climate-ref-core/src/climate_ref_core/output_files.py index 18a4f42c5..a4eeac106 100644 --- a/packages/climate-ref-core/src/climate_ref_core/output_files.py +++ b/packages/climate-ref-core/src/climate_ref_core/output_files.py @@ -116,28 +116,32 @@ class PlaceholderMap: """ Bidirectional map between absolute runtime directories and portable ```` placeholders. - A committed regression bundle is made machine-independent by replacing host-specific absolute - paths with stable tokens -- ````, ```` and (optionally) - ````. The *same* map drives every direction: + A committed regression bundle is made machine-independent + by replacing host-specific absolute paths with stable tokens -- + ````, ```` and (optionally) ````. + The *same* map drives every direction: - :meth:`sanitise` rewrites absolute paths to tokens (capture / mint). - :meth:`hydrate` rewrites tokens back to absolute paths (replay / rebuild). - - :meth:`as_replacements` yields the ``{absolute: token}`` mapping the bundle and series - comparators apply to a freshly regenerated artefact before diffing. + - :meth:`as_replacements` yields the ``{absolute: token}`` mapping + the bundle and series comparators apply to a freshly regenerated artefact before diffing. - The token set is declared in one place (:meth:`for_baseline` then :meth:`with_output`), so the - capture side and the verification side cannot declare different sets and drift apart. Adding a - placeholder is a one-line change here, not a new parameter threaded through every caller. + The token set is declared in one place (:meth:`for_baseline` then :meth:`with_output`), + so the capture side and the verification side cannot declare different sets and drift apart. + Adding a placeholder is a one-line change here, + not a new parameter threaded through every caller. - The two configuration-stable tokens (```` / ````) are fixed for - a whole run; the per-execution ```` is late-bound with :meth:`with_output`. + The two configuration-stable tokens (```` / ````) + are fixed for a whole run; + the per-execution ```` is late-bound with :meth:`with_output`. """ pairs: tuple[tuple[str, Path], ...] """Ordered ``(token, absolute_directory)`` pairs. - Application order is not significant: all rewriting goes through :func:`rewrite_tree`, which - re-sorts longest-match-first so an overlapping shorter path cannot shadow a longer one. + Application order is not significant: + all rewriting goes through :func:`rewrite_tree`, + which re-sorts longest-match-first so an overlapping shorter path cannot shadow a longer one. """ @classmethod @@ -145,10 +149,12 @@ def for_baseline(cls, *, test_data_dir: Path, software_root_dir: Path | None = N """ Build the configuration-stable placeholder set for a committed baseline. - Holds every token except the per-execution output directory, which is added with - :meth:`with_output`. ``software_root_dir`` is optional: when ``None`` no - ```` substitution is applied -- a verification context that does not know - the shared-software root relies on this, and the omission is explicit and declared once. + Holds every token except the per-execution output directory, + which is added with :meth:`with_output`. + ``software_root_dir`` is optional: + when ``None`` no ```` substitution is applied -- + a verification context that does not know the shared-software root relies on this, + and the omission is explicit and declared once. """ pairs: list[tuple[str, Path]] = [(PLACEHOLDER_TEST_DATA_DIR, test_data_dir)] if software_root_dir is not None: @@ -162,22 +168,24 @@ def with_output(self, output_dir: Path) -> PlaceholderMap: def as_replacements(self) -> dict[str, str]: """Return the ``{absolute_directory: token}`` mapping (real path -> placeholder). - This is what the bundle and series comparators apply to a regenerated artefact before - diffing it against the committed (already-placeholdered) baseline. + This is what the bundle and series comparators apply to a regenerated artefact + before diffing it against the committed (already-placeholdered) baseline. """ return {str(abs_dir): token for token, abs_dir in self.pairs} def sanitise(self, directory: Path, globs: tuple[str, ...] = SANITISED_FILE_GLOBS) -> None: """Rewrite absolute paths to ```` placeholders in every text artefact under ``directory``. - Binary files are never touched. In-place. + Binary files are never touched. + In-place. """ rewrite_tree(directory, self.as_replacements(), globs) def hydrate(self, directory: Path, globs: tuple[str, ...] = SANITISED_FILE_GLOBS) -> None: """Inverse of :meth:`sanitise`: rewrite ```` placeholders back to absolute paths. - Binary files are never touched. In-place. + Binary files are never touched. + In-place. """ rewrite_tree(directory, {token: str(abs_dir) for token, abs_dir in self.pairs}, globs) From 7b18affcb42fe890a5a301a62dba7d8983310b89 Mon Sep 17 00:00:00 2001 From: Jared Lewis Date: Thu, 25 Jun 2026 21:57:36 +1000 Subject: [PATCH 4/4] chore: fix review findings --- changelog/759.trivial.md | 2 +- .../src/climate_ref_core/output_files.py | 18 +++++++++++++-- .../climate_ref_core/regression/capture.py | 13 +++++++++++ .../tests/unit/regression/test_capture.py | 12 ++++++++++ .../tests/unit/test_output_files.py | 23 +++++++++++++++++++ 5 files changed, 65 insertions(+), 3 deletions(-) diff --git a/changelog/759.trivial.md b/changelog/759.trivial.md index 37603089f..58259e876 100644 --- a/changelog/759.trivial.md +++ b/changelog/759.trivial.md @@ -1 +1 @@ -Collapsed the regression-baseline path placeholders (`` / `` / ``) into a single `PlaceholderMap` value object shared by the capture and verification paths, replacing the directory parameters previously threaded through both. Committed baselines and their digests are unchanged. +Collapsed the regression-baseline path placeholders (`` / `` / ``) into a single `PlaceholderMap` value object shared by the capture and verification paths, replacing the directory parameters previously threaded through both. diff --git a/packages/climate-ref-core/src/climate_ref_core/output_files.py b/packages/climate-ref-core/src/climate_ref_core/output_files.py index a4eeac106..47efbaec5 100644 --- a/packages/climate-ref-core/src/climate_ref_core/output_files.py +++ b/packages/climate-ref-core/src/climate_ref_core/output_files.py @@ -162,8 +162,22 @@ def for_baseline(cls, *, test_data_dir: Path, software_root_dir: Path | None = N return cls(pairs=tuple(pairs)) def with_output(self, output_dir: Path) -> PlaceholderMap: - """Return a new map that also binds ```` to the per-execution ``output_dir``.""" - return PlaceholderMap(pairs=((PLACEHOLDER_OUTPUT_DIR, output_dir), *self.pairs)) + """Return a new map that also binds ```` to the per-execution ``output_dir``. + + Rebinding replaces any existing ```` entry, + so a second call hydrates to the latest directory rather than the first. + """ + rest = tuple((token, path) for token, path in self.pairs if token != PLACEHOLDER_OUTPUT_DIR) + return PlaceholderMap(pairs=((PLACEHOLDER_OUTPUT_DIR, output_dir), *rest)) + + @property + def is_output_bound(self) -> bool: + """Whether ```` has been bound via :meth:`with_output`. + + The committed-bundle writer requires this: + an unbound map would leave execution-specific output paths in the committed bundle. + """ + return any(token == PLACEHOLDER_OUTPUT_DIR for token, _ in self.pairs) def as_replacements(self) -> dict[str, str]: """Return the ``{absolute_directory: token}`` mapping (real path -> placeholder). diff --git a/packages/climate-ref-core/src/climate_ref_core/regression/capture.py b/packages/climate-ref-core/src/climate_ref_core/regression/capture.py index 17e739a18..9b299ca14 100644 --- a/packages/climate-ref-core/src/climate_ref_core/regression/capture.py +++ b/packages/climate-ref-core/src/climate_ref_core/regression/capture.py @@ -228,7 +228,20 @@ def write_committed_bundle( : The committed digests ``{filename: sha256}`` of the bytes just written, suitable for :attr:`Manifest.committed`. + + Raises + ------ + ValueError + If ``placeholders`` is not bound to an output directory. + An unbound map would leave execution-specific output paths in the committed bundle + and digest those machine-specific bytes. """ + if not placeholders.is_output_bound: + raise ValueError( + "placeholders must be bound to an output directory via with_output() " + "before writing a committed bundle" + ) + regression_dir.mkdir(parents=True, exist_ok=True) for filename in COMMITTED_BUNDLE_FILES: diff --git a/packages/climate-ref-core/tests/unit/regression/test_capture.py b/packages/climate-ref-core/tests/unit/regression/test_capture.py index 9f5a95d34..cd7c9dbc5 100644 --- a/packages/climate-ref-core/tests/unit/regression/test_capture.py +++ b/packages/climate-ref-core/tests/unit/regression/test_capture.py @@ -69,6 +69,18 @@ def test_write_committed_bundle_sanitises_and_digests(tmp_path): assert digests["diagnostic.json"] == sha256_file(regression_dir / "diagnostic.json") +def test_write_committed_bundle_rejects_unbound_placeholders(tmp_path): + output_dir = (tmp_path / "scratch" / "frag").resolve() + test_data_dir = (tmp_path / "test-data").resolve() + source = _seed_execution(tmp_path / "scratch", "frag", output_dir=output_dir, test_data_dir=test_data_dir) + regression_dir = tmp_path / "regression" + + with pytest.raises(ValueError, match="with_output"): + write_committed_bundle( + source, regression_dir, placeholders=PlaceholderMap.for_baseline(test_data_dir=test_data_dir) + ) + + def _sig_figs(value: float) -> int: """Count the significant figures in ``value``'s shortest round-trip repr.""" # ``repr`` gives Python's shortest decimal that round-trips to the same float, diff --git a/packages/climate-ref-core/tests/unit/test_output_files.py b/packages/climate-ref-core/tests/unit/test_output_files.py index 32b1af691..0ec551811 100644 --- a/packages/climate-ref-core/tests/unit/test_output_files.py +++ b/packages/climate-ref-core/tests/unit/test_output_files.py @@ -186,6 +186,29 @@ def test_placeholder_map_with_output_returns_a_new_map(tmp_path): assert PLACEHOLDER_OUTPUT_DIR in {token for token, _ in bound.pairs} +def test_placeholder_map_with_output_rebinding_replaces(tmp_path): + # A second with_output replaces the first: one entry, hydrating to the latest dir. + first = tmp_path / "first" + second = tmp_path / "second" + base = PlaceholderMap.for_baseline(test_data_dir=tmp_path / "td") + rebound = base.with_output(first).with_output(second) + + output_dirs = [path for token, path in rebound.pairs if token == PLACEHOLDER_OUTPUT_DIR] + assert output_dirs == [second] + + directory = tmp_path / "regression" + directory.mkdir() + (directory / "a.json").write_text(PLACEHOLDER_OUTPUT_DIR) + rebound.hydrate(directory) + assert (directory / "a.json").read_text() == str(second) + + +def test_placeholder_map_is_output_bound(tmp_path): + base = PlaceholderMap.for_baseline(test_data_dir=tmp_path / "td") + assert not base.is_output_bound + assert base.with_output(tmp_path / "out").is_output_bound + + @pytest.mark.parametrize("filename", ("bundle.zip", "nested/bundle.zip")) def test_copy_output_file_returns_relpath(filename, tmp_path): scratch = (tmp_path / "scratch").resolve()