Repository: materialsvirtuallab/monty

Issue:
[Dev] Unit test for `tempfile` alters test file in place
Unit test for `tempfile` (`test_tempfile.py`) alters test file `3000_lines.txt.gz` in place (because of re-gzip), could lead to repeated commit of this file, for example in fd6b16ddde887573d1f8b986d00bdf5a2407f840.

Triggered by the following test: https://github.com/materialsvirtuallab/monty/blob/09d36926b6e613738adf19e61da61bc9f85f11e2/tests/test_tempfile.py#L47-L66

Discussion:

Patch:
diff --git a/src/monty/shutil.py b/src/monty/shutil.py
index 37d28a3..98047f5 100644
--- a/src/monty/shutil.py
+++ b/src/monty/shutil.py
@@ -54,11 +54,14 @@ def gzip_dir(path: str | Path, compresslevel: int = 6) -> None:
         compresslevel (int): Level of compression, 1-9. 9 is default for
             GzipFile, 6 is default for gzip.
     """
-    path = Path(path)
-    for root, _, files in os.walk(path):
+    for root, _, files in os.walk(Path(path)):
         for f in files:
             full_f = Path(root, f).resolve()
             if Path(f).suffix.lower() != ".gz" and not full_f.is_dir():
+                if os.path.exists(f"{full_f}.gz"):
+                    warnings.warn(f"Both {f} and {f}.gz exist.", stacklevel=2)
+                    continue
+
                 with (
                     open(full_f, "rb") as f_in,
                     GzipFile(


Test patch:
diff --git a/tests/test_shutil.py b/tests/test_shutil.py
index 98ddc1a..16cd9aa 100644
--- a/tests/test_shutil.py
+++ b/tests/test_shutil.py
@@ -127,7 +127,7 @@ class TestGzipDir:
 
         self.mtime = os.path.getmtime(os.path.join(test_dir, "gzip_dir", "tempfile"))
 
-    def test_gzip(self):
+    def test_gzip_dir(self):
         full_f = os.path.join(test_dir, "gzip_dir", "tempfile")
         gzip_dir(os.path.join(test_dir, "gzip_dir"))
 
@@ -139,6 +139,29 @@ class TestGzipDir:
 
         assert os.path.getmtime(f"{full_f}.gz") == pytest.approx(self.mtime, 4)
 
+    def test_gzip_dir_file_coexist(self):
+        """Test case where both file and file.gz exist."""
+        full_f = os.path.join(test_dir, "gzip_dir", "temptestfile")
+        gz_f = f"{full_f}.gz"
+
+        # Create both the file and its gzipped version
+        with open(full_f, "w") as f:
+            f.write("not gzipped")
+        with GzipFile(gz_f, "wb") as g:
+            g.write(b"gzipped")
+
+        with pytest.warns(
+            UserWarning, match="Both temptestfile and temptestfile.gz exist."
+        ):
+            gzip_dir(os.path.join(test_dir, "gzip_dir"))
+
+        # Verify contents of the files
+        with open(full_f, "r") as f:
+            assert f.read() == "not gzipped"
+
+        with GzipFile(gz_f, "rb") as g:
+            assert g.read() == b"gzipped"
+
     def test_handle_sub_dirs(self):
         sub_dir = os.path.join(test_dir, "gzip_dir", "sub_dir")
         sub_file = os.path.join(sub_dir, "new_tempfile")
diff --git a/tests/test_tempfile.py b/tests/test_tempfile.py
index 4bce408..4dfd3b1 100644
--- a/tests/test_tempfile.py
+++ b/tests/test_tempfile.py
@@ -3,6 +3,7 @@ from __future__ import annotations
 import os
 import shutil
 
+import pytest
 from monty.tempfile import ScratchDir
 
 TEST_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "test_files")
@@ -49,23 +50,24 @@ class TestScratchDir:
         # We write a pre-scratch file.
         with open("pre_scratch_text", "w") as f:
             f.write("write")
-        init_gz = [f for f in os.listdir(os.getcwd()) if f.endswith(".gz")]
-        with (
-            ScratchDir(
-                self.scratch_root,
-                copy_from_current_on_enter=True,
-                copy_to_current_on_exit=True,
-                gzip_on_exit=True,
-            ),
-            open("scratch_text", "w") as f,
-        ):
-            f.write("write")
+        init_gz_files = [f for f in os.listdir(os.getcwd()) if f.endswith(".gz")]
+        with pytest.warns(match="Both 3000_lines.txt and 3000_lines.txt.gz exist."):
+            with (
+                ScratchDir(
+                    self.scratch_root,
+                    copy_from_current_on_enter=True,
+                    copy_to_current_on_exit=True,
+                    gzip_on_exit=True,
+                ),
+                open("scratch_text", "w") as f,
+            ):
+                f.write("write")
         files = os.listdir(os.getcwd())
 
-        # Make sure the stratch_text.gz exists
+        # Make sure the scratch_text.gz exists
         assert "scratch_text.gz" in files
         for f in files:
-            if f.endswith(".gz") and f not in init_gz:
+            if f.endswith(".gz") and f not in init_gz_files:
                 os.remove(f)
         os.remove("pre_scratch_text")