Skip to content

Commit fabf0fd

Browse files
GWealecopybara-github
authored andcommitted
test: stop the cross-loop startup tests from racing on their own mock
Both cross-loop startup tests entered mock.patch.object on the shared plugin instance from inside each worker thread. patch.object swaps and restores one attribute on one object and is not thread safe: when two threads read the original before either installs its mock, both record that the attribute was absent from the instance, and both delete it on exit. The second delete raises, so the test failed with AttributeError: object has no attribute "_lazy_setup" which is the mock unwinding itself, not anything about coalescing. Install the mock once from the test thread and let the worker threads race only on _ensure_started, which is what these tests are for. The behaviour under test is unchanged: both loops still call in concurrently and setup still has to coalesce to a single run. Co-authored-by: George Weale <gweale@google.com> PiperOrigin-RevId: 953746627
1 parent 472e463 commit fabf0fd

1 file changed

Lines changed: 30 additions & 26 deletions

File tree

tests/unittests/plugins/test_bigquery_agent_analytics_plugin.py

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10460,20 +10460,21 @@ async def fake_lazy_setup(**kwargs):
1046010460

1046110461
def run_in_fresh_loop():
1046210462
try:
10463-
with mock.patch.object(
10464-
plugin, "_lazy_setup", side_effect=fake_lazy_setup
10465-
):
10466-
asyncio.run(plugin._ensure_started())
10463+
asyncio.run(plugin._ensure_started())
1046710464
except BaseException as e: # noqa: BLE001 - collecting for assertion
1046810465
errors.append(e)
1046910466

10470-
threads = [
10471-
platform_thread.create_thread(run_in_fresh_loop) for _ in range(2)
10472-
]
10473-
for t in threads:
10474-
t.start()
10475-
for t in threads:
10476-
t.join(timeout=10)
10467+
# Patch from this thread only. patch.object swaps a single shared
10468+
# attribute and is not itself thread safe, so entering it from both
10469+
# threads raced on _lazy_setup instead of on the code under test.
10470+
with mock.patch.object(plugin, "_lazy_setup", side_effect=fake_lazy_setup):
10471+
threads = [
10472+
platform_thread.create_thread(run_in_fresh_loop) for _ in range(2)
10473+
]
10474+
for t in threads:
10475+
t.start()
10476+
for t in threads:
10477+
t.join(timeout=10)
1047710478
assert not errors, f"cross-loop startup raised: {errors}"
1047810479

1047910480
def test_concurrent_stale_cleanup_folds_once(
@@ -10593,24 +10594,27 @@ async def slow_setup(**kwargs):
1059310594

1059410595
def run_in_fresh_loop():
1059510596
try:
10596-
with mock.patch.object(plugin, "_lazy_setup", side_effect=slow_setup):
10597-
barrier.wait(timeout=5)
10598-
asyncio.run(plugin._ensure_started())
10597+
barrier.wait(timeout=5)
10598+
asyncio.run(plugin._ensure_started())
1059910599
except BaseException as e: # noqa: BLE001
1060010600
errors.append(e)
1060110601

10602-
threads = [
10603-
platform_thread.create_thread(run_in_fresh_loop) for _ in range(2)
10604-
]
10605-
for t in threads:
10606-
t.start()
10607-
# Deterministic rendezvous: hold the owner inside setup until BOTH
10608-
# threads have entered _ensure_started.
10609-
entered.wait(timeout=5)
10610-
release.set()
10611-
for t in threads:
10612-
t.join(timeout=10)
10613-
assert not t.is_alive(), "thread failed to terminate"
10602+
# Patch from this thread only. patch.object swaps a single shared
10603+
# attribute and is not itself thread safe, so entering it from both
10604+
# threads raced on _lazy_setup instead of on the code under test.
10605+
with mock.patch.object(plugin, "_lazy_setup", side_effect=slow_setup):
10606+
threads = [
10607+
platform_thread.create_thread(run_in_fresh_loop) for _ in range(2)
10608+
]
10609+
for t in threads:
10610+
t.start()
10611+
# Deterministic rendezvous: hold the owner inside setup until BOTH
10612+
# threads have entered _ensure_started.
10613+
entered.wait(timeout=5)
10614+
release.set()
10615+
for t in threads:
10616+
t.join(timeout=10)
10617+
assert not t.is_alive(), "thread failed to terminate"
1061410618

1061510619
assert not errors, f"cross-loop startup raised: {errors}"
1061610620
assert len(setup_calls) == 1, f"shared setup ran {len(setup_calls)} times"

0 commit comments

Comments
 (0)