test: make the assertions that could not fail able to fail

CONTRIBUTING.md

@@ -94,6 +94,11 @@ Every behavioural change needs a test, and tests here are expected to be determi
 - **No hardware, no timing luck.** Use the `virtual://` loopback adapter for anything about real
   bus behaviour, and `ControllableBus` (`tests/CanKit.Pro.Tests/Infrastructure/`) when the test
   needs to control what the bus does — echo frames, bus state, whether a transmit is accepted.
+  `ControllableBus.DeferredEchoCapable(...)` parks each TX echo in a `DeferredEchoQueue` instead
+  of raising it inside `Transmit`, which is the only way to have two sends pending at once: a
+  synchronous echo re-enters `CanBusService`'s pending-send lock on the transmitting thread, so
+  the pending list never holds more than that thread's own entry. Reach for it whenever the
+  behaviour under test is about how several in-flight sends relate to each other.
 - **Do not test through an adapter's internals.** If a test needs reflection into another
   package's private state, it is testing that package, not ours; drive the scenario through the
   double instead.

tests/CanKit.Pro.Tests/Infrastructure/ControllableBus.cs

@@ -9,6 +9,27 @@
 
 namespace CanKit.Pro.Tests.Infrastructure;
 
+/// <summary>
+/// When the TX echo of an accepted transmit reaches <see cref="ICanBus.FrameObserved"/>.
+/// </summary>
+public enum EchoDelivery
+{
+    /// <summary>
+    /// Raised from inside <see cref="ControllableBus.Transmit(in CanFrame)"/> — on the
+    /// transmitting thread, inside whatever lock the caller holds while transmitting. What a real
+    /// echo-mode adapter (CanKit.Adapter.Virtual in <c>ChannelWorkMode.Echo</c>) does, and what
+    /// makes the reentrancy in <c>CanBusService.SendWithEchoConfirmAsync</c> observable.
+    /// </summary>
+    Synchronous,
+
+    /// <summary>
+    /// Parked in <see cref="ControllableBus.DeferredEchoes"/> instead of being raised. The test
+    /// chooses when each echo is delivered, which lets more than one pending send exist at once —
+    /// see <see cref="DeferredEchoQueue"/> for why that is not achievable synchronously.
+    /// </summary>
+    Deferred,
+}
+
 /// <summary>
 /// An <see cref="ICanBus"/> the test drives directly: it decides whether a transmit is accepted,
 /// whether (and when) a TX echo comes back, and what <see cref="BusState"/> the controller reports.
@@ -40,30 +61,60 @@ public sealed class ControllableBus : ICanBus
     private readonly IBusRTOptionsConfigurator _options;
     private int _disposed;
 
-    private ControllableBus(ICanBus configurationSource)
+    private ControllableBus(ICanBus configurationSource, EchoDelivery echoDelivery)
     {
         _configurationSource = configurationSource;
         _options = new EchoCapableOptions(configurationSource.Options);
+        EchoMode = echoDelivery;
+        DeferredEchoes = new DeferredEchoQueue(frame => RaiseObserved(frame, isEcho: true));
         // What a healthy CAN controller reports; tests move it from here.
         BusState = BusState.ErrActive;
     }
 
     /// <summary>
     /// Creates a double whose <see cref="Options"/> report <c>ChannelWorkMode.Echo</c> and the
     /// <c>CanFeature.Echo</c> capability — the combination that makes <c>SendConfirmed</c> take
-    /// the real-echo-matching path (FR-RAW-031).
+    /// the real-echo-matching path (FR-RAW-031) — and that echoes synchronously from inside
+    /// <see cref="Transmit(in CanFrame)"/>, exactly as a real echo-mode adapter does.
     /// </summary>
     public static ControllableBus EchoCapable(string session)
-        => new(VirtualAdapterFixture.Open(session, 0, ChannelWorkMode.Echo));
-
+        => new(VirtualAdapterFixture.Open(session, 0, ChannelWorkMode.Echo), EchoDelivery.Synchronous);
 
+    /// <summary>
+    /// Same echo-capable configuration as <see cref="EchoCapable"/>, but every accepted transmit's
+    /// echo is parked in <see cref="DeferredEchoes"/> until the test releases it.
+    /// </summary>
+    /// <remarks>
+    /// Use this whenever the behaviour under test needs two or more sends to be pending at the
+    /// same time. A synchronous echo makes that impossible — it re-enters
+    /// <c>CanBusService</c>'s pending-send lock on the transmitting thread before that thread ever
+    /// leaves <c>Transmit</c>, so the pending list only ever holds the entry belonging to the
+    /// thread currently inside it, no matter how many callers race. See
+    /// <see cref="DeferredEchoQueue"/>.
+    /// </remarks>
+    public static ControllableBus DeferredEchoCapable(string session)
+        => new(VirtualAdapterFixture.Open(session, 0, ChannelWorkMode.Echo), EchoDelivery.Deferred);
 
     /// <summary>Whether <see cref="Transmit(in CanFrame)"/> reports the frame as accepted.</summary>
     public bool AcceptTransmit { get; set; } = true;
 
     /// <summary>Whether an accepted frame is echoed back through <see cref="FrameObserved"/>.</summary>
     public bool EchoAcceptedFrames { get; set; } = true;
 
+    /// <summary>
+    /// Whether an accepted frame's echo is raised inside <see cref="Transmit(in CanFrame)"/> or
+    /// parked in <see cref="DeferredEchoes"/>. Settable mid-test so a scenario can, for example,
+    /// let the first send confirm normally and only defer the ones it needs to overlap.
+    /// </summary>
+    public EchoDelivery EchoMode { get; set; }
+
+    /// <summary>
+    /// Echoes parked by <see cref="EchoDelivery.Deferred"/> mode, and the handle that releases
+    /// them. Always present; stays empty while <see cref="EchoMode"/> is
+    /// <see cref="EchoDelivery.Synchronous"/>.
+    /// </summary>
+    public DeferredEchoQueue DeferredEchoes { get; }
+
     /// <summary>Number of frames handed to <see cref="Transmit(in CanFrame)"/>.</summary>
     public int TransmitCount => Volatile.Read(ref _transmitCount);
 
@@ -92,9 +143,17 @@ public int Transmit(in CanFrame frame)
         Interlocked.Increment(ref _transmitCount);
         if (!AcceptTransmit) return 0;
 
-        // A real echo-mode adapter delivers the echo synchronously from inside Transmit; matching
-        // that is what makes the reentrancy in CanBusService.SendWithEchoConfirmAsync observable.
-        if (EchoAcceptedFrames) RaiseObserved(frame, isEcho: true);
+        if (EchoAcceptedFrames)
+        {
+            // Synchronous is the default because that is what a real echo-mode adapter does, and
+            // matching it is what makes the reentrancy in CanBusService.SendWithEchoConfirmAsync
+            // observable. Deferred parks the echo instead, so the caller leaves Transmit — and
+            // releases the service's pending-send lock — with its entry still pending; see
+            // DeferredEchoQueue for why some FR-RAW-031 behaviour is only reachable that way.
+            if (EchoMode == EchoDelivery.Deferred) DeferredEchoes.Park(frame);
+            else RaiseObserved(frame, isEcho: true);
+        }
+
         return 1;
     }
 

tests/CanKit.Pro.Tests/Infrastructure/DeferredEchoQueue.cs

@@ -0,0 +1,180 @@
+using System;
+using System.Collections.Generic;
+using System.Threading;
+using System.Threading.Tasks;
+using CanKit.Abstractions.API.Can.Definitions;
+
+namespace CanKit.Pro.Tests.Infrastructure;
+
+/// <summary>
+/// The parking lot for TX echoes of a <see cref="ControllableBus"/> running in
+/// <see cref="EchoDelivery.Deferred"/> mode: <c>Transmit</c> hands the frame here instead of
+/// echoing it, and the test decides when — and in which order — each echo reaches
+/// <c>FrameObserved</c>.
+///
+/// <para>
+/// Why this exists: a real echo-mode adapter delivers the echo synchronously from inside
+/// <c>Transmit</c>, and <c>CanBusService.SendWithEchoConfirmAsync</c> transmits while holding its
+/// pending-send lock. A synchronous echo therefore re-enters that lock <em>on the transmitting
+/// thread</em>, so the pending list can never hold more than the one entry that thread just
+/// registered. Every property that is only observable with two or more entries queued for the same
+/// key — FIFO matching of byte-identical concurrent sends (SRS FR-RAW-031), an expired pending
+/// send poisoning the FIFO for a later one — is therefore untestable against a synchronous echo:
+/// the assertion holds no matter what the matching code does. Deferring the echo is what puts the
+/// second entry in the list.
+/// </para>
+///
+/// <para>
+/// Everything here is deliberately explicit rather than time-based: <see cref="WaitForEnqueuedAsync"/>
+/// is the only wait, and it waits on a transmit actually having happened rather than on a delay
+/// that "should be long enough". Nothing in this class starts a timer, a thread, or a task — an
+/// echo moves only when the test says so, on the test's own thread.
+/// </para>
+/// </summary>
+/// <remarks>
+/// Reusable beyond FR-RAW-031: any scenario that needs a pending send to still be pending while a
+/// second one is registered (a late echo arriving after its own send already timed out, an echo
+/// that never arrives at all while later ones do — see <see cref="DiscardNext"/>) is expressed by
+/// parking, then releasing or discarding, in whatever order the scenario calls for.
+/// </remarks>
+public sealed class DeferredEchoQueue
+{
+    private readonly Action<CanFrame> _deliver;
+
+    private readonly object _gate = new();
+    private readonly List<CanFrame> _parked = new();
+    private readonly List<(int Threshold, TaskCompletionSource Tcs)> _waiters = new();
+
+    // Monotonic: counts every frame ever parked, so a waiter's threshold cannot be un-met by a
+    // subsequent release. "Two sends have been transmitted" must stay true once it is true.
+    private int _enqueued;
+
+    internal DeferredEchoQueue(Action<CanFrame> deliver) => _deliver = deliver;
+
+    /// <summary>Echoes parked and not yet released or discarded, oldest first.</summary>
+    public int Count
+    {
+        get { lock (_gate) return _parked.Count; }
+    }
+
+    /// <summary>
+    /// Total number of echoes ever parked — i.e. accepted transmits observed while in
+    /// <see cref="EchoDelivery.Deferred"/> mode. Never decreases.
+    /// </summary>
+    public int Enqueued
+    {
+        get { lock (_gate) return _enqueued; }
+    }
+
+    /// <summary>
+    /// Completes once <see cref="Enqueued"/> has reached <paramref name="count"/> — the
+    /// deterministic replacement for "sleep a bit and hope the sends got that far".
+    /// </summary>
+    /// <remarks>
+    /// A transmit is parked from inside <c>ICanBus.Transmit</c>, which
+    /// <c>CanBusService.SendWithEchoConfirmAsync</c> calls after it has registered its pending
+    /// entry and while still holding the pending-send lock. So "n echoes parked" is a hard
+    /// guarantee that n pending sends are registered, not an approximation of it.
+    /// </remarks>
+    /// <param name="count">Number of parked echoes to wait for.</param>
+    /// <param name="timeout">How long to wait before failing; a bound against a hang, not a
+    /// scheduling assumption.</param>
+    /// <exception cref="TimeoutException">Fewer than <paramref name="count"/> echoes were parked
+    /// within <paramref name="timeout"/>.</exception>
+    public async Task WaitForEnqueuedAsync(int count, TimeSpan timeout)
+    {
+        if (count <= 0) throw new ArgumentOutOfRangeException(nameof(count), count, "Count must be positive.");
+
+        Task wait;
+        lock (_gate)
+        {
+            if (_enqueued >= count) return;
+            var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+            _waiters.Add((count, tcs));
+            wait = tcs.Task;
+        }
+
+        try
+        {
+            await wait.WaitAsync(timeout).ConfigureAwait(false);
+        }
+        catch (TimeoutException)
+        {
+            throw new TimeoutException(
+                $"Only {Enqueued} of {count} expected echoes were parked within {timeout}.");
+        }
+    }
+
+    /// <summary>
+    /// Delivers the oldest parked echo through the bus's <c>FrameObserved</c> event, on the
+    /// calling thread. Returns <c>false</c> when nothing is parked.
+    /// </summary>
+    public bool ReleaseNext()
+    {
+        CanFrame frame;
+        lock (_gate)
+        {
+            if (_parked.Count == 0) return false;
+            frame = _parked[0];
+            _parked.RemoveAt(0);
+        }
+
+        // Delivered outside the lock: the echo runs the service's whole match-and-complete path
+        // (and any continuation it resumes) on this thread, and none of that may be serialized
+        // against a concurrent Transmit parking the next echo.
+        _deliver(frame);
+        return true;
+    }
+
+    /// <summary>
+    /// Delivers every parked echo, oldest first. Returns how many were delivered.
+    /// </summary>
+    public int ReleaseAll()
+    {
+        var released = 0;
+        while (ReleaseNext()) released++;
+        return released;
+    }
+
+    /// <summary>
+    /// Drops the oldest parked echo without ever delivering it — the frame reached the wire but
+    /// its echo is lost, while later echoes still arrive normally. Returns <c>false</c> when
+    /// nothing is parked.
+    /// </summary>
+    public bool DiscardNext()
+    {
+        lock (_gate)
+        {
+            if (_parked.Count == 0) return false;
+            _parked.RemoveAt(0);
+            return true;
+        }
+    }
+
+    /// <summary>Parks <paramref name="frame"/>; called by <see cref="ControllableBus.Transmit"/>.</summary>
+    internal void Park(in CanFrame frame)
+    {
+        (int Threshold, TaskCompletionSource Tcs)[]? satisfied = null;
+        lock (_gate)
+        {
+            _parked.Add(frame);
+            _enqueued++;
+
+            if (_waiters.Count > 0)
+            {
+                var met = _waiters.FindAll(w => w.Threshold <= _enqueued);
+                if (met.Count > 0)
+                {
+                    satisfied = met.ToArray();
+                    _waiters.RemoveAll(w => w.Threshold <= _enqueued);
+                }
+            }
+        }
+
+        // Never complete a TCS under the lock: the waiter's continuation may call straight back
+        // into ReleaseNext/Count, and Park runs inside the service's pending-send lock.
+        if (satisfied is null) return;
+        foreach (var (_, tcs) in satisfied)
+            tcs.TrySetResult();
+    }
+}