scuba: Use Path objects in ScubaVolume

scuba/config.py

@@ -314,7 +314,7 @@ def _get_str(data: CfgData, key: str, default: Optional[str] = None) -> Optional
     return _get_typed_val(data, key, str, default)
 
 
-def _get_dict(data: CfgData, key: str) -> Optional[dict]:
+def _get_dict(data: CfgData, key: str) -> Optional[dict[str, Any]]:
     return _get_typed_val(data, key, dict)
 
 
@@ -323,19 +323,19 @@ def _get_delimited_str_list(data: CfgData, key: str, sep: str) -> List[str]:
     return s.split(sep) if s else []
 
 
-def _get_volumes(data: CfgData) -> Optional[Dict[str, ScubaVolume]]:
+def _get_volumes(data: CfgData) -> Optional[Dict[Path, ScubaVolume]]:
     voldata = _get_dict(data, "volumes")
     if voldata is None:
         return None
 
     vols = {}
-    for cpath, v in voldata.items():
-        cpath = _expand_path(cpath)
+    for cpath_str, v in voldata.items():
+        cpath = _expand_path(cpath_str)
         vols[cpath] = ScubaVolume.from_dict(cpath, v)
     return vols
 
 
-def _expand_path(in_str: str) -> str:
+def _expand_path(in_str: str) -> Path:
     try:
         output = expand_env_vars(in_str)
     except KeyError as ke:
@@ -348,17 +348,17 @@ def _expand_path(in_str: str) -> str:
             f"Unable to expand string '{in_str}' due to parsing errors"
         ) from ve
 
-    return output
+    return Path(output)
 
 
 @dataclasses.dataclass(frozen=True)
 class ScubaVolume:
-    container_path: str
-    host_path: str  # TODO: Optional for anonymous volume
+    container_path: Path
+    host_path: Path  # TODO: Optional for anonymous volume
     options: List[str] = dataclasses.field(default_factory=list)
 
     @classmethod
-    def from_dict(cls, cpath: str, node: CfgNode) -> ScubaVolume:
+    def from_dict(cls, cpath: Path, node: CfgNode) -> ScubaVolume:
         # Treat a null node as an empty dict
         if node is None:
             node = {}
@@ -390,10 +390,7 @@ def from_dict(cls, cpath: str, node: CfgNode) -> ScubaVolume:
         raise ConfigError(f"{cpath}: must be string or dict")
 
     def get_vol_opt(self) -> str:
-        # TODO: change host_path and container_path to Path objects
-        return make_vol_opt(
-            Path(self.host_path), Path(self.container_path), self.options
-        )
+        return make_vol_opt(self.host_path, self.container_path, self.options)
 
 
 @dataclasses.dataclass(frozen=True)
@@ -406,7 +403,7 @@ class ScubaAlias:
     shell: Optional[str] = None
     as_root: bool = False
     docker_args: Optional[List[str]] = None
-    volumes: Optional[Dict[str, ScubaVolume]] = None
+    volumes: Optional[Dict[Path, ScubaVolume]] = None
 
     @classmethod
     def from_dict(cls, name: str, node: CfgNode) -> ScubaAlias:
@@ -434,7 +431,7 @@ class ScubaConfig:
     shell: str
     entrypoint: Optional[str]
     docker_args: Optional[List[str]]  # TODO: drop Optional?
-    volumes: Optional[Dict[str, ScubaVolume]]  # TODO: drop Optional? Dict?
+    volumes: Optional[Dict[Path, ScubaVolume]]  # TODO: drop Optional? Dict?
     aliases: Dict[str, ScubaAlias]
     hooks: Dict[str, List[str]]
     environment: Environment

scuba/scuba.py

@@ -191,7 +191,7 @@ def try_create_volumes(self) -> None:
             return
 
         for vol in self.context.volumes.values():
-            if os.path.exists(vol.host_path):
+            if vol.host_path.exists():
                 continue
 
             try:
@@ -403,7 +403,7 @@ def get_docker_cmdline(self) -> Sequence[str]:
 class ScubaContext:
     image: str
     environment: Dict[str, str]  # key: value
-    volumes: Dict[str, ScubaVolume]
+    volumes: Dict[Path, ScubaVolume]
     shell: str
     docker_args: List[str]
     script: Optional[List[str]] = None  # TODO: drop Optional?
@@ -435,7 +435,7 @@ def process_command(
         entrypoint = cfg.entrypoint
         environment = copy.copy(cfg.environment)
         docker_args = copy.copy(cfg.docker_args) or []
-        volumes: Dict[str, ScubaVolume] = copy.copy(cfg.volumes or {})
+        volumes: Dict[Path, ScubaVolume] = copy.copy(cfg.volumes or {})
         as_root = False
 
         if command:

tests/test_config.py

@@ -892,9 +892,7 @@ def test_volumes_simple_volume(self) -> None:
         assert config.volumes is not None
         assert len(config.volumes) == 1
 
-        v = config.volumes["/cpath"]
-        assert v.container_path == "/cpath"
-        assert v.host_path == "/hpath"
+        assert_vol(config.volumes, "/cpath", "/hpath")
 
     def test_volumes_complex(self) -> None:
         """volumes can be set using the complex form"""
@@ -917,23 +915,9 @@ def test_volumes_complex(self) -> None:
         assert vols is not None
         assert len(vols) == 3
 
-        v = vols["/foo"]
-        assert isinstance(v, scuba.config.ScubaVolume)
-        assert v.container_path == "/foo"
-        assert v.host_path == "/host/foo"
-        assert v.options == []
-
-        v = vols["/bar"]
-        assert isinstance(v, scuba.config.ScubaVolume)
-        assert v.container_path == "/bar"
-        assert v.host_path == "/host/bar"
-        assert v.options == []
-
-        v = vols["/snap"]
-        assert isinstance(v, scuba.config.ScubaVolume)
-        assert v.container_path == "/snap"
-        assert v.host_path == "/host/snap"
-        assert v.options == ["z", "ro"]
+        assert_vol(vols, "/foo", "/host/foo")
+        assert_vol(vols, "/bar", "/host/bar")
+        assert_vol(vols, "/snap", "/host/snap", ["z", "ro"])
 
     def test_alias_volumes_set(self) -> None:
         """docker_args can be set via alias"""
@@ -958,17 +942,8 @@ def test_alias_volumes_set(self) -> None:
         assert vols is not None
         assert len(vols) == 2
 
-        v = vols["/foo"]
-        assert isinstance(v, scuba.config.ScubaVolume)
-        assert v.container_path == "/foo"
-        assert v.host_path == "/host/foo"
-        assert v.options == []
-
-        v = vols["/bar"]
-        assert isinstance(v, scuba.config.ScubaVolume)
-        assert v.container_path == "/bar"
-        assert v.host_path == "/host/bar"
-        assert v.options == ["z", "ro"]
+        assert_vol(vols, "/foo", "/host/foo")
+        assert_vol(vols, "/bar", "/host/bar", ["z", "ro"])
 
     def test_volumes_with_env_vars_simple(self, monkeypatch) -> None:
         """volume definitions can contain environment variables"""
@@ -988,11 +963,7 @@ def test_volumes_with_env_vars_simple(self, monkeypatch) -> None:
         assert vols is not None
         assert len(vols) == 1
 
-        v = list(vols.values())[0]
-        assert isinstance(v, scuba.config.ScubaVolume)
-        assert v.container_path == "/bar/baz/foo"
-        assert v.host_path == "/moo/doo/foo"
-        assert v.options == []
+        assert_vol(vols, "/bar/baz/foo", "/moo/doo/foo")
 
     def test_volumes_with_env_vars_complex(self, monkeypatch) -> None:
         """complex volume definitions can contain environment variables"""
@@ -1019,23 +990,11 @@ def test_volumes_with_env_vars_complex(self, monkeypatch) -> None:
         assert vols is not None
         assert len(vols) == 3
 
-        v = vols["/home/testuser/.config"]
-        assert isinstance(v, scuba.config.ScubaVolume)
-        assert v.container_path == "/home/testuser/.config"
-        assert v.host_path == "/home/testuser/.config"
-        assert v.options == []
-
-        v = vols["/tmp/"]
-        assert isinstance(v, scuba.config.ScubaVolume)
-        assert v.container_path == "/tmp/"
-        assert v.host_path == "/home/testuser/scuba/myproject/tmp"
-        assert v.options == []
-
-        v = vols["/var/spool/mail/container"]
-        assert isinstance(v, scuba.config.ScubaVolume)
-        assert v.container_path == "/var/spool/mail/container"
-        assert v.host_path == "/var/spool/mail/testuser"
-        assert v.options == ["z", "ro"]
+        assert_vol(vols, "/home/testuser/.config", "/home/testuser/.config")
+        assert_vol(vols, "/tmp", "/home/testuser/scuba/myproject/tmp")
+        assert_vol(
+            vols, "/var/spool/mail/container", "/var/spool/mail/testuser", ["z", "ro"]
+        )
 
     def test_volumes_with_invalid_env_vars(self, monkeypatch) -> None:
         """Volume definitions cannot include unset env vars"""

tests/test_scuba.py

@@ -1,5 +1,6 @@
 import pytest
 import shlex
+from .utils import assert_vol
 
 from scuba.config import ScubaConfig, ConfigError, OverrideStr
 from scuba.scuba import ScubaContext
@@ -340,13 +341,8 @@ def test_process_command_alias_extends_volumes(self) -> None:
         vols = result.volumes
         assert len(vols) == 2
 
-        v = vols["/foo"]
-        assert v.container_path == "/foo"
-        assert v.host_path == "/host/foo"
-
-        v = vols["/bar"]
-        assert v.container_path == "/bar"
-        assert v.host_path == "/host/bar"
+        assert_vol(vols, "/foo", "/host/foo")
+        assert_vol(vols, "/bar", "/host/bar")
 
     def test_process_command_alias_updates_volumes(self) -> None:
         """aliases can extend the volumes"""
@@ -370,6 +366,4 @@ def test_process_command_alias_updates_volumes(self) -> None:
         vols = result.volumes
         assert len(vols) == 1
 
-        v = vols["/foo"]
-        assert v.container_path == "/foo"
-        assert v.host_path == "/alternate/foo"
+        assert_vol(vols, "/foo", "/alternate/foo")

tests/utils.py

@@ -1,6 +1,7 @@
 import os
 import sys
 from os.path import normpath
+from pathlib import Path
 import tempfile
 import shutil
 import unittest
@@ -9,6 +10,8 @@
 from typing import Any, Sequence, Union
 from unittest import mock
 
+from scuba.config import ScubaVolume
+
 PathStr = Union[Path, str]
 
 
@@ -26,6 +29,21 @@ def assert_str_equalish(exp: Any, act: Any) -> None:
     assert exp == act
 
 
+def assert_vol(
+    vols: dict[Path, ScubaVolume],
+    cpath_str: str,
+    hpath_str: str,
+    options: list[str] = [],
+) -> None:
+    cpath = Path(cpath_str)
+    hpath = Path(hpath_str)
+    v = vols[cpath]
+    assert isinstance(v, ScubaVolume)
+    assert v.container_path == cpath
+    assert v.host_path == hpath
+    assert v.options == options
+
+
 def make_executable(path: PathStr) -> None:
     mode = os.stat(path).st_mode
     mode |= (mode & 0o444) >> 2  # copy R bits to X