Merge branch 'fixup_filelock_tests' into picklable-shared-membucket---with-tests---test

Author: amaximciuc

python/tests/bucket_tester.py

@@ -10,6 +10,7 @@
 from queue import Queue
 from typing import BinaryIO
 from unittest import TestCase
+import uuid
 
 import pyarrow as pa
 import pyarrow.parquet as pq
@@ -87,7 +88,7 @@ def __init__(self, storage: IBucket, test_case: TestCase) -> None:
         self.storage = storage
         self.test_case = test_case
         # Next is a unique suffix to be used in the names of dirs and files, so they will be unique
-        self.us = f"{iTSms.now() % 100_000_000:08d}"
+        self.us = uuid.uuid4().hex
 
     def cleanup(self) -> None:
         self.storage.remove_prefix(f"dir{self.us}")

python/tests/test_append_only_fs_bucket.py

@@ -7,6 +7,7 @@
 
 from bucketbase import MemoryBucket
 from bucketbase.fs_bucket import AppendOnlyFSBucket, FSBucket
+from bucketbase.named_lock_manager import FileLockManager
 
 
 class TestAppendOnlyFSBucket(unittest.TestCase):
@@ -44,7 +45,8 @@ def put_object(self2, name, content):
         # put object is expected to create a lock file before calling base_bucket.put_object, and remove it after
         bucket_in_test.put_object(object_name, content)
 
-        self.assertFalse(lock_file_path.exists())
+        verifier_manager = FileLockManager(self.locks_path)
+        self.assertTrue(verifier_manager.get_lock(object_name).acquire(timeout=0.1), "Lock should have been released after put_object")
         self.assertEqual(base_bucket_put_calls, [(object_name, content)])
 
     def test_put_object_twice_raises_exception(self):
@@ -89,9 +91,12 @@ def test_lock_object_creates_lock_and_unlock_releases(self):
         # Check if the lock file was created
         lock_file_path = self.locks_path / (object_name.replace(bucket_in_test.SEP, FSBucket.TEMP_SEP) + ".lock")
         self.assertTrue(lock_file_path.exists())
+        verifier_manager = FileLockManager(self.locks_path)
+        with self.assertRaises(TimeoutError):
+            verifier_manager.get_lock(object_name).acquire(timeout=0.1)
 
         bucket_in_test._unlock_object(object_name)
-        self.assertFalse(lock_file_path.exists())
+        self.assertTrue(verifier_manager.get_lock(object_name).acquire(timeout=0.1), "Lock should have been released after _unlock_object")
 
     def test_unlocking_unlocked_object_raises_assertion(self):
         bucket_in_test = AppendOnlyFSBucket(self.base_bucket, self.locks_path)
@@ -213,3 +218,7 @@ def thread2_func():
         t2.join()
         self.assertListEqual(results, ["thread1_done", "thread2_file_exists"])
         self.assertEqual(bucket_in_test.get_object(object_name), content1)
+
+
+if __name__ == "__main__":
+    unittest.main()

python/tests/test_namedlock.py

@@ -71,16 +71,19 @@ def test_sanitizes_path_characters(self):
         lock1.acquire()
         lock2.acquire()
 
-        # Verify both lock files were created with sanitized names
-        files = list(self.temp_dir.glob("*"))
-        sanitized_names = [f.name for f in files]
+        verifier_manager = FileLockManager(self.temp_dir)
+        with self.assertRaises(TimeoutError):
+            verifier_manager.get_lock(name1).acquire(timeout=0.1)
+        with self.assertRaises(TimeoutError):
+            verifier_manager.get_lock(name2).acquire(timeout=0.1)
 
-        self.assertIn("path#with#slashes.lock", sanitized_names)
-        self.assertIn("path#with#slashes2.lock", sanitized_names)
         lock1.release()
         lock2.release()
 
-        self.assertEqual(0, len(list(self.temp_dir.glob("*"))))
+        self.assertTrue(verifier_manager.get_lock(name1).acquire(timeout=0.1))
+        verifier_manager.get_lock(name1).release()
+        self.assertTrue(verifier_manager.get_lock(name2).acquire(timeout=0.1))
+        verifier_manager.get_lock(name2).release()
 
     def test_lock_directory_created(self):
         # Delete the directory to test creation
@@ -93,3 +96,18 @@ def test_lock_directory_created(self):
 
         self.assertTrue(self.temp_dir.exists())
         self.assertTrue(self.temp_dir.is_dir())
+
+    def test_possibly_stale_lock_file_does_not_block_new_manager(self):
+        name = "some_lock"
+        lock1 = self.lock_manager.get_lock(name)
+        lock1.acquire()
+        lock1.release()
+
+        new_manager = FileLockManager(self.temp_dir)
+        lock2 = new_manager.get_lock(name)
+        self.assertTrue(lock2.acquire(timeout=0.1))
+        lock2.release()
+
+
+if __name__ == "__main__":
+    unittest.main()