Skip to content

Commit 9128c3d

Browse files
CrispStrobeclaude
andcommitted
fix(test): hoist auth patches out of per-thread bodies (mock thread-safety)
`test_isolated_session_separate_threads_get_separate_clients` failed intermittently on the 3.10 lane (passed 3.11/3.12) with KeyError: 2 — the assertion `clients[1] is not clients[2]` couldn't find the second slot because thread 2 had errored mid-flight. Root cause: each thread independently entered a `with patch('services.auth.auth_service.get_auth_details', ...)` block. `unittest.mock.patch` is not thread-safe — its __enter__ saves the original attribute and __exit__ restores it, with no locking between threads. When t1's __exit__ races with t2's __enter__ (or vice versa), the real `get_auth_details` ends up callable for a brief window, raising MissingCredentialsError on the unauthenticated CI runner. That exception killed thread 2 silently, leaving the clients[2] slot unset, which produced the KeyError on the post-join assertion (instead of surfacing the actual error). Two changes: 1. Hoist both patches (`get_auth_details` and `refresh_tokens`) out of the per-thread function so they wrap the entire join window. The threads see a stable mock auth surface for their full lifetime, no patch/unpatch racing. 2. Capture per-thread exceptions in an `errors` dict, asserted before the slot comparison. When this test ever does fail in the future for an unrelated reason, the failure message will name the real exception instead of just "KeyError". Verified locally: passes 5/5 reruns of the targeted test, plus the full suite stays green (`pytest tests/ -q --ignore=tests/test_live_smoke.py` → 100%). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent c68f588 commit 9128c3d

1 file changed

Lines changed: 27 additions & 11 deletions

File tree

tests/test_webdav_misc.py

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -201,25 +201,41 @@ def test_get_content_writes_error_message_to_returned_bytesio():
201201
# ---------- isolated session: 'no refresh' variant honors thread-local boundary ----------
202202

203203
def test_isolated_session_separate_threads_get_separate_clients():
204-
"""Thread-local design: different threads get different clients."""
204+
"""Thread-local design: different threads get different clients.
205+
206+
Both auth-service patches are hoisted out of the threaded body —
207+
`unittest.mock.patch` is not thread-safe, and races between
208+
`__enter__` / `__exit__` would let real auth code leak into one of
209+
the threads, raise MissingCredentialsError, and leave the
210+
`clients[2]` slot unpopulated → KeyError on the assertion. Patching
211+
once for the whole test gives both threads a stable mocked auth
212+
surface for the entire window in which they call _get_isolated_session.
213+
"""
205214
import threading
206215

207216
api = WebDAVAPIClient()
208217
fake_creds = {'token': 't', 'newToken': 'nt', 'user': {'email': 'u'}}
209218

210219
clients = {}
220+
errors = {}
221+
211222
def grab_client(thread_id):
212-
with patch('services.auth.auth_service.get_auth_details',
213-
return_value=fake_creds), \
214-
patch('services.auth.auth_service.refresh_tokens'):
223+
try:
215224
clients[thread_id] = api._get_isolated_session()
225+
except Exception as e: # surface, don't silently lose the slot
226+
errors[thread_id] = e
216227

217-
t1 = threading.Thread(target=grab_client, args=(1,))
218-
t2 = threading.Thread(target=grab_client, args=(2,))
219-
t1.start()
220-
t2.start()
221-
t1.join()
222-
t2.join()
223-
228+
with patch('services.auth.auth_service.get_auth_details',
229+
return_value=fake_creds), \
230+
patch('services.auth.auth_service.refresh_tokens'):
231+
t1 = threading.Thread(target=grab_client, args=(1,))
232+
t2 = threading.Thread(target=grab_client, args=(2,))
233+
t1.start()
234+
t2.start()
235+
t1.join()
236+
t2.join()
237+
238+
# Surface in-thread errors so the failure is the actual cause, not KeyError.
239+
assert not errors, f"thread errors: {errors}"
224240
# Each thread got its own ApiClient
225241
assert clients[1] is not clients[2]

0 commit comments

Comments
 (0)