From 4f86073333a4c841f4e41ee4f367ae2c77f644b0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 17 Aug 2026 05:33:45 -0400 Subject: [PATCH] test(s7): pin the span/multi-var cancellation reopen contract, one-bad-decode-in-chunk, and multi-var shape mismatch --- .../S7SpanReadParityTests.cs | 197 +++++++++++++++++- 1 file changed, 192 insertions(+), 5 deletions(-) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs index 00160f68..ac18e4ba 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs @@ -36,6 +36,11 @@ public sealed class S7SpanReadParityTests private const uint StatusBadDeviceFailure = 0x808B0000u; /// OPC UA BadNodeIdUnknown — what an unresolvable reference stamps. private const uint StatusBadNodeIdUnknown = 0x80340000u; + /// + /// OPC UA BadCommunicationError — what the driver's catch-all failure mapping stamps, + /// which is where a malformed multi-var answer (an InvalidDataException) lands. + /// + private const uint StatusBadCommunicationError = 0x80050000u; // ── The mixed tag set ──────────────────────────────────────────────────────────────── @@ -373,6 +378,137 @@ public sealed class S7SpanReadParityTests factory.ReadBytesCalls.ShouldBe(2); } + // ── Malformed multi-var answers ────────────────────────────────────────────────────── + + /// + /// ONE undecodable item in an otherwise-good multi-var chunk spoils only its own tag: its + /// chunk-mates keep the values the batch already carried, and NOBODY is re-read individually. + /// A per-ITEM decode fault is not a chunk fault — the whole-chunk fallback exists for a + /// failed REQUEST, and firing it here would re-read tags that already have good values. + /// + /// + /// Health ends Healthy rather than Degraded because the decode loop stamps + /// health per item in chunk order, so the good item after the bad one re-stamps it. That is + /// the same last-writer-wins behaviour the per-tag loop has always had; it is pinned here so + /// a change to it is a deliberate one. + /// + [Fact] + public async Task One_undecodable_multi_var_item_stamps_only_its_own_tag_Bad() + { + var ct = TestContext.Current.CancellationToken; + var images = Images(); + var factory = new ImageS7PlcFactory(images) { NullItemIndex = 1 }; + using var driver = new S7Driver(Options(ScatteredWords(3), maxGap: 2), "s7-mv-item-fault", factory); + await driver.InitializeAsync("{}", ct); + + var results = await driver.ReadAsync(ScatteredRefs(3), ct); + + // The one item the CPU answered with nothing. + results[1].StatusCode.ShouldBe(StatusBadCommunicationError); + results[1].Value.ShouldBeNull(); + + // Its chunk-mates are untouched, values and all (S{i} lives at DB1.DBW{i * 8}). + var db1 = images[(S7NetDataType.DataBlock, 1)]; + static ushort Word(byte[] image, int at) => (ushort)((image[at] << 8) | image[at + 1]); + results[0].StatusCode.ShouldBe(0u); + results[0].Value.ShouldBe(Word(db1, 0)); + results[2].StatusCode.ShouldBe(0u); + results[2].Value.ShouldBe(Word(db1, 16)); + + // One batch, and NO per-tag fallback at all — not even for the tag that failed to decode, + // which would have failed identically on the per-tag path. + factory.MultiVarCalls.ShouldBe(1); + factory.StringReadCalls.ShouldBe(0); + factory.ReadBytesCalls.ShouldBe(0); + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// + /// An answer of the wrong SHAPE — one item short — cannot be mapped back onto the request's + /// indexes, so the driver refuses to guess which tag got which value and falls the whole + /// chunk back to the per-tag path, which still serves every tag Good. + /// + [Fact] + public async Task A_short_multi_var_answer_falls_back_to_per_tag_reads_and_still_serves_good() + { + var ct = TestContext.Current.CancellationToken; + var factory = new ImageS7PlcFactory(Images()) { ShortAnswer = true }; + using var driver = new S7Driver(Options(ScatteredWords(5), maxGap: 2), "s7-mv-shape", factory); + await driver.InitializeAsync("{}", ct); + + var results = await driver.ReadAsync(ScatteredRefs(5), ct); + + results.ShouldAllBe(s => s.StatusCode == 0u); + results.ShouldAllBe(s => s.Value != null); + factory.MultiVarCalls.ShouldBe(1); + // Every tag of the rejected chunk was re-read individually — including the four the short + // answer DID carry, because a mis-shaped answer says nothing trustworthy about any of them. + factory.StringReadCalls.ShouldBe(5); + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + // ── Cancellation mid-read (STAB-15a) ───────────────────────────────────────────────── + + /// + /// Cancellation observed while a SPAN's block read is in flight abandons a half-read + /// ISO-on-TCP response on the single gated stream, so the handle can't be reused: the OCE + /// must propagate (the caller cancelled — nobody consumes the batch) AND the next read must + /// open a fresh connection. The per-tag path's sibling of this is pinned by + /// S7DriverReconnectTests.Cancelled_read_marks_handle_dead_and_next_read_reopens. + /// + [Fact] + public async Task A_cancelled_span_block_read_marks_the_handle_dead_and_the_next_read_reopens() + { + var ct = TestContext.Current.CancellationToken; + var factory = new ImageS7PlcFactory(Images()) { HangReadBytes = true }; + using var driver = new S7Driver(Options(NarrowTags(), maxGap: 16), "s7-span-cancel", factory); + await driver.InitializeAsync("{}", ct); + factory.CreatedPlcCount.ShouldBe(1); + + string[] refs = ["A", "B", "C"]; + using var cts = new CancellationTokenSource(); + var reading = driver.ReadAsync(refs, cts.Token); + await factory.ReadBytesEntered.Task.WaitAsync(TimeSpan.FromSeconds(10), ct); + await cts.CancelAsync(); + + await Should.ThrowAsync(async () => await reading); + + // The reopen is the observable half of the contract: the desynced handle was discarded, so + // the next read builds a NEW connection rather than reusing it. + factory.HangReadBytes = false; + var recovered = await driver.ReadAsync(refs, ct); + recovered.ShouldAllBe(s => s.StatusCode == 0u); + factory.CreatedPlcCount.ShouldBe(2); + } + + /// + /// The same contract on the MULTI-VAR leftover path: a cancelled batch request propagates + /// the OCE and marks the handle dead, so the next read reopens. Distinct from the per-chunk + /// fallback — cancellation is the one request fault that must NOT be retried per tag. + /// + [Fact] + public async Task A_cancelled_multi_var_read_marks_the_handle_dead_and_the_next_read_reopens() + { + var ct = TestContext.Current.CancellationToken; + var factory = new ImageS7PlcFactory(Images()) { HangMultiVar = true }; + using var driver = new S7Driver(Options(ScatteredWords(5), maxGap: 2), "s7-mv-cancel", factory); + await driver.InitializeAsync("{}", ct); + factory.CreatedPlcCount.ShouldBe(1); + + var refs = ScatteredRefs(5); + using var cts = new CancellationTokenSource(); + var reading = driver.ReadAsync(refs, cts.Token); + await factory.MultiVarEntered.Task.WaitAsync(TimeSpan.FromSeconds(10), ct); + await cts.CancelAsync(); + + await Should.ThrowAsync(async () => await reading); + + factory.HangMultiVar = false; + var recovered = await driver.ReadAsync(refs, ct); + recovered.ShouldAllBe(s => s.StatusCode == 0u); + factory.CreatedPlcCount.ShouldBe(2); + } + // ── Configurability ────────────────────────────────────────────────────────────────── /// @@ -473,9 +609,43 @@ public sealed class S7SpanReadParityTests /// When set, only the MULTI-VAR read throws this — the per-tag fallback still serves the image. public Exception? MultiVarFault { get; init; } + /// + /// When true, ReadBytesAsync hangs until its token is cancelled, so cancellation is + /// the only way out of a block read (STAB-15a). Settable so a test can turn it back off + /// after observing the cancellation and let the reopened handle read normally. + /// + public bool HangReadBytes { get; set; } + + /// When true, ReadMultipleVarsAsync hangs until its token is cancelled (STAB-15a). + public bool HangMultiVar { get; set; } + + /// Completed the moment a hanging ReadBytesAsync is entered, so a test cancels a read that is genuinely in flight. + public TaskCompletionSource ReadBytesEntered { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + /// Completed the moment a hanging ReadMultipleVarsAsync is entered. + public TaskCompletionSource MultiVarEntered { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + /// + /// When set, the multi-var answer comes back the right shape but with THIS item's + /// Value left null — the per-item decode fault, which must spoil only that tag. + /// + public int? NullItemIndex { get; init; } + + /// + /// When true, the multi-var answer comes back ONE item short — a shape the driver cannot + /// map back onto the request's indexes, so it must reject the whole chunk. + /// + public bool ShortAnswer { get; init; } + + /// How many connections this factory has been asked to build — the reopen observable. + public int CreatedPlcCount; + /// public IS7Plc Create(S7CpuType cpuType, string host, int port, short rack, short slot, TimeSpan timeout) - => new ImageS7Plc(this); + { + Interlocked.Increment(ref CreatedPlcCount); + return new ImageS7Plc(this); + } private sealed class ImageS7Plc(ImageS7PlcFactory owner) : IS7Plc { @@ -489,11 +659,18 @@ public sealed class S7SpanReadParityTests public void Close() => IsConnected = false; /// - public Task ReadBytesAsync(S7NetDataType area, int db, int startByteAdr, int count, CancellationToken ct) + public async Task ReadBytesAsync(S7NetDataType area, int db, int startByteAdr, int count, CancellationToken ct) { // Counted BEFORE the injected fault so a test can assert how many block reads were // ATTEMPTED — a failure test that counted only successes would read zero. Interlocked.Increment(ref owner.ReadBytesCalls); + if (owner.HangReadBytes) + { + // Signal first, then hang: the test cancels only once the read is genuinely in + // flight, so it exercises cancel-mid-I/O rather than racing a timer. + owner.ReadBytesEntered.TrySetResult(); + await Task.Delay(System.Threading.Timeout.Infinite, ct).ConfigureAwait(false); + } if (owner.ReadFault is { } fault) throw fault; var image = owner.Images[(area, db)]; if (startByteAdr + count > image.Length) @@ -501,7 +678,7 @@ public sealed class S7SpanReadParityTests throw new PlcException(ErrorCode.ReadData, $"fake image overrun: {area} db{db} @{startByteAdr}+{count} exceeds {image.Length}"); } - return Task.FromResult(image[startByteAdr..(startByteAdr + count)]); + return image[startByteAdr..(startByteAdr + count)]; } /// @@ -512,13 +689,18 @@ public sealed class S7SpanReadParityTests /// ushort, DWord a uint. Like the real library it populates each /// item's Value in place and hands the same list back. /// - public Task> ReadMultipleVarsAsync( + public async Task> ReadMultipleVarsAsync( List items, CancellationToken ct) { // Counted (and captured) BEFORE the injected fault so a fallback test can assert the // batch was ATTEMPTED — counting only successes would read zero. Interlocked.Increment(ref owner.MultiVarCalls); lock (owner.MultiVarItems) owner.MultiVarItems.Add([.. items]); + if (owner.HangMultiVar) + { + owner.MultiVarEntered.TrySetResult(); + await Task.Delay(System.Threading.Timeout.Infinite, ct).ConfigureAwait(false); + } if (owner.MultiVarFault is { } mvFault) throw mvFault; if (owner.ReadFault is { } fault) throw fault; @@ -537,7 +719,12 @@ public sealed class S7SpanReadParityTests }; } - return Task.FromResult>(items); + // The two malformed-answer shapes, injected AFTER decoding so the answer is as good + // as any other batch's apart from the one defect under test. + if (owner.NullItemIndex is { } nullAt) items[nullAt].Value = null; + if (owner.ShortAnswer) return items.GetRange(0, items.Count - 1); + + return items; } ///