From 1211cd9a83acca5d7895b6e958de90cc38596852 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 7 Oct 2026 17:10:35 -0400 Subject: [PATCH] test(abcip): Task 13 review r2 - non-vacuous exclusive-packing tests Scheduler tests (exclusive mid-queue and at head) and a template-reader test that genuinely queues a Read Template behind a small read; bounded QueuedCount wait replaces the fixed delay. Falsified: removing exclusive from the reader turns the reader test red; removing the Exclusive handling in RequestScheduler turns all three red. --- .../Cip/RequestSchedulerTests.cs | 47 +++++++++++++++ .../NativeAbCipTemplateReaderTests.cs | 60 ++++++++++++------- 2 files changed, 87 insertions(+), 20 deletions(-) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/RequestSchedulerTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/RequestSchedulerTests.cs index 5d40cfb5..8518f296 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/RequestSchedulerTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Cip/RequestSchedulerTests.cs @@ -390,4 +390,51 @@ public sealed class RequestSchedulerTests ch.SentCount.ShouldBe(1); Should.Throw(() => s.SendAsync(Req(8), CancellationToken.None)); } + + private static bool IsMsp(byte[] r) => r[0] == 0x0A; + + [Fact] + public async Task An_exclusive_request_goes_out_bare_and_splits_the_packing_around_it_in_fifo_order() + { + var ch = new FakeCipChannel(); + await using var s = new RequestScheduler(ch, allowPacking: true); + var a = await HoldFirstAsync(s, ch, Req(8, 1)); + + var b1 = s.SendAsync(Req(8, 2), Ct); + var b2 = s.SendAsync(Req(8, 3), Ct); + var c = s.SendAsync(Req(8, 4), Ct, exclusive: true); + var d1 = s.SendAsync(Req(8, 5), Ct); + var d2 = s.SendAsync(Req(8, 6), Ct); + s.QueuedCount.ShouldBe(5); + ch.Gate!.SetResult(); + await Task.WhenAll(a, b1, b2, c, d1, d2).WaitAsync(Timeout, Ct); + + // A, then [B1 B2] packed, C bare, then [D1 D2] packed: the exclusive request is a batch boundary. + ch.Sent.Count.ShouldBe(4); + IsMsp(ch.Sent[1]).ShouldBeTrue(); + FakeCipChannel.SplitMsp(ch.Sent[1]).Select(e => e[^1]).ShouldBe(new byte[] { 2, 3 }); + IsMsp(ch.Sent[2]).ShouldBeFalse(); + ch.Sent[2][^1].ShouldBe((byte)4); + IsMsp(ch.Sent[3]).ShouldBeTrue(); + FakeCipChannel.SplitMsp(ch.Sent[3]).Select(e => e[^1]).ShouldBe(new byte[] { 5, 6 }); + } + + [Fact] + public async Task An_exclusive_request_at_the_head_of_the_queue_does_not_take_followers_with_it() + { + var ch = new FakeCipChannel(); + await using var s = new RequestScheduler(ch, allowPacking: true); + var a = await HoldFirstAsync(s, ch, Req(8, 1)); + + var c = s.SendAsync(Req(8, 2), Ct, exclusive: true); + var d1 = s.SendAsync(Req(8, 3), Ct); + var d2 = s.SendAsync(Req(8, 4), Ct); + ch.Gate!.SetResult(); + await Task.WhenAll(a, c, d1, d2).WaitAsync(Timeout, Ct); + + ch.Sent.Count.ShouldBe(3); + IsMsp(ch.Sent[1]).ShouldBeFalse(); + ch.Sent[1][^1].ShouldBe((byte)2); + FakeCipChannel.SplitMsp(ch.Sent[2]).Select(e => e[^1]).ShouldBe(new byte[] { 3, 4 }); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTemplateReaderTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTemplateReaderTests.cs index b68aab94..fe0cbaa4 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTemplateReaderTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTemplateReaderTests.cs @@ -21,6 +21,13 @@ public sealed class NativeAbCipTemplateReaderTests /// Builds a channel serving (header + raw) the way a controller would. private static FakeCipChannel ServeTemplate(byte[] captured, int maxRequestBytes, int? statusOverrideAtFirst = null) + { + var ch = new FakeCipChannel(maxRequestBytes); + ch.RespondPerService = ServeTemplatePerService(captured, maxRequestBytes); + return ch; + } + + private static Func ServeTemplatePerService(byte[] captured, int maxRequestBytes) { var id = BinaryPrimitives.ReadUInt16LittleEndian(captured); var defWords = BinaryPrimitives.ReadUInt32LittleEndian(captured.AsSpan(2)); @@ -29,9 +36,7 @@ public sealed class NativeAbCipTemplateReaderTests var handle = BinaryPrimitives.ReadUInt16LittleEndian(captured.AsSpan(12)); var raw = captured.AsSpan(14).ToArray(); var total = (int)(defWords * 4 - 23); - - var ch = new FakeCipChannel(maxRequestBytes); - ch.RespondPerService = req => + return req => { if (req[0] == 0x03) { @@ -56,7 +61,6 @@ public sealed class NativeAbCipTemplateReaderTests var done = offset + n >= total; return FakeCipChannel.Reply(0x4C, done ? (byte)0x00 : (byte)0x06, raw[offset..(offset + n)]); }; - return ch; } private static async Task ReadNativeAsync(FakeCipChannel ch, ushort id) @@ -130,32 +134,48 @@ public sealed class NativeAbCipTemplateReaderTests } [Fact] - public async Task A_template_read_queued_behind_a_small_read_on_a_packing_scheduler_goes_out_alone() + public async Task A_read_template_queued_behind_a_small_read_on_a_packing_scheduler_goes_out_alone() { var captured = Convert.FromHexString(CipTemplateObjectWireCaptureTests.LookupMemberTemplate); var ch = ServeTemplate(captured, 504); - ch.Gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var served = ch.RespondPerService; - ch.RespondPerService = r => r[0] == 0x0E ? FakeCipChannel.OkReply(r) : served(r); await using var sched = new RequestScheduler(ch, allowPacking: true); var ct = TestContext.Current.CancellationToken; + var small = new byte[] { 0x0E, 0x02, 0x20, 0x01, 0x24, 0x01 }; // Get Attribute Single + var hold = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var inner = ch.RespondPerService; + var firstSmall = true; + Task? s1 = null; + ch.Respond = async req => + { + // The Get Attributes reply is delivered with a small read already queued behind it, so the + // pump takes that read next (S1) and holds it in flight while the reader submits its first + // Read Template: that template is genuinely queued behind another request. + if (req[0] == 0x03) + { + s1 = sched.SendAsync(small, ct); + return await ch.DefaultRespond(req); + } + if (req[0] == 0x0E && firstSmall) + { + firstSmall = false; + await hold.Task; + } + return await ch.DefaultRespond(req); + }; + ch.RespondPerService = r => r[0] == 0x0E ? FakeCipChannel.OkReply(r) : inner(r); using var reader = new NativeAbCipTemplateReader(_ => (sched, ch, () => ValueTask.CompletedTask)); - // A is in flight (gated); B queues behind it; the template read then queues behind B. - var small = new byte[] { 0x0E, 0x02, 0x20, 0x01, 0x24, 0x01 }; // Get Attribute Single - var a = sched.SendAsync(small, ct); - await ch.WaitForSendsAsync(1); - var b = sched.SendAsync(small, ct); var read = reader.ReadAsync(Params, 3386, ct); - await Task.Delay(200, ct); - ch.Gate.SetResult(); - await Task.WhenAll(a, b); + await ch.WaitForSendsAsync(2); // Get Attributes, then S1 held in flight + var s2 = sched.SendAsync(small, ct); + using var bound = CancellationTokenSource.CreateLinkedTokenSource(ct); + bound.CancelAfter(TimeSpan.FromSeconds(10)); + while (sched.QueuedCount < 2) await Task.Delay(1, bound.Token); // S2 + the first Read Template + hold.SetResult(); await read; + await Task.WhenAll(s1!, s2); - // Packing was live (the attribute query shared an MSP with B), yet no Read Template rode in one. - ch.Sent.ShouldContain(r => r[0] == 0x0A); - foreach (var msp in ch.Sent.Where(r => r[0] == 0x0A)) - FakeCipChannel.SplitMsp(msp).ShouldNotContain(e => e[0] == 0x4C); + ch.Sent.Where(r => r[0] == 0x0A).ShouldAllBe(msp => FakeCipChannel.SplitMsp(msp).All(e => e[0] != 0x4C)); ch.Sent.ShouldContain(r => r[0] == 0x4C); // the Read Template requests went out bare }