Merge pull request #16 from samsrabin/add-rimport-tests

Add rimport tests

Author: samsrabin

rimport

@@ -1,4 +1,6 @@
 #!/glade/u/apps/derecho/24.12/opt/view/bin/python
+# TODO: Move all the Python into new file rimport.py for simpler testing. Keep rimport as a
+# convenience wrapper.
 """
 A drop-in CLI replacement for the legacy `rimport` csh tool.
 
@@ -77,7 +79,7 @@ def build_parser() -> argparse.ArgumentParser:
         help=argparse.SUPPRESS,
     )
 
-    # Provide -help to mirror legacy behavior (no -h)
+    # Provide -help to mirror legacy behavior
     parser.add_argument("-help", action="help", help=argparse.SUPPRESS)
 
     return parser
@@ -122,6 +124,8 @@ def stage_data(src: Path, inputdata_root: Path, staging_root: Path) -> None:
     try:
         rel = src.resolve().relative_to(inputdata_root.resolve())
     except ValueError:
+        # TODO: Do not hard-code string here
+        # TODO: Check whether it's IN THE DIRECTORY, not whether the path contains a string
         if "d651077" in str(src):
             raise RuntimeError(f"Source file {src.name} is already published.")
         else:
@@ -137,17 +141,21 @@ def ensure_running_as(target_user: str, argv: list[str]) -> None:
     try:
         target_uid = pwd.getpwnam(target_user).pw_uid
     except KeyError:
+        # TODO: Raise Python error instead of SystemExit
         print(f"rimport: target user '{target_user}' not found on this system", file=sys.stderr)
         raise SystemExit(2)
 
     if os.geteuid() != target_uid:
         if not sys.stdin.isatty():
+            # TODO: Do not hard-code "cesmdata" here
             print("rimport: need interactive TTY to authenticate as 'cesmdata' (2FA).\n"
                   "         Try: sudo -u cesmdata rimport …", file=sys.stderr)
+            # TODO: Raise Python error instead of SystemExit
             raise SystemExit(2)
         # Re-exec under target user; this invokes sudo’s normal password/2FA flow.
         os.execvp("sudo", ["sudo", "-u", target_user, "--"] + argv)
 
+# TODO: Unused; delete.
 def safe_mvandlink(src: Path, dst: Path) -> None:
     dst.parent.mkdir(parents=True, exist_ok=True)
     # Move (handles cross-filesystem with copy2+remove under the hood)
@@ -163,6 +171,7 @@ def get_staging_root() -> Path:
     env = os.getenv("RIMPORT_STAGING")
     if env:
         return Path(env).expanduser().resolve()
+    # TODO: This should be a module-level variable.
     return Path("/glade/campaign/collections/gdex/data/d651077/cesmdata/inputdata")
 
 
@@ -171,8 +180,10 @@ def main(argv: List[str] | None = None) -> int:
     args = parser.parse_args(argv)
 
     # Ensure we are running as the cesmdata account before touching the tree
-    # Comment out the next line if you prefer to run `sudox -u cesmdata rimport …` explicitly.
-    ensure_running_as("cesmdata", sys.argv)
+    # Set env var RIMPORT_SKIP_USER_CHECK=1 if you prefer to run `sudox -u cesmdata rimport …`
+    # explicitly (or for testing).
+    if os.getenv("RIMPORT_SKIP_USER_CHECK") != "1":
+        ensure_running_as("cesmdata", sys.argv)
 
     root = Path(args.inputdata).expanduser().resolve()
     if not root.exists():

tests/rimport/__init__.py

@@ -0,0 +1 @@
+"""Tests for rimport module."""

tests/rimport/test_build_parser.py

@@ -0,0 +1,113 @@
+"""
+Tests for build_parser() function in rimport script.
+"""
+
+import os
+import sys
+import argparse
+import importlib.util
+from importlib.machinery import SourceFileLoader
+
+import pytest
+
+# Import rimport module from file without .py extension
+rimport_path = os.path.join(
+    os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))),
+    "rimport",
+)
+loader = SourceFileLoader("rimport", rimport_path)
+spec = importlib.util.spec_from_loader("rimport", loader)
+if spec is None:
+    raise ImportError(f"Could not create spec for rimport from {rimport_path}")
+rimport = importlib.util.module_from_spec(spec)
+sys.modules["rimport"] = rimport
+loader.exec_module(rimport)
+
+
+class TestBuildParser:
+    """Test suite for build_parser() function."""
+
+    def test_parser_creation(self):
+        """Test that build_parser creates an ArgumentParser."""
+        parser = rimport.build_parser()
+        assert isinstance(parser, argparse.ArgumentParser)
+
+    def test_parser_prog_name(self):
+        """Test that parser has correct program name."""
+        parser = rimport.build_parser()
+        assert parser.prog == "rimport"
+
+    def test_file_argument_accepted(self):
+        """Test that -file argument is accepted."""
+        parser = rimport.build_parser()
+        args = parser.parse_args(["-file", "test.txt"])
+        assert args.file == "test.txt"
+        assert args.filelist is None
+
+    def test_list_argument_accepted(self):
+        """Test that -list argument is accepted."""
+        parser = rimport.build_parser()
+        args = parser.parse_args(["-list", "files.txt"])
+        assert args.filelist == "files.txt"
+        assert args.file is None
+
+    def test_file_and_list_mutually_exclusive(self, capsys):
+        """Test that -file and -list cannot be used together."""
+        parser = rimport.build_parser()
+        with pytest.raises(SystemExit):
+            parser.parse_args(["-file", "test.txt", "-list", "files.txt"])
+
+        # Check that the error message explains the problem
+        captured = capsys.readouterr()
+        stderr_lines = captured.err.strip().split("\n")
+        assert "not allowed with argument" in stderr_lines[-1]
+
+    def test_file_or_list_required(self, capsys):
+        """Test that either -file or -list is required."""
+        parser = rimport.build_parser()
+        with pytest.raises(SystemExit):
+            parser.parse_args([])
+
+        # Check that the error message explains the problem
+        captured = capsys.readouterr()
+        stderr_lines = captured.err.strip().split("\n")
+        assert "rimport: error: one of the arguments" in stderr_lines[-1]
+
+    def test_inputdata_default(self):
+        """Test that -inputdata has correct default value."""
+        parser = rimport.build_parser()
+        args = parser.parse_args(["-file", "test.txt"])
+        expected_default = os.path.join(
+            "/glade", "campaign", "cesm", "cesmdata", "cseg", "inputdata"
+        )
+        assert args.inputdata == expected_default
+
+    def test_inputdata_custom(self):
+        """Test that -inputdata can be customized."""
+        parser = rimport.build_parser()
+        custom_path = "/custom/path"
+        args = parser.parse_args(["-file", "test.txt", "-inputdata", custom_path])
+        assert args.inputdata == custom_path
+
+    @pytest.mark.parametrize("help_flag", ["-help", "-h"])
+    def test_help_flags_show_help(self, help_flag):
+        """Test that -help and -h flags trigger help."""
+        parser = rimport.build_parser()
+        with pytest.raises(SystemExit) as exc_info:
+            parser.parse_args([help_flag])
+        # Help should exit with code 0
+        assert exc_info.value.code == 0
+
+    def test_file_with_inputdata(self):
+        """Test combining -file with -inputdata."""
+        parser = rimport.build_parser()
+        args = parser.parse_args(["-file", "data.nc", "-inputdata", "/my/data"])
+        assert args.file == "data.nc"
+        assert args.inputdata == "/my/data"
+
+    def test_list_with_inputdata(self):
+        """Test combining -list with -inputdata."""
+        parser = rimport.build_parser()
+        args = parser.parse_args(["-list", "files.txt", "-inputdata", "/my/data"])
+        assert args.filelist == "files.txt"
+        assert args.inputdata == "/my/data"