Merge pull request #22 from samsrabin/fix-check-for-published-files

Fix --check for published files

Author: samsrabin

rimport

@@ -210,10 +210,15 @@ def stage_data(
     dst = staging_root / rel
 
     if dst.exists():
-        logger.info("File is already published but NOT linked; linking now.")
-        replace_one_file_with_symlink(inputdata_root, staging_root, str(src))
-        print_can_file_be_downloaded(can_file_be_downloaded(rel, staging_root))
-        check_relink_worked(src, dst)
+        msg = "File is already published but NOT linked"
+        if check:
+            logger.info("%s; would link.", msg)
+            print_can_file_be_downloaded(can_file_be_downloaded(rel, staging_root))
+        else:
+            logger.info("%s; linking now.", msg)
+            replace_one_file_with_symlink(inputdata_root, staging_root, str(src))
+            check_relink_worked(src, dst)
+            print_can_file_be_downloaded(can_file_be_downloaded(rel, staging_root))
         return
 
     if check:

tests/rimport/test_can_file_be_downloaded.py

@@ -43,23 +43,26 @@ def test_existing_file_exists(self):
     def test_true_abspath(self):
         """Test that can_file_be_downloaded() is true for an existing file given absolute path"""
         file_abspath = Path(os.path.join(DEFAULT_STAGING_ROOT, RELPATH_THAT_DOES_EXIST))
-        assert rimport.can_file_be_downloaded(
+        result = rimport.can_file_be_downloaded(
             file_abspath,
             DEFAULT_STAGING_ROOT,
         )
+        assert result, f"Maybe the server is down? Try again. Result was: {result}"
 
     def test_true_relpath(self):
         """Test that can_file_be_downloaded() is true for an existing file given relative path"""
         file_relpath = Path(RELPATH_THAT_DOES_EXIST)
-        assert rimport.can_file_be_downloaded(
+        result = rimport.can_file_be_downloaded(
             file_relpath,
             DEFAULT_STAGING_ROOT,
         )
+        assert result, f"Maybe the server is down? Try again. Result was: {result}"
 
     def test_false_nonexistent(self):
         """Test that can_file_be_downloaded() is false for a nonexistent file"""
         file_relpath = Path("weurueridniduafnea/smfnigsroerij/msdif8ernnr.nc")
-        assert not rimport.can_file_be_downloaded(
+        result = rimport.can_file_be_downloaded(
             file_relpath,
             DEFAULT_STAGING_ROOT,
         )
+        assert not result, f"Unexpected result: {result}"

tests/rimport/test_cmdline.py

@@ -455,13 +455,14 @@ def test_error_symlink_pointing_outside_staging(
         msg = "is outside staging directory"
         assert msg in result.stderr
 
-    def test_check_doesnt_copy(self, rimport_script, test_env, rimport_env):
-        """Test that a file is NOT copied to the staging directory if check is True."""
+    def test_check_doesnt_copy_unpublished(self, rimport_script, test_env, rimport_env):
+        """Test that an unpublished file is not copied to the staging directory if check is True."""
         inputdata_root = test_env["inputdata_root"]
         staging_root = test_env["staging_root"]
 
         # Create a file in inputdata
-        test_file = inputdata_root / "test.nc"
+        file_basename = "test.nc"
+        test_file = inputdata_root / file_basename
         test_file.write_text("test data")
 
         # Make sure --check skips ensure_running_as()
@@ -472,7 +473,7 @@ def test_check_doesnt_copy(self, rimport_script, test_env, rimport_env):
             sys.executable,
             rimport_script,
             "-file",
-            "test.nc",
+            file_basename,
             "-inputdata",
             str(inputdata_root),
             "--check",
@@ -490,11 +491,66 @@ def test_check_doesnt_copy(self, rimport_script, test_env, rimport_env):
         assert result.returncode == 0, f"Command failed: {result.stderr}"
 
         # Verify file was not staged
-        staged_file = staging_root / "test.nc"
+        staged_file = staging_root / file_basename
         assert not staged_file.exists()
 
         # Verify file was not replaced with a symlink
         assert not test_file.is_symlink()
 
         # Verify message was printed
         assert "not already published" in result.stdout
+
+        # Verify messages weren't printed
+        assert "already published but NOT linked".lower() not in result.stdout.lower()
+        assert "Deleted original file".lower() not in result.stdout.lower()
+        assert "Created symbolic link".lower() not in result.stdout.lower()
+        assert "Error creating symlink".lower() not in result.stdout.lower()
+
+    def test_check_doesnt_relink_published(self, rimport_script, test_env, rimport_env):
+        """Test that published file is not relinked if check is True."""
+        inputdata_root = test_env["inputdata_root"]
+        staging_root = test_env["staging_root"]
+
+        # Create a file in inputdata and staging
+        file_basename = "test.nc"
+        test_file = inputdata_root / file_basename
+        test_file.write_text("test data")
+        staged_file = staging_root / file_basename
+        staged_file.write_text("test data")
+
+        # Make sure --check skips ensure_running_as()
+        del rimport_env["RIMPORT_SKIP_USER_CHECK"]
+
+        # Run rimport with --check option
+        command = [
+            sys.executable,
+            rimport_script,
+            "-file",
+            file_basename,
+            "-inputdata",
+            str(inputdata_root),
+            "--check",
+        ]
+
+        result = subprocess.run(
+            command,
+            capture_output=True,
+            text=True,
+            check=False,
+            env=rimport_env,
+        )
+
+        # Verify success
+        assert result.returncode == 0, f"Command failed: {result.stderr}"
+
+        # Verify file was not replaced with a symlink
+        assert not test_file.is_symlink()
+
+        # Verify message was printed
+        assert "already published but NOT linked".lower() in result.stdout.lower()
+
+        # Verify messages weren't printed
+        assert "linking now".lower() not in result.stdout.lower()
+        assert "Deleted original file".lower() not in result.stdout.lower()
+        assert "Created symbolic link".lower() not in result.stdout.lower()
+        assert "Error creating symlink".lower() not in result.stdout.lower()

tests/rimport/test_stage_data.py

@@ -54,8 +54,8 @@ def fixture_staging_root(tmp_path):
 class TestStageData:
     """Test suite for stage_data() function."""
 
-    def test_copies_file_to_staging(self, inputdata_root, staging_root):
-        """Test that a file is copied to the staging directory."""
+    def test_copies_and_relinks_unpublished(self, inputdata_root, staging_root):
+        """Test that an unpublished file is copied to the staging directory and relinked."""
         # Create file in inputdata root
         src = inputdata_root / "file.nc"
         src.write_text("data content")
@@ -72,22 +72,35 @@ def test_copies_file_to_staging(self, inputdata_root, staging_root):
         assert src.is_symlink()
         assert src.resolve() == dst
 
-    def test_check_doesnt_copy(self, inputdata_root, staging_root, caplog):
-        """Test that a file is NOT copied to the staging directory if check is True"""
+    def test_check_doesnt_copy_unpublished(self, inputdata_root, staging_root, caplog):
+        """Test that an unpublished file is NOT copied to the staging directory if check is True"""
         # Create file in inputdata root
-        src = inputdata_root / "file.nc"
+        file_basename = "file.nc"
+        src = inputdata_root / file_basename
         src.write_text("data content")
 
         # Check the file
         rimport.stage_data(src, inputdata_root, staging_root, check=True)
 
-        # Verify file was NOT copied to staging
-        dst = staging_root / "file.nc"
+        # Verify file was NOT copied to staging or relinked
+        dst = staging_root / file_basename
         assert not dst.exists()
         assert not src.is_symlink()
 
-        # Verify message was logged
-        assert "not already published" in caplog.text
+    def test_check_doesnt_relink_published(self, inputdata_root, staging_root, caplog):
+        """Test that a published file is NOT relinked if check is True"""
+        # Create file in inputdata root and staging
+        file_basename = "file.nc"
+        src = inputdata_root / file_basename
+        src.write_text("data content")
+        dst = staging_root / file_basename
+        dst.write_text("data content")
+
+        # Check the file
+        rimport.stage_data(src, inputdata_root, staging_root, check=True)
+
+        # Verify file was NOT relinked
+        assert not src.is_symlink()
 
     def test_preserves_directory_structure(self, inputdata_root, staging_root):
         """Test that directory structure is preserved in staging."""