Repository navigation
tests: replace timing sleeps with condition-based waits - #1252
Merged
Carter Tinney (cartertinney) merged 15 commits intoSep 1, 2026
Merged
Carter Tinney (cartertinney) merged 15 commits into
Carter Tinney (cartertinney) merged 15 commits into
Conversation
Carter Tinney (cartertinney)
changed the base branch from
main
to
ct/stabilize-uv-cache
September 1, 2026 13:17
Carter Tinney (cartertinney)
force-pushed
the
agents/unit-test-failure-analysis-python312
branch
from
September 1, 2026 13:17
97a22f5 to
a8070b5
Compare
Ewerton Scaboro da Silva (ewertons)
requested a balanced review from Copilot
September 1, 2026 16:00
Copilot started reviewing on behalf of
Ewerton Scaboro da Silva (ewertons)
September 1, 2026 16:01
View session
Ewerton Scaboro da Silva (ewertons)
approved these changes
Sep 1, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Replaces timing sleeps with bounded, condition-based synchronization to make unit tests more deterministic.
Changes:
- Adds reusable polling, thread-running, and blocking-call fixtures.
- Uses explicit completion waits and Janus queue cleanup.
- Strengthens handler-removal and alarm synchronization tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/conftest.py |
Adds synchronization fixtures. |
tests/unit/helpers.py |
Defines shared timeout constants. |
tests/unit/iothub/test_sync_inbox.py |
Tests blocking inbox retrieval deterministically. |
tests/unit/iothub/test_sync_handler_manager.py |
Replaces handler timing sleeps. |
tests/unit/iothub/test_sync_clients.py |
Uses blocking-call assertions. |
tests/unit/iothub/aio/test_async_inbox.py |
Adds bounded waits and queue cleanup. |
tests/unit/iothub/aio/test_async_handler_manager.py |
Adds outcome-based handler waits. |
tests/unit/iothub/aio/test_async_clients.py |
Bounds asynchronous receive operations. |
tests/unit/common/test_async_adapter.py |
Awaits callback completion directly. |
tests/unit/common/test_alarm.py |
Waits on alarm completion events. |
tests/unit/common/pipeline/test_pipeline_stages_base.py |
Polls for event-handler invocation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Avishek (avishekpant)
approved these changes
Sep 1, 2026
Carter Tinney (cartertinney)
force-pushed
the
agents/unit-test-failure-analysis-python312
branch
from
September 1, 2026 17:26
619cfd7 to
f08deb3
Compare
Member
Author
|
/azp run Python E2E |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Avoid flaky assertions that assume cross-thread and cross-event-loop work completes within a fixed delay. Wait for observable outcomes instead, directly await callback futures, and explicitly synchronize blocking inbox tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Retain not-before and negative observation assertions where elapsed time is part of the behavior. Use explicit runner barriers to prove handler removal occurs while inbox work is still pending. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use local deadlines for prompt and cross-loop async completion. Run deliberately blocking synchronous calls in daemon-backed futures so timeout failures cannot hang executor shutdown. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Delegate async condition deadlines to asyncio.wait_for and remove future-state assertions already guaranteed by successful bounded awaits. Keep direct exception-state checks where storing the exact exception is under test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename predicate-polling fixtures to poll_until and async_poll_until so they are clearly distinct from asyncio.wait_for, which applies a deadline to an awaitable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add succinct setup, action, and state comments around the non-obvious polling, blocking, and runner-control mechanics introduced by the test hardening. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use explicit 100 ms deadlines for inbox reads and receive operations arranged to complete immediately. This preserves promptness expectations while still allowing the cross-loop scheduling required by AsyncClientInbox. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Define the 100 ms prompt-completion policy once and reuse it across async adapter, inbox, and receive-client tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace historical teardown sleeps with the Janus 2.0 aclose API, scheduled on the internal loop where each queue's async side is bound. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the repeated call-start tracking and daemon-thread fixture protocol with one assert_call_blocks helper that synchronizes, releases, and propagates results with bounded waits. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make every poll_until and async_poll_until call state its timeout so bounded waiting is visible at the assertion site rather than hidden by a helper default. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use the 100 ms prompt deadline for single-hop pipeline callbacks and a one-second eventual deadline for handler work crossing queues, event loops, and worker threads. Keep five-second bounds only for deliberate deadlock-safety orchestration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use the original 100 ms expectation for single handler operations. Reserve a 500 ms batch deadline for five-item tests, including the case observed exceeding 100 ms in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
After handler removal returns, directly assert that the handler and runner have been cleared before queuing new items. This proves no worker can invoke the removed handler without an observation sleep. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Receiver runner removal only guarantees coroutine handlers were scheduled on the handler loop. Wait for those invocations to complete before asserting the final call count. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Carter Tinney (cartertinney)
force-pushed
the
agents/unit-test-failure-analysis-python312
branch
from
September 1, 2026 19:29
f08deb3 to
88fa1e2
Compare
Carter Tinney (cartertinney)
deleted the
agents/unit-test-failure-analysis-python312
branch
September 1, 2026 19:29
Carter Tinney (cartertinney)
added a commit
that referenced
this pull request
Sep 1, 2026
## Stack - Stacked on #1252; review this PR after or against that branch. ## What changed - replace one-second connection polling and unbounded event/future waits with bounded condition-based synchronization - remove method subscription sleeps because receive-handler setters synchronously wait for feature subscription - signal Event Hub readiness after the first active receive cycle instead of sleeping three seconds - start each helper from its construction timestamp so later partitions retain new events without replaying prior test traffic - filter raw-string telemetry waits by expected payload so delayed unrelated events cannot satisfy them - guarantee service-helper shutdown if readiness fails before fixture yield - run the Event Hub receiver on a daemon-backed future so its 30-second shutdown timeout cannot be bypassed - bound twin matching/retry loops with absolute deadlines - remove import-time iptables mutation; existing per-test setup restores network state - co-locate focused wait-helper and Event Hub lifecycle tests under the IoT Hub E2E suite ## Validation - Python 3.12 unit suite: 5437 passed, 6 skipped - E2E infrastructure tests: 13 passed - Black and Ruff passed - all 132 IoT Hub E2E tests collected without cloud access - previous natural build, Python E2E, DPS E2E, and Horton E2E gates passed --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
What changed
poll_untilandasync_poll_untilfixtures with required explicit timeoutsassert_call_blocksfor blocking sync API assertionsQueue.aclose()Why
A Python 3.12 unit-test run observed only 2 of 5 handler invocations after a fixed 100 ms sleep. Positive completion tests now poll for outcomes while retaining meaningful timing bounds. Negative post-removal behavior is verified structurally: the setter does not return until the runner exits, after which both handler and runner references must be
None.Validation