Improve golden master tests

Dockerfile

@@ -4,7 +4,7 @@ FROM ghcr.io/diamondlightsource/ubuntu-devcontainer:noble AS developer
 
 # Add any system dependencies for the developer/build environment here
 RUN apt-get update -y && apt-get install -y --no-install-recommends \
-    graphviz environment-modules wget \
+    graphviz environment-modules wget file \
     && cd /tmp \
     && wget https://github.com/apptainer/apptainer/releases/download/v1.3.3/apptainer_1.3.3_amd64.deb \
     && apt install -y ./apptainer_1.3.3_amd64.deb \

pyproject.toml

@@ -174,6 +174,7 @@ commands = [
 src = ["src", "tests"]
 line-length = 88
 lint.select = [
+    "ANN", # flake8-annotations - https://docs.astral.sh/ruff/rules/#flake8-annotations-ann
     "B",   # flake8-bugbear - https://docs.astral.sh/ruff/rules/#flake8-bugbear-b
     "C4",  # flake8-comprehensions - https://docs.astral.sh/ruff/rules/#flake8-comprehensions-c4
     "E",   # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e
@@ -190,3 +191,5 @@ lint.select = [
 # See https://github.com/DiamondLightSource/python-copier-template/issues/154
 # Remove this line to forbid private member access in tests
 "tests/**/*" = ["SLF001"]
+# Managed by the copier template; left unannotated to avoid divergence
+".github/pages/make_switcher.py" = ["ANN"]

src/deploy_tools/__main__.py

@@ -1,11 +1,13 @@
 import logging
+import sys
 from pathlib import Path
 from typing import Annotated
 
 import typer
 
 from . import __version__
 from .compare import compare_to_snapshot
+from .errors import DeployToolsError
 from .models.schema import generate_schema
 from .sync import synchronise
 from .validate import validate_and_test_configuration
@@ -203,7 +205,14 @@ def common(
 
 
 def main() -> None:
-    app()
+    try:
+        app()
+    except DeployToolsError as exc:
+        # Expected, actionable failures carry a complete user-facing message, so present
+        # just that and exit non-zero. Any other exception propagates to Typer's
+        # excepthook and surfaces as a full traceback, as befits an unexpected bug.
+        typer.secho(f"Error: {exc}", fg=typer.colors.RED, err=True)
+        sys.exit(1)
 
 
 if __name__ == "__main__":

src/deploy_tools/app_builder.py

@@ -7,6 +7,7 @@
 from deploy_tools.models.binary_app import BinaryApp, HashType
 
 from .apptainer import create_sif_file
+from .errors import DeployToolsError
 from .layout import ModuleBuildLayout
 from .models.apptainer_app import ApptainerApp
 from .models.module import Application, Module
@@ -16,7 +17,7 @@
 ALL_READ_EXECUTE_PERMISSIONS = 0o555
 
 
-class AppBuilderError(Exception):
+class AppBuilderError(DeployToolsError):
     """Raised when building an application's files fails."""
 
 
@@ -27,7 +28,7 @@ def __init__(self, templater: Templater, build_layout: ModuleBuildLayout) -> Non
         self._templater = templater
         self._build_layout = build_layout
 
-    def create_application_files(self, app: Application, module: Module):
+    def create_application_files(self, app: Application, module: Module) -> None:
         """Create the entrypoint and supporting files for an application.
 
         The files produced depend on the application type (Apptainer, shell or binary).

src/deploy_tools/apptainer.py

@@ -1,8 +1,10 @@
-import subprocess
 from pathlib import Path
 
+from .errors import DeployToolsError
+from .external_tools import run_command
 
-class ApptainerError(Exception):
+
+class ApptainerError(DeployToolsError):
     """Raised when building an Apptainer SIF file fails."""
 
 
@@ -27,4 +29,4 @@ def create_sif_file(
         )
 
     commands = ["apptainer", "pull", output_path, container_url]
-    subprocess.run(commands, check=True)
+    run_command(commands, check=True)

src/deploy_tools/compare.py

@@ -7,6 +7,7 @@
 import yaml
 from pydantic import TypeAdapter
 
+from .errors import DeployToolsError
 from .layout import Layout
 from .models.deployment import (
     DefaultVersionsByName,
@@ -23,7 +24,7 @@
 logger = logging.getLogger(__name__)
 
 
-class ComparisonError(Exception):
+class ComparisonError(DeployToolsError):
     """Raised when comparing the deployment area to its snapshot fails."""
 
 
@@ -182,7 +183,7 @@ def _compare_default_versions(snapshot: Deployment, actual: Deployment) -> None:
         )
 
 
-def _get_dict_diff(d1: dict[str, Any], d2: dict[str, Any]):
+def _get_dict_diff(d1: dict[str, Any], d2: dict[str, Any]) -> str:
     return "\n" + "\n".join(
         difflib.ndiff(
             _yaml_dumps(d1).splitlines(),

src/deploy_tools/errors.py

@@ -0,0 +1,10 @@
+class DeployToolsError(Exception):
+    """Base class for expected, user-facing deploy-tools errors.
+
+    These represent actionable failure conditions, such as invalid configuration or a
+    corrupt deployment area, where the exception message is itself the guidance the
+    operator needs. The CLI catches this base class at the top level and prints the
+    message without a traceback (see ``deploy_tools.__main__.main``). Any exception that
+    is *not* a ``DeployToolsError`` is treated as an unexpected bug and surfaces with a
+    full traceback.
+    """

src/deploy_tools/external_tools.py

@@ -0,0 +1,51 @@
+import subprocess
+from pathlib import Path
+from typing import Any
+
+from .errors import DeployToolsError
+
+
+class ExternalToolError(DeployToolsError):
+    """Raised when a required external command-line tool is missing or fails."""
+
+
+def run_command(
+    command: list[str | Path],
+    *,
+    check: bool = False,
+    capture_output: bool = False,
+    text: bool = False,
+) -> subprocess.CompletedProcess[Any]:
+    """Run an external command, surfacing tool problems as clean domain errors.
+
+    Wraps ``subprocess.run`` so that a missing executable (or, when ``check`` is set, a
+    non-zero exit) is reported as an ``ExternalToolError`` rather than a raw
+    ``FileNotFoundError``/``CalledProcessError`` traceback.
+
+    Args:
+        command: The command to run, as a list whose first element is the executable.
+        check: If True, raise when the command exits with a non-zero status.
+        capture_output: If True, capture the command's stdout and stderr.
+        text: If True, decode captured output as text rather than bytes.
+
+    Returns:
+        The ``subprocess.CompletedProcess`` for the finished command.
+
+    Raises:
+        ExternalToolError: If the executable cannot be found, or ``check`` is set and
+            the command exits with a non-zero status.
+    """
+    executable = str(command[0])
+    try:
+        return subprocess.run(
+            command, check=check, capture_output=capture_output, text=text
+        )
+    except FileNotFoundError as exc:
+        raise ExternalToolError(
+            f"Required external tool not found: '{executable}'.\n"
+            f"Ensure it is installed and available on PATH."
+        ) from exc
+    except subprocess.CalledProcessError as exc:
+        raise ExternalToolError(
+            f"External tool '{executable}' failed with exit status {exc.returncode}."
+        ) from exc

src/deploy_tools/layout.py

@@ -47,7 +47,7 @@ def get_sif_files_folder(self, name: str, version: str) -> Path:
         return self.get_module_folder(name, version) / self.SIF_FILES_FOLDER
 
     @property
-    def build_root(self):
+    def build_root(self) -> Path:
         """Root path of the build area."""
         return self._root
 

src/deploy_tools/models/save_and_load.py

@@ -6,6 +6,7 @@
 import yaml
 from pydantic import BaseModel
 
+from ..errors import DeployToolsError
 from .deployment import (
     Deployment,
     DeploymentSettings,
@@ -17,7 +18,7 @@
 DEPLOYMENT_SETTINGS = "settings" + YAML_FILE_SUFFIX
 
 
-class LoadError(Exception):
+class LoadError(DeployToolsError):
     """Raised when configuration files cannot be loaded into the model."""
 
 

src/deploy_tools/print_updates.py

@@ -25,7 +25,10 @@ def _print_module_updates(release_changes: ReleaseChanges) -> None:
         if releases:
             print(f"Modules to be {action}:")
 
-            for release in releases:
+            # Sort for stable output: release lists follow config load (glob) order.
+            for release in sorted(
+                releases, key=lambda r: (r.module.name, r.module.version)
+            ):
                 print(f"{release.module.name}/{release.module.version}")
             print()
 
@@ -36,10 +39,10 @@ def _print_module_updates(release_changes: ReleaseChanges) -> None:
 def _print_version_updates(
     old_defaults: DefaultVersionsByName, new_defaults: DefaultVersionsByName
 ) -> None:
-    module_names = old_defaults.keys() | new_defaults.keys()
+    module_names: set[str] = old_defaults.keys() | new_defaults.keys()
 
     update_messages: list[str] = []
-    for name in module_names:
+    for name in sorted(module_names):
         old = old_defaults.get(name, "None")
         new = new_defaults.get(name, "None")
         if not old == new:

src/deploy_tools/snapshot.py

@@ -3,14 +3,15 @@
 
 from git import Repo
 
+from .errors import DeployToolsError
 from .layout import Layout
 from .models.deployment import Deployment, DeploymentSettings
 from .models.save_and_load import load_from_yaml, save_as_yaml
 
 logger = logging.getLogger(__name__)
 
 
-class SnapshotError(Exception):
+class SnapshotError(DeployToolsError):
     """Raised when a deployment snapshot is missing or in an unexpected state."""
 
 

src/deploy_tools/validate.py

@@ -1,11 +1,12 @@
 import logging
-import subprocess
 from pathlib import Path
 from tempfile import TemporaryDirectory
 
 from natsort import natsorted
 
 from .build import build
+from .errors import DeployToolsError
+from .external_tools import run_command
 from .layout import Layout
 from .models.changes import DeploymentChanges, ReleaseChanges
 from .models.deployment import (
@@ -25,7 +26,7 @@
 SCRIPT_INDICATOR_PHRASE = "shell script"
 
 
-class ValidationError(Exception):
+class ValidationError(DeployToolsError):
     """Raised when the deployment configuration fails validation."""
 
 
@@ -242,9 +243,7 @@ def _is_shell_script(file: Path) -> bool:
     This uses the output of the Linux 'file' command, which is dependent on the
     shebang line as well as the OS. Both are controlled in our deployment.
     """
-    result = subprocess.run(
-        ["file", f"{file.absolute()}"], capture_output=True, text=True
-    )
+    result = run_command(["file", str(file.absolute())], capture_output=True, text=True)
     return SCRIPT_INDICATOR_PHRASE in result.stdout
 
 
@@ -257,12 +256,12 @@ def _check_bash_syntax(file: Path) -> None:
     This is also unable to check that all required functions and tools are available, so
     some typos are likely to pass.
     """
-    result = subprocess.run(
-        ["bash", "-n", f"{file.absolute()}"],
+    result = run_command(
+        ["bash", "-n", str(file.absolute())],
         capture_output=True,
         text=True,
     )
-    if result.stderr:
+    if result.returncode != 0:
         raise ValidationError(
             f"Output script {file.absolute()} is invalid with errors:\n{result.stderr}"
         )

tests/configs/golden-master/01-initial/apps/0.1.yaml

@@ -0,0 +1,37 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json
+
+module:
+  name: apps
+  version: "0.1"
+  description: Module demonstrating shell and Apptainer applications
+
+  env_vars:
+    - name: OTHER_VALUE
+      value: Version 0.1
+
+  applications:
+    - app_type: apptainer
+
+      container:
+        path: docker://ghcr.io/apptainer/lolcow
+        version: latest
+
+      entrypoints:
+        - name: cowsay-hello
+          command: cowsay
+          options:
+            command_args: Hello
+
+    - app_type: shell
+
+      name: test-echo-module-var
+      script:
+        - echo $OTHER_VALUE
+
+    - app_type: shell
+
+      name: test-shell-script
+      script:
+        - "echo This is the first line of a shell script"
+        - "echo and this is the second line."
+        - "echo Your input is ${1}"

tests/configs/golden-master/01-initial/deps/0.2.yaml

@@ -0,0 +1,14 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json
+
+module:
+  name: deps
+  version: "0.2"
+  description: Module demonstrating dependencies
+
+  dependencies:
+    - name: apps
+      version: "0.1"
+    # A version-less dependency is valid only for modules not managed by deploy-tools.
+    - name: external-module
+
+  applications: []

tests/configs/golden-master/01-initial/settings.yaml

@@ -0,0 +1,3 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/deployment-settings.json
+
+default_versions: {}

tests/configs/golden-master/01-initial/updatable/1.0.yaml

@@ -0,0 +1,21 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json
+
+module:
+  name: updatable
+  version: "1.0"
+  description: Module demonstrating in-place updates (allow_updates)
+
+  env_vars:
+    - name: UPDATABLE_VALUE
+      value: Set at initial deployment
+
+  applications:
+    - app_type: shell
+
+      name: test-updatable-echo
+      script:
+        - echo $UPDATABLE_VALUE
+
+  # allow_updates lets a later sync change this version's configuration in place,
+  # without bumping the version number.
+  allow_updates: true

tests/configs/golden-master/01-initial/versions/1.0.yaml

@@ -0,0 +1,8 @@
+# yaml-language-server: $schema=/workspaces/deploy-tools/src/deploy_tools/models/schemas/release.json
+
+module:
+  name: versions
+  version: "1.0"
+  description: Module demonstrating multiple versions and default-version selection
+
+  applications: []