test(agent): the worker-start-failure test intercepts the callback worker again

`kwargs.get("target") is sched._run_callback` is always False (a bound method is a fresh object
per access), so the fake never returned Boom and the _dispatch failure branch went untested;
the test passed on the normal worker. Compare with == and assert the interception happened
(mutation: retiring the handle on start failure now fails the test).

The thread-count assertions sampled while per-fire workers were still live; quiesce every
handle with cancel(wait=) before sampling so the count is deterministic (AGENTS.md: timing tests
must not assume a quiet runner).

Follow-up to #106308.
This commit is contained in:
kshitijk4poor
2026-09-09 12:35:42 +05:30
committed by kshitij
parent 8077206073
commit 7f3e0bb59a

View File

@@ -24,18 +24,18 @@ def test_two_intervals_fire_proportionally_and_cancel_stops_one():
assert _wait_until(lambda: len(slow) >= 3)
assert len(fast) > len(slow) # 5x interval ratio -> clearly more fast ticks
# Scheduling another timer creates no persistent per-handle thread.
before = threading.active_count()
sched.schedule(lambda: None, 0.01).cancel()
assert threading.active_count() == before
assert sched._thread is not None and sched._thread.is_alive()
h_fast.cancel()
h_fast.cancel(wait=1.0)
n_fast = len(fast)
time.sleep(0.1)
assert len(fast) == n_fast, "cancelled callback kept firing"
assert len(slow) > 3, "sibling callback stopped when another was cancelled"
h_slow.cancel()
h_slow.cancel(wait=1.0)
# With every handle quiesced, scheduling + cancelling adds no persistent thread.
before = threading.active_count()
sched.schedule(lambda: None, 0.01).cancel(wait=1.0)
assert threading.active_count() == before
def test_raising_callback_is_rescheduled_and_does_not_kill_sibling():
@@ -81,15 +81,15 @@ def test_module_level_schedule_uses_shared_default():
hits = []
h = schedule(lambda: hits.append(1), 0.01)
assert _wait_until(lambda: hits)
h.cancel()
h.cancel(wait=1.0)
thread = periodic_scheduler._DEFAULT._thread
assert thread is not None and thread.name == "hermes-periodic-scheduler"
# Scheduling more timers on the shared default adds no OS threads.
# Scheduling more timers on the shared default adds no persistent OS threads.
before = threading.active_count()
handles = [schedule(lambda: None, 0.01) for _ in range(20)]
assert threading.active_count() == before
for handle in handles:
handle.cancel()
handle.cancel(wait=1.0)
assert threading.active_count() == before
def test_blocked_callback_does_not_stall_due_sibling(monkeypatch):
@@ -132,7 +132,8 @@ def test_worker_start_failure_keeps_timer(monkeypatch):
def flaky(*args, **kwargs):
# Only this scheduler's own callback worker fails, once; a leaked handle on the shared
# _DEFAULT scheduler must not be the one that consumes the single Boom.
if kwargs.get("target") is sched._run_callback and attempts["n"] == 0:
# Bound methods are fresh objects per access: compare with ==, never `is`.
if kwargs.get("target") == sched._run_callback and attempts["n"] == 0:
attempts["n"] += 1
class Boom:
@@ -148,6 +149,7 @@ def test_worker_start_failure_keeps_timer(monkeypatch):
assert _wait_until(lambda: bool(fired), timeout=3.0), (
"worker-start failure silently retired the timer"
)
assert attempts["n"] == 1, "the fake never intercepted the callback worker"
assert not handle.cancelled
finally:
handle.cancel()
handle.cancel(wait=1.0)