Improve robustness of relink test imports.

Author: samsrabin

tests/rimport/test_build_parser.py

@@ -3,9 +3,7 @@
 """
 
 import os
-import sys
 import argparse
-from pathlib import Path
 import importlib.util
 from importlib.machinery import SourceFileLoader
 
@@ -21,7 +19,7 @@
 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
+# Don't add to sys.modules to avoid conflict with other test files
 loader.exec_module(rimport)
 
 

tests/rimport/test_can_file_be_downloaded.py

@@ -3,7 +3,6 @@
 """
 
 import os
-import sys
 import importlib.util
 from importlib.machinery import SourceFileLoader
 from pathlib import Path
@@ -22,7 +21,7 @@
 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
+# Don't add to sys.modules to avoid conflict with other test files
 loader.exec_module(rimport)
 
 RELPATH_THAT_DOES_EXIST = os.path.join(

tests/rimport/test_ensure_running_as.py

@@ -3,7 +3,6 @@
 """
 
 import os
-import sys
 import importlib.util
 from importlib.machinery import SourceFileLoader
 from unittest.mock import patch, MagicMock
@@ -20,7 +19,7 @@
 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
+# Don't add to sys.modules to avoid conflict with other test files
 loader.exec_module(rimport)
 
 

tests/rimport/test_get_staging_root.py

@@ -3,7 +3,6 @@
 """
 
 import os
-import sys
 import importlib.util
 from importlib.machinery import SourceFileLoader
 from pathlib import Path
@@ -20,7 +19,7 @@
 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
+# Don't add to sys.modules to avoid conflict with other test files
 loader.exec_module(rimport)
 
 

tests/rimport/test_normalize_paths.py

@@ -3,7 +3,6 @@
 """
 
 import os
-import sys
 import importlib.util
 from importlib.machinery import SourceFileLoader
 from pathlib import Path
@@ -20,7 +19,7 @@
 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
+# Don't add to sys.modules to avoid conflict with other test files
 loader.exec_module(rimport)
 
 

tests/rimport/test_read_filelist.py

@@ -3,7 +3,6 @@
 """
 
 import os
-import sys
 import importlib.util
 from importlib.machinery import SourceFileLoader
 
@@ -17,7 +16,7 @@
 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
+# Don't add to sys.modules to avoid conflict with other test files
 loader.exec_module(rimport)
 
 

tests/rimport/test_stage_data.py

@@ -3,13 +3,15 @@
 """
 
 import os
-import sys
+import logging
 import importlib.util
 from importlib.machinery import SourceFileLoader
 from unittest.mock import patch
 
 import pytest
 
+import shared
+
 # 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__)))),
@@ -20,10 +22,19 @@
 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
+# Don't add to sys.modules to avoid conflict with other test files
 loader.exec_module(rimport)
 
 
+@pytest.fixture(autouse=True)
+def configure_logging_for_tests():
+    """Configure logging for all tests in this module."""
+    shared.configure_logging(logging.INFO)
+    yield
+    # Cleanup
+    rimport.logger.handlers.clear()
+
+
 @pytest.fixture(name="inputdata_root")
 def fixture_inputdata_root(tmp_path):
     """Create and return an inputdata root directory."""
@@ -119,8 +130,9 @@ def test_prints_live_symlink_already_published_not_downloadable(
             # Verify that shutil.copy2 was never called (function returned early)
             mock_copy.assert_not_called()
 
+    @patch.object(rimport, "can_file_be_downloaded")
     def test_prints_live_symlink_already_published_is_downloadable(
-        self, inputdata_root, staging_root, caplog
+        self, mock_can_file_be_downloaded, inputdata_root, staging_root, caplog
     ):
         """
         Like test_prints_live_symlink_already_published_not_downloadable, but mocks
@@ -132,15 +144,16 @@ def test_prints_live_symlink_already_published_is_downloadable(
         src = inputdata_root / "link.nc"
         src.symlink_to(real_file)
 
+        # Mock can_file_be_downloaded to return True
+        mock_can_file_be_downloaded.return_value = True
+
         # Mock shutil.copy2 to verify it's never called
         with patch("shutil.copy2") as mock_copy:
-            # Mock can_file_be_downloaded to return True
-            with patch("rimport.can_file_be_downloaded", return_value=True):
-                # Should print message for live symlink and return early
-                rimport.stage_data(src, inputdata_root, staging_root)
+            # Should print message for live symlink and return early
+            rimport.stage_data(src, inputdata_root, staging_root)
 
-                # Verify that shutil.copy2 was never called (function returned early)
-                mock_copy.assert_not_called()
+            # Verify that shutil.copy2 was never called (function returned early)
+            mock_copy.assert_not_called()
 
         # Verify the right messages were logged
         msg = "File is already published and linked"
@@ -152,8 +165,9 @@ def test_prints_live_symlink_already_published_is_downloadable(
         msg = "is already under staging directory"
         assert msg not in caplog.text
 
+    @patch.object(rimport, "can_file_be_downloaded")
     def test_prints_published_but_not_linked(
-        self, inputdata_root, staging_root, caplog
+        self, mock_can_file_be_downloaded, inputdata_root, staging_root, caplog
     ):
         """
         Tests printed message for when a file has been published (copied to staging root) but not
@@ -166,15 +180,17 @@ def test_prints_published_but_not_linked(
         inputdata = inputdata_root / filename
         inputdata.write_text("data")
 
+        # Mock can_file_be_downloaded to return True
+        mock_can_file_be_downloaded.return_value = True
+
         # Mock shutil.copy2 to verify it's never called
         with patch("shutil.copy2") as mock_copy:
-            # Mock can_file_be_downloaded to return True
-            with patch("rimport.can_file_be_downloaded", return_value=True):
-                # Should print message for live symlink and return early
-                rimport.stage_data(inputdata, inputdata_root, staging_root)
 
-                # Verify that shutil.copy2 was never called (function returned early)
-                mock_copy.assert_not_called()
+            # Should print message for live symlink and return early
+            rimport.stage_data(inputdata, inputdata_root, staging_root)
+
+            # Verify that shutil.copy2 was never called (function returned early)
+            mock_copy.assert_not_called()
 
         # Verify the right messages were logged or not
         msg = "File is already published and linked"