diff --git a/mbproxy/tests/Mbproxy.Tests/Bcd/BcdCodecTests.cs b/mbproxy/tests/Mbproxy.Tests/Bcd/BcdCodecTests.cs index ee362f9..038d872 100644 --- a/mbproxy/tests/Mbproxy.Tests/Bcd/BcdCodecTests.cs +++ b/mbproxy/tests/Mbproxy.Tests/Bcd/BcdCodecTests.cs @@ -44,6 +44,20 @@ public sealed class BcdCodecTests .ParamName.ShouldBe("value"); } + /// + /// Phase 12 (W3 test gap #11) — locks the boundary contract for the `(uint)value > Max16` + /// range check. `int.MinValue` cast to `uint` becomes `0x80000000`, which is well above + /// `Max16` (= 9999), so the throw fires cleanly without arithmetic surprise. Prevents + /// regressions if the bounds check is ever rewritten with a two-sided int comparison + /// that would underflow on extreme negatives. + /// + [Fact] + public void Encode16_IntMinValue_Throws_OutOfRange_NoArithmeticSurprise() + { + Should.Throw(() => BcdCodec.Encode16(int.MinValue)) + .ParamName.ShouldBe("value"); + } + // ── Decode16 ──────────────────────────────────────────────────────────── [Fact] diff --git a/mbproxy/tests/Mbproxy.Tests/Configuration/ConfigReconcilerTests.cs b/mbproxy/tests/Mbproxy.Tests/Configuration/ConfigReconcilerTests.cs index a8ea22b..01c1ea0 100644 --- a/mbproxy/tests/Mbproxy.Tests/Configuration/ConfigReconcilerTests.cs +++ b/mbproxy/tests/Mbproxy.Tests/Configuration/ConfigReconcilerTests.cs @@ -280,6 +280,72 @@ public sealed class ConfigReconcilerTests : IAsyncDisposable // under concurrent load. Assert.Equal(5, counters.ReloadAppliedCount); } + + /// + /// Phase 12 (W3 test gap #16) — stress-test the W2.3 ConcurrentDictionary fix and the + /// W2.1 coalescing-accessor wiring. Many concurrent Apply calls drive add/remove of + /// many distinct PLCs; without W2.3's ConcurrentDictionary the inner Task.WhenAll + /// continuations would corrupt the dictionary and crash with KeyNotFoundException or + /// ArgumentException. The test asserts: all applies complete, no exceptions are + /// thrown, and the reload counter is exactly the apply count. + /// + [Fact(Timeout = 30_000)] + public async Task Apply_ManyConcurrentReloads_With_PlcChurn_NoCorruption() + { + // Empty initial — first Apply will Add all PLCs. + var initial = MakeOptions([]); + var monitor = new FakeOptionsMonitor(initial); + + var supervisors = new System.Collections.Concurrent.ConcurrentDictionary(StringComparer.Ordinal); + + var counters = new ServiceCounters(); + var reconciler = BuildReconciler(monitor, counters); + _reconcilers.Add(reconciler); + reconciler.Attach(supervisors, initial); + + // Build 8 different option snapshots, each a different PLC roster. + // Each Apply will trigger Add+Remove churn against the live supervisor dict — + // exactly the path that W2.3's ConcurrentDictionary was needed for. + const int snapshots = 8; + const int plcsPerSnapshot = 4; + var snaps = new MbproxyOptions[snapshots]; + var allPlcs = new List(); + for (int s = 0; s < snapshots; s++) + { + var plcsForSnap = new PlcOptions[plcsPerSnapshot]; + for (int p = 0; p < plcsPerSnapshot; p++) + { + plcsForSnap[p] = MakePlc($"PLC-{s}-{p}", PickFreePort()); + allPlcs.Add(plcsForSnap[p]); + } + snaps[s] = MakeOptions(plcsForSnap); + } + + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(25)); + + // Fire 16 concurrent applies cycling through the 8 snapshots so each is + // submitted twice. Inner per-PLC Task.WhenAll continuations from W2.3 will run + // in parallel and stress-test the dictionary mutation safety. + var tasks = Enumerable.Range(0, 16) + .Select(i => Task.Run(() => reconciler.ApplyAsync(snaps[i % snapshots], cts.Token), cts.Token)) + .ToArray(); + + var results = await Task.WhenAll(tasks); + + Assert.All(results, r => Assert.True(r, "every Apply must succeed")); + Assert.Equal(16, counters.ReloadAppliedCount); + + // Final dictionary state: all keys present must come from the last-applied snapshot. + // The "last-applied snapshot" depends on scheduling so we just verify NO orphan + // entries — every supervisor in the dict must correspond to some snapshot's PLCs. + var validNames = new HashSet(allPlcs.Select(p => p.Name)); + foreach (var name in supervisors.Keys) + Assert.Contains(name, validNames); + + // Track supervisors for cleanup. + foreach (var s in supervisors.Values) + _supervisors.Add(s); + } } /// diff --git a/mbproxy/tests/Mbproxy.Tests/Configuration/HotReloadE2ETests.cs b/mbproxy/tests/Mbproxy.Tests/Configuration/HotReloadE2ETests.cs index 3027874..2782661 100644 --- a/mbproxy/tests/Mbproxy.Tests/Configuration/HotReloadE2ETests.cs +++ b/mbproxy/tests/Mbproxy.Tests/Configuration/HotReloadE2ETests.cs @@ -363,6 +363,68 @@ public sealed class HotReloadE2ETests : IAsyncLifetime await host.StopAsync(stopCts.Token); } + // ── Phase 12 (W3 test gap #10) — ReadCoalescing.Enabled hot-reload flip ───────────── + + /// + /// W3 — verifies that flipping Mbproxy.Resilience.ReadCoalescing.Enabled at + /// runtime via hot-reload propagates to the live + /// snapshot. The W2.1 fix wires the accessor through to add/restart supervisors; + /// the multiplexer reads it per-PDU. Proving the IOptionsMonitor sees the new value + /// is sufficient — the per-PDU read path is unit-tested at the multiplexer level. + /// + [Fact(Timeout = 8_000)] + public async Task E2E_ReadCoalescingEnabled_FlipAtRuntime_PropagatesToOptionsMonitor() + { + int port = PickFreePort(); + int adminPort = PickFreePort(); + + WriteConfigWithCoalescing(_configPath, port, adminPort, enabled: true); + + using var host = BuildHost(_configPath); + using var startCts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); + await host.StartAsync(startCts.Token); + await WaitForAsync(() => CanConnect(port), TimeSpan.FromSeconds(5), + "listener should be reachable after startup"); + + var monitor = host.Services + .GetRequiredService>(); + monitor.CurrentValue.Resilience.ReadCoalescing.Enabled.ShouldBeTrue( + "initial config sets Enabled=true"); + + // Flip to false and re-save. + WriteConfigWithCoalescing(_configPath, port, adminPort, enabled: false); + + await WaitForAsync( + () => monitor.CurrentValue.Resilience.ReadCoalescing.Enabled == false, + TimeSpan.FromSeconds(5), + "IOptionsMonitor.CurrentValue must reflect Enabled=false after hot-reload"); + + using var stopCts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); + await host.StopAsync(stopCts.Token); + } + + private static void WriteConfigWithCoalescing( + string path, int listenPort, int adminPort, bool enabled) + { + var doc = new + { + Mbproxy = new + { + AdminPort = adminPort, + BcdTags = new { Global = Array.Empty() }, + Plcs = new[] { new { Name = "PLC-A", ListenPort = listenPort, Host = "127.0.0.1", Port = 502 } }, + Connection = new { BackendConnectTimeoutMs = 500, BackendRequestTimeoutMs = 500 }, + Resilience = new + { + ReadCoalescing = new { Enabled = enabled, MaxParties = 32 }, + }, + }, + }; + string tmp = path + ".tmp"; + File.WriteAllText(tmp, JsonSerializer.Serialize(doc, new JsonSerializerOptions { WriteIndented = true })); + File.Move(tmp, path, overwrite: true); + } + private static void WriteConfigWithCacheableTag( string path, int listenPort, int adminPort, int address, int cacheTtlMs) { diff --git a/mbproxy/tests/Mbproxy.Tests/Proxy/BcdPduPipelineTests.cs b/mbproxy/tests/Mbproxy.Tests/Proxy/BcdPduPipelineTests.cs index bb0c031..390d202 100644 --- a/mbproxy/tests/Mbproxy.Tests/Proxy/BcdPduPipelineTests.cs +++ b/mbproxy/tests/Mbproxy.Tests/Proxy/BcdPduPipelineTests.cs @@ -404,6 +404,27 @@ public sealed class BcdPduPipelineTests ctx.Counters.Snapshot().RewrittenSlots.ShouldBe(0); } + /// + /// Phase 12 (W3 test gap #15) — DL205/DL260 caps FC03/FC04 reads at qty=128 (above + /// Modbus spec's 125; documented in DL260/dl205.md). The proxy must NOT truncate the + /// qty field — a request with qty > 128 at non-BCD addresses must pass through + /// unchanged so the PLC's own validator returns exception 03 to the client. This is + /// the transparent-pass-through contract for FCs and addresses the rewriter doesn't + /// own. + /// + [Fact] + public void FC03_Request_QtyAbove128_AtNonBcdAddress_PassesThroughUnchanged() + { + var ctx = MakeContext(BcdTag.Create(1024, 16)); // tag elsewhere; not in this read + var req = Fc03Request(address: 5000, qty: 200); + byte[] original = [..req]; + + Pipeline.Process(MbapDirection.RequestToBackend, ReadOnlySpan.Empty, req.AsSpan(), ctx); + + req.ShouldBe(original, "qty must NOT be truncated; the PLC validates and returns ex03"); + ctx.Counters.Snapshot().RewrittenSlots.ShouldBe(0); + } + /// /// Phase 12 (W3 test gap) — symmetric inverse of the existing partial-overlap test: /// the write range starts ON the high register of a 32-bit pair (low word is BEFORE diff --git a/mbproxy/tests/Mbproxy.Tests/Proxy/Supervision/SupervisorTests.cs b/mbproxy/tests/Mbproxy.Tests/Proxy/Supervision/SupervisorTests.cs index 35c4811..2a5966c 100644 --- a/mbproxy/tests/Mbproxy.Tests/Proxy/Supervision/SupervisorTests.cs +++ b/mbproxy/tests/Mbproxy.Tests/Proxy/Supervision/SupervisorTests.cs @@ -6,6 +6,7 @@ using Mbproxy.Proxy.Supervision; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Polly; +using Shouldly; using Xunit; namespace Mbproxy.Tests.Proxy.Supervision; @@ -174,28 +175,63 @@ public sealed class SupervisorTests // ── Test 4: runtime fault triggers recovery ────────────────────────────────────────── + /// + /// Phase 12 (W3 test gap #4) — replaces the previous placeholder. Genuinely faults + /// the running listener mid-life by stopping its underlying + /// via reflection (the only externally-observable hook to force the accept loop's + /// to throw ). + /// The supervisor must observe the fault, transition to , + /// and re-bind on the next Polly attempt — emitting one + /// mbproxy.listener.recovered event and bumping RecoveryAttempts. + /// [Fact] - public async Task Supervisor_RuntimeFault_TriggersRecovery() + public async Task Supervisor_RuntimeFault_OnRunningListener_RecoversAndRebinds() { - // This test verifies that a supervisor that starts successfully stays Bound - // and that recovery mechanics are wired. For a full runtime-fault scenario, - // see the E2E tests. Here we verify: - // 1. Supervisor reaches Bound. - // 2. After StopAsync, transitions to Stopped. - // 3. RecoveryAttempts is 0 when no fault occurred. - int port = PickFreePort(); + // Fast retry so the test completes in a few hundred ms. var pipeline = FastRecoveryPipeline(initialMs: 100, steadyMs: 100); await using var supervisor = BuildSupervisor(port, pipeline); - using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); await supervisor.StartAsync(cts.Token); await supervisor.WaitForInitialBindAttemptAsync(cts.Token); Assert.Equal(SupervisorState.Bound, supervisor.Snapshot().State); - var snap = supervisor.Snapshot(); - Assert.Equal(SupervisorState.Bound, snap.State); - Assert.Equal(0, snap.RecoveryAttempts); + // Force the accept loop to fault by reaching into PlcListener._listener via + // reflection and calling Stop(). PlcListener.RunAsync's catch handler observes + // the resulting ObjectDisposedException, returns normally without cancellation, + // and the supervisor's "RunAsync returned without cancellation" branch fires — + // transitioning to Recovering and re-throwing into Polly. + var supervisorType = typeof(PlcListenerSupervisor); + var currentListenerField = supervisorType.GetField("_currentListener", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!; + var listener = currentListenerField.GetValue(supervisor); + listener.ShouldNotBeNull("_currentListener must be set after Bound"); + + var listenerType = listener.GetType(); + var innerListenerField = listenerType.GetField("_listener", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!; + var inner = (TcpListener?)innerListenerField.GetValue(listener); + inner.ShouldNotBeNull("PlcListener._listener must be set after StartAsync"); + inner!.Stop(); + + // Wait for re-bind: state must return to Bound and recoveryAttempts must be ≥ 1. + bool recovered = false; + for (int i = 0; i < 50; i++) + { + var snap = supervisor.Snapshot(); + if (snap.State == SupervisorState.Bound && snap.RecoveryAttempts >= 1) + { + recovered = true; + break; + } + await Task.Delay(50, cts.Token); + } + recovered.ShouldBeTrue("supervisor must rebind after the underlying listener is faulted"); + + var final = supervisor.Snapshot(); + final.State.ShouldBe(SupervisorState.Bound); + final.RecoveryAttempts.ShouldBeGreaterThanOrEqualTo(1); await supervisor.StopAsync(cts.Token); Assert.Equal(SupervisorState.Stopped, supervisor.Snapshot().State);