tests: fix monitor harness races, tox py313 only, agent guidance - #152
Merged
Conversation
- _wait_for_monitors_idle now also detects still-alive ConnectionMonitor-* threads and raises on timeout instead of returning silently; a leaked thread consumes the next scenario's mocks (the delayed-connect flake) - replace wall-clock mock polling with Event-synchronized side_effects in the three real-thread scenarios; thread-safe wpa status sequence - scope-suppress expected pydantic serializer warnings in the three intentionally-invalid validation scenarios - tox: envlist py313 only (py39/py311 were skip noise) - add AGENTS.md testing/PR rules and CLAUDE.md pointer for coding agents
This was referenced Jul 19, 2026
joshschmelzle
added a commit
that referenced
this pull request
Jul 19, 2026
…153) Two lifecycle defects found during adversarial review of the delayed-connect test flake (see #152): - monitor_loop removed its registry entries BEFORE running the dhcp/route/ autostart phase (and early-return paths skipped cleanup entirely), so a thread could be invisible to stop helpers while side effects were still pending. Cleanup now runs in a finally after the body, on every path. - stop_connection_monitor and stop_all_connection_monitors held _monitor_lock across join(), but the monitor thread's own deregistration needs that lock, guaranteeing the join times out in exactly the window it matters. Joins now happen outside the lock.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the delayed-connect flake that failed CI on dev, plus cleanup.
Root cause of the flake
Single-thread, no zombie required:
patch("...monitor.time.sleep")patchessleepon the global stdlibtimemodule (monitor.py does plainimport time), so the test's owntime.sleep(0.01)poll loop was a no-op. Both the monitor thread and the test busy-spun on the GIL; on a loaded runner a ~2s stall could expire the old wall-clock deadline before the monitor reached the dhcp step.monitor_looppops itself from the registry before running the dhcp/route/app phase, andstop_all_connection_monitorsholds_monitor_lockacross itsjoin(2.0)while the thread's own cleanup wants that lock: a guaranteed-timeout lock convoy that releases both threads simultaneously into patch teardown. The old_wait_for_monitors_idlewatched only the registry and returned instantly.The nested
with patchblocks unwind innermost first:start_app_in_namespaceis unpatched beforerestart_dhcp_with_timeout. The thread's dhcp call landed on the still-patched mock (1 call); its start_app call landed after restore (mock: 0 calls). Exactly the CI evidence:dhcp called once, start_app called 0 times.Changes
_wait_for_monitors_idlerequires the registry empty and no liveConnectionMonitor-*threads (they are already named), and raises on timeout instead of returning silently. This also closes the pop-before-actions blind spot.threading.Eventset by the mock side_effect.Event.waitblocks in C and yields the GIL, so it is immune to the global sleep mock. Cleanup runs inside the patch stack and before assertions, so a failing run cannot tear down mocks under a live thread or leak one.envlist = py313only (py39/py311 were permanent SKIP noise; CI and the shipped target are 3.13).AGENTS.mdwith branch/PR and testing rules (including the globaltime.sleepaliasing gotcha), plus aCLAUDE.mdpointer.Follow-ups (separate PRs)
finallyafter the action phase; stop holding_monitor_lockacrossjoin().requires-python = ">=3.9"and 3.9/3.11 classifiers.