From 2f82538dd6e474ebf14aa43e999b7543bcaba8ef Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Tue, 16 Jun 2026 11:19:49 +0000
Subject: [PATCH 01/16] Add test_sync.py to test double-deprecation and removal
 issues

---
 tests/configs/empty/settings.yaml             |  3 +++
 .../example-module-multi/1.0.yaml             |  8 ++++++
 .../example-module-multi/2.0.yaml             |  8 ++++++
 .../multi-version-active/settings.yaml        |  3 +++
 .../example-module-multi/1.0.yaml             | 12 +++++++++
 .../example-module-multi/2.0.yaml             | 12 +++++++++
 .../multi-version-deprecated/settings.yaml    |  3 +++
 tests/test_sync.py                            | 27 +++++++++++++++++++
 8 files changed, 76 insertions(+)
 create mode 100644 tests/configs/empty/settings.yaml
 create mode 100644 tests/configs/multi-version-active/example-module-multi/1.0.yaml
 create mode 100644 tests/configs/multi-version-active/example-module-multi/2.0.yaml
 create mode 100644 tests/configs/multi-version-active/settings.yaml
 create mode 100644 tests/configs/multi-version-deprecated/example-module-multi/1.0.yaml
 create mode 100644 tests/configs/multi-version-deprecated/example-module-multi/2.0.yaml
 create mode 100644 tests/configs/multi-version-deprecated/settings.yaml
 create mode 100644 tests/test_sync.py

diff --git a/tests/configs/empty/settings.yaml b/tests/configs/empty/settings.yaml
new file mode 100644
index 00000000..70f60017
--- /dev/null
+++ b/tests/configs/empty/settings.yaml
@@ -0,0 +1,3 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/deployment-settings.json
+
+default_versions: {}
diff --git a/tests/configs/multi-version-active/example-module-multi/1.0.yaml b/tests/configs/multi-version-active/example-module-multi/1.0.yaml
new file mode 100644
index 00000000..d006a584
--- /dev/null
+++ b/tests/configs/multi-version-active/example-module-multi/1.0.yaml
@@ -0,0 +1,8 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json
+
+module:
+  name: example-module-multi
+  version: "1.0"
+  description: Module with two versions, used to exercise name-folder cleanup
+
+  applications: []
diff --git a/tests/configs/multi-version-active/example-module-multi/2.0.yaml b/tests/configs/multi-version-active/example-module-multi/2.0.yaml
new file mode 100644
index 00000000..c6cd894b
--- /dev/null
+++ b/tests/configs/multi-version-active/example-module-multi/2.0.yaml
@@ -0,0 +1,8 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json
+
+module:
+  name: example-module-multi
+  version: "2.0"
+  description: Module with two versions, used to exercise name-folder cleanup
+
+  applications: []
diff --git a/tests/configs/multi-version-active/settings.yaml b/tests/configs/multi-version-active/settings.yaml
new file mode 100644
index 00000000..70f60017
--- /dev/null
+++ b/tests/configs/multi-version-active/settings.yaml
@@ -0,0 +1,3 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/deployment-settings.json
+
+default_versions: {}
diff --git a/tests/configs/multi-version-deprecated/example-module-multi/1.0.yaml b/tests/configs/multi-version-deprecated/example-module-multi/1.0.yaml
new file mode 100644
index 00000000..4452fbbc
--- /dev/null
+++ b/tests/configs/multi-version-deprecated/example-module-multi/1.0.yaml
@@ -0,0 +1,12 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json
+
+module:
+  name: example-module-multi
+  version: "1.0"
+  description: Module with two versions, used to exercise name-folder cleanup
+
+  applications: []
+
+# Deprecating both versions together moves their modulefile links into the deprecated
+# area and clears the now-spent live modulefiles name folder.
+deprecated: true
diff --git a/tests/configs/multi-version-deprecated/example-module-multi/2.0.yaml b/tests/configs/multi-version-deprecated/example-module-multi/2.0.yaml
new file mode 100644
index 00000000..de2c0844
--- /dev/null
+++ b/tests/configs/multi-version-deprecated/example-module-multi/2.0.yaml
@@ -0,0 +1,12 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json
+
+module:
+  name: example-module-multi
+  version: "2.0"
+  description: Module with two versions, used to exercise name-folder cleanup
+
+  applications: []
+
+# Deprecating both versions together moves their modulefile links into the deprecated
+# area and clears the now-spent live modulefiles name folder.
+deprecated: true
diff --git a/tests/configs/multi-version-deprecated/settings.yaml b/tests/configs/multi-version-deprecated/settings.yaml
new file mode 100644
index 00000000..70f60017
--- /dev/null
+++ b/tests/configs/multi-version-deprecated/settings.yaml
@@ -0,0 +1,3 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/deployment-settings.json
+
+default_versions: {}
diff --git a/tests/test_sync.py b/tests/test_sync.py
new file mode 100644
index 00000000..abf34b82
--- /dev/null
+++ b/tests/test_sync.py
@@ -0,0 +1,27 @@
+from pathlib import Path
+
+from conftest import run_cli
+from deploy_tools.layout import Layout
+
+MODULE_NAME = "example-module-multi"
+
+
+def test_deprecate_then_remove_multiple_versions_of_same_module(
+    tmp_path: Path, configs: Path
+):
+    # Regression test for issues with removing name folders for the same module name
+    # twice.
+    run_cli("sync", "--from-scratch", tmp_path, configs / "multi-version-active")
+
+    layout = Layout(tmp_path)
+    live_name_folder = layout.modulefiles_root / MODULE_NAME
+    deprecated_name_folder = layout.deprecated_modulefiles_root / MODULE_NAME
+
+    run_cli("sync", tmp_path, configs / "multi-version-deprecated")
+    assert not live_name_folder.exists()
+    assert {p.name for p in deprecated_name_folder.iterdir()} == {"1.0", "2.0"}
+
+    run_cli("sync", tmp_path, configs / "empty")
+    assert not deprecated_name_folder.exists()
+    assert not (layout.modules_root / MODULE_NAME).exists()
+    assert run_cli("compare", tmp_path) == ""

From 203af40fed9cd15eb4e3456a00cf3bdeb5368740 Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Mon, 29 Jun 2026 10:45:22 +0000
Subject: [PATCH 02/16] Add test for external tool calls

---
 tests/test_external_tools.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 tests/test_external_tools.py

diff --git a/tests/test_external_tools.py b/tests/test_external_tools.py
new file mode 100644
index 00000000..6c8b4afd
--- /dev/null
+++ b/tests/test_external_tools.py
@@ -0,0 +1,17 @@
+import pytest
+
+from deploy_tools.external_tools import ExternalToolError, run_command
+
+
+def test_run_command_reports_missing_tool():
+    # A required external tool that is not installed must surface as a clean
+    # ExternalToolError naming the tool, not a raw FileNotFoundError traceback.
+    with pytest.raises(ExternalToolError, match="deploy-tools-no-such-binary"):
+        run_command(["deploy-tools-no-such-binary"])
+
+
+def test_run_command_reports_failed_tool():
+    # With check=True, a non-zero exit is reported as an ExternalToolError rather than a
+    # raw CalledProcessError.
+    with pytest.raises(ExternalToolError, match="exit status"):
+        run_command(["bash", "-c", "exit 3"], check=True)

From fb92d9767f28f7ef3560231b25678c95a7695fe1 Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Wed, 8 Jul 2026 17:14:56 +0000
Subject: [PATCH 03/16] Fix type hinting in test_sync and test_external_tools

---
 tests/test_external_tools.py | 4 ++--
 tests/test_sync.py           | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/test_external_tools.py b/tests/test_external_tools.py
index 6c8b4afd..89d079b2 100644
--- a/tests/test_external_tools.py
+++ b/tests/test_external_tools.py
@@ -3,14 +3,14 @@
 from deploy_tools.external_tools import ExternalToolError, run_command
 
 
-def test_run_command_reports_missing_tool():
+def test_run_command_reports_missing_tool() -> None:
     # A required external tool that is not installed must surface as a clean
     # ExternalToolError naming the tool, not a raw FileNotFoundError traceback.
     with pytest.raises(ExternalToolError, match="deploy-tools-no-such-binary"):
         run_command(["deploy-tools-no-such-binary"])
 
 
-def test_run_command_reports_failed_tool():
+def test_run_command_reports_failed_tool() -> None:
     # With check=True, a non-zero exit is reported as an ExternalToolError rather than a
     # raw CalledProcessError.
     with pytest.raises(ExternalToolError, match="exit status"):
diff --git a/tests/test_sync.py b/tests/test_sync.py
index abf34b82..35ecaddc 100644
--- a/tests/test_sync.py
+++ b/tests/test_sync.py
@@ -8,7 +8,7 @@
 
 def test_deprecate_then_remove_multiple_versions_of_same_module(
     tmp_path: Path, configs: Path
-):
+) -> None:
     # Regression test for issues with removing name folders for the same module name
     # twice.
     run_cli("sync", "--from-scratch", tmp_path, configs / "multi-version-active")

From 83efbcf9e81fef4a4d27c827c4329df3bc9fe029 Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Wed, 17 Jun 2026 18:48:05 +0000
Subject: [PATCH 04/16] Have dedicated test for ensuring apptainer pulls are
 successful

---
 .../apptainer-pull/example-apptainer/1.0.yaml | 19 +++++++++++++++++++
 tests/configs/apptainer-pull/settings.yaml    |  3 +++
 tests/test_apptainer_pull.py                  | 14 ++++++++++++++
 3 files changed, 36 insertions(+)
 create mode 100644 tests/configs/apptainer-pull/example-apptainer/1.0.yaml
 create mode 100644 tests/configs/apptainer-pull/settings.yaml
 create mode 100644 tests/test_apptainer_pull.py

diff --git a/tests/configs/apptainer-pull/example-apptainer/1.0.yaml b/tests/configs/apptainer-pull/example-apptainer/1.0.yaml
new file mode 100644
index 00000000..58d70c18
--- /dev/null
+++ b/tests/configs/apptainer-pull/example-apptainer/1.0.yaml
@@ -0,0 +1,19 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json
+
+module:
+  name: example-apptainer
+  version: "1.0"
+  description: Minimal single-image module exercising a real apptainer pull
+
+  applications:
+    - app_type: apptainer
+
+      container:
+        path: docker://ghcr.io/apptainer/lolcow
+        version: latest
+
+      entrypoints:
+        - name: cowsay-hello
+          command: cowsay
+          options:
+            command_args: Hello
diff --git a/tests/configs/apptainer-pull/settings.yaml b/tests/configs/apptainer-pull/settings.yaml
new file mode 100644
index 00000000..70f60017
--- /dev/null
+++ b/tests/configs/apptainer-pull/settings.yaml
@@ -0,0 +1,3 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/deployment-settings.json
+
+default_versions: {}
diff --git a/tests/test_apptainer_pull.py b/tests/test_apptainer_pull.py
new file mode 100644
index 00000000..1dc5c2b9
--- /dev/null
+++ b/tests/test_apptainer_pull.py
@@ -0,0 +1,14 @@
+from pathlib import Path
+
+from conftest import run_cli
+
+
+def test_sync_builds_real_sif_file(configs: Path, tmp_path: Path) -> None:
+    # The golden-master test stubs the apptainer pull, so this is the only test that
+    # exercises a real one. It needs apptainer installed and network access to ghcr.io;
+    # a missing binary fails the test (via run_cli) rather than skipping it, by design.
+    run_cli("sync", "--from-scratch", tmp_path, configs / "apptainer-pull")
+
+    sif_files = list(tmp_path.glob("**/*.sif"))
+    assert sif_files, "sync did not produce a .sif file"
+    assert all(f.stat().st_size > 0 for f in sif_files), "produced .sif file is empty"

From 8b757c3d4f4303bf095fa91ad7b3e72d0932df0f Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Wed, 17 Jun 2026 19:08:57 +0000
Subject: [PATCH 05/16] Add test for BinaryApp using a local generated file

---
 tests/test_binary.py | 100 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 tests/test_binary.py

diff --git a/tests/test_binary.py b/tests/test_binary.py
new file mode 100644
index 00000000..1dd0ca19
--- /dev/null
+++ b/tests/test_binary.py
@@ -0,0 +1,100 @@
+import hashlib
+from pathlib import Path
+
+import pytest
+import yaml
+
+from conftest import run_cli
+from deploy_tools.app_builder import AppBuilderError
+
+# A small payload served as a local file:// "download" source, so the binary builder's
+# real urlretrieve and hashing run with no network access.
+BINARY_CONTENT = b"#!/bin/sh\necho 'hello from a binary module'\n"
+
+DIGESTS = {
+    "sha256": hashlib.sha256(BINARY_CONTENT).hexdigest(),
+    "sha512": hashlib.sha512(BINARY_CONTENT).hexdigest(),
+    "md5": hashlib.md5(BINARY_CONTENT).hexdigest(),
+}
+
+
+def _write_binary_deployment(
+    tmp_path: Path, hash_type: str, hash_value: str
+) -> tuple[Path, Path]:
+    """Build a deployment root and a single binary-module config under ``tmp_path``.
+
+    Binary modules download from a URL and verify a hash, so the config must reference a
+    real file whose digest is known at test time. A committed config cannot carry such a
+    machine-specific ``file://`` path, so it is generated per test.
+
+    Args:
+        tmp_path: The per-test scratch directory.
+        hash_type: The ``hash_type`` to record in the config.
+        hash_value: The hash to record in the config.
+
+    Returns:
+        The ``(deployment_root, config_folder)`` pair to pass to ``sync``.
+    """
+    source = tmp_path / "downloads" / "example-tool"
+    source.parent.mkdir()
+    source.write_bytes(BINARY_CONTENT)
+
+    config_folder = tmp_path / "config"
+    module_dir = config_folder / "example-binary"
+    module_dir.mkdir(parents=True)
+    (config_folder / "settings.yaml").write_text("default_versions: {}\n")
+
+    release = {
+        "module": {
+            "name": "example-binary",
+            "version": "1.0",
+            "description": "Single binary module for builder coverage",
+            "applications": [
+                {
+                    "app_type": "binary",
+                    "name": "example-tool",
+                    "url": source.as_uri(),
+                    "hash": hash_value,
+                    "hash_type": hash_type,
+                }
+            ],
+        }
+    }
+    (module_dir / "1.0.yaml").write_text(yaml.safe_dump(release))
+
+    deployment_root = tmp_path / "deployment"
+    deployment_root.mkdir()
+    return deployment_root, config_folder
+
+
+@pytest.mark.parametrize("hash_type", ["sha256", "sha512", "md5"])
+def test_sync_builds_binary_with_hash_check(tmp_path: Path, hash_type: str) -> None:
+    # Each supported algorithm should validate the download and deploy an executable
+    # binary entrypoint with the original contents.
+    deployment_root, config_folder = _write_binary_deployment(
+        tmp_path, hash_type, DIGESTS[hash_type]
+    )
+    run_cli("sync", "--from-scratch", deployment_root, config_folder)
+
+    binary = deployment_root / "modules/example-binary/1.0/entrypoints/example-tool"
+    assert binary.read_bytes() == BINARY_CONTENT
+    assert binary.stat().st_mode & 0o111, "deployed binary is not executable"
+
+
+def test_sync_builds_binary_without_hash_check(tmp_path: Path) -> None:
+    # hash_type "none" skips verification; the binary is still downloaded and deployed.
+    deployment_root, config_folder = _write_binary_deployment(tmp_path, "none", "")
+    run_cli("sync", "--from-scratch", deployment_root, config_folder)
+
+    binary = deployment_root / "modules/example-binary/1.0/entrypoints/example-tool"
+    assert binary.read_bytes() == BINARY_CONTENT
+
+
+def test_sync_rejects_binary_with_wrong_hash(tmp_path: Path) -> None:
+    # A digest that does not match the download must fail the build rather than deploy
+    # an unverified binary.
+    deployment_root, config_folder = _write_binary_deployment(
+        tmp_path, "sha256", "0" * 64
+    )
+    with pytest.raises(AppBuilderError, match="hash check failed"):
+        run_cli("sync", "--from-scratch", deployment_root, config_folder)

From c7bf8708385294d8c5a175322a7749fe41fd508e Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Wed, 8 Jul 2026 17:18:03 +0000
Subject: [PATCH 06/16] Add test for syncing with nonexistent .git folder

---
 tests/test_sync.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/test_sync.py b/tests/test_sync.py
index 35ecaddc..6d863b49 100644
--- a/tests/test_sync.py
+++ b/tests/test_sync.py
@@ -1,7 +1,11 @@
+import shutil
 from pathlib import Path
 
+import pytest
+
 from conftest import run_cli
 from deploy_tools.layout import Layout
+from deploy_tools.sync import SyncError
 
 MODULE_NAME = "example-module-multi"
 
@@ -25,3 +29,15 @@ def test_deprecate_then_remove_multiple_versions_of_same_module(
     assert not deprecated_name_folder.exists()
     assert not (layout.modules_root / MODULE_NAME).exists()
     assert run_cli("compare", tmp_path) == ""
+
+
+def test_sync_rejects_deployment_area_without_git_repo(
+    tmp_path: Path, configs: Path
+) -> None:
+    # An area whose .git has been lost still has a snapshot but no history to commit
+    # against: a corrupt state surfaced as a clean error, not a GitPython traceback.
+    run_cli("sync", "--from-scratch", tmp_path, configs / "multi-version-active")
+    shutil.rmtree(tmp_path / ".git")
+
+    with pytest.raises(SyncError, match="not a git repository"):
+        run_cli("sync", tmp_path, configs / "multi-version-active")

From 55190b26a80daa07b0408a0548641f94ec698498 Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Fri, 19 Jun 2026 15:37:39 +0000
Subject: [PATCH 07/16] Add tests for Module options not previously covered

---
 tests/test_module_options.py | 129 +++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)
 create mode 100644 tests/test_module_options.py

diff --git a/tests/test_module_options.py b/tests/test_module_options.py
new file mode 100644
index 00000000..b543eae4
--- /dev/null
+++ b/tests/test_module_options.py
@@ -0,0 +1,129 @@
+"""Tests for Module/app options that drive otherwise-untested template branches.
+
+These CLI-driven ``sync`` tests build a config from typed models, set each option to a
+representative value, and assert the rendered output. They are kept out of the
+golden master tests to avoid bloat.
+"""
+
+from pathlib import Path
+
+from conftest import run_cli
+from deploy_tools.models.apptainer_app import (
+    ApptainerApp,
+    ContainerImage,
+    Entrypoint,
+    EntrypointOptions,
+)
+from deploy_tools.models.deployment import DeploymentSettings
+from deploy_tools.models.module import Module, Release
+from deploy_tools.models.save_and_load import (
+    DEPLOYMENT_SETTINGS,
+    YAML_FILE_SUFFIX,
+    save_as_yaml,
+)
+from deploy_tools.models.shell_app import ShellApp
+
+
+def _sync(tmp_path: Path, release: Release) -> Path:
+    """Serialise a single-module config under ``tmp_path`` and ``sync`` it from scratch.
+
+    Args:
+        tmp_path: The per-test scratch directory.
+        release: The Release to deploy as the sole module.
+
+    Returns:
+        The deployment root the module was synced into.
+    """
+    module = release.module
+    config_folder = tmp_path / "config"
+    save_as_yaml(DeploymentSettings(), config_folder / DEPLOYMENT_SETTINGS, True)
+    save_as_yaml(
+        release,
+        config_folder / module.name / f"{module.version}{YAML_FILE_SUFFIX}",
+        True,
+    )
+
+    deployment_root = tmp_path / "deployment"
+    deployment_root.mkdir()
+    run_cli("sync", "--from-scratch", deployment_root, config_folder)
+    return deployment_root
+
+
+def test_load_and_unload_scripts_render_tcl_blocks(tmp_path: Path) -> None:
+    release = Release(
+        module=Module(
+            name="example-scripted",
+            version="1.0",
+            description="Module exercising load/unload script hooks",
+            load_script=["set example_loaded 1"],
+            unload_script=["unset example_loaded"],
+            applications=[
+                ShellApp(app_type="shell", name="example-cmd", script=["echo hello"])
+            ],
+        )
+    )
+    deployment_root = _sync(tmp_path, release)
+
+    modulefile = (
+        deployment_root / "modules/example-scripted/1.0/modulefile"
+    ).read_text()
+
+    # The unload-mode guard is produced only by the unload_script branch, so its
+    # presence confirms that branch rendered; the script lines confirm both blocks
+    # carry content.
+    assert "if { [module-info mode unload] } {" in modulefile
+    assert "set example_loaded 1" in modulefile
+    assert "unset example_loaded" in modulefile
+
+
+def test_apptainer_options_merge_global_and_entrypoint(
+    tmp_path: Path, stub_apptainer_pull: None
+) -> None:
+    # The Apptainer entrypoint combines global_options with per-entrypoint options
+    release = Release(
+        module=Module(
+            name="example-apptainer-opts",
+            version="1.0",
+            description="Module exercising apptainer entrypoint option merge",
+            applications=[
+                ApptainerApp(
+                    app_type="apptainer",
+                    container=ContainerImage(
+                        path="docker://ghcr.io/apptainer/lolcow",  # type: ignore[arg-type]
+                        version="latest",
+                    ),
+                    global_options=EntrypointOptions(
+                        mounts=["/global/mount"],
+                        host_binaries=["globalbin"],
+                        apptainer_args="--global-arg",
+                        command_args="--global-cmd",
+                    ),
+                    entrypoints=[
+                        Entrypoint(
+                            name="example-cmd",
+                            command="realcmd",
+                            options=EntrypointOptions(
+                                mounts=["/local/mount"],
+                                host_binaries=["localbin"],
+                                apptainer_args="--local-arg",
+                                command_args="--local-cmd",
+                            ),
+                        )
+                    ],
+                )
+            ],
+        )
+    )
+    deployment_root = _sync(tmp_path, release)
+
+    entrypoint = (
+        deployment_root / "modules/example-apptainer-opts/1.0/entrypoints/example-cmd"
+    ).read_text()
+
+    # Global and per-entrypoint values are concatenated, global first: comma-joined for
+    # mounts, space-joined for host_binaries and the arg strings.
+    assert 'mounts="/global/mount,/local/mount"' in entrypoint
+    assert 'apptainer_args="--global-arg --local-arg"' in entrypoint
+    assert 'command_args="--global-cmd --local-cmd"' in entrypoint
+    assert "for i in globalbin localbin; do" in entrypoint
+    assert 'command="realcmd"' in entrypoint

From 8a8a5dd3922224d1815c196b3986b4c6f5a6eee4 Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Wed, 8 Jul 2026 17:22:26 +0000
Subject: [PATCH 08/16] Reorganise test configuration for consistency

---
 .../apptainer-pull/example-apptainer/1.0.yaml      |  0
 .../{ => valid}/apptainer-pull/settings.yaml       |  0
 tests/configs/{ => valid}/empty/settings.yaml      |  0
 .../example-module-multi/1.0.yaml                  |  0
 .../example-module-multi/2.0.yaml                  |  0
 .../{ => valid}/multi-version-active/settings.yaml |  0
 .../example-module-multi/1.0.yaml                  |  0
 .../example-module-multi/2.0.yaml                  |  0
 .../multi-version-deprecated/settings.yaml         |  0
 tests/test_apptainer_pull.py                       |  2 +-
 tests/test_sync.py                                 | 14 +++++++++-----
 11 files changed, 10 insertions(+), 6 deletions(-)
 rename tests/configs/{ => valid}/apptainer-pull/example-apptainer/1.0.yaml (100%)
 rename tests/configs/{ => valid}/apptainer-pull/settings.yaml (100%)
 rename tests/configs/{ => valid}/empty/settings.yaml (100%)
 rename tests/configs/{ => valid}/multi-version-active/example-module-multi/1.0.yaml (100%)
 rename tests/configs/{ => valid}/multi-version-active/example-module-multi/2.0.yaml (100%)
 rename tests/configs/{ => valid}/multi-version-active/settings.yaml (100%)
 rename tests/configs/{ => valid}/multi-version-deprecated/example-module-multi/1.0.yaml (100%)
 rename tests/configs/{ => valid}/multi-version-deprecated/example-module-multi/2.0.yaml (100%)
 rename tests/configs/{ => valid}/multi-version-deprecated/settings.yaml (100%)

diff --git a/tests/configs/apptainer-pull/example-apptainer/1.0.yaml b/tests/configs/valid/apptainer-pull/example-apptainer/1.0.yaml
similarity index 100%
rename from tests/configs/apptainer-pull/example-apptainer/1.0.yaml
rename to tests/configs/valid/apptainer-pull/example-apptainer/1.0.yaml
diff --git a/tests/configs/apptainer-pull/settings.yaml b/tests/configs/valid/apptainer-pull/settings.yaml
similarity index 100%
rename from tests/configs/apptainer-pull/settings.yaml
rename to tests/configs/valid/apptainer-pull/settings.yaml
diff --git a/tests/configs/empty/settings.yaml b/tests/configs/valid/empty/settings.yaml
similarity index 100%
rename from tests/configs/empty/settings.yaml
rename to tests/configs/valid/empty/settings.yaml
diff --git a/tests/configs/multi-version-active/example-module-multi/1.0.yaml b/tests/configs/valid/multi-version-active/example-module-multi/1.0.yaml
similarity index 100%
rename from tests/configs/multi-version-active/example-module-multi/1.0.yaml
rename to tests/configs/valid/multi-version-active/example-module-multi/1.0.yaml
diff --git a/tests/configs/multi-version-active/example-module-multi/2.0.yaml b/tests/configs/valid/multi-version-active/example-module-multi/2.0.yaml
similarity index 100%
rename from tests/configs/multi-version-active/example-module-multi/2.0.yaml
rename to tests/configs/valid/multi-version-active/example-module-multi/2.0.yaml
diff --git a/tests/configs/multi-version-active/settings.yaml b/tests/configs/valid/multi-version-active/settings.yaml
similarity index 100%
rename from tests/configs/multi-version-active/settings.yaml
rename to tests/configs/valid/multi-version-active/settings.yaml
diff --git a/tests/configs/multi-version-deprecated/example-module-multi/1.0.yaml b/tests/configs/valid/multi-version-deprecated/example-module-multi/1.0.yaml
similarity index 100%
rename from tests/configs/multi-version-deprecated/example-module-multi/1.0.yaml
rename to tests/configs/valid/multi-version-deprecated/example-module-multi/1.0.yaml
diff --git a/tests/configs/multi-version-deprecated/example-module-multi/2.0.yaml b/tests/configs/valid/multi-version-deprecated/example-module-multi/2.0.yaml
similarity index 100%
rename from tests/configs/multi-version-deprecated/example-module-multi/2.0.yaml
rename to tests/configs/valid/multi-version-deprecated/example-module-multi/2.0.yaml
diff --git a/tests/configs/multi-version-deprecated/settings.yaml b/tests/configs/valid/multi-version-deprecated/settings.yaml
similarity index 100%
rename from tests/configs/multi-version-deprecated/settings.yaml
rename to tests/configs/valid/multi-version-deprecated/settings.yaml
diff --git a/tests/test_apptainer_pull.py b/tests/test_apptainer_pull.py
index 1dc5c2b9..406d24ac 100644
--- a/tests/test_apptainer_pull.py
+++ b/tests/test_apptainer_pull.py
@@ -7,7 +7,7 @@ def test_sync_builds_real_sif_file(configs: Path, tmp_path: Path) -> None:
     # The golden-master test stubs the apptainer pull, so this is the only test that
     # exercises a real one. It needs apptainer installed and network access to ghcr.io;
     # a missing binary fails the test (via run_cli) rather than skipping it, by design.
-    run_cli("sync", "--from-scratch", tmp_path, configs / "apptainer-pull")
+    run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "apptainer-pull")
 
     sif_files = list(tmp_path.glob("**/*.sif"))
     assert sif_files, "sync did not produce a .sif file"
diff --git a/tests/test_sync.py b/tests/test_sync.py
index 6d863b49..ca0cac7e 100644
--- a/tests/test_sync.py
+++ b/tests/test_sync.py
@@ -15,17 +15,19 @@ def test_deprecate_then_remove_multiple_versions_of_same_module(
 ) -> None:
     # Regression test for issues with removing name folders for the same module name
     # twice.
-    run_cli("sync", "--from-scratch", tmp_path, configs / "multi-version-active")
+    run_cli(
+        "sync", "--from-scratch", tmp_path, configs / "valid" / "multi-version-active"
+    )
 
     layout = Layout(tmp_path)
     live_name_folder = layout.modulefiles_root / MODULE_NAME
     deprecated_name_folder = layout.deprecated_modulefiles_root / MODULE_NAME
 
-    run_cli("sync", tmp_path, configs / "multi-version-deprecated")
+    run_cli("sync", tmp_path, configs / "valid" / "multi-version-deprecated")
     assert not live_name_folder.exists()
     assert {p.name for p in deprecated_name_folder.iterdir()} == {"1.0", "2.0"}
 
-    run_cli("sync", tmp_path, configs / "empty")
+    run_cli("sync", tmp_path, configs / "valid" / "empty")
     assert not deprecated_name_folder.exists()
     assert not (layout.modules_root / MODULE_NAME).exists()
     assert run_cli("compare", tmp_path) == ""
@@ -36,8 +38,10 @@ def test_sync_rejects_deployment_area_without_git_repo(
 ) -> None:
     # An area whose .git has been lost still has a snapshot but no history to commit
     # against: a corrupt state surfaced as a clean error, not a GitPython traceback.
-    run_cli("sync", "--from-scratch", tmp_path, configs / "multi-version-active")
+    run_cli(
+        "sync", "--from-scratch", tmp_path, configs / "valid" / "multi-version-active"
+    )
     shutil.rmtree(tmp_path / ".git")
 
     with pytest.raises(SyncError, match="not a git repository"):
-        run_cli("sync", tmp_path, configs / "multi-version-active")
+        run_cli("sync", tmp_path, configs / "valid" / "multi-version-active")

From 6604ea48a52708da452aa5708259b1c4dc5c2062 Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Fri, 19 Jun 2026 18:17:34 +0000
Subject: [PATCH 09/16] Expand test_module_options to cover template branches

---
 tests/test_module_options.py | 82 ++++++++++++++++++++++++++++++------
 1 file changed, 69 insertions(+), 13 deletions(-)

diff --git a/tests/test_module_options.py b/tests/test_module_options.py
index b543eae4..477f4d6b 100644
--- a/tests/test_module_options.py
+++ b/tests/test_module_options.py
@@ -1,8 +1,7 @@
 """Tests for Module/app options that drive otherwise-untested template branches.
 
 These CLI-driven ``sync`` tests build a config from typed models, set each option to a
-representative value, and assert the rendered output. They are kept out of the
-golden master tests to avoid bloat.
+representative value, and assert the rendered output.
 """
 
 from pathlib import Path
@@ -15,7 +14,7 @@
     EntrypointOptions,
 )
 from deploy_tools.models.deployment import DeploymentSettings
-from deploy_tools.models.module import Module, Release
+from deploy_tools.models.module import EnvVar, Module, ModuleDependency, Release
 from deploy_tools.models.save_and_load import (
     DEPLOYMENT_SETTINGS,
     YAML_FILE_SUFFIX,
@@ -24,24 +23,25 @@
 from deploy_tools.models.shell_app import ShellApp
 
 
-def _sync(tmp_path: Path, release: Release) -> Path:
-    """Serialise a single-module config under ``tmp_path`` and ``sync`` it from scratch.
+def _sync(tmp_path: Path, *releases: Release) -> Path:
+    """Serialise a config of the given releases under ``tmp_path`` and ``sync`` it.
 
     Args:
         tmp_path: The per-test scratch directory.
-        release: The Release to deploy as the sole module.
+        releases: The Releases to deploy from scratch.
 
     Returns:
-        The deployment root the module was synced into.
+        The deployment root the modules were synced into.
     """
-    module = release.module
     config_folder = tmp_path / "config"
     save_as_yaml(DeploymentSettings(), config_folder / DEPLOYMENT_SETTINGS, True)
-    save_as_yaml(
-        release,
-        config_folder / module.name / f"{module.version}{YAML_FILE_SUFFIX}",
-        True,
-    )
+    for release in releases:
+        module = release.module
+        save_as_yaml(
+            release,
+            config_folder / module.name / f"{module.version}{YAML_FILE_SUFFIX}",
+            True,
+        )
 
     deployment_root = tmp_path / "deployment"
     deployment_root.mkdir()
@@ -127,3 +127,59 @@ def test_apptainer_options_merge_global_and_entrypoint(
     assert 'command_args="--global-cmd --local-cmd"' in entrypoint
     assert "for i in globalbin localbin; do" in entrypoint
     assert 'command="realcmd"' in entrypoint
+
+
+def test_dependencies_render_module_load_lines(tmp_path: Path) -> None:
+    # The modulefile dependency block branches on whether a version is given: a managed
+    # dependency pins a version, while a version-less dependency (valid only for modules
+    # not managed by deploy-tools) renders the bare module name.
+    dependency = Release(
+        module=Module(
+            name="example-dependency",
+            version="1.0",
+            description="Managed module used as a versioned dependency target",
+            applications=[],
+        )
+    )
+    dependent = Release(
+        module=Module(
+            name="example-dependent",
+            version="1.0",
+            description="Module with a managed and an external dependency",
+            dependencies=[
+                ModuleDependency(name="example-dependency", version="1.0"),
+                ModuleDependency(name="external-module"),
+            ],
+            applications=[],
+        )
+    )
+    deployment_root = _sync(tmp_path, dependency, dependent)
+
+    modulefile = (
+        deployment_root / "modules/example-dependent/1.0/modulefile"
+    ).read_text()
+
+    # Versioned dependency renders with its version. The version-less external one
+    # renders the bare module name.
+    assert "module load example-dependency/1.0" in modulefile
+    assert "module load external-module\n" in modulefile
+
+
+def test_env_vars_render_setenv(tmp_path: Path) -> None:
+    # env_vars render as setenv lines in the modulefile; assert one is emitted.
+    release = Release(
+        module=Module(
+            name="example-env-vars",
+            version="1.0",
+            description="Module exercising environment variable rendering",
+            env_vars=[EnvVar(name="EXAMPLE_VAR", value="example-value")],
+            applications=[],
+        )
+    )
+    deployment_root = _sync(tmp_path, release)
+
+    modulefile = (
+        deployment_root / "modules/example-env-vars/1.0/modulefile"
+    ).read_text()
+
+    assert 'setenv EXAMPLE_VAR "example-value"' in modulefile

From fc81679b5a95a7d9ee9b3c4d595359cfc44e50e8 Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Mon, 22 Jun 2026 10:39:43 +0000
Subject: [PATCH 10/16] Add test for deploying from scratch into dir with
 snapshot

---
 tests/test_sync.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/test_sync.py b/tests/test_sync.py
index ca0cac7e..ea4b29ff 100644
--- a/tests/test_sync.py
+++ b/tests/test_sync.py
@@ -5,6 +5,7 @@
 
 from conftest import run_cli
 from deploy_tools.layout import Layout
+from deploy_tools.snapshot import SnapshotError
 from deploy_tools.sync import SyncError
 
 MODULE_NAME = "example-module-multi"
@@ -45,3 +46,14 @@ def test_sync_rejects_deployment_area_without_git_repo(
 
     with pytest.raises(SyncError, match="not a git repository"):
         run_cli("sync", tmp_path, configs / "valid" / "multi-version-active")
+
+
+def test_sync_from_scratch_rejects_existing_snapshot(
+    tmp_path: Path, configs: Path
+) -> None:
+    # --from-scratch must refuse to run when the area already holds a snapshot.
+    run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "minimal")
+    with pytest.raises(
+        SnapshotError, match="must not exist when deploying from scratch"
+    ):
+        run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "minimal")

From 0ba2aa376ab33a17810e6e1aae009026200e0859 Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Mon, 22 Jun 2026 11:53:32 +0000
Subject: [PATCH 11/16] Add unit test for defensive check in
 apply_default_versions()

---
 tests/test_modulefile.py | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 tests/test_modulefile.py

diff --git a/tests/test_modulefile.py b/tests/test_modulefile.py
new file mode 100644
index 00000000..86ca79a8
--- /dev/null
+++ b/tests/test_modulefile.py
@@ -0,0 +1,29 @@
+"""Direct-call unit tests for ``modulefile`` helpers."""
+
+from pathlib import Path
+
+from deploy_tools.layout import Layout
+from deploy_tools.modulefile import apply_default_versions
+
+
+def test_apply_default_versions_removes_default_for_unlisted_module(
+    tmp_path: Path,
+) -> None:
+    # apply_default_versions removes the stale default (.version) file for a deployed
+    # module that is absent from the default map. The CLI never reaches this branch
+    # because validate_default_versions assigns a default to every deployed module
+    # (raising otherwise), so call the function directly to pin its cleanup contract.
+    layout = Layout(tmp_path)
+    name, version = "example", "1.0"
+
+    # A deployed module: a live modulefile link plus an existing default (.version)
+    # file.
+    mf_link = layout.get_modulefile_link(name, version)
+    mf_link.parent.mkdir(parents=True, exist_ok=True)
+    mf_link.touch()
+    default_file = layout.get_default_version_file(name)
+    default_file.write_text("#%Module1.0\nset ModulesVersion 1.0\n")
+
+    apply_default_versions({}, layout)
+
+    assert not default_file.exists()

From 52a667b73bdeae7629234daa4243cf4c9127a16f Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Tue, 30 Jun 2026 14:07:46 +0000
Subject: [PATCH 12/16] Add explicit default configuration to test_sync.py

---
 .../example-module-multi/1.0.yaml                |  8 ++++++++
 .../example-module-multi/2.0.yaml                |  8 ++++++++
 .../multi-version-explicit-default/settings.yaml |  5 +++++
 tests/test_sync.py                               | 16 ++++++++++++++++
 4 files changed, 37 insertions(+)
 create mode 100644 tests/configs/valid/multi-version-explicit-default/example-module-multi/1.0.yaml
 create mode 100644 tests/configs/valid/multi-version-explicit-default/example-module-multi/2.0.yaml
 create mode 100644 tests/configs/valid/multi-version-explicit-default/settings.yaml

diff --git a/tests/configs/valid/multi-version-explicit-default/example-module-multi/1.0.yaml b/tests/configs/valid/multi-version-explicit-default/example-module-multi/1.0.yaml
new file mode 100644
index 00000000..675a9eec
--- /dev/null
+++ b/tests/configs/valid/multi-version-explicit-default/example-module-multi/1.0.yaml
@@ -0,0 +1,8 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json
+
+module:
+  name: example-module-multi
+  version: "1.0"
+  description: Module with two versions, used to exercise explicit default selection
+
+  applications: []
diff --git a/tests/configs/valid/multi-version-explicit-default/example-module-multi/2.0.yaml b/tests/configs/valid/multi-version-explicit-default/example-module-multi/2.0.yaml
new file mode 100644
index 00000000..1335942e
--- /dev/null
+++ b/tests/configs/valid/multi-version-explicit-default/example-module-multi/2.0.yaml
@@ -0,0 +1,8 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json
+
+module:
+  name: example-module-multi
+  version: "2.0"
+  description: Module with two versions, used to exercise explicit default selection
+
+  applications: []
diff --git a/tests/configs/valid/multi-version-explicit-default/settings.yaml b/tests/configs/valid/multi-version-explicit-default/settings.yaml
new file mode 100644
index 00000000..f3215dbe
--- /dev/null
+++ b/tests/configs/valid/multi-version-explicit-default/settings.yaml
@@ -0,0 +1,5 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/deployment-settings.json
+
+# Pin the default to 1.0 even though 2.0 is present and would be auto-selected.
+default_versions:
+  example-module-multi: "1.0"
diff --git a/tests/test_sync.py b/tests/test_sync.py
index ea4b29ff..9bd2984b 100644
--- a/tests/test_sync.py
+++ b/tests/test_sync.py
@@ -34,6 +34,22 @@ def test_deprecate_then_remove_multiple_versions_of_same_module(
     assert run_cli("compare", tmp_path) == ""
 
 
+def test_sync_applies_explicit_default_version_over_auto_selection(
+    tmp_path: Path, configs: Path
+) -> None:
+    # settings.yaml pins the default to 1.0 even though 2.0 is deployed and would be
+    # auto-selected. Confirm the explicit pin is set in the on-disk .version file.
+    run_cli(
+        "sync",
+        "--from-scratch",
+        tmp_path,
+        configs / "valid" / "multi-version-explicit-default",
+    )
+
+    default_version_file = Layout(tmp_path).get_default_version_file(MODULE_NAME)
+    assert "set ModulesVersion 1.0" in default_version_file.read_text()
+
+
 def test_sync_rejects_deployment_area_without_git_repo(
     tmp_path: Path, configs: Path
 ) -> None:

From 8cbd6d0930f73b54ad739f40581a653dd618c9fc Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Mon, 6 Jul 2026 15:33:48 +0000
Subject: [PATCH 13/16] Use more specific pytest.raises matches in sync-related
 tests

---
 tests/test_binary.py         | 5 ++++-
 tests/test_external_tools.py | 4 +++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/test_binary.py b/tests/test_binary.py
index 1dd0ca19..67abf0ce 100644
--- a/tests/test_binary.py
+++ b/tests/test_binary.py
@@ -96,5 +96,8 @@ def test_sync_rejects_binary_with_wrong_hash(tmp_path: Path) -> None:
     deployment_root, config_folder = _write_binary_deployment(
         tmp_path, "sha256", "0" * 64
     )
-    with pytest.raises(AppBuilderError, match="hash check failed"):
+    with pytest.raises(
+        AppBuilderError,
+        match="Downloaded Binary .*/downloads/example-tool hash check failed",
+    ):
         run_cli("sync", "--from-scratch", deployment_root, config_folder)
diff --git a/tests/test_external_tools.py b/tests/test_external_tools.py
index 89d079b2..1ab95b05 100644
--- a/tests/test_external_tools.py
+++ b/tests/test_external_tools.py
@@ -13,5 +13,7 @@ def test_run_command_reports_missing_tool() -> None:
 def test_run_command_reports_failed_tool() -> None:
     # With check=True, a non-zero exit is reported as an ExternalToolError rather than a
     # raw CalledProcessError.
-    with pytest.raises(ExternalToolError, match="exit status"):
+    with pytest.raises(
+        ExternalToolError, match="External tool 'bash' failed with exit status 3"
+    ):
         run_command(["bash", "-c", "exit 3"], check=True)

From 0dbb91fb223f779caca391c385d28522a214e421 Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Wed, 8 Jul 2026 17:52:04 +0000
Subject: [PATCH 14/16] Reword test config descriptions to clarify use

---
 .../valid/multi-version-active/example-module-multi/1.0.yaml    | 2 +-
 .../valid/multi-version-active/example-module-multi/2.0.yaml    | 2 +-
 .../multi-version-deprecated/example-module-multi/1.0.yaml      | 2 +-
 .../multi-version-deprecated/example-module-multi/2.0.yaml      | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/configs/valid/multi-version-active/example-module-multi/1.0.yaml b/tests/configs/valid/multi-version-active/example-module-multi/1.0.yaml
index d006a584..0c60ddcf 100644
--- a/tests/configs/valid/multi-version-active/example-module-multi/1.0.yaml
+++ b/tests/configs/valid/multi-version-active/example-module-multi/1.0.yaml
@@ -3,6 +3,6 @@
 module:
   name: example-module-multi
   version: "1.0"
-  description: Module with two versions, used to exercise name-folder cleanup
+  description: Module with two versions
 
   applications: []
diff --git a/tests/configs/valid/multi-version-active/example-module-multi/2.0.yaml b/tests/configs/valid/multi-version-active/example-module-multi/2.0.yaml
index c6cd894b..b7e528c3 100644
--- a/tests/configs/valid/multi-version-active/example-module-multi/2.0.yaml
+++ b/tests/configs/valid/multi-version-active/example-module-multi/2.0.yaml
@@ -3,6 +3,6 @@
 module:
   name: example-module-multi
   version: "2.0"
-  description: Module with two versions, used to exercise name-folder cleanup
+  description: Module with two versions
 
   applications: []
diff --git a/tests/configs/valid/multi-version-deprecated/example-module-multi/1.0.yaml b/tests/configs/valid/multi-version-deprecated/example-module-multi/1.0.yaml
index 4452fbbc..99ff567c 100644
--- a/tests/configs/valid/multi-version-deprecated/example-module-multi/1.0.yaml
+++ b/tests/configs/valid/multi-version-deprecated/example-module-multi/1.0.yaml
@@ -3,7 +3,7 @@
 module:
   name: example-module-multi
   version: "1.0"
-  description: Module with two versions, used to exercise name-folder cleanup
+  description: Module with two versions
 
   applications: []
 
diff --git a/tests/configs/valid/multi-version-deprecated/example-module-multi/2.0.yaml b/tests/configs/valid/multi-version-deprecated/example-module-multi/2.0.yaml
index de2c0844..55ca590d 100644
--- a/tests/configs/valid/multi-version-deprecated/example-module-multi/2.0.yaml
+++ b/tests/configs/valid/multi-version-deprecated/example-module-multi/2.0.yaml
@@ -3,7 +3,7 @@
 module:
   name: example-module-multi
   version: "2.0"
-  description: Module with two versions, used to exercise name-folder cleanup
+  description: Module with two versions
 
   applications: []
 

From 119e3bef0ef9f320ed5a6e8fd89fcac5b04d3519 Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Tue, 14 Jul 2026 12:20:29 +0000
Subject: [PATCH 15/16] Update docstrings and comments for newer dependency
 pinning behaviour

Previously, all modules managed by the current Deployment required a
specific, pinned version if they were a dependency. This was then
relaxed, but docstrings and comments were left out of date.
---
 src/deploy_tools/models/module.py               | 11 +++++++----
 src/deploy_tools/models/schemas/deployment.json |  4 ++--
 src/deploy_tools/models/schemas/module.json     |  4 ++--
 src/deploy_tools/models/schemas/release.json    |  4 ++--
 src/deploy_tools/validate.py                    |  9 ++++-----
 5 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/src/deploy_tools/models/module.py b/src/deploy_tools/models/module.py
index cca0da37..a9f78857 100644
--- a/src/deploy_tools/models/module.py
+++ b/src/deploy_tools/models/module.py
@@ -20,16 +20,19 @@
 class ModuleDependency(ParentModel):
     """Specify an Environment Module to include as a dependency.
 
-    If the dependent Environment Module is managed by this same Deployment (i.e. is a
-    Module), you must specify a specific version in order to pass validation.
+    The version is optional. If a version is given and the dependency is a Module
+    managed by this same Deployment, that version must exist among the deployed
+    (non-deprecated) versions to pass validation. If no version is given, the
+    dependency's default version is resolved at load time and is not validated.
     """
 
     name: Annotated[str, Field(description="Name of module dependency")]
     version: Annotated[
         str | None,
         Field(
-            description="Version of dependency. Will use default if none specified, "
-            "but this is only valid for modules not managed using deploy-tools"
+            description="Version of dependency. If omitted, the default version is "
+            "resolved at load time. If given for a deploy-tools-managed module, the "
+            "version must exist in the deployment."
         ),
     ] = None
 
diff --git a/src/deploy_tools/models/schemas/deployment.json b/src/deploy_tools/models/schemas/deployment.json
index 346a5eeb..22fea3e2 100644
--- a/src/deploy_tools/models/schemas/deployment.json
+++ b/src/deploy_tools/models/schemas/deployment.json
@@ -354,7 +354,7 @@
     },
     "ModuleDependency": {
       "additionalProperties": false,
-      "description": "Specify an Environment Module to include as a dependency.\n\nIf the dependent Environment Module is managed by this same Deployment (i.e. is a\nModule), you must specify a specific version in order to pass validation.",
+      "description": "Specify an Environment Module to include as a dependency.\n\nThe version is optional. If a version is given and the dependency is a Module\nmanaged by this same Deployment, that version must exist among the deployed\n(non-deprecated) versions to pass validation. If no version is given, the\ndependency's default version is resolved at load time and is not validated.",
       "properties": {
         "name": {
           "description": "Name of module dependency",
@@ -371,7 +371,7 @@
             }
           ],
           "default": null,
-          "description": "Version of dependency. Will use default if none specified, but this is only valid for modules not managed using deploy-tools",
+          "description": "Version of dependency. If omitted, the default version is resolved at load time. If given for a deploy-tools-managed module, the version must exist in the deployment.",
           "title": "Version"
         }
       },
diff --git a/src/deploy_tools/models/schemas/module.json b/src/deploy_tools/models/schemas/module.json
index 1f5e5e7d..ccac6706 100644
--- a/src/deploy_tools/models/schemas/module.json
+++ b/src/deploy_tools/models/schemas/module.json
@@ -223,7 +223,7 @@
     },
     "ModuleDependency": {
       "additionalProperties": false,
-      "description": "Specify an Environment Module to include as a dependency.\n\nIf the dependent Environment Module is managed by this same Deployment (i.e. is a\nModule), you must specify a specific version in order to pass validation.",
+      "description": "Specify an Environment Module to include as a dependency.\n\nThe version is optional. If a version is given and the dependency is a Module\nmanaged by this same Deployment, that version must exist among the deployed\n(non-deprecated) versions to pass validation. If no version is given, the\ndependency's default version is resolved at load time and is not validated.",
       "properties": {
         "name": {
           "description": "Name of module dependency",
@@ -240,7 +240,7 @@
             }
           ],
           "default": null,
-          "description": "Version of dependency. Will use default if none specified, but this is only valid for modules not managed using deploy-tools",
+          "description": "Version of dependency. If omitted, the default version is resolved at load time. If given for a deploy-tools-managed module, the version must exist in the deployment.",
           "title": "Version"
         }
       },
diff --git a/src/deploy_tools/models/schemas/release.json b/src/deploy_tools/models/schemas/release.json
index 0176ee9a..c625f215 100644
--- a/src/deploy_tools/models/schemas/release.json
+++ b/src/deploy_tools/models/schemas/release.json
@@ -335,7 +335,7 @@
     },
     "ModuleDependency": {
       "additionalProperties": false,
-      "description": "Specify an Environment Module to include as a dependency.\n\nIf the dependent Environment Module is managed by this same Deployment (i.e. is a\nModule), you must specify a specific version in order to pass validation.",
+      "description": "Specify an Environment Module to include as a dependency.\n\nThe version is optional. If a version is given and the dependency is a Module\nmanaged by this same Deployment, that version must exist among the deployed\n(non-deprecated) versions to pass validation. If no version is given, the\ndependency's default version is resolved at load time and is not validated.",
       "properties": {
         "name": {
           "description": "Name of module dependency",
@@ -352,7 +352,7 @@
             }
           ],
           "default": null,
-          "description": "Version of dependency. Will use default if none specified, but this is only valid for modules not managed using deploy-tools",
+          "description": "Version of dependency. If omitted, the default version is resolved at load time. If given for a deploy-tools-managed module, the version must exist in the deployment.",
           "title": "Version"
         }
       },
diff --git a/src/deploy_tools/validate.py b/src/deploy_tools/validate.py
index 7883a72e..35518220 100644
--- a/src/deploy_tools/validate.py
+++ b/src/deploy_tools/validate.py
@@ -88,12 +88,11 @@ def _validate_release_changes(
 
 
 def _validate_module_dependencies(deployment: Deployment) -> None:
-    """Ensure that all module dependencies are set appropriately.
+    """Ensure that all module dependencies reference versions that will exist.
 
-    This checks any module dependency names that come from current configuration to
-    ensure they exist and are not deprecated. Not specifying a particular version is
-    only valid for dependencies that are managed outside of the current deployment
-    configuration.
+    Only dependencies that pin a version are checked: if the dependency is a Module
+    managed by this deployment, the pinned version must be present among the final
+    deployed (non-deprecated) versions.
     """
     final_deployed_versions = deployment.get_final_deployed_versions()
 

From a00da36e3ed4bc2b073c1eb5a1df2005323ea71c Mon Sep 17 00:00:00 2001
From: Martin Gaughran <martin.gaughran@diamond.ac.uk>
Date: Tue, 14 Jul 2026 12:22:17 +0000
Subject: [PATCH 16/16] Fix test_dependencies_render_module_load_lines comments
 for accuracy

---
 tests/test_module_options.py | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/tests/test_module_options.py b/tests/test_module_options.py
index 477f4d6b..d0f1f3c5 100644
--- a/tests/test_module_options.py
+++ b/tests/test_module_options.py
@@ -130,14 +130,21 @@ def test_apptainer_options_merge_global_and_entrypoint(
 
 
 def test_dependencies_render_module_load_lines(tmp_path: Path) -> None:
-    # The modulefile dependency block branches on whether a version is given: a managed
-    # dependency pins a version, while a version-less dependency (valid only for modules
-    # not managed by deploy-tools) renders the bare module name.
-    dependency = Release(
+    # The modulefile dependency block branches on whether a version is given: a pinned
+    # dependency renders `name/version`, while a version-less one renders the bare name.
+    pinned = Release(
         module=Module(
-            name="example-dependency",
+            name="example-pinned-dep",
             version="1.0",
-            description="Managed module used as a versioned dependency target",
+            description="Managed module referenced with a pinned version",
+            applications=[],
+        )
+    )
+    default = Release(
+        module=Module(
+            name="example-default-dep",
+            version="1.0",
+            description="Managed module referenced without a version",
             applications=[],
         )
     )
@@ -145,24 +152,24 @@ def test_dependencies_render_module_load_lines(tmp_path: Path) -> None:
         module=Module(
             name="example-dependent",
             version="1.0",
-            description="Module with a managed and an external dependency",
+            description="Module with a pinned and a version-less managed dependency",
             dependencies=[
-                ModuleDependency(name="example-dependency", version="1.0"),
-                ModuleDependency(name="external-module"),
+                ModuleDependency(name="example-pinned-dep", version="1.0"),
+                ModuleDependency(name="example-default-dep"),
             ],
             applications=[],
         )
     )
-    deployment_root = _sync(tmp_path, dependency, dependent)
+    deployment_root = _sync(tmp_path, pinned, default, dependent)
 
     modulefile = (
         deployment_root / "modules/example-dependent/1.0/modulefile"
     ).read_text()
 
-    # Versioned dependency renders with its version. The version-less external one
-    # renders the bare module name.
-    assert "module load example-dependency/1.0" in modulefile
-    assert "module load external-module\n" in modulefile
+    # Pinned dependency renders with its version; the version-less one renders the bare
+    # module name (and loads the default).
+    assert "module load example-pinned-dep/1.0" in modulefile
+    assert "module load example-default-dep\n" in modulefile
 
 
 def test_env_vars_render_setenv(tmp_path: Path) -> None: