From 709bd37e8057e55d20186dff0b717719a09fdd2e Mon Sep 17 00:00:00 2001 From: Nathan Esquenazi Date: Thu, 23 Apr 2026 08:59:36 -0700 Subject: [PATCH] fix(tests): patch Session.save outside thread loop in issue798 test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_concurrent_new_sessions_get_correct_profiles nests `with patch.object(m.Session, 'save', return_value=None)` inside each of two threads. unittest.mock has a known thread-safety bug here: if thread B enters its `with` before thread A exits, thread A's __exit__ can capture thread B's mock as the "original" and restore that. End state: the class attribute is permanently a MagicMock, breaking every later test that writes a real session file. This was the root cause of the "pre-existing ordering-sensitive flaky tests" observed in every prior review — test_issue856, test_sprint46, test_provider_management all hit it with different symptoms. Fix: apply the patch ONCE outside the thread loop, covering both workers. Race being reproduced (explicit profile vs global) is unaffected — the threads still run concurrently, they just share the single patch scope. Full suite before: 2-4 ordering flakes depending on exact subset. Full suite after: 1905 passed, 0 failed. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_issue798.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/test_issue798.py b/tests/test_issue798.py index cfe51446..fef8a00b 100644 --- a/tests/test_issue798.py +++ b/tests/test_issue798.py @@ -133,18 +133,24 @@ def test_concurrent_new_sessions_get_correct_profiles(): results = {} errors = [] + # Patch Session.save ONCE around both threads — not once per thread. + # Per-thread `with patch.object(...)` nested across threads has a known + # concurrency bug in unittest.mock where one thread's __exit__ can capture + # the other thread's mock as the "original" and leave the class attribute + # permanently pointing at a MagicMock, breaking every later test that + # calls Session.save (any test writing a real session file). def make_session(profile_name, key): try: - with patch.object(m.Session, 'save', return_value=None): - s = m.new_session(profile=profile_name) + s = m.new_session(profile=profile_name) results[key] = s.profile except Exception as exc: errors.append(exc) - t1 = threading.Thread(target=make_session, args=('alice', 'alice')) - t2 = threading.Thread(target=make_session, args=('bob', 'bob')) - t1.start(); t2.start() - t1.join(timeout=5); t2.join(timeout=5) + with patch.object(m.Session, 'save', return_value=None): + t1 = threading.Thread(target=make_session, args=('alice', 'alice')) + t2 = threading.Thread(target=make_session, args=('bob', 'bob')) + t1.start(); t2.start() + t1.join(timeout=5); t2.join(timeout=5) assert not errors, f"Threads raised: {errors}" assert results.get('alice') == 'alice', f"alice session had profile {results.get('alice')!r}"