test(device): give the time-dependent internals an injectable clock so seconds-long deadlines are testable

[tylerkron]

What was wrong

Every timeout, deadline and retry backoff in Daqifi.Core read the ambient system clock directly, so the only way to test one was to actually wait it out. That made the library's longest-running safety nets the least-tested code in it: the text exchange's first-response timeout, the SD download watchdog from #399, and the reconnect backoff ladder are all counted in seconds to minutes, so they were asserted at their short-timeout edges — or, in the ladder's case, not really asserted at all. Every reconnect test in the suite runs a millisecond-scale stand-in policy and checks only that each wait is longer than the last, which a ladder that never honoured its ceiling would satisfy just as well.

The same missing seam is what makes the tests that do touch a deadline fragile. They have to leave slack for a loaded runner, and the slack is a guess — the SD watchdog test carries a comment explaining that its budget was raised from 200 ms to 2 s after the old margin lost a race on macOS CI and turned main red after every test in the run had passed.

How it was fixed

TimeProvider is injected into the internals that read a clock — the text exchange, the operation serializer's drain barrier, the error throttle, the reconnect supervisor, the channel-population wait and the SD card operations — defaulting to TimeProvider.System everywhere.

Nothing about production timing changes. TimeProvider.System.GetTimestamp() is Stopwatch.GetTimestamp(), and Task.Delay(d, TimeProvider.System, ct) is Task.Delay(d, ct). No timeout is shortened, no delay is skipped, no ordering moves. The deadlines that were deliberately monotonic stay monotonic — the two that used DateTime.UtcNow (the drain barrier and the reconnect duration) actually get stronger, since they now use an elapsed-time source a clock correction cannot move under a request in flight.

The seam is internal, so there is no public API movement and PublicAPI.Shipped.txt is untouched. Exposing a TimeProvider on DeviceConnectionOptions / ReconnectOptions is a real API decision now that the surface is tracked (#636), and #637 explicitly defers it.

The thing a reviewer should push on: the pump loops. A test cannot advance a fake clock to one precise instant, because the code under test registers each wait only when it reaches it — so FakeClockPump steps the clock in slices until the work completes. That overshoots, always in the same direction: a jump landing before a wait is registered just moves the clock, and the wait is then created relative to the new now. So overshoot can only grant the code under test more device time, never less, and every assertion built on it is one-sided ("it did not give up before its budget"). The helper's remarks say this out loud; the reasoning is what makes the assertions sound.

Verification

Full suite green on both target frameworks: 4143 passed, 3 skipped (Daqifi.Core.Tests) and 217 passed (Daqifi.Mcp.Tests), on net9.0 and net10.0. CI green on ubuntu, windows and macOS.

origin/main is merged in. #701 landed the text exchange's terminator short-circuit while this was open, and it logs the exchange's elapsed time through the Stopwatch this branch replaced — the two merged cleanly and did not compile. Caught by CI on the merge ref, since the branch built green on its own base; its log now goes through ElapsedMs() like every other elapsed read in the engine.

Nine new tests, over deadlines that were previously unreachable:

what it now covers	device time	real time
the shipping reconnect ladder walked in full, including its MaxDelay ceiling	91 s	~130 ms
a successful reconnect reports an Outage covering the whole backoff	31 s	(same test)
the SD download watchdog on the shipping 30-minute budget	30 min	18 ms
the drain barrier's bound, and that it stops the moment the producer idles	30 min	52 ms
the error throttle's five-second interval, at its exact tick boundary	1 min	20 ms

Two existing tests were converted rather than added to: the silent-device response-timeout test went from 3 s of real waiting to 77 ms, and now asserts the full 3000 ms window exactly instead of allowing the exchange to give up 500 ms early; the throttle's collapse test dropped its two Thread.Sleeps and now asserts the interval the device actually ships with rather than a 150 ms stand-in.

Two of the issue's success criteria are not met and were not attempted here, since they are the bulk test conversion rather than the seam:

• the Task.Delay/Thread.Sleep count in Daqifi.Core.Tests is unchanged at 185 (three real sleeps removed, two yields added inside the shared pump helper)
• suite wall time is not materially down — it was already 34 s at baseline and measures 32–37 s across runs, inside the noise on this machine

Worth noting for anyone reading #637 later: all five flake tickets it cites (#634, #632, #589, #559, #516) are already closed, and #634's fix was a test seam rather than a widened bound, so it is already immune to scheduler load. The prize this PR actually collects is the third cost the issue names — the long-timeout paths — not the flakes.

No bench run: this is a test-infrastructure change with no wire-format or device-behaviour component, and the production default is byte-for-byte the previous behaviour.

closes #637

🤖 Generated with Claude Code

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Inject clocks into device internals for deterministic deadline tests

🧪 Tests ✨ Enhancement 🕐 40+ Minutes

AI Description

• Routes device deadlines, delays, and backoffs through injectable TimeProvider clocks.
• Adds stepped-clock coverage for production timeout budgets and exact throttle boundaries.
• Preserves production timing while replacing wall-clock duration checks with monotonic timestamps.

Diagram

graph TD
  Fake["Fake TimeProvider"] --> Pump["Clock pump"] --> Tests["Deadline tests"]
  Fake --> Device["Device clock"] --> Hosts["Host seams"] --> Engines["Text and drain"]
  Device --> Reconnect["Reconnect waits"]
  Hosts --> Sd["SD operations"]
  Fake --> Throttle["Error throttle"]

Loading

High-Level Assessment

The internal TimeProvider seam is the best fit: it uses the framework abstraction, preserves TimeProvider.System behavior in production, supports provider-aware Task.Delay and cancellation APIs, and avoids expanding the tracked public API. A custom clock interface would duplicate framework functionality, while exposing the provider through public connection options would impose an unnecessary compatibility commitment for a testability change.

Files changed (17) +852 / -71

Enhancement (1) +16 / -3

DeviceErrorThrottle.csMake throttle timing provider-driven +16/-3

Make throttle timing provider-driven

• Accepts an optional TimeProvider and uses it for timestamp and elapsed-time calculations. TimeProvider.System remains the default to preserve production behavior.

src/Daqifi.Core/Device/DeviceErrorThrottle.cs

Bug fix (1) +16 / -3

OperationSerializer.csUse monotonic provider timing for queue draining +16/-3

Use monotonic provider timing for queue draining

• Replaces DateTime-based deadline checks and ambient delays with host-provider elapsed timing. Names the unchanged ten-millisecond drain poll interval.

src/Daqifi.Core/Device/Internal/OperationSerializer.cs

Refactor (7) +193 / -53

DaqifiDevice.csCentralize device timing on an internal provider +69/-13

Centralize device timing on an internal provider

• Adds an internal TimeProvider seam and exposes it through collaborator host interfaces. Routes reconnect, initialization, capability spacing, and channel-population timing through provider-aware delays and monotonic elapsed measurements.

src/Daqifi.Core/Device/DaqifiDevice.cs

DaqifiStreamingDevice.csExpose the device clock to SD operations +3/-0

Expose the device clock to SD operations

• Implements the SD operation host clock contract using the base device's internal provider.

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs

IOperationSerializationHost.csAdd a clock to the serializer host contract +10/-0

Add a clock to the serializer host contract

• Extends the internal host seam with the TimeProvider used to measure bounded outbound draining.

src/Daqifi.Core/Device/Internal/IOperationSerializationHost.cs

ITextExchangeHost.csAdd a clock to the text exchange host +14/-0

Add a clock to the text exchange host

• Extends the internal text exchange contract with a monotonic TimeProvider for settle delays and response deadlines.

src/Daqifi.Core/Device/Internal/ITextExchangeHost.cs

TextExchangeEngine.csRoute text exchange deadlines through TimeProvider +35/-17

Route text exchange deadlines through TimeProvider

• Replaces Stopwatch and ambient waits with the host clock for elapsed logging, settle delays, line waits, inactivity windows, and overall response ceilings. Production remains monotonic through TimeProvider.System.

src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs

ISdCardOperationHost.csAdd a clock to the SD operation host +13/-0

Add a clock to the SD operation host

• Extends the internal SD collaborator contract with the provider used for settle delays, transfer budgets, and watchdog deadlines.

src/Daqifi.Core/Device/SdCard/ISdCardOperationHost.cs

SdCardOperations.csRoute SD timing and watchdogs through TimeProvider +49/-23

Route SD timing and watchdogs through TimeProvider

• Uses the host clock for interface settling, command spacing, generated log timestamps, download elapsed time, remaining budgets, cancellation deadlines, and abandonment races. Names the unchanged logging command spacing interval.

src/Daqifi.Core/Device/SdCard/SdCardOperations.cs

Tests (6) +517 / -12

DeviceErrorThrottleTests.csTest production throttle intervals with fake time +78/-3

Test production throttle intervals with fake time

• Replaces wall-clock sleeps with FakeTimeProvider advances and tests the exact five-second boundary. Adds sustained-storm coverage for raise frequency and suppressed occurrence accounting.

src/Daqifi.Core.Tests/Device/DeviceErrorThrottleTests.cs

DeviceReconnectTests.csExercise the full shipping reconnect ladder +131/-0

Exercise the full shipping reconnect ladder

• Drives the production-scale backoff policy through fake time, including repeated MaxDelay saturation. Also verifies successful reconnect outage reporting covers the complete backoff without real waiting.

src/Daqifi.Core.Tests/Device/DeviceReconnectTests.cs

ConnectionGuardTests.csSupply the text host system clock +1/-0

Supply the text host system clock

• Updates the guard-only ITextExchangeHost test double for the new TimeProvider contract.

src/Daqifi.Core.Tests/Device/Internal/ConnectionGuardTests.cs

OperationSerializerDrainBarrierTests.csCover drain barrier timing and cancellation +205/-0

Cover drain barrier timing and cancellation

• Adds focused tests for the outbound drain barrier's production budget, immediate idle path, mid-wait completion, and caller cancellation. Minimal host and producer stubs isolate the serializer behavior.

src/Daqifi.Core.Tests/Device/Internal/OperationSerializerDrainBarrierTests.cs

SdCardOperationsCollaboratorTests.csTest the shipping SD download watchdog +73/-0

Test the shipping SD download watchdog

• Adds fake-time coverage for abandoning a stalled transfer at the production 30-minute hard deadline without unsafe cleanup. Extends the SD host fake with an injectable clock.

src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsCollaboratorTests.cs

TextExchangeLineFramingTests.csDrive silent-response timeout with fake time +29/-9

Drive silent-response timeout with fake time

• Reworks the silent-device test to advance the full response window deterministically. Removes scheduler slack while preserving assertions that no response was recognized.

src/Daqifi.Core.Tests/Device/TextExchangeLineFramingTests.cs

Other (2) +110 / -0

Daqifi.Core.Tests.csprojAdd fake-time testing dependency +5/-0

Add fake-time testing dependency

• Adds Microsoft.Extensions.TimeProvider.Testing for deterministic deadline and delay tests. The production project gains no dependency.

src/Daqifi.Core.Tests/Daqifi.Core.Tests.csproj

FakeClockPump.csAdd sliced fake-clock pump helpers +105/-0

Add sliced fake-clock pump helpers

• Introduces helpers that advance fake time in slices while asynchronous loops register successive waits. Real-time bounds convert stalled test scenarios into failures and returned elapsed device time supports one-sided assertions.

src/Daqifi.Core.Tests/TestSupport/FakeClockPump.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Reconnect tests can hang ✓ Resolved 🐞 Bug ☼ Reliability

Description

FakeClockPump.UntilAsync returns normally when its real-time bound expires, but both new reconnect
tests then await the still-incomplete event task without any timeout. A reconnect regression that
prevents ReconnectFailed or Reconnected from firing therefore hangs the test run instead of
failing after GateTimeout.

Code

src/Daqifi.Core.Tests/Device/DeviceReconnectTests.cs[1300]

+        var result = await failed;

Relevance

●●● Strong

Exact precedent accepted bounding unbounded coordination awaits to turn reconnect/test hangs into
failures.

PR-#364
PR-#440
PR-#454

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The pump loop exits when wall time reaches its limit and unconditionally returns, without completing
or canceling the target. The two added reconnect tests subsequently perform unbounded awaits on
those targets; this is the same test-hang pattern previously fixed by bounding coordination awaits.

src/Daqifi.Core.Tests/TestSupport/FakeClockPump.cs[67-81]
src/Daqifi.Core.Tests/Device/DeviceReconnectTests.cs[1293-1300]
src/Daqifi.Core.Tests/Device/DeviceReconnectTests.cs[1330-1337]
PR-#364

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new reconnect tests await their event tasks indefinitely after `FakeClockPump.UntilAsync` returns, even though that helper may return because its real-time limit expired while the event task remains incomplete.

## Issue Context
Make timeout expiry fail the tests rather than hang the runner. Either make `UntilAsync` throw/assert when its target is incomplete, or explicitly bound/check both event awaits after pumping.

## Fix Focus Areas
- src/Daqifi.Core.Tests/Device/DeviceReconnectTests.cs[1298-1300]
- src/Daqifi.Core.Tests/Device/DeviceReconnectTests.cs[1336-1337]
- src/Daqifi.Core.Tests/TestSupport/FakeClockPump.cs[71-81]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 15/18, lines 528/200; both must reach the floor). Router rationale: This push adds substantial, concurrency-sensitive exchange termination logic across the device, host contract, and asynchronous engine, with multiple independent boundary and state-tracking paths where redundant review could catch subtle defects.

Tip of the day

💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 8dccc52

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit fb88add