Simplify tests #2

lib/crewai/tests/utilities/test_lock_store.py

@@ -1,13 +1,14 @@
-"""Tests for lock_store."""
+"""Tests for lock_store.
+
+We verify our own logic: the _redis_available guard and which portalocker
+backend is selected. We trust portalocker to handle actual locking mechanics.
+"""
 
 from __future__ import annotations
 
 import sys
-import threading
-import time
 from unittest import mock
 
-import portalocker.exceptions
 import pytest
 
 import crewai.utilities.lock_store as lock_store
@@ -16,7 +17,6 @@ from crewai.utilities.lock_store import lock
 
 @pytest.fixture(autouse=True)
 def no_redis_url(monkeypatch):
-    """Unset REDIS_URL for every test so file-based locking is used by default."""
     monkeypatch.setattr(lock_store, "_REDIS_URL", None)
 
 
@@ -31,8 +31,7 @@ def test_redis_not_available_without_url():
 
 def test_redis_not_available_when_package_missing(monkeypatch):
     monkeypatch.setattr(lock_store, "_REDIS_URL", "redis://localhost:6379")
-    # Setting a key to None in sys.modules causes ImportError on import
-    monkeypatch.setitem(sys.modules, "redis", None)
+    monkeypatch.setitem(sys.modules, "redis", None)  # None → ImportError on import
     assert lock_store._redis_available() is False
 
 
@@ -43,89 +42,21 @@ def test_redis_available_with_url_and_package(monkeypatch):
 
 
 # ---------------------------------------------------------------------------
-# file-based lock
+# lock strategy selection
 # ---------------------------------------------------------------------------
 
 
-def test_lock_yields():
-    with lock("basic"):
-        pass
+def test_uses_file_lock_when_redis_unavailable():
+    with mock.patch("portalocker.Lock") as mock_lock:
+        with lock("file_test"):
+            pass
+
+    mock_lock.assert_called_once()
+    assert "crewai:" in mock_lock.call_args.args[0]
 
 
-def test_lock_releases_on_exception():
-    with pytest.raises(ValueError):
-        with lock("on_exception"):
-            raise ValueError("boom")
-
-    # would hang or raise LockException if the lock was not released
-    with lock("on_exception"):
-        pass
-
-
-def test_lock_is_mutually_exclusive_across_threads():
-    concurrent_holders = 0
-    max_concurrent = 0
-    counter_lock = threading.Lock()
-    barrier = threading.Barrier(5)
-
-    def worker():
-        nonlocal concurrent_holders, max_concurrent
-        barrier.wait()  # all threads compete at the same time
-        with lock("mutex", timeout=10):
-            with counter_lock:
-                concurrent_holders += 1
-                max_concurrent = max(max_concurrent, concurrent_holders)
-            time.sleep(0.01)  # hold briefly so overlap is detectable if locking fails
-            with counter_lock:
-                concurrent_holders -= 1
-
-    threads = [threading.Thread(target=worker) for _ in range(5)]
-    for t in threads:
-        t.start()
-    for t in threads:
-        t.join(timeout=10)
-    assert all(not t.is_alive() for t in threads), "threads did not finish in time"
-
-    assert max_concurrent == 1
-
-
-def test_lock_timeout_raises_when_held():
-    acquired = threading.Event()
-    release = threading.Event()
-
-    def holder():
-        with lock("timeout_test", timeout=10):
-            acquired.set()
-            release.wait()
-
-    t = threading.Thread(target=holder)
-    t.start()
-    acquired.wait()
-
-    try:
-        with pytest.raises(portalocker.exceptions.LockException):
-            with lock("timeout_test", timeout=0.1):
-                pass
-    finally:
-        release.set()
-        t.join(timeout=10)
-    assert not t.is_alive(), "holder thread did not finish in time"
-
-
-def test_different_names_are_independent():
-    with lock("alpha"):
-        with lock("beta"):
-            pass  # would deadlock if names mapped to the same lock
-
-
-# ---------------------------------------------------------------------------
-# Redis path
-# ---------------------------------------------------------------------------
-
-
-def test_redis_lock_used_when_available(monkeypatch):
+def test_uses_redis_lock_when_redis_available(monkeypatch):
     fake_conn = mock.MagicMock()
-    monkeypatch.setattr(lock_store, "_REDIS_URL", "redis://localhost:6379")
     monkeypatch.setattr(lock_store, "_redis_available", mock.Mock(return_value=True))
     monkeypatch.setattr(lock_store, "_redis_connection", mock.Mock(return_value=fake_conn))