From 9e54a3d1d49fd6a665493b1074c8b575006fc9b8 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Mon, 29 Jun 2026 11:01:23 +0000 Subject: [PATCH 01/28] Add test for CLI error handling --- tests/test_cli.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index cdbd77b1..be6ca5e6 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,5 +1,6 @@ import subprocess import sys +from pathlib import Path from deploy_tools import __version__ @@ -7,3 +8,16 @@ def test_cli_version() -> None: cmd = [sys.executable, "-m", "deploy_tools", "--version"] assert subprocess.check_output(cmd).decode().strip() == __version__ + + +def test_cli_reports_domain_error_without_traceback(tmp_path: Path) -> None: + # compare on an existing area with no snapshot raises SnapshotError, a + # DeployToolsError. Run the real entry point and confirm the message is presented as + # a clean 'Error: ...' on stderr with exit 1, and no traceback is dumped. + cmd = [sys.executable, "-m", "deploy_tools", "compare", str(tmp_path)] + result = subprocess.run(cmd, capture_output=True, text=True) + + assert result.returncode == 1 + assert "Error:" in result.stderr + assert "snapshot not found" in result.stderr + assert "Traceback" not in result.stderr From c1698424d21fed45cd56c9594eba366acff01fc9 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Fri, 26 Jun 2026 17:04:22 +0000 Subject: [PATCH 02/28] Add behavioural tests for validate command This covers cases that aren't easily tested with golden master tests. --- .../example-module-shell/1.0.yaml | 19 ++++++++ .../all-excluded-defaults/settings.yaml | 3 ++ .../example-module-shell/1.0.yaml | 17 +++++++ .../invalid/missing-default/settings.yaml | 4 ++ .../example-module-shell/1.0.yaml | 17 +++++++ .../modified-no-allow-updates/settings.yaml | 3 ++ .../removed-no-deprecation/settings.yaml | 3 ++ .../example-module-base/1.0.yaml | 8 ++++ .../example-module-dependent/1.0.yaml | 12 +++++ .../invalid/unknown-dependency/settings.yaml | 3 ++ .../minimal/example-module-shell/1.0.yaml | 17 +++++++ tests/configs/minimal/settings.yaml | 3 ++ tests/test_validate.py | 47 +++++++++++++++++++ 13 files changed, 156 insertions(+) create mode 100644 tests/configs/invalid/all-excluded-defaults/example-module-shell/1.0.yaml create mode 100644 tests/configs/invalid/all-excluded-defaults/settings.yaml create mode 100644 tests/configs/invalid/missing-default/example-module-shell/1.0.yaml create mode 100644 tests/configs/invalid/missing-default/settings.yaml create mode 100644 tests/configs/invalid/modified-no-allow-updates/example-module-shell/1.0.yaml create mode 100644 tests/configs/invalid/modified-no-allow-updates/settings.yaml create mode 100644 tests/configs/invalid/removed-no-deprecation/settings.yaml create mode 100644 tests/configs/invalid/unknown-dependency/example-module-base/1.0.yaml create mode 100644 tests/configs/invalid/unknown-dependency/example-module-dependent/1.0.yaml create mode 100644 tests/configs/invalid/unknown-dependency/settings.yaml create mode 100644 tests/configs/minimal/example-module-shell/1.0.yaml create mode 100644 tests/configs/minimal/settings.yaml create mode 100644 tests/test_validate.py diff --git a/tests/configs/invalid/all-excluded-defaults/example-module-shell/1.0.yaml b/tests/configs/invalid/all-excluded-defaults/example-module-shell/1.0.yaml new file mode 100644 index 00000000..92da05df --- /dev/null +++ b/tests/configs/invalid/all-excluded-defaults/example-module-shell/1.0.yaml @@ -0,0 +1,19 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json + +module: + name: example-module-shell + version: "1.0" + description: Shell module excluded from defaults with no eligible fallback + + env_vars: + - name: SHELL_VALUE + value: hello + + applications: + - app_type: shell + + name: test-shell-echo + script: + - echo $SHELL_VALUE + + exclude_from_defaults: true diff --git a/tests/configs/invalid/all-excluded-defaults/settings.yaml b/tests/configs/invalid/all-excluded-defaults/settings.yaml new file mode 100644 index 00000000..70f60017 --- /dev/null +++ b/tests/configs/invalid/all-excluded-defaults/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/invalid/missing-default/example-module-shell/1.0.yaml b/tests/configs/invalid/missing-default/example-module-shell/1.0.yaml new file mode 100644 index 00000000..e8f3c4a7 --- /dev/null +++ b/tests/configs/invalid/missing-default/example-module-shell/1.0.yaml @@ -0,0 +1,17 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json + +module: + name: example-module-shell + version: "1.0" + description: Minimal shell-only module used by the validate tests + + env_vars: + - name: SHELL_VALUE + value: hello + + applications: + - app_type: shell + + name: test-shell-echo + script: + - echo $SHELL_VALUE diff --git a/tests/configs/invalid/missing-default/settings.yaml b/tests/configs/invalid/missing-default/settings.yaml new file mode 100644 index 00000000..33cecd08 --- /dev/null +++ b/tests/configs/invalid/missing-default/settings.yaml @@ -0,0 +1,4 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/deployment-settings.json + +default_versions: + example-module-shell: "2.0" diff --git a/tests/configs/invalid/modified-no-allow-updates/example-module-shell/1.0.yaml b/tests/configs/invalid/modified-no-allow-updates/example-module-shell/1.0.yaml new file mode 100644 index 00000000..69c07817 --- /dev/null +++ b/tests/configs/invalid/modified-no-allow-updates/example-module-shell/1.0.yaml @@ -0,0 +1,17 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json + +module: + name: example-module-shell + version: "1.0" + description: Minimal shell-only module used by the validate tests + + env_vars: + - name: SHELL_VALUE + value: changed + + applications: + - app_type: shell + + name: test-shell-echo + script: + - echo $SHELL_VALUE diff --git a/tests/configs/invalid/modified-no-allow-updates/settings.yaml b/tests/configs/invalid/modified-no-allow-updates/settings.yaml new file mode 100644 index 00000000..70f60017 --- /dev/null +++ b/tests/configs/invalid/modified-no-allow-updates/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/invalid/removed-no-deprecation/settings.yaml b/tests/configs/invalid/removed-no-deprecation/settings.yaml new file mode 100644 index 00000000..70f60017 --- /dev/null +++ b/tests/configs/invalid/removed-no-deprecation/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/invalid/unknown-dependency/example-module-base/1.0.yaml b/tests/configs/invalid/unknown-dependency/example-module-base/1.0.yaml new file mode 100644 index 00000000..7e8c3115 --- /dev/null +++ b/tests/configs/invalid/unknown-dependency/example-module-base/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-base + version: "1.0" + description: Base module that the dependent module references + + applications: [] diff --git a/tests/configs/invalid/unknown-dependency/example-module-dependent/1.0.yaml b/tests/configs/invalid/unknown-dependency/example-module-dependent/1.0.yaml new file mode 100644 index 00000000..dc38a528 --- /dev/null +++ b/tests/configs/invalid/unknown-dependency/example-module-dependent/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-dependent + version: "1.0" + description: References a version of example-module-base that does not exist + + dependencies: + - name: example-module-base + version: "9.9" + + applications: [] diff --git a/tests/configs/invalid/unknown-dependency/settings.yaml b/tests/configs/invalid/unknown-dependency/settings.yaml new file mode 100644 index 00000000..70f60017 --- /dev/null +++ b/tests/configs/invalid/unknown-dependency/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/minimal/example-module-shell/1.0.yaml b/tests/configs/minimal/example-module-shell/1.0.yaml new file mode 100644 index 00000000..e8f3c4a7 --- /dev/null +++ b/tests/configs/minimal/example-module-shell/1.0.yaml @@ -0,0 +1,17 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json + +module: + name: example-module-shell + version: "1.0" + description: Minimal shell-only module used by the validate tests + + env_vars: + - name: SHELL_VALUE + value: hello + + applications: + - app_type: shell + + name: test-shell-echo + script: + - echo $SHELL_VALUE diff --git a/tests/configs/minimal/settings.yaml b/tests/configs/minimal/settings.yaml new file mode 100644 index 00000000..70f60017 --- /dev/null +++ b/tests/configs/minimal/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_validate.py b/tests/test_validate.py new file mode 100644 index 00000000..d0ef0b15 --- /dev/null +++ b/tests/test_validate.py @@ -0,0 +1,47 @@ +from pathlib import Path + +import pytest + +from conftest import run_cli +from deploy_tools.validate import ValidationError + +# Configurations that 'validate' should reject when deployed from scratch into an empty +# area. Each maps a config folder under configs/invalid to a substring expected in the +# resulting ValidationError. +INVALID_INITIAL_CONFIGS = [ + ("unknown-dependency", "unknown module dependency"), + ("missing-default", "Unable to configure"), + ("all-excluded-defaults", "exclude_from_defaults"), +] + + +@pytest.mark.parametrize("config_name, message", INVALID_INITIAL_CONFIGS) +def test_validate_rejects_invalid_initial_config( + tmp_path: Path, configs: Path, config_name: str, message: str +): + with pytest.raises(ValidationError, match=message): + run_cli( + "validate", "--from-scratch", tmp_path, configs / "invalid" / config_name + ) + + +def test_validate_rejects_update_without_allow_updates(tmp_path: Path, configs: Path): + # Deploy a baseline whose module did not opt in to allow_updates, then validate a + # config that changes that module in place: this must be rejected. + run_cli("sync", "--from-scratch", tmp_path, configs / "minimal") + with pytest.raises(ValidationError, match="modified without updating version"): + run_cli("validate", tmp_path, configs / "invalid" / "modified-no-allow-updates") + + +def test_validate_rejects_removal_without_deprecation(tmp_path: Path, configs: Path): + # Deploy a baseline module, then validate a config that drops it without first + # deprecating it (and without --allow-all): this must be rejected. + run_cli("sync", "--from-scratch", tmp_path, configs / "minimal") + with pytest.raises(ValidationError, match="removed without prior deprecation"): + run_cli("validate", tmp_path, configs / "invalid" / "removed-no-deprecation") + + +def test_validate_test_build(tmp_path: Path, configs: Path): + # --test-build actually builds the modules into a temporary area and syntax-checks + # the generated shell entrypoints, so exercise it on a valid shell-only config. + run_cli("validate", "--test-build", "--from-scratch", tmp_path, configs / "minimal") From a08460b65a38f3187c8d7201e0f4e42d1cca8945 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Mon, 15 Jun 2026 18:13:01 +0000 Subject: [PATCH 03/28] Have compare command dump values in JSON mode to support special types --- src/deploy_tools/compare.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/deploy_tools/compare.py b/src/deploy_tools/compare.py index 61a0ce96..16ddaa0e 100644 --- a/src/deploy_tools/compare.py +++ b/src/deploy_tools/compare.py @@ -194,4 +194,6 @@ def _get_dict_diff(d1: dict[str, Any], d2: dict[str, Any]) -> str: def _yaml_dumps(obj: dict[str, Any], indent: int | None = None) -> str: ta = TypeAdapter(dict[str, Any]) - return yaml.safe_dump(ta.dump_python(obj), indent=indent) + # Dump in JSON mode so pydantic special types (e.g. AnyUrl) are coerced to YAML-safe + # primitives + return yaml.safe_dump(ta.dump_python(obj, mode="json"), indent=indent) From 04c3d6cda990b91e61110f43513d257db0896b26 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Mon, 15 Jun 2026 19:04:17 +0000 Subject: [PATCH 04/28] Add separate tests for the compare command Tests relating to non-existent output dir use the compare function as otherwise the Typer configuration prevents the exception being raised. --- tests/test_compare.py | 140 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 tests/test_compare.py diff --git a/tests/test_compare.py b/tests/test_compare.py new file mode 100644 index 00000000..c6a858d4 --- /dev/null +++ b/tests/test_compare.py @@ -0,0 +1,140 @@ +from pathlib import Path +from shutil import rmtree + +import pytest + +from conftest import run_cli +from deploy_tools.compare import ComparisonError, compare_to_snapshot +from deploy_tools.layout import Layout +from deploy_tools.snapshot import SnapshotError + +# The minimal config deploys a single shell-only module. Tests that start from a clean +# deployment and then corrupt it in one specific way share these coordinates. +MODULE_NAME = "example-module-shell" +MODULE_VERSION = "1.0" + + +def _sync_minimal(area: Path, configs: Path) -> Layout: + """Deploy the minimal shell-only config into ``area`` and return its ``Layout``. + + Several tests below need a single, cleanly-deployed module that they then corrupt in + one specific way, so this captures the shared from-scratch sync. + + Args: + area: Empty deployment area to deploy into. + configs: The ``configs`` fixture: the directory holding sample configurations. + + Returns: + A ``Layout`` describing the freshly-deployed ``area``. + """ + run_cli("sync", "--from-scratch", area, configs / "minimal") + return Layout(area) + + +def test_compare_accepts_clean_deployment(tmp_path: Path, configs: Path): + # A deployment area is self-consistent immediately after a sync, so compare should + # succeed and print nothing. + _sync_minimal(tmp_path, configs) + assert run_cli("compare", tmp_path) == "" + + +def test_compare_accepts_deprecated_modules(tmp_path: Path, configs: Path): + # Ensure compare runs successfully with deprecated modulefile links. + run_cli("sync", "--from-scratch", tmp_path, configs / "04-deprecated") + assert run_cli("compare", tmp_path) == "" + + +def test_compare_from_scratch_accepts_empty_area(tmp_path: Path): + # --from-scratch only checks that the area is an existing, empty directory. + assert run_cli("compare", "--from-scratch", tmp_path) == "" + + +def test_compare_from_scratch_rejects_non_empty_area(tmp_path: Path): + # Any pre-existing content means the area is not ready for a from-scratch deploy. + (tmp_path / "stray-file").touch() + with pytest.raises(ComparisonError, match="not empty"): + run_cli("compare", "--from-scratch", tmp_path) + + +def test_compare_rejects_missing_snapshot(tmp_path: Path): + # An existing area with no deployment.yaml has no snapshot to compare against. + with pytest.raises(SnapshotError, match="snapshot not found"): + run_cli("compare", tmp_path) + + +def test_compare_rejects_module_without_modulefile(tmp_path: Path, configs: Path): + # Removing a module's modulefile link leaves a built module that is not exposed. + layout = _sync_minimal(tmp_path, configs) + layout.get_modulefile_link(MODULE_NAME, MODULE_VERSION).unlink() + with pytest.raises(ComparisonError, match="No modulefile found"): + run_cli("compare", tmp_path) + + +def test_compare_rejects_modulefile_without_module(tmp_path: Path, configs: Path): + # Removing the built module leaves a dangling modulefile link + layout = _sync_minimal(tmp_path, configs) + rmtree(layout.get_module_folder(MODULE_NAME, MODULE_VERSION)) + with pytest.raises(ComparisonError, match="without corresponding built module"): + run_cli("compare", tmp_path) + + +def test_compare_rejects_duplicate_modulefiles(tmp_path: Path, configs: Path): + # The same modulefile must not appear in both the live and deprecated areas. + layout = _sync_minimal(tmp_path, configs) + deprecated_link = layout.get_modulefile_link( + MODULE_NAME, MODULE_VERSION, from_deprecated=True + ) + deprecated_link.parent.mkdir(parents=True, exist_ok=True) + deprecated_link.touch() + with pytest.raises(ComparisonError, match="Duplicate modulefiles"): + run_cli("compare", tmp_path) + + +def test_compare_rejects_release_mismatch(tmp_path: Path, configs: Path): + # Tamper with the snapshot's record of the module so it no longer matches the module + # configuration reconstructed from the deployment area. + layout = _sync_minimal(tmp_path, configs) + snapshot = layout.deployment_snapshot_path + contents = snapshot.read_text() + assert "Minimal shell-only module" in contents + snapshot.write_text( + contents.replace("Minimal shell-only module", "Tampered module") + ) + with pytest.raises(ComparisonError, match="release configuration do not match"): + run_cli("compare", tmp_path) + + +def test_compare_rejects_default_version_mismatch(tmp_path: Path, configs: Path): + # Point the module's .version file at a different version than the snapshot expects. + layout = _sync_minimal(tmp_path, configs) + version_file = layout.get_default_version_file(MODULE_NAME) + version_file.write_text("#%Module1.0\nset ModulesVersion 9.9\n") + with pytest.raises(ComparisonError, match="default versions do not match"): + run_cli("compare", tmp_path) + + +def test_compare_use_ref_detects_drift(tmp_path: Path, configs: Path): + # sync commits a snapshot to the deployment area's git repo on every run. After two + # syncs the area matches its own (HEAD) snapshot, but not the previous commit's + # snapshot, which predates the module added by the second sync. + run_cli("sync", "--from-scratch", tmp_path, configs / "01-initial") + run_cli("sync", tmp_path, configs / "02-added") + + assert run_cli("compare", tmp_path) == "" + assert run_cli("compare", "--use-ref", "HEAD", tmp_path) == "" + + with pytest.raises(ComparisonError, match="release configuration do not match"): + run_cli("compare", "--use-ref", "HEAD~1", tmp_path) + + +def test_compare_from_scratch_rejects_missing_root(tmp_path: Path): + # The CLI's argument validation rejects a non-existent path before the command runs, + # so exercise this guard by calling the function directly. + with pytest.raises(ComparisonError, match="does not exist"): + compare_to_snapshot(tmp_path / "does-not-exist", from_scratch=True) + + +def test_compare_rejects_missing_root(tmp_path: Path): + # As above, but for the snapshot-loading path used by a non from-scratch compare. + with pytest.raises(SnapshotError, match="does not exist"): + compare_to_snapshot(tmp_path / "does-not-exist") From bd94f8bb5f9083e0b6d8a61e3e57b56ee2102491 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Mon, 15 Jun 2026 19:05:17 +0000 Subject: [PATCH 05/28] Only search non-deprecated modules for default versions in compare --- src/deploy_tools/compare.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/deploy_tools/compare.py b/src/deploy_tools/compare.py index 16ddaa0e..dbdd9432 100644 --- a/src/deploy_tools/compare.py +++ b/src/deploy_tools/compare.py @@ -87,10 +87,15 @@ def _reconstruct_deployment_config_from_modules(layout: Layout) -> Deployment: Note that the default versions will be different to those in initial configuration. """ releases = _collect_releases(layout) - default_versions = _collect_default_modulefile_versions(layout, list(releases)) - settings = DeploymentSettings(default_versions=default_versions) + deployment = Deployment(settings=DeploymentSettings(), releases=releases) - return Deployment(settings=settings, releases=releases) + # Only non-deprecated (live) modules are associated with a .version file + live_names = list(deployment.get_final_deployed_modules()) + deployment.settings.default_versions = _collect_default_modulefile_versions( + layout, live_names + ) + + return deployment def _collect_releases(layout: Layout) -> ReleasesByNameAndVersion: From 06cbd0c11b4222796342e55e8484c2532ab313c8 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Tue, 16 Jun 2026 11:19:49 +0000 Subject: [PATCH 06/28] Add test_sync.py to test double-deprecation and removal issues --- src/deploy_tools/deploy.py | 4 +++ 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 +++++++++++++++++++ 9 files changed, 80 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/src/deploy_tools/deploy.py b/src/deploy_tools/deploy.py index 939c13cd..e85220f2 100644 --- a/src/deploy_tools/deploy.py +++ b/src/deploy_tools/deploy.py @@ -155,6 +155,10 @@ def _delete_modulefiles_name_folder( ) -> None: modulefiles_name_path = layout.get_modulefiles_root(from_deprecated) / name + # Handle several versions of the same module being processed together + if not modulefiles_name_path.exists(): + return + # Ignore the default version file when checking for existing modulefile links if next(modulefiles_name_path.glob(MODULE_VERSIONS_GLOB), None) is None: shutil.rmtree(modulefiles_name_path) 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 8f8ca4beaa67b98dc50f0b798c7d44ad4ae336ee Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Wed, 17 Jun 2026 10:02:31 +0000 Subject: [PATCH 07/28] Remove previous snapshot dead code from Layout This feature was replaced with the git ref option to the compare command. --- src/deploy_tools/layout.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/deploy_tools/layout.py b/src/deploy_tools/layout.py index 3d8b98d7..e57a0300 100644 --- a/src/deploy_tools/layout.py +++ b/src/deploy_tools/layout.py @@ -62,7 +62,6 @@ class Layout: DEFAULT_VERSION_FILENAME = ".version" DEPLOYMENT_SNAPSHOT_FILENAME = "deployment.yaml" - PREVIOUS_DEPLOYMENT_SNAPSHOT_FILENAME = "previous-deployment.yaml" def __init__(self, deployment_root: Path, build_root: Path | None = None) -> None: self._root = deployment_root From 96420879a59692d8152e3ac130c4ccae97e9212b Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Wed, 17 Jun 2026 10:08:34 +0000 Subject: [PATCH 08/28] Improve exception handling for compare command and snapshots --- src/deploy_tools/compare.py | 7 ++++++- src/deploy_tools/snapshot.py | 22 ++++++++++++++++++---- tests/test_compare.py | 26 ++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/src/deploy_tools/compare.py b/src/deploy_tools/compare.py index dbdd9432..5123a066 100644 --- a/src/deploy_tools/compare.py +++ b/src/deploy_tools/compare.py @@ -157,7 +157,12 @@ def _collect_default_modulefile_versions( default_versions: dict[str, str] = {} for name in names: - default_version = get_default_modulefile_version(name, layout) + try: + default_version = get_default_modulefile_version(name, layout) + except FileNotFoundError as exc: + raise ComparisonError( + f"Live module '{name}' has no default version file" + ) from exc if default_version is not None: default_versions[name] = default_version diff --git a/src/deploy_tools/snapshot.py b/src/deploy_tools/snapshot.py index ad2c2540..e29fcc51 100644 --- a/src/deploy_tools/snapshot.py +++ b/src/deploy_tools/snapshot.py @@ -1,7 +1,9 @@ import io import logging -from git import Repo +import yaml +from git import BadName, Repo +from pydantic import ValidationError as PydanticValidationError from .errors import DeployToolsError from .layout import Layout @@ -54,14 +56,26 @@ def load_snapshot(layout: Layout, from_scratch: bool = False) -> Deployment: ) logger.debug("Loading snapshot: %s", layout.deployment_snapshot_path) - with open(layout.deployment_snapshot_path) as f: - return load_from_yaml(Deployment, f) + try: + with open(layout.deployment_snapshot_path) as f: + return load_from_yaml(Deployment, f) + except (OSError, yaml.YAMLError, PydanticValidationError, TypeError) as exc: + raise SnapshotError( + f"Deployment snapshot could not be read:\n{layout.deployment_snapshot_path}" + ) from exc def load_snapshot_from_ref(layout: Layout, ref: str) -> Deployment: """Load the deployment snapshot from the given git ref of the deployment area.""" logger.debug("Loading snapshot from ref: %s", ref) repo = Repo(layout.deployment_root) - ref_snapshot = repo.commit(ref).tree[layout.DEPLOYMENT_SNAPSHOT_FILENAME] + + try: + ref_snapshot = repo.commit(ref).tree[layout.DEPLOYMENT_SNAPSHOT_FILENAME] + except (BadName, KeyError) as exc: + raise SnapshotError( + f"Deployment snapshot not found at git ref:\n{ref}" + ) from exc + with io.BytesIO(ref_snapshot.data_stream.read()) as snapshot_f: # type: ignore return load_from_yaml(Deployment, snapshot_f) diff --git a/tests/test_compare.py b/tests/test_compare.py index c6a858d4..6432955b 100644 --- a/tests/test_compare.py +++ b/tests/test_compare.py @@ -138,3 +138,29 @@ def test_compare_rejects_missing_root(tmp_path: Path): # As above, but for the snapshot-loading path used by a non from-scratch compare. with pytest.raises(SnapshotError, match="does not exist"): compare_to_snapshot(tmp_path / "does-not-exist") + + +def test_compare_rejects_missing_default_version_file(tmp_path: Path, configs: Path): + # A live module records its default version in a .version file; a missing one is + # corruption that compare must report cleanly, not as a raw FileNotFoundError. + layout = _sync_minimal(tmp_path, configs) + layout.get_default_version_file(MODULE_NAME).unlink() + with pytest.raises(ComparisonError, match="default version file"): + run_cli("compare", tmp_path) + + +def test_compare_rejects_unreadable_snapshot(tmp_path: Path, configs: Path): + # A corrupt deployment.yaml (e.g. truncated by a failed write) must surface as a + # SnapshotError rather than a raw parser error. + layout = _sync_minimal(tmp_path, configs) + layout.deployment_snapshot_path.write_text("") + with pytest.raises(SnapshotError, match="could not be read"): + run_cli("compare", tmp_path) + + +def test_compare_rejects_unknown_git_ref(tmp_path: Path, configs: Path): + # An unresolvable --use-ref should fail with a clear SnapshotError, not a raw + # GitPython BadName error. + _sync_minimal(tmp_path, configs) + with pytest.raises(SnapshotError, match="git ref"): + run_cli("compare", "--use-ref", "no-such-ref", tmp_path) From 5f243d7ec9cc10918a8a1921865def82eb459771 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Wed, 17 Jun 2026 12:14:09 +0000 Subject: [PATCH 09/28] Fix typo in validate.py for allow_all, and provide relevant tests --- src/deploy_tools/validate.py | 4 ++-- .../example-module-shell/1.0.yaml | 17 +++++++++++++++ .../example-module-shell/2.0.yaml | 21 +++++++++++++++++++ .../invalid/added-deprecated/settings.yaml | 3 +++ tests/test_validate.py | 20 ++++++++++++++++++ 5 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 tests/configs/invalid/added-deprecated/example-module-shell/1.0.yaml create mode 100644 tests/configs/invalid/added-deprecated/example-module-shell/2.0.yaml create mode 100644 tests/configs/invalid/added-deprecated/settings.yaml diff --git a/src/deploy_tools/validate.py b/src/deploy_tools/validate.py index 84bdd533..7883a72e 100644 --- a/src/deploy_tools/validate.py +++ b/src/deploy_tools/validate.py @@ -157,11 +157,11 @@ def _get_release_changes( return release_changes -def _validate_added_modules(releases: list[Release], from_scratch: bool) -> None: +def _validate_added_modules(releases: list[Release], allow_all: bool) -> None: for release in releases: module = release.module if release.deprecated: - if not from_scratch: + if not allow_all: raise ValidationError( f"Module {module.name}/{module.version} cannot have deprecated " f"status on initial creation." diff --git a/tests/configs/invalid/added-deprecated/example-module-shell/1.0.yaml b/tests/configs/invalid/added-deprecated/example-module-shell/1.0.yaml new file mode 100644 index 00000000..e8f3c4a7 --- /dev/null +++ b/tests/configs/invalid/added-deprecated/example-module-shell/1.0.yaml @@ -0,0 +1,17 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json + +module: + name: example-module-shell + version: "1.0" + description: Minimal shell-only module used by the validate tests + + env_vars: + - name: SHELL_VALUE + value: hello + + applications: + - app_type: shell + + name: test-shell-echo + script: + - echo $SHELL_VALUE diff --git a/tests/configs/invalid/added-deprecated/example-module-shell/2.0.yaml b/tests/configs/invalid/added-deprecated/example-module-shell/2.0.yaml new file mode 100644 index 00000000..c857eea9 --- /dev/null +++ b/tests/configs/invalid/added-deprecated/example-module-shell/2.0.yaml @@ -0,0 +1,21 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json + +module: + name: example-module-shell + version: "2.0" + description: Minimal shell-only module used by the validate tests + + env_vars: + - name: SHELL_VALUE + value: hello + + applications: + - app_type: shell + + name: test-shell-echo + script: + - echo $SHELL_VALUE + +# A brand-new release introduced directly in a deprecated state. This is only permitted +# with --allow-all (or a from-scratch deploy); a normal deploy must reject it. +deprecated: true diff --git a/tests/configs/invalid/added-deprecated/settings.yaml b/tests/configs/invalid/added-deprecated/settings.yaml new file mode 100644 index 00000000..70f60017 --- /dev/null +++ b/tests/configs/invalid/added-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_validate.py b/tests/test_validate.py index d0ef0b15..a1eb7d0c 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -41,6 +41,26 @@ def test_validate_rejects_removal_without_deprecation(tmp_path: Path, configs: P run_cli("validate", tmp_path, configs / "invalid" / "removed-no-deprecation") +def test_validate_rejects_added_deprecated_module(tmp_path: Path, configs: Path): + # Deploy a baseline module, then validate a config that introduces a brand-new + # release already in a deprecated state. Deprecating a module on initial creation + # is only allowed with --allow-all, so a normal validate must reject it. + run_cli("sync", "--from-scratch", tmp_path, configs / "minimal") + with pytest.raises(ValidationError, match="cannot have deprecated status"): + run_cli("validate", tmp_path, configs / "invalid" / "added-deprecated") + + +def test_validate_allows_added_deprecated_module_with_allow_all( + tmp_path: Path, configs: Path +): + # The --allow-all flag deliberately permits introducing an already-deprecated + # release on initial creation. + run_cli("sync", "--from-scratch", tmp_path, configs / "minimal") + run_cli( + "validate", "--allow-all", tmp_path, configs / "invalid" / "added-deprecated" + ) + + def test_validate_test_build(tmp_path: Path, configs: Path): # --test-build actually builds the modules into a temporary area and syntax-checks # the generated shell entrypoints, so exercise it on a valid shell-only config. From 0fc31b4b7da55bb44924b191667378cabe4d1e8f Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Wed, 17 Jun 2026 13:36:44 +0000 Subject: [PATCH 10/28] Use pathlib symlink_to instead of os function --- src/deploy_tools/deploy.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/deploy_tools/deploy.py b/src/deploy_tools/deploy.py index e85220f2..9ea3c58b 100644 --- a/src/deploy_tools/deploy.py +++ b/src/deploy_tools/deploy.py @@ -1,5 +1,4 @@ import logging -import os import shutil from pathlib import Path @@ -69,7 +68,7 @@ def _deploy_new_releases(to_add: list[Release], layout: Layout) -> None: mf_link = layout.get_modulefile_link(name, version, from_deprecated=deprecated) mf_link.parent.mkdir(parents=True, exist_ok=True) - os.symlink(modulefile, mf_link) + mf_link.symlink_to(modulefile) def _deploy_releases( From 6f85eff7e29fbec87ecc77297aa29550ddd624d4 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Wed, 17 Jun 2026 13:39:40 +0000 Subject: [PATCH 11/28] Use Layout variables to calculate .gitignore paths --- src/deploy_tools/sync.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/deploy_tools/sync.py b/src/deploy_tools/sync.py index fb7d105a..e17a6fc6 100644 --- a/src/deploy_tools/sync.py +++ b/src/deploy_tools/sync.py @@ -5,7 +5,7 @@ from .build import build, clean_build_area from .deploy import deploy_changes -from .layout import Layout +from .layout import Layout, ModuleBuildLayout from .models.save_and_load import load_deployment from .snapshot import create_snapshot, load_snapshot from .templater import Templater, TemplateType @@ -15,7 +15,12 @@ logger = logging.getLogger(__name__) -IGNORE_DIRS = ["/build", "/modules/*/*/sif_files"] +# Files not tracked in the deployment area's git repo (kept only as a change reference): +# the transient build area and the large Apptainer .sif images. +IGNORE_DIRS = [ + f"/{Layout.DEFAULT_BUILD_ROOT_NAME}", + f"/{Layout.MODULES_ROOT_NAME}/*/*/{ModuleBuildLayout.SIF_FILES_FOLDER}", +] def synchronise( From 662c578fcfe0972fb9fd32c6fb586ec2d42e1f74 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Mon, 29 Jun 2026 10:45:22 +0000 Subject: [PATCH 12/28] 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 3662b0116e72803c8366250f1b61a5d495bc4942 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Fri, 26 Jun 2026 17:23:33 +0000 Subject: [PATCH 13/28] Add consistent type hinting to all CLI tests --- tests/test_compare.py | 40 +++++++++++++++++++++--------------- tests/test_external_tools.py | 4 ++-- tests/test_sync.py | 2 +- tests/test_validate.py | 18 ++++++++++------ 4 files changed, 39 insertions(+), 25 deletions(-) diff --git a/tests/test_compare.py b/tests/test_compare.py index 6432955b..89546cd2 100644 --- a/tests/test_compare.py +++ b/tests/test_compare.py @@ -31,38 +31,40 @@ def _sync_minimal(area: Path, configs: Path) -> Layout: return Layout(area) -def test_compare_accepts_clean_deployment(tmp_path: Path, configs: Path): +def test_compare_accepts_clean_deployment(tmp_path: Path, configs: Path) -> None: # A deployment area is self-consistent immediately after a sync, so compare should # succeed and print nothing. _sync_minimal(tmp_path, configs) assert run_cli("compare", tmp_path) == "" -def test_compare_accepts_deprecated_modules(tmp_path: Path, configs: Path): +def test_compare_accepts_deprecated_modules(tmp_path: Path, configs: Path) -> None: # Ensure compare runs successfully with deprecated modulefile links. run_cli("sync", "--from-scratch", tmp_path, configs / "04-deprecated") assert run_cli("compare", tmp_path) == "" -def test_compare_from_scratch_accepts_empty_area(tmp_path: Path): +def test_compare_from_scratch_accepts_empty_area(tmp_path: Path) -> None: # --from-scratch only checks that the area is an existing, empty directory. assert run_cli("compare", "--from-scratch", tmp_path) == "" -def test_compare_from_scratch_rejects_non_empty_area(tmp_path: Path): +def test_compare_from_scratch_rejects_non_empty_area(tmp_path: Path) -> None: # Any pre-existing content means the area is not ready for a from-scratch deploy. (tmp_path / "stray-file").touch() with pytest.raises(ComparisonError, match="not empty"): run_cli("compare", "--from-scratch", tmp_path) -def test_compare_rejects_missing_snapshot(tmp_path: Path): +def test_compare_rejects_missing_snapshot(tmp_path: Path) -> None: # An existing area with no deployment.yaml has no snapshot to compare against. with pytest.raises(SnapshotError, match="snapshot not found"): run_cli("compare", tmp_path) -def test_compare_rejects_module_without_modulefile(tmp_path: Path, configs: Path): +def test_compare_rejects_module_without_modulefile( + tmp_path: Path, configs: Path +) -> None: # Removing a module's modulefile link leaves a built module that is not exposed. layout = _sync_minimal(tmp_path, configs) layout.get_modulefile_link(MODULE_NAME, MODULE_VERSION).unlink() @@ -70,7 +72,9 @@ def test_compare_rejects_module_without_modulefile(tmp_path: Path, configs: Path run_cli("compare", tmp_path) -def test_compare_rejects_modulefile_without_module(tmp_path: Path, configs: Path): +def test_compare_rejects_modulefile_without_module( + tmp_path: Path, configs: Path +) -> None: # Removing the built module leaves a dangling modulefile link layout = _sync_minimal(tmp_path, configs) rmtree(layout.get_module_folder(MODULE_NAME, MODULE_VERSION)) @@ -78,7 +82,7 @@ def test_compare_rejects_modulefile_without_module(tmp_path: Path, configs: Path run_cli("compare", tmp_path) -def test_compare_rejects_duplicate_modulefiles(tmp_path: Path, configs: Path): +def test_compare_rejects_duplicate_modulefiles(tmp_path: Path, configs: Path) -> None: # The same modulefile must not appear in both the live and deprecated areas. layout = _sync_minimal(tmp_path, configs) deprecated_link = layout.get_modulefile_link( @@ -90,7 +94,7 @@ def test_compare_rejects_duplicate_modulefiles(tmp_path: Path, configs: Path): run_cli("compare", tmp_path) -def test_compare_rejects_release_mismatch(tmp_path: Path, configs: Path): +def test_compare_rejects_release_mismatch(tmp_path: Path, configs: Path) -> None: # Tamper with the snapshot's record of the module so it no longer matches the module # configuration reconstructed from the deployment area. layout = _sync_minimal(tmp_path, configs) @@ -104,7 +108,9 @@ def test_compare_rejects_release_mismatch(tmp_path: Path, configs: Path): run_cli("compare", tmp_path) -def test_compare_rejects_default_version_mismatch(tmp_path: Path, configs: Path): +def test_compare_rejects_default_version_mismatch( + tmp_path: Path, configs: Path +) -> None: # Point the module's .version file at a different version than the snapshot expects. layout = _sync_minimal(tmp_path, configs) version_file = layout.get_default_version_file(MODULE_NAME) @@ -113,7 +119,7 @@ def test_compare_rejects_default_version_mismatch(tmp_path: Path, configs: Path) run_cli("compare", tmp_path) -def test_compare_use_ref_detects_drift(tmp_path: Path, configs: Path): +def test_compare_use_ref_detects_drift(tmp_path: Path, configs: Path) -> None: # sync commits a snapshot to the deployment area's git repo on every run. After two # syncs the area matches its own (HEAD) snapshot, but not the previous commit's # snapshot, which predates the module added by the second sync. @@ -127,20 +133,22 @@ def test_compare_use_ref_detects_drift(tmp_path: Path, configs: Path): run_cli("compare", "--use-ref", "HEAD~1", tmp_path) -def test_compare_from_scratch_rejects_missing_root(tmp_path: Path): +def test_compare_from_scratch_rejects_missing_root(tmp_path: Path) -> None: # The CLI's argument validation rejects a non-existent path before the command runs, # so exercise this guard by calling the function directly. with pytest.raises(ComparisonError, match="does not exist"): compare_to_snapshot(tmp_path / "does-not-exist", from_scratch=True) -def test_compare_rejects_missing_root(tmp_path: Path): +def test_compare_rejects_missing_root(tmp_path: Path) -> None: # As above, but for the snapshot-loading path used by a non from-scratch compare. with pytest.raises(SnapshotError, match="does not exist"): compare_to_snapshot(tmp_path / "does-not-exist") -def test_compare_rejects_missing_default_version_file(tmp_path: Path, configs: Path): +def test_compare_rejects_missing_default_version_file( + tmp_path: Path, configs: Path +) -> None: # A live module records its default version in a .version file; a missing one is # corruption that compare must report cleanly, not as a raw FileNotFoundError. layout = _sync_minimal(tmp_path, configs) @@ -149,7 +157,7 @@ def test_compare_rejects_missing_default_version_file(tmp_path: Path, configs: P run_cli("compare", tmp_path) -def test_compare_rejects_unreadable_snapshot(tmp_path: Path, configs: Path): +def test_compare_rejects_unreadable_snapshot(tmp_path: Path, configs: Path) -> None: # A corrupt deployment.yaml (e.g. truncated by a failed write) must surface as a # SnapshotError rather than a raw parser error. layout = _sync_minimal(tmp_path, configs) @@ -158,7 +166,7 @@ def test_compare_rejects_unreadable_snapshot(tmp_path: Path, configs: Path): run_cli("compare", tmp_path) -def test_compare_rejects_unknown_git_ref(tmp_path: Path, configs: Path): +def test_compare_rejects_unknown_git_ref(tmp_path: Path, configs: Path) -> None: # An unresolvable --use-ref should fail with a clear SnapshotError, not a raw # GitPython BadName error. _sync_minimal(tmp_path, configs) 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") diff --git a/tests/test_validate.py b/tests/test_validate.py index a1eb7d0c..51b945a7 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -18,14 +18,16 @@ @pytest.mark.parametrize("config_name, message", INVALID_INITIAL_CONFIGS) def test_validate_rejects_invalid_initial_config( tmp_path: Path, configs: Path, config_name: str, message: str -): +) -> None: with pytest.raises(ValidationError, match=message): run_cli( "validate", "--from-scratch", tmp_path, configs / "invalid" / config_name ) -def test_validate_rejects_update_without_allow_updates(tmp_path: Path, configs: Path): +def test_validate_rejects_update_without_allow_updates( + tmp_path: Path, configs: Path +) -> None: # Deploy a baseline whose module did not opt in to allow_updates, then validate a # config that changes that module in place: this must be rejected. run_cli("sync", "--from-scratch", tmp_path, configs / "minimal") @@ -33,7 +35,9 @@ def test_validate_rejects_update_without_allow_updates(tmp_path: Path, configs: run_cli("validate", tmp_path, configs / "invalid" / "modified-no-allow-updates") -def test_validate_rejects_removal_without_deprecation(tmp_path: Path, configs: Path): +def test_validate_rejects_removal_without_deprecation( + tmp_path: Path, configs: Path +) -> None: # Deploy a baseline module, then validate a config that drops it without first # deprecating it (and without --allow-all): this must be rejected. run_cli("sync", "--from-scratch", tmp_path, configs / "minimal") @@ -41,7 +45,9 @@ def test_validate_rejects_removal_without_deprecation(tmp_path: Path, configs: P run_cli("validate", tmp_path, configs / "invalid" / "removed-no-deprecation") -def test_validate_rejects_added_deprecated_module(tmp_path: Path, configs: Path): +def test_validate_rejects_added_deprecated_module( + tmp_path: Path, configs: Path +) -> None: # Deploy a baseline module, then validate a config that introduces a brand-new # release already in a deprecated state. Deprecating a module on initial creation # is only allowed with --allow-all, so a normal validate must reject it. @@ -52,7 +58,7 @@ def test_validate_rejects_added_deprecated_module(tmp_path: Path, configs: Path) def test_validate_allows_added_deprecated_module_with_allow_all( tmp_path: Path, configs: Path -): +) -> None: # The --allow-all flag deliberately permits introducing an already-deprecated # release on initial creation. run_cli("sync", "--from-scratch", tmp_path, configs / "minimal") @@ -61,7 +67,7 @@ def test_validate_allows_added_deprecated_module_with_allow_all( ) -def test_validate_test_build(tmp_path: Path, configs: Path): +def test_validate_test_build(tmp_path: Path, configs: Path) -> None: # --test-build actually builds the modules into a temporary area and syntax-checks # the generated shell entrypoints, so exercise it on a valid shell-only config. run_cli("validate", "--test-build", "--from-scratch", tmp_path, configs / "minimal") From 4516b07f4c959c678510b400ceca841c200cc499 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Wed, 17 Jun 2026 18:40:57 +0000 Subject: [PATCH 14/28] Ensure GitPython repos are closed properly --- src/deploy_tools/snapshot.py | 19 +++++++++---------- src/deploy_tools/sync.py | 17 +++++++++-------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/deploy_tools/snapshot.py b/src/deploy_tools/snapshot.py index e29fcc51..134bced1 100644 --- a/src/deploy_tools/snapshot.py +++ b/src/deploy_tools/snapshot.py @@ -68,14 +68,13 @@ def load_snapshot(layout: Layout, from_scratch: bool = False) -> Deployment: def load_snapshot_from_ref(layout: Layout, ref: str) -> Deployment: """Load the deployment snapshot from the given git ref of the deployment area.""" logger.debug("Loading snapshot from ref: %s", ref) - repo = Repo(layout.deployment_root) - - try: - ref_snapshot = repo.commit(ref).tree[layout.DEPLOYMENT_SNAPSHOT_FILENAME] - except (BadName, KeyError) as exc: - raise SnapshotError( - f"Deployment snapshot not found at git ref:\n{ref}" - ) from exc + with Repo(layout.deployment_root) as repo: + try: + ref_snapshot = repo.commit(ref).tree[layout.DEPLOYMENT_SNAPSHOT_FILENAME] + except (BadName, KeyError) as exc: + raise SnapshotError( + f"Deployment snapshot not found at git ref:\n{ref}" + ) from exc - with io.BytesIO(ref_snapshot.data_stream.read()) as snapshot_f: # type: ignore - return load_from_yaml(Deployment, snapshot_f) + with io.BytesIO(ref_snapshot.data_stream.read()) as snapshot_f: # type: ignore + return load_from_yaml(Deployment, snapshot_f) diff --git a/src/deploy_tools/sync.py b/src/deploy_tools/sync.py index e17a6fc6..cb633fcf 100644 --- a/src/deploy_tools/sync.py +++ b/src/deploy_tools/sync.py @@ -52,16 +52,17 @@ def synchronise( else: repo = Repo(layout.deployment_root) - logger.info("Creating snapshot") - create_snapshot(deployment, layout) + with repo: + logger.info("Creating snapshot") + create_snapshot(deployment, layout) - logger.info("Deploying changes") - deploy_changes(deployment_changes, layout) + logger.info("Deploying changes") + deploy_changes(deployment_changes, layout) - logger.info("Committing changes to git (for reference)") - repo.git.add("--all") - commit = repo.index.commit("Performed sync process") - logger.info("Commit SHA: %s", commit.hexsha) + logger.info("Committing changes to git (for reference)") + repo.git.add("--all") + commit = repo.index.commit("Performed sync process") + logger.info("Commit SHA: %s", commit.hexsha) logger.info("Sync process finished") From 0a2bf295c734cbfae9533163497d075e0826c058 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Wed, 17 Jun 2026 18:48:05 +0000 Subject: [PATCH 15/28] 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 6db1cb6987d97ea68c57999bbeb3f708c84702e8 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Wed, 17 Jun 2026 19:08:57 +0000 Subject: [PATCH 16/28] 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 2c4d4936b886a30d1227e3e4f431dd59c619468c Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Fri, 19 Jun 2026 12:04:37 +0000 Subject: [PATCH 17/28] Add error handling when loading deployment configuration with tests --- src/deploy_tools/sync.py | 16 ++++++++++-- .../invalid/name-mismatch/settings.yaml | 3 +++ .../name-mismatch/wrong-folder-name/1.0.yaml | 15 +++++++++++ .../stray-file/example-module-shell/1.0.yaml | 13 ++++++++++ .../example-module-shell/README.txt | 2 ++ .../configs/invalid/stray-file/settings.yaml | 3 +++ .../example-module-shell/2.0.yaml | 15 +++++++++++ .../invalid/version-mismatch/settings.yaml | 3 +++ tests/test_load.py | 25 +++++++++++++++++++ tests/test_sync.py | 16 ++++++++++++ 10 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 tests/configs/invalid/name-mismatch/settings.yaml create mode 100644 tests/configs/invalid/name-mismatch/wrong-folder-name/1.0.yaml create mode 100644 tests/configs/invalid/stray-file/example-module-shell/1.0.yaml create mode 100644 tests/configs/invalid/stray-file/example-module-shell/README.txt create mode 100644 tests/configs/invalid/stray-file/settings.yaml create mode 100644 tests/configs/invalid/version-mismatch/example-module-shell/2.0.yaml create mode 100644 tests/configs/invalid/version-mismatch/settings.yaml create mode 100644 tests/test_load.py diff --git a/src/deploy_tools/sync.py b/src/deploy_tools/sync.py index cb633fcf..b2dd2dd7 100644 --- a/src/deploy_tools/sync.py +++ b/src/deploy_tools/sync.py @@ -1,10 +1,11 @@ import logging from pathlib import Path -from git import Repo +from git import InvalidGitRepositoryError, Repo from .build import build, clean_build_area from .deploy import deploy_changes +from .errors import DeployToolsError from .layout import Layout, ModuleBuildLayout from .models.save_and_load import load_deployment from .snapshot import create_snapshot, load_snapshot @@ -15,6 +16,11 @@ logger = logging.getLogger(__name__) + +class SyncError(DeployToolsError): + """Raised when the deployment area is not in a state the sync process can use.""" + + # Files not tracked in the deployment area's git repo (kept only as a change reference): # the transient build area and the large Apptainer .sif images. IGNORE_DIRS = [ @@ -50,7 +56,13 @@ def synchronise( logger.info("Creating git repository in deployment area") repo = _initialise_git_repo(layout.deployment_root, IGNORE_DIRS) else: - repo = Repo(layout.deployment_root) + try: + repo = Repo(layout.deployment_root) + except InvalidGitRepositoryError as exc: + raise SyncError( + f"Deployment area has a snapshot but is not a git repository " + f"(git metadata missing or corrupt):\n{layout.deployment_root}" + ) from exc with repo: logger.info("Creating snapshot") diff --git a/tests/configs/invalid/name-mismatch/settings.yaml b/tests/configs/invalid/name-mismatch/settings.yaml new file mode 100644 index 00000000..70f60017 --- /dev/null +++ b/tests/configs/invalid/name-mismatch/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/invalid/name-mismatch/wrong-folder-name/1.0.yaml b/tests/configs/invalid/name-mismatch/wrong-folder-name/1.0.yaml new file mode 100644 index 00000000..c9cab129 --- /dev/null +++ b/tests/configs/invalid/name-mismatch/wrong-folder-name/1.0.yaml @@ -0,0 +1,15 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json + +# The module name does not match its parent folder (wrong-folder-name), which the +# loader requires to match. +module: + name: example-module-shell + version: "1.0" + description: Module whose name does not match its containing folder + + applications: + - app_type: shell + + name: test-shell-echo + script: + - echo hello diff --git a/tests/configs/invalid/stray-file/example-module-shell/1.0.yaml b/tests/configs/invalid/stray-file/example-module-shell/1.0.yaml new file mode 100644 index 00000000..16c35a7b --- /dev/null +++ b/tests/configs/invalid/stray-file/example-module-shell/1.0.yaml @@ -0,0 +1,13 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json + +module: + name: example-module-shell + version: "1.0" + description: Minimal shell-only module sat alongside a stray non-yaml file + + applications: + - app_type: shell + + name: test-shell-echo + script: + - echo hello diff --git a/tests/configs/invalid/stray-file/example-module-shell/README.txt b/tests/configs/invalid/stray-file/example-module-shell/README.txt new file mode 100644 index 00000000..40412955 --- /dev/null +++ b/tests/configs/invalid/stray-file/example-module-shell/README.txt @@ -0,0 +1,2 @@ +A stray non-YAML file in a module directory. The config loader treats every entry +under a module folder as a release file, so this must be rejected with a LoadError. diff --git a/tests/configs/invalid/stray-file/settings.yaml b/tests/configs/invalid/stray-file/settings.yaml new file mode 100644 index 00000000..70f60017 --- /dev/null +++ b/tests/configs/invalid/stray-file/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/invalid/version-mismatch/example-module-shell/2.0.yaml b/tests/configs/invalid/version-mismatch/example-module-shell/2.0.yaml new file mode 100644 index 00000000..8fd14619 --- /dev/null +++ b/tests/configs/invalid/version-mismatch/example-module-shell/2.0.yaml @@ -0,0 +1,15 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json + +# The module version does not match its filename (2.0.yaml), which the loader requires +# to match. +module: + name: example-module-shell + version: "1.0" + description: Module whose version does not match its filename + + applications: + - app_type: shell + + name: test-shell-echo + script: + - echo hello diff --git a/tests/configs/invalid/version-mismatch/settings.yaml b/tests/configs/invalid/version-mismatch/settings.yaml new file mode 100644 index 00000000..70f60017 --- /dev/null +++ b/tests/configs/invalid/version-mismatch/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_load.py b/tests/test_load.py new file mode 100644 index 00000000..d826c1d3 --- /dev/null +++ b/tests/test_load.py @@ -0,0 +1,25 @@ +from pathlib import Path + +import pytest + +from conftest import run_cli +from deploy_tools.models.save_and_load import LoadError + +# Configs whose on-disk layout is malformed: loading must fail with a clear LoadError +# before any validation logic runs. Each maps a folder under configs/invalid to a +# substring expected in the error. +MALFORMED_CONFIGS = [ + ("stray-file", "Unexpected file in configuration directory"), + ("name-mismatch", "Module name .* does not match path"), + ("version-mismatch", "Module version .* does not match path"), +] + + +@pytest.mark.parametrize("config_name, message", MALFORMED_CONFIGS) +def test_load_rejects_malformed_config_layout( + tmp_path: Path, configs: Path, config_name: str, message: str +) -> None: + with pytest.raises(LoadError, match=message): + run_cli( + "validate", "--from-scratch", tmp_path, configs / "invalid" / config_name + ) 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 b7f52a912c7cdd2ac0e31a0eb60f35a7e0014ead Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Fri, 19 Jun 2026 12:06:23 +0000 Subject: [PATCH 18/28] Wrap load logic with standard deploy-tools error handling --- src/deploy_tools/models/save_and_load.py | 13 ++++++++----- .../example-module-shell/1.0.yaml | 15 +++++++++++++++ .../invalid/invalid-module-field/settings.yaml | 3 +++ tests/test_load.py | 16 ++++++++++++++++ 4 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 tests/configs/invalid/invalid-module-field/example-module-shell/1.0.yaml create mode 100644 tests/configs/invalid/invalid-module-field/settings.yaml diff --git a/src/deploy_tools/models/save_and_load.py b/src/deploy_tools/models/save_and_load.py index 818cd969..6aabf80f 100644 --- a/src/deploy_tools/models/save_and_load.py +++ b/src/deploy_tools/models/save_and_load.py @@ -5,6 +5,7 @@ import yaml from pydantic import BaseModel +from pydantic import ValidationError as PydanticValidationError from ..errors import DeployToolsError from .deployment import ( @@ -67,8 +68,13 @@ def _load_release(path: Path) -> Release: if path.is_dir() or not path.suffix == YAML_FILE_SUFFIX: raise LoadError(f"Unexpected file in configuration directory:\n{path}") - with open(path) as f: - return load_from_yaml(Release, f) + try: + with open(path) as f: + return load_from_yaml(Release, f) + except (OSError, yaml.YAMLError, PydanticValidationError, TypeError) as exc: + # Include exc: under the top-level handler the traceback (and chained cause) is + # suppressed, so the failing field is only visible if surfaced in the message. + raise LoadError(f"Module configuration is invalid:\n{path}\n{exc}") from exc def _check_filepath_matches(version_path: Path, module: Module) -> None: @@ -76,9 +82,6 @@ def _check_filepath_matches(version_path: Path, module: Module) -> None: It should be the Module's name and version as ///.yaml """ - if version_path.is_dir() and version_path.suffix == YAML_FILE_SUFFIX: - raise LoadError(f"Module directory has incorrect suffix:\n{version_path}") - if not module.name == version_path.parent.name: raise LoadError( f"Module name {module.name} does not match path:\n{version_path}" diff --git a/tests/configs/invalid/invalid-module-field/example-module-shell/1.0.yaml b/tests/configs/invalid/invalid-module-field/example-module-shell/1.0.yaml new file mode 100644 index 00000000..b4ea58ac --- /dev/null +++ b/tests/configs/invalid/invalid-module-field/example-module-shell/1.0.yaml @@ -0,0 +1,15 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json + +# The module name contains a dot, which MODULE_NAME_REGEX forbids; loading must fail +# with a clear LoadError that names the offending field. +module: + name: example.module.shell + version: "1.0" + description: Module with a name that violates the name constraint + + applications: + - app_type: shell + + name: test-shell-echo + script: + - echo hello diff --git a/tests/configs/invalid/invalid-module-field/settings.yaml b/tests/configs/invalid/invalid-module-field/settings.yaml new file mode 100644 index 00000000..70f60017 --- /dev/null +++ b/tests/configs/invalid/invalid-module-field/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_load.py b/tests/test_load.py index d826c1d3..80e1b806 100644 --- a/tests/test_load.py +++ b/tests/test_load.py @@ -23,3 +23,19 @@ def test_load_rejects_malformed_config_layout( run_cli( "validate", "--from-scratch", tmp_path, configs / "invalid" / config_name ) + + +def test_load_surfaces_invalid_field_in_error(tmp_path: Path, configs: Path) -> None: + # A config that fails model validation must raise a clean LoadError that names the + # offending field; the top-level handler hides the traceback, so the field detail is + # only visible if carried in the message. + with pytest.raises(LoadError) as exc_info: + run_cli( + "validate", + "--from-scratch", + tmp_path, + configs / "invalid" / "invalid-module-field", + ) + message = str(exc_info.value) + assert "Module configuration is invalid" in message + assert "module.name" in message From c5ce9ad765785183ff0ea6e00cadeb7294027e93 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Fri, 19 Jun 2026 12:50:57 +0000 Subject: [PATCH 19/28] Add dedicated unit tests for default version logic --- tests/test_default_versions.py | 126 +++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 tests/test_default_versions.py diff --git a/tests/test_default_versions.py b/tests/test_default_versions.py new file mode 100644 index 00000000..2025ce6e --- /dev/null +++ b/tests/test_default_versions.py @@ -0,0 +1,126 @@ +"""Unit tests for default-version selection. + +These call the function directly rather than through the CLI. Here that is to isolate +pure, non-trivial logic (e.g. natsort ordering of non-SemVer versions): a targeted unit +test states the outcome precisely and pins any regression to a line, rather than +surfacing it as an opaque golden-master diff. +""" + +from collections import defaultdict + +import pytest + +from deploy_tools.models.deployment import ( + Deployment, + DeploymentSettings, + ReleasesByNameAndVersion, +) +from deploy_tools.models.module import Module, Release +from deploy_tools.validate import ValidationError, validate_default_versions + + +def _release( + name: str, version: str, *, deprecated: bool = False, excluded: bool = False +) -> Release: + """Build a minimal Release with no applications, for selection tests.""" + return Release( + module=Module( + name=name, + version=version, + applications=[], + exclude_from_defaults=excluded, + ), + deprecated=deprecated, + ) + + +def _deployment( + *releases: Release, default_versions: dict[str, str] | None = None +) -> Deployment: + """Assemble a Deployment from loose Releases and optional explicit defaults.""" + by_name: ReleasesByNameAndVersion = defaultdict(dict) + for release in releases: + by_name[release.module.name][release.module.version] = release + + return Deployment( + settings=DeploymentSettings(default_versions=default_versions or {}), + releases=by_name, + ) + + +def test_auto_selects_latest_version_by_natsort() -> None: + # natsort, not string ordering: "10.0" must beat "2.0" (which sorts last lexically). + deployment = _deployment( + _release("mod", "1.0"), + _release("mod", "2.0"), + _release("mod", "10.0"), + ) + assert validate_default_versions(deployment) == {"mod": "10.0"} + + +def test_prerelease_sorts_before_final_release() -> None: + # The documented non-SemVer case: "1.2rc1" must come before "1.2", so the final + # release is chosen as default over its own release candidate. + deployment = _deployment( + _release("mod", "1.2rc1"), + _release("mod", "1.2"), + ) + assert validate_default_versions(deployment) == {"mod": "1.2"} + + +def test_excluded_versions_are_not_auto_selected() -> None: + # The highest version opts out of defaults, so the next-highest is chosen instead. + deployment = _deployment( + _release("mod", "1.0"), + _release("mod", "2.0", excluded=True), + ) + assert validate_default_versions(deployment) == {"mod": "1.0"} + + +def test_deprecated_versions_are_not_auto_selected() -> None: + # Deprecated releases are excluded from the deployed set, so a higher deprecated + # version must not become the default over a lower active one. + deployment = _deployment( + _release("mod", "1.0"), + _release("mod", "2.0", deprecated=True), + ) + assert validate_default_versions(deployment) == {"mod": "1.0"} + + +def test_explicit_default_is_preserved_over_auto_selection() -> None: + # An explicit default wins even when a higher version exists. + deployment = _deployment( + _release("mod", "1.0"), + _release("mod", "2.0"), + default_versions={"mod": "1.0"}, + ) + assert validate_default_versions(deployment) == {"mod": "1.0"} + + +def test_explicit_default_for_nonexistent_version_raises() -> None: + # Pointing the default at a version that will not be deployed is rejected. + deployment = _deployment( + _release("mod", "1.0"), + default_versions={"mod": "9.9"}, + ) + with pytest.raises(ValidationError, match="Unable to configure"): + validate_default_versions(deployment) + + +def test_explicit_default_for_deprecated_version_raises() -> None: + # A deprecated version is not in the deployed set, so it cannot be an explicit + # default either. + deployment = _deployment( + _release("mod", "1.0", deprecated=True), + default_versions={"mod": "1.0"}, + ) + with pytest.raises(ValidationError, match="Unable to configure"): + validate_default_versions(deployment) + + +def test_all_versions_excluded_raises() -> None: + # With every version opting out and no explicit default, there is no eligible + # version to make default. + deployment = _deployment(_release("mod", "1.0", excluded=True)) + with pytest.raises(ValidationError, match="exclude_from_defaults"): + validate_default_versions(deployment) From 3b3e043c45293337952055772452ce1c907dfb0e Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Fri, 19 Jun 2026 15:37:39 +0000 Subject: [PATCH 20/28] 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 a5b1673e2b63a16c21e0314c3b8087be2ce7a75d Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Mon, 29 Jun 2026 09:50:03 +0000 Subject: [PATCH 21/28] Reorganise CLI test configuration for consistency --- tests/configs/README.md | 16 ++++++++++++++++ .../apptainer-pull/example-apptainer/1.0.yaml | 0 .../{ => valid}/apptainer-pull/settings.yaml | 0 tests/configs/{ => valid}/empty/settings.yaml | 0 .../minimal/example-module-shell/1.0.yaml | 0 tests/configs/{ => valid}/minimal/settings.yaml | 0 .../example-module-multi/1.0.yaml | 0 .../example-module-multi/2.0.yaml | 0 .../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_compare.py | 12 ++++++++---- tests/test_sync.py | 14 +++++++++----- tests/test_validate.py | 16 +++++++++++----- 16 files changed, 45 insertions(+), 15 deletions(-) create mode 100644 tests/configs/README.md 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}/minimal/example-module-shell/1.0.yaml (100%) rename tests/configs/{ => valid}/minimal/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/README.md b/tests/configs/README.md new file mode 100644 index 00000000..eb592576 --- /dev/null +++ b/tests/configs/README.md @@ -0,0 +1,16 @@ +# Test configurations + +Sample `deploy-tools` configuration folders used by the test suite, grouped by purpose: + +- **`golden-master/`** — the `01-initial` … `06-removed` lifecycle sequence. These are + **cumulative and ordered**: each stage is synced on top of the previous one to model a + real deployment lifecycle (only `01-initial` uses `--from-scratch`). Driven by + `test_golden_master.py` and regenerated by `generate_samples.sh`. Do not reorder or make + independent. +- **`valid/`** — standalone, self-contained configs that should deploy/validate cleanly, + each serving one focused test (e.g. `minimal`, `empty`, `multi-version-active`). +- **`invalid/`** — configs that a command must reject; each maps to a test asserting the + specific error message. + +When adding a fixture, prefer reusing an existing one; otherwise add a single-purpose +folder under `valid/` or `invalid/`. Leave `golden-master/` to the lifecycle sequence. 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/minimal/example-module-shell/1.0.yaml b/tests/configs/valid/minimal/example-module-shell/1.0.yaml similarity index 100% rename from tests/configs/minimal/example-module-shell/1.0.yaml rename to tests/configs/valid/minimal/example-module-shell/1.0.yaml diff --git a/tests/configs/minimal/settings.yaml b/tests/configs/valid/minimal/settings.yaml similarity index 100% rename from tests/configs/minimal/settings.yaml rename to tests/configs/valid/minimal/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_compare.py b/tests/test_compare.py index 89546cd2..490c7b8d 100644 --- a/tests/test_compare.py +++ b/tests/test_compare.py @@ -27,7 +27,7 @@ def _sync_minimal(area: Path, configs: Path) -> Layout: Returns: A ``Layout`` describing the freshly-deployed ``area``. """ - run_cli("sync", "--from-scratch", area, configs / "minimal") + run_cli("sync", "--from-scratch", area, configs / "valid" / "minimal") return Layout(area) @@ -40,7 +40,9 @@ def test_compare_accepts_clean_deployment(tmp_path: Path, configs: Path) -> None def test_compare_accepts_deprecated_modules(tmp_path: Path, configs: Path) -> None: # Ensure compare runs successfully with deprecated modulefile links. - run_cli("sync", "--from-scratch", tmp_path, configs / "04-deprecated") + run_cli( + "sync", "--from-scratch", tmp_path, configs / "golden-master" / "04-deprecated" + ) assert run_cli("compare", tmp_path) == "" @@ -123,8 +125,10 @@ def test_compare_use_ref_detects_drift(tmp_path: Path, configs: Path) -> None: # sync commits a snapshot to the deployment area's git repo on every run. After two # syncs the area matches its own (HEAD) snapshot, but not the previous commit's # snapshot, which predates the module added by the second sync. - run_cli("sync", "--from-scratch", tmp_path, configs / "01-initial") - run_cli("sync", tmp_path, configs / "02-added") + run_cli( + "sync", "--from-scratch", tmp_path, configs / "golden-master" / "01-initial" + ) + run_cli("sync", tmp_path, configs / "golden-master" / "02-added") assert run_cli("compare", tmp_path) == "" assert run_cli("compare", "--use-ref", "HEAD", tmp_path) == "" 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") diff --git a/tests/test_validate.py b/tests/test_validate.py index 51b945a7..8d1d3528 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -30,7 +30,7 @@ def test_validate_rejects_update_without_allow_updates( ) -> None: # Deploy a baseline whose module did not opt in to allow_updates, then validate a # config that changes that module in place: this must be rejected. - run_cli("sync", "--from-scratch", tmp_path, configs / "minimal") + run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "minimal") with pytest.raises(ValidationError, match="modified without updating version"): run_cli("validate", tmp_path, configs / "invalid" / "modified-no-allow-updates") @@ -40,7 +40,7 @@ def test_validate_rejects_removal_without_deprecation( ) -> None: # Deploy a baseline module, then validate a config that drops it without first # deprecating it (and without --allow-all): this must be rejected. - run_cli("sync", "--from-scratch", tmp_path, configs / "minimal") + run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "minimal") with pytest.raises(ValidationError, match="removed without prior deprecation"): run_cli("validate", tmp_path, configs / "invalid" / "removed-no-deprecation") @@ -51,7 +51,7 @@ def test_validate_rejects_added_deprecated_module( # Deploy a baseline module, then validate a config that introduces a brand-new # release already in a deprecated state. Deprecating a module on initial creation # is only allowed with --allow-all, so a normal validate must reject it. - run_cli("sync", "--from-scratch", tmp_path, configs / "minimal") + run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "minimal") with pytest.raises(ValidationError, match="cannot have deprecated status"): run_cli("validate", tmp_path, configs / "invalid" / "added-deprecated") @@ -61,7 +61,7 @@ def test_validate_allows_added_deprecated_module_with_allow_all( ) -> None: # The --allow-all flag deliberately permits introducing an already-deprecated # release on initial creation. - run_cli("sync", "--from-scratch", tmp_path, configs / "minimal") + run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "minimal") run_cli( "validate", "--allow-all", tmp_path, configs / "invalid" / "added-deprecated" ) @@ -70,4 +70,10 @@ def test_validate_allows_added_deprecated_module_with_allow_all( def test_validate_test_build(tmp_path: Path, configs: Path) -> None: # --test-build actually builds the modules into a temporary area and syntax-checks # the generated shell entrypoints, so exercise it on a valid shell-only config. - run_cli("validate", "--test-build", "--from-scratch", tmp_path, configs / "minimal") + run_cli( + "validate", + "--test-build", + "--from-scratch", + tmp_path, + configs / "valid" / "minimal", + ) From db9aa4b2144504ca792f656b50e3ef870f9beb4d Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Fri, 19 Jun 2026 17:42:44 +0000 Subject: [PATCH 22/28] Improve names of configuration folders --- .../example-module-shell/1.0.yaml | 0 .../settings.yaml | 0 .../example-module-shell/1.0.yaml | 0 .../example-module-shell/2.0.yaml | 0 .../settings.yaml | 0 .../example-module-shell/1.0.yaml | 0 .../settings.yaml | 0 .../example-module-shell/1.0.yaml | 0 .../settings.yaml | 0 .../example-module-shell/1.0.yaml | 0 .../settings.yaml | 0 .../settings.yaml | 0 tests/test_load.py | 2 +- tests/test_validate.py | 19 +++++++++++++------ 14 files changed, 14 insertions(+), 7 deletions(-) rename tests/configs/invalid/{added-deprecated => default-version-not-deployed}/example-module-shell/1.0.yaml (100%) rename tests/configs/invalid/{missing-default => default-version-not-deployed}/settings.yaml (100%) rename tests/configs/invalid/{missing-default => deprecated-on-creation}/example-module-shell/1.0.yaml (100%) rename tests/configs/invalid/{added-deprecated => deprecated-on-creation}/example-module-shell/2.0.yaml (100%) rename tests/configs/invalid/{added-deprecated => deprecated-on-creation}/settings.yaml (100%) rename tests/configs/invalid/{invalid-module-field => invalid-module-name}/example-module-shell/1.0.yaml (100%) rename tests/configs/invalid/{all-excluded-defaults => invalid-module-name}/settings.yaml (100%) rename tests/configs/invalid/{modified-no-allow-updates => modified-without-allow-updates}/example-module-shell/1.0.yaml (100%) rename tests/configs/invalid/{invalid-module-field => modified-without-allow-updates}/settings.yaml (100%) rename tests/configs/invalid/{all-excluded-defaults => no-eligible-default}/example-module-shell/1.0.yaml (100%) rename tests/configs/invalid/{modified-no-allow-updates => no-eligible-default}/settings.yaml (100%) rename tests/configs/invalid/{removed-no-deprecation => removed-without-deprecation}/settings.yaml (100%) diff --git a/tests/configs/invalid/added-deprecated/example-module-shell/1.0.yaml b/tests/configs/invalid/default-version-not-deployed/example-module-shell/1.0.yaml similarity index 100% rename from tests/configs/invalid/added-deprecated/example-module-shell/1.0.yaml rename to tests/configs/invalid/default-version-not-deployed/example-module-shell/1.0.yaml diff --git a/tests/configs/invalid/missing-default/settings.yaml b/tests/configs/invalid/default-version-not-deployed/settings.yaml similarity index 100% rename from tests/configs/invalid/missing-default/settings.yaml rename to tests/configs/invalid/default-version-not-deployed/settings.yaml diff --git a/tests/configs/invalid/missing-default/example-module-shell/1.0.yaml b/tests/configs/invalid/deprecated-on-creation/example-module-shell/1.0.yaml similarity index 100% rename from tests/configs/invalid/missing-default/example-module-shell/1.0.yaml rename to tests/configs/invalid/deprecated-on-creation/example-module-shell/1.0.yaml diff --git a/tests/configs/invalid/added-deprecated/example-module-shell/2.0.yaml b/tests/configs/invalid/deprecated-on-creation/example-module-shell/2.0.yaml similarity index 100% rename from tests/configs/invalid/added-deprecated/example-module-shell/2.0.yaml rename to tests/configs/invalid/deprecated-on-creation/example-module-shell/2.0.yaml diff --git a/tests/configs/invalid/added-deprecated/settings.yaml b/tests/configs/invalid/deprecated-on-creation/settings.yaml similarity index 100% rename from tests/configs/invalid/added-deprecated/settings.yaml rename to tests/configs/invalid/deprecated-on-creation/settings.yaml diff --git a/tests/configs/invalid/invalid-module-field/example-module-shell/1.0.yaml b/tests/configs/invalid/invalid-module-name/example-module-shell/1.0.yaml similarity index 100% rename from tests/configs/invalid/invalid-module-field/example-module-shell/1.0.yaml rename to tests/configs/invalid/invalid-module-name/example-module-shell/1.0.yaml diff --git a/tests/configs/invalid/all-excluded-defaults/settings.yaml b/tests/configs/invalid/invalid-module-name/settings.yaml similarity index 100% rename from tests/configs/invalid/all-excluded-defaults/settings.yaml rename to tests/configs/invalid/invalid-module-name/settings.yaml diff --git a/tests/configs/invalid/modified-no-allow-updates/example-module-shell/1.0.yaml b/tests/configs/invalid/modified-without-allow-updates/example-module-shell/1.0.yaml similarity index 100% rename from tests/configs/invalid/modified-no-allow-updates/example-module-shell/1.0.yaml rename to tests/configs/invalid/modified-without-allow-updates/example-module-shell/1.0.yaml diff --git a/tests/configs/invalid/invalid-module-field/settings.yaml b/tests/configs/invalid/modified-without-allow-updates/settings.yaml similarity index 100% rename from tests/configs/invalid/invalid-module-field/settings.yaml rename to tests/configs/invalid/modified-without-allow-updates/settings.yaml diff --git a/tests/configs/invalid/all-excluded-defaults/example-module-shell/1.0.yaml b/tests/configs/invalid/no-eligible-default/example-module-shell/1.0.yaml similarity index 100% rename from tests/configs/invalid/all-excluded-defaults/example-module-shell/1.0.yaml rename to tests/configs/invalid/no-eligible-default/example-module-shell/1.0.yaml diff --git a/tests/configs/invalid/modified-no-allow-updates/settings.yaml b/tests/configs/invalid/no-eligible-default/settings.yaml similarity index 100% rename from tests/configs/invalid/modified-no-allow-updates/settings.yaml rename to tests/configs/invalid/no-eligible-default/settings.yaml diff --git a/tests/configs/invalid/removed-no-deprecation/settings.yaml b/tests/configs/invalid/removed-without-deprecation/settings.yaml similarity index 100% rename from tests/configs/invalid/removed-no-deprecation/settings.yaml rename to tests/configs/invalid/removed-without-deprecation/settings.yaml diff --git a/tests/test_load.py b/tests/test_load.py index 80e1b806..c8f2d0bf 100644 --- a/tests/test_load.py +++ b/tests/test_load.py @@ -34,7 +34,7 @@ def test_load_surfaces_invalid_field_in_error(tmp_path: Path, configs: Path) -> "validate", "--from-scratch", tmp_path, - configs / "invalid" / "invalid-module-field", + configs / "invalid" / "invalid-module-name", ) message = str(exc_info.value) assert "Module configuration is invalid" in message diff --git a/tests/test_validate.py b/tests/test_validate.py index 8d1d3528..6ddba61f 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -10,8 +10,8 @@ # resulting ValidationError. INVALID_INITIAL_CONFIGS = [ ("unknown-dependency", "unknown module dependency"), - ("missing-default", "Unable to configure"), - ("all-excluded-defaults", "exclude_from_defaults"), + ("default-version-not-deployed", "Unable to configure"), + ("no-eligible-default", "exclude_from_defaults"), ] @@ -32,7 +32,9 @@ def test_validate_rejects_update_without_allow_updates( # config that changes that module in place: this must be rejected. run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "minimal") with pytest.raises(ValidationError, match="modified without updating version"): - run_cli("validate", tmp_path, configs / "invalid" / "modified-no-allow-updates") + run_cli( + "validate", tmp_path, configs / "invalid" / "modified-without-allow-updates" + ) def test_validate_rejects_removal_without_deprecation( @@ -42,7 +44,9 @@ def test_validate_rejects_removal_without_deprecation( # deprecating it (and without --allow-all): this must be rejected. run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "minimal") with pytest.raises(ValidationError, match="removed without prior deprecation"): - run_cli("validate", tmp_path, configs / "invalid" / "removed-no-deprecation") + run_cli( + "validate", tmp_path, configs / "invalid" / "removed-without-deprecation" + ) def test_validate_rejects_added_deprecated_module( @@ -53,7 +57,7 @@ def test_validate_rejects_added_deprecated_module( # is only allowed with --allow-all, so a normal validate must reject it. run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "minimal") with pytest.raises(ValidationError, match="cannot have deprecated status"): - run_cli("validate", tmp_path, configs / "invalid" / "added-deprecated") + run_cli("validate", tmp_path, configs / "invalid" / "deprecated-on-creation") def test_validate_allows_added_deprecated_module_with_allow_all( @@ -63,7 +67,10 @@ def test_validate_allows_added_deprecated_module_with_allow_all( # release on initial creation. run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "minimal") run_cli( - "validate", "--allow-all", tmp_path, configs / "invalid" / "added-deprecated" + "validate", + "--allow-all", + tmp_path, + configs / "invalid" / "deprecated-on-creation", ) From 5f5fcaadbb805a24d12505410fa59a741468c454 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Fri, 19 Jun 2026 17:44:53 +0000 Subject: [PATCH 23/28] Reduce irrelevant parameters in configuration --- .../example-module-shell/1.0.yaml | 6 +----- .../deprecated-on-creation/example-module-shell/1.0.yaml | 6 +----- .../deprecated-on-creation/example-module-shell/2.0.yaml | 8 +------- .../example-module-shell/1.0.yaml | 8 +++----- .../no-eligible-default/example-module-shell/1.0.yaml | 6 +----- tests/configs/valid/minimal/example-module-shell/1.0.yaml | 6 +----- 6 files changed, 8 insertions(+), 32 deletions(-) diff --git a/tests/configs/invalid/default-version-not-deployed/example-module-shell/1.0.yaml b/tests/configs/invalid/default-version-not-deployed/example-module-shell/1.0.yaml index e8f3c4a7..deda7448 100644 --- a/tests/configs/invalid/default-version-not-deployed/example-module-shell/1.0.yaml +++ b/tests/configs/invalid/default-version-not-deployed/example-module-shell/1.0.yaml @@ -5,13 +5,9 @@ module: version: "1.0" description: Minimal shell-only module used by the validate tests - env_vars: - - name: SHELL_VALUE - value: hello - applications: - app_type: shell name: test-shell-echo script: - - echo $SHELL_VALUE + - echo hello diff --git a/tests/configs/invalid/deprecated-on-creation/example-module-shell/1.0.yaml b/tests/configs/invalid/deprecated-on-creation/example-module-shell/1.0.yaml index e8f3c4a7..deda7448 100644 --- a/tests/configs/invalid/deprecated-on-creation/example-module-shell/1.0.yaml +++ b/tests/configs/invalid/deprecated-on-creation/example-module-shell/1.0.yaml @@ -5,13 +5,9 @@ module: version: "1.0" description: Minimal shell-only module used by the validate tests - env_vars: - - name: SHELL_VALUE - value: hello - applications: - app_type: shell name: test-shell-echo script: - - echo $SHELL_VALUE + - echo hello diff --git a/tests/configs/invalid/deprecated-on-creation/example-module-shell/2.0.yaml b/tests/configs/invalid/deprecated-on-creation/example-module-shell/2.0.yaml index c857eea9..ec135cff 100644 --- a/tests/configs/invalid/deprecated-on-creation/example-module-shell/2.0.yaml +++ b/tests/configs/invalid/deprecated-on-creation/example-module-shell/2.0.yaml @@ -5,17 +5,11 @@ module: version: "2.0" description: Minimal shell-only module used by the validate tests - env_vars: - - name: SHELL_VALUE - value: hello - applications: - app_type: shell name: test-shell-echo script: - - echo $SHELL_VALUE + - echo hello -# A brand-new release introduced directly in a deprecated state. This is only permitted -# with --allow-all (or a from-scratch deploy); a normal deploy must reject it. deprecated: true diff --git a/tests/configs/invalid/modified-without-allow-updates/example-module-shell/1.0.yaml b/tests/configs/invalid/modified-without-allow-updates/example-module-shell/1.0.yaml index 69c07817..7f3194e6 100644 --- a/tests/configs/invalid/modified-without-allow-updates/example-module-shell/1.0.yaml +++ b/tests/configs/invalid/modified-without-allow-updates/example-module-shell/1.0.yaml @@ -5,13 +5,11 @@ module: version: "1.0" description: Minimal shell-only module used by the validate tests - env_vars: - - name: SHELL_VALUE - value: changed - applications: - app_type: shell name: test-shell-echo + # Differs from the valid/minimal baseline (echo hello) so that deploying this over + # it is an in-place modification, rejected without allow_updates / a version bump. script: - - echo $SHELL_VALUE + - echo modified diff --git a/tests/configs/invalid/no-eligible-default/example-module-shell/1.0.yaml b/tests/configs/invalid/no-eligible-default/example-module-shell/1.0.yaml index 92da05df..77d40f52 100644 --- a/tests/configs/invalid/no-eligible-default/example-module-shell/1.0.yaml +++ b/tests/configs/invalid/no-eligible-default/example-module-shell/1.0.yaml @@ -5,15 +5,11 @@ module: version: "1.0" description: Shell module excluded from defaults with no eligible fallback - env_vars: - - name: SHELL_VALUE - value: hello - applications: - app_type: shell name: test-shell-echo script: - - echo $SHELL_VALUE + - echo hello exclude_from_defaults: true diff --git a/tests/configs/valid/minimal/example-module-shell/1.0.yaml b/tests/configs/valid/minimal/example-module-shell/1.0.yaml index e8f3c4a7..deda7448 100644 --- a/tests/configs/valid/minimal/example-module-shell/1.0.yaml +++ b/tests/configs/valid/minimal/example-module-shell/1.0.yaml @@ -5,13 +5,9 @@ module: version: "1.0" description: Minimal shell-only module used by the validate tests - env_vars: - - name: SHELL_VALUE - value: hello - applications: - app_type: shell name: test-shell-echo script: - - echo $SHELL_VALUE + - echo hello From 76a29a6392dec6563c5ef85d9867dd2c7b6b0626 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Fri, 19 Jun 2026 18:17:34 +0000 Subject: [PATCH 24/28] 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 3352347d1b71a0817624a74973095fa563be6a10 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Mon, 22 Jun 2026 10:39:43 +0000 Subject: [PATCH 25/28] 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 5081ea800c341be5df9311e544e150a96335ac18 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Mon, 22 Jun 2026 10:40:38 +0000 Subject: [PATCH 26/28] Add tests for validation edge cases --- .../example-module-shell/1.0.yaml | 15 ++++++++++++ .../invalid-shell-script/settings.yaml | 3 +++ tests/test_validate.py | 24 +++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 tests/configs/invalid/invalid-shell-script/example-module-shell/1.0.yaml create mode 100644 tests/configs/invalid/invalid-shell-script/settings.yaml diff --git a/tests/configs/invalid/invalid-shell-script/example-module-shell/1.0.yaml b/tests/configs/invalid/invalid-shell-script/example-module-shell/1.0.yaml new file mode 100644 index 00000000..8b749cd2 --- /dev/null +++ b/tests/configs/invalid/invalid-shell-script/example-module-shell/1.0.yaml @@ -0,0 +1,15 @@ +# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json + +module: + name: example-module-shell + version: "1.0" + description: Module whose shell script is not valid bash + + applications: + - app_type: shell + + name: test-shell-echo + # 'fi' with no matching 'if' is invalid bash, so `validate --test-build` must + # reject the generated entrypoint when it runs `bash -n` over it. + script: + - fi diff --git a/tests/configs/invalid/invalid-shell-script/settings.yaml b/tests/configs/invalid/invalid-shell-script/settings.yaml new file mode 100644 index 00000000..70f60017 --- /dev/null +++ b/tests/configs/invalid/invalid-shell-script/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_validate.py b/tests/test_validate.py index 6ddba61f..63df526d 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -84,3 +84,27 @@ def test_validate_test_build(tmp_path: Path, configs: Path) -> None: tmp_path, configs / "valid" / "minimal", ) + + +def test_validate_test_build_rejects_invalid_script( + tmp_path: Path, configs: Path +) -> None: + # --test-build runs `bash -n` over each generated shell entrypoint, so a script that + # is not valid bash must be rejected. + with pytest.raises(ValidationError, match="is invalid with errors"): + run_cli( + "validate", + "--test-build", + "--from-scratch", + tmp_path, + configs / "invalid" / "invalid-shell-script", + ) + + +def test_validate_reports_no_actions_when_unchanged( + tmp_path: Path, configs: Path +) -> None: + # Validating a config that is already fully deployed previews no changes. + run_cli("sync", "--from-scratch", tmp_path, configs / "valid" / "minimal") + output = run_cli("validate", tmp_path, configs / "valid" / "minimal") + assert "No release actions required" in output From d7c76e86452b8a7982ef5014f473ad8ded7c3243 Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Mon, 22 Jun 2026 11:53:32 +0000 Subject: [PATCH 27/28] Add unit test for defensive check in apply_default_versions() --- src/deploy_tools/modulefile.py | 2 ++ tests/test_modulefile.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 tests/test_modulefile.py diff --git a/src/deploy_tools/modulefile.py b/src/deploy_tools/modulefile.py index 5597de89..c8842408 100644 --- a/src/deploy_tools/modulefile.py +++ b/src/deploy_tools/modulefile.py @@ -60,6 +60,8 @@ def apply_default_versions( overwrite=True, ) else: + # Defensive: remove a stale default file for a deployed module that has no + # entry in the default map. This ought to be unreachable via the CLI default_version_file.unlink(missing_ok=True) 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 df673cc6273f22f4956bdb81eb624e217107005c Mon Sep 17 00:00:00 2001 From: Martin Gaughran Date: Tue, 30 Jun 2026 14:07:46 +0000 Subject: [PATCH 28/28] 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: