From 566ab20d726fe1b8716d654a0c1417db38cd1a85 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 7 Oct 2026 18:24:25 -0400 Subject: [PATCH] test(abcip): differential stops on first connection failure, paces + caps requests, reserves array/UDT slots, fails vacuous template parity; write tests opt-in (Task 18 review r1) --- ...-10-07-native-ab-transport-gate-results.md | 67 ++++ .../Differential/DifferentialRun.cs | 372 +++++++++++++----- .../Differential/DifferentialSafetyTests.cs | 179 +++++++++ .../Differential/DifferentialTarget.cs | 38 +- .../TransportDifferentialTests.cs | 33 +- .../Docker/README.md | 13 + .../Emulate/AbCipEmulateAlmdTests.cs | 1 + .../IntegrationWriteGate.cs | 26 ++ .../AbLegacyReadSmokeTests.cs | 1 + .../Docker/README.md | 2 + .../IntegrationWriteGate.cs | 26 ++ 11 files changed, 648 insertions(+), 110 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialSafetyTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/IntegrationWriteGate.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/IntegrationWriteGate.cs diff --git a/docs/plans/2026-10-07-native-ab-transport-gate-results.md b/docs/plans/2026-10-07-native-ab-transport-gate-results.md index b0993d34..06826e51 100644 --- a/docs/plans/2026-10-07-native-ab-transport-gate-results.md +++ b/docs/plans/2026-10-07-native-ab-transport-gate-results.md @@ -118,6 +118,73 @@ fresh Forward Open) per tag: 9+ sessions, which the netstat sampling showed. The on `@tags` for its whole duration, which mirrors the native anchor lease. That leaves the one-session capture above. +### Review round 1 re-run (press-safety fixes) + +Re-run 2026-10-07 after the review-r1 fixes, with the default pacing. `AB_SERVER_ENDPOINT=127.0.0.1:1` +kept the sibling suites' discovery probes off the network: + +```bash +AB_SERVER_ENDPOINT=127.0.0.1:1 AB_DIFF_ENDPOINT=10.100.0.35:44818 AB_DIFF_COUNT_OPENS=1 \ + dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests \ + --filter "FullyQualifiedName~Differential" --logger "console;verbosity=detailed" +``` + +**Result: 12/12 passed — 7 live tests and 5 offline safety tests — with 0 unexpected and 0 expected divergences.** + +| Check | Result | +|---|---| +| Settings | pace 20 ms, max tags 200, max templates 50 | +| Browse | 9 = 9 | +| Selection (array round-robin first) | 1 array + 8 scalars. ab_server has no UDTs, so there were no UDT members and the coverage check had nothing to require | +| Reads | 9/9 equal | +| Templates | 0 ids, not truncated. Not vacuous by the new rule: ab_server's `STRING` is not a structure, so browse found no structured tags | +| Native Forward Opens | **1** | +| TCP connections to the target before native | 0 (baseline 0) | + +**Wire capture of this run** (tcpdump in the ab_server container's namespace, decoded with tshark): + +| TCP stream | Session | Forward Open | Forward Close | Requests | Window (s) | +|---|---|---|---|---|---| +| 0 | libplctag | 1 (Large, 0x5B) | 1 | 1× 0x55, 18× 0x52 | 0.000 → 0.487 | +| 1 | native | 1 (0x54) | 1 | 1× 0x55, 18× 0x52 | 0.510 → 0.984 | + +- No discovery probes. +- Zero write services: no 0x4D, 0x53, 0x4B or 0x10. +- The two sessions are 23 ms apart and do not overlap. + +**The Forward Opens differ between the transports, and that is expected.** libplctag always uses a *Large* +Forward Open. Native uses a regular one because the `CompactLogix` profile's connection size (504) is at most 511. Task 21 will +see the same split on the press. + +**What changed for press safety (review r1):** + +- **Stop on the first connection-level failure, on either transport.** That covers a refused connect, + RegisterSession or Forward Open, a timeout, and a lost session. Any status other than + NotFound/NotAllowed/OutOfBounds/TooLarge/TooSmall/Unsupported/TypeMismatch aborts the whole run with the + failing operation and status. Nothing is retried. +- **The native phase is capped at one open attempt.** It sets the new process-wide + `CipConnection.OpenAttemptBudget`, so a reconnect cannot reach the wire at all, even from inside an + enumerator that swallows a per-program failure. +- **The run also aborts the moment `ForwardOpenCount` passes 1.** This is checked after every browse entry, + template and tag. +- **Offline pin:** against a loopback listener that accepts and hangs up, the native phase aborts at browse after exactly + **one** TCP connect. Removing the harness abort turns the test red. Removing both the abort and the budget + turns the connect count red. +- **Pacing:** `AB_DIFF_PACE_MS` (default 20) is applied before every harness-issued request: browse, + each template read, each tag's create-read and read. Listing *pages* inside the enumerators are not + paced. +- **Volume caps:** `AB_DIFF_MAX_TEMPLATES` defaults to 50. `AB_DIFF_MAX_TAGS` and `AB_DIFF_MAX_TEMPLATES` must be at most 500; + a higher value fails fast instead of being clamped. +- **Selection:** the default selection takes arrays (up to 20), UDT members (up to 20) and scalars round-robin, so any cap + of 3 or more exercises all three kinds. The reads test fails if browse offered arrays or UDT members + but the selection holds none. +- **Template parity** fails when browse found structured tags but no template id was compared, and treats an + undecodable shape on either side as a mismatch. +- **Write-capable integration tests** now require `AB_SERVER_ALLOW_WRITES=1` and refuse while + `AB_DIFF_ENDPOINT` is set. Those tests are the Logix Emulate ALMD test and the AbLegacy SLC500 N7:5 round trip. + The ab_server controllogix matrix is read-only and unaffected: 15 passed and 11 skipped with + `AB_SERVER_COMPOSE_PROFILE=controllogix`. + ## AdminUI /run (T19) _(pending)_ diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialRun.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialRun.cs index ee988d06..03b72db1 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialRun.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialRun.cs @@ -11,7 +11,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.Differential; internal sealed record SelectedTag(string Name, AbCipDataType? Type, int ElementCount, bool IsArray, string Origin); /// What one transport observed for one tag. -internal sealed record ReadObservation(int Status, string? Error, byte[]? Raw, object? Decoded) +internal sealed record ReadObservation(int Status, string? Error, byte[]? Raw, object? Decoded, string? DecodeError = null) { public bool Succeeded => Status == (int)AbStatus.Ok && Error is null; } @@ -23,6 +23,9 @@ internal sealed class TransportObservations public string? BrowseError { get; set; } public Dictionary Shapes { get; } = []; public Dictionary TemplateErrors { get; } = []; + + /// Template ids that were queued but not read because AB_DIFF_MAX_TEMPLATES was reached. + public int TemplatesTruncated { get; set; } public Dictionary Reads { get; } = new(StringComparer.Ordinal); } @@ -43,16 +46,28 @@ internal sealed class DifferentialResult public required TimeSpan LibplctagDrainWait { get; init; } } +/// +/// The run stopped at its first connection-level failure, or when the native side opened more than once. +/// Thrown, never swallowed: every live test then fails with this message. +/// +internal sealed class DifferentialAbortedException(string message, Exception? inner = null) + : Exception(message, inner); + /// /// Runs the whole comparison ONCE: every libplctag operation first (handles disposed), then — only /// once libplctag's session is gone — every native operation under one anchor lease. Reads only: /// browse listings, template reads and tag reads. Nothing on this path encodes or sends a write. +/// Stop on first failure (review r1 C1). The first connection-level failure on either side — +/// connect / RegisterSession / Forward Open refused, timeout, session lost — aborts the whole run with the +/// operation and status; nothing is retried. The native side is additionally capped at ONE open attempt +/// process-wide (), so a reconnect cannot reach the wire even +/// from inside an enumerator that swallows a per-program failure, and the run aborts the moment +/// passes 1. /// internal static class DifferentialRun { private const int MaxArrays = 20; private const int MaxUdtMembers = 20; - private const int MaxTemplates = 256; /// Upper bound on waiting for libplctag's session to close before native work may start. private static readonly TimeSpan LibplctagDrainBudget = TimeSpan.FromSeconds(30); @@ -60,58 +75,34 @@ internal static class DifferentialRun /// Settle used when the platform cannot enumerate TCP connections. private static readonly TimeSpan FallbackSettle = TimeSpan.FromSeconds(5); + /// Bound on resolving the endpoint's host name (M5). + private static readonly TimeSpan ResolveBudget = TimeSpan.FromSeconds(5); + + /// + /// Statuses that describe ONE tag and leave the session healthy. Everything else — Timeout, BadConnection, + /// Abort, BadReply, Pending, any unexpected exception — is connection-level and aborts the run. + /// + private static readonly HashSet TagLevelStatuses = + [ + AbStatus.NotFound, AbStatus.NotAllowed, AbStatus.OutOfBounds, AbStatus.TooLarge, AbStatus.TooSmall, + AbStatus.Unsupported, AbStatus.TypeMismatch, + ]; + public static async Task ExecuteAsync(DifferentialTarget target, CancellationToken ct) { - var baseline = CountConnections(target); + var addresses = await ResolveAsync(target, ct).ConfigureAwait(false); + var baseline = LocalEndpointsTo(target, addresses); // ---- Phase 1: libplctag only. Every handle is disposed before the phase ends. ---- - // libplctag shares one session (and one Forward Open) among the handles alive at the same time, and - // closes it when the last one is destroyed. The per-tag create/read/dispose below would therefore open - // a fresh session per tag; an anchor handle on the listing pseudo-tag, held for the whole phase, keeps - // every libplctag operation on ONE session — the mirror of the native anchor lease. - var lib = new TransportObservations(); - var libFactories = AbCipTransportFactories.For(AbCipTransport.Libplctag); - IReadOnlyList selection; - using (var libAnchor = libFactories.Tags.Create(target.Params("@tags"))) - { - try - { - await libAnchor.InitializeAsync(ct).ConfigureAwait(false); - } - catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested) - { - throw new InvalidOperationException( - $"libplctag could not open its anchor session to {target}: {Describe(ex)}", ex); - } - await BrowseAsync(target, libFactories.Enumerators, lib, ct).ConfigureAwait(false); - await ReadTemplatesAsync(target, libFactories.Templates, lib, ct).ConfigureAwait(false); - selection = Select(target, lib); - await ReadTagsAsync(target, libFactories.Tags, selection, lib, ct).ConfigureAwait(false); - } + var (lib, selection) = await RunLibplctagPhaseAsync(target, ct).ConfigureAwait(false); // ---- Barrier: libplctag's session must be gone before the native side connects. ---- var drain = Stopwatch.StartNew(); - var beforeNative = await WaitForLibplctagSessionToCloseAsync(target, baseline, ct).ConfigureAwait(false); + var beforeNative = await WaitForLibplctagSessionToCloseAsync(target, addresses, baseline, ct).ConfigureAwait(false); drain.Stop(); // ---- Phase 2: native only, every operation sharing ONE connection held open by the anchor. ---- - var native = new TransportObservations(); - var nativeFactories = AbCipTransportFactories.For(AbCipTransport.Native); - var anchorParams = target.Params("@tags"); - Interlocked.Exchange(ref CipConnection.ForwardOpenCount, 0); - int opens; - var anchor = DeviceSessionPool.Shared.Acquire(NativeAbCipDevice.Key(anchorParams), NativeAbCipDevice.Options(anchorParams)); - try - { - await BrowseAsync(target, nativeFactories.Enumerators, native, ct).ConfigureAwait(false); - await ReadTemplatesAsync(target, nativeFactories.Templates, native, ct).ConfigureAwait(false); - await ReadTagsAsync(target, nativeFactories.Tags, selection, native, ct).ConfigureAwait(false); - } - finally - { - opens = Volatile.Read(ref CipConnection.ForwardOpenCount); - await anchor.DisposeAsync().ConfigureAwait(false); - } + var (native, opens) = await RunNativePhaseAsync(target, selection, ct).ConfigureAwait(false); return new DifferentialResult { @@ -120,60 +111,147 @@ internal static class DifferentialRun Native = native, Selection = selection, NativeForwardOpens = opens, - ConnectionsBaseline = baseline, + ConnectionsBaseline = baseline?.Count, ConnectionsBeforeNative = beforeNative, LibplctagDrainWait = drain.Elapsed, }; } - private static async Task BrowseAsync( - DifferentialTarget target, IAbCipTagEnumeratorFactory factory, TransportObservations into, CancellationToken ct) + /// + /// libplctag shares one session (and one Forward Open) among the handles alive at the same time, and + /// closes it when the last one is destroyed. The per-tag create/read/dispose below would therefore open + /// a fresh session per tag; an anchor handle on the listing pseudo-tag, held for the whole phase, keeps + /// every libplctag operation on ONE session — the mirror of the native anchor lease. + /// + internal static async Task<(TransportObservations Observations, IReadOnlyList Selection)> RunLibplctagPhaseAsync( + DifferentialTarget target, CancellationToken ct) { + const string phase = "libplctag"; + var lib = new TransportObservations(); + var factories = AbCipTransportFactories.For(AbCipTransport.Libplctag); + using var anchor = factories.Tags.Create(target.Params("@tags")); + try + { + await anchor.InitializeAsync(ct).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested) + { + throw Abort(phase, "anchor session (@tags)", ex); + } + await BrowseAsync(phase, target, factories.Enumerators, lib, afterEach: null, ct).ConfigureAwait(false); + await ReadTemplatesAsync(phase, target, factories.Templates, lib, afterEach: null, ct).ConfigureAwait(false); + var selection = Select(target, lib); + await ReadTagsAsync(phase, target, factories.Tags, selection, lib, afterEach: null, ct).ConfigureAwait(false); + return (lib, selection); + } + + /// The native phase alone; internal so the offline abort test can drive it against a fake endpoint. + internal static async Task<(TransportObservations Observations, int ForwardOpens)> RunNativePhaseAsync( + DifferentialTarget target, IReadOnlyList selection, CancellationToken ct) + { + const string phase = "native"; + var native = new TransportObservations(); + var factories = AbCipTransportFactories.For(AbCipTransport.Native); + var anchorParams = target.Params("@tags"); + + void CheckOpens(string operation) + { + var opens = Volatile.Read(ref CipConnection.ForwardOpenCount); + if (opens > 1) + throw new DifferentialAbortedException( + $"Differential ABORTED in the {phase} phase after {operation}: {opens} Forward Open frames on the wire, " + + "expected exactly one (a reconnect, or a Large→regular fallback). Nothing was retried."); + } + + Interlocked.Exchange(ref CipConnection.ForwardOpenCount, 0); + Volatile.Write(ref CipConnection.OpenAttemptBudget, Volatile.Read(ref CipConnection.OpenAttempts) + 1); + int forwardOpens; + try + { + var anchor = DeviceSessionPool.Shared.Acquire(NativeAbCipDevice.Key(anchorParams), NativeAbCipDevice.Options(anchorParams)); + try + { + await BrowseAsync(phase, target, factories.Enumerators, native, CheckOpens, ct).ConfigureAwait(false); + await ReadTemplatesAsync(phase, target, factories.Templates, native, CheckOpens, ct).ConfigureAwait(false); + await ReadTagsAsync(phase, target, factories.Tags, selection, native, CheckOpens, ct).ConfigureAwait(false); + } + finally + { + forwardOpens = Volatile.Read(ref CipConnection.ForwardOpenCount); + await anchor.DisposeAsync().ConfigureAwait(false); + } + } + finally + { + Volatile.Write(ref CipConnection.OpenAttemptBudget, -1); + } + return (native, forwardOpens); + } + + private static async Task BrowseAsync( + string phase, DifferentialTarget target, IAbCipTagEnumeratorFactory factory, TransportObservations into, + Action? afterEach, CancellationToken ct) + { + await PaceAsync(target, ct).ConfigureAwait(false); using var enumerator = factory.Create(); try { await foreach (var tag in enumerator.EnumerateAsync(target.Params("@tags"), ct).ConfigureAwait(false)) + { into.Browse.Add(tag); + afterEach?.Invoke($"browse entry '{tag.Name}'"); + } } - catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } - catch (Exception ex) + catch (Exception ex) when (ex is not DifferentialAbortedException && (ex is not OperationCanceledException || !ct.IsCancellationRequested)) { + if (IsConnectionLevel(ex)) throw Abort(phase, "browse (@tags)", ex); into.BrowseError = Describe(ex); } + afterEach?.Invoke("browse"); } /// Reads every template id the browse surfaced, then every nested id those shapes name. private static async Task ReadTemplatesAsync( - DifferentialTarget target, IAbCipTemplateReaderFactory factory, TransportObservations into, CancellationToken ct) + string phase, DifferentialTarget target, IAbCipTemplateReaderFactory factory, TransportObservations into, + Action? afterEach, CancellationToken ct) { var queue = new Queue(into.Browse .Where(t => t.TemplateInstanceId is not null) .Select(t => t.TemplateInstanceId!.Value) .Distinct() .Order()); + var seen = new HashSet(queue); using var reader = factory.Create(); - while (queue.Count > 0 && into.Shapes.Count + into.TemplateErrors.Count < MaxTemplates) + while (queue.Count > 0) { + if (into.Shapes.Count + into.TemplateErrors.Count >= target.MaxTemplates) + { + into.TemplatesTruncated = queue.Count; + break; + } var id = queue.Dequeue(); - if (into.Shapes.ContainsKey(id) || into.TemplateErrors.ContainsKey(id)) continue; + await PaceAsync(target, ct).ConfigureAwait(false); try { var shape = CipTemplateObjectDecoder.Decode(await reader.ReadAsync(target.Params($"@udt/{id}"), id, ct).ConfigureAwait(false)); into.Shapes[id] = shape; foreach (var nested in shape?.Members ?? []) - if (nested.NestedTemplateId is { } n) queue.Enqueue(n); + if (nested.NestedTemplateId is { } n && seen.Add(n)) queue.Enqueue(n); } - catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } - catch (Exception ex) + catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested) { + if (IsConnectionLevel(ex)) throw Abort(phase, $"template {id}", ex); into.TemplateErrors[id] = Describe(ex); } + afterEach?.Invoke($"template {id}"); } } /// /// The tags to read, chosen from the libplctag browse (the run's first phase), so both transports - /// read exactly the same list. + /// read exactly the same list. Without AB_DIFF_TAGS the three kinds are taken ROUND-ROBIN + /// (array, UDT member, scalar, ...) under their quotas — up to 20 arrays and 20 UDT members — so a + /// press with thousands of scalars still exercises arrays and UDT members at any cap (review r1 I2). /// internal static IReadOnlyList Select(DifferentialTarget target, TransportObservations browsed) { @@ -223,15 +301,63 @@ internal static class DifferentialRun } } - IEnumerable chosen = target.Tags is { } explicitNames - ? explicitNames.Select(n => byName.GetValueOrDefault(n) ?? new SelectedTag(n, null, 1, false, "explicit (type unknown)")) - : scalars.Concat(arrays.Take(MaxArrays)).Concat(members.Take(MaxUdtMembers)); - return chosen.DistinctBy(t => t.Name).Take(target.MaxTags).ToList(); + if (target.Tags is { } explicitNames) + return explicitNames + .Select(n => byName.GetValueOrDefault(n) ?? new SelectedTag(n, null, 1, false, "explicit (type unknown)")) + .DistinctBy(t => t.Name).Take(target.MaxTags).ToList(); + + var chosen = new List(); + var seen = new HashSet(StringComparer.Ordinal); + // Reference-type enumerators created ONCE: a boxed List.Enumerator struct would restart per round. + IEnumerator[] sources = + [ + arrays.Take(MaxArrays).GetEnumerator(), + members.Take(MaxUdtMembers).GetEnumerator(), + scalars.AsEnumerable().Take(int.MaxValue).GetEnumerator(), + ]; + try + { + var live = true; + while (live && chosen.Count < target.MaxTags) + { + live = false; + foreach (var source in sources) + { + if (chosen.Count >= target.MaxTags) break; + while (source.MoveNext()) + { + live = true; + if (seen.Add(source.Current.Name)) { chosen.Add(source.Current); break; } + } + } + } + } + finally + { + foreach (var source in sources) source.Dispose(); + } + return chosen; + } + + /// + /// Coverage problems of a default (browse-driven) selection: browse offered arrays or UDT members but + /// none were selected. Empty for an explicit AB_DIFF_TAGS list, which is the operator's choice. + /// + internal static List SelectionCoverageProblems( + DifferentialTarget target, TransportObservations browsed, IReadOnlyList selection) + { + var problems = new List(); + if (target.Tags is not null) return problems; + var available = Select(target with { MaxTags = DifferentialTarget.Ceiling }, browsed); + foreach (var origin in new[] { "array", "udt member" }) + if (available.Any(t => t.Origin == origin) && selection.All(t => t.Origin != origin)) + problems.Add($"browse offered {origin}s but the selection holds none"); + return problems; } private static async Task ReadTagsAsync( - DifferentialTarget target, IAbCipTagFactory factory, IReadOnlyList selection, - TransportObservations into, CancellationToken ct) + string phase, DifferentialTarget target, IAbCipTagFactory factory, IReadOnlyList selection, + TransportObservations into, Action? afterEach, CancellationToken ct) { foreach (var tag in selection) { @@ -242,30 +368,41 @@ internal static class DifferentialRun string? error = null; try { + await PaceAsync(target, ct).ConfigureAwait(false); await runtime.InitializeAsync(ct).ConfigureAwait(false); + await PaceAsync(target, ct).ConfigureAwait(false); await runtime.ReadAsync(ct).ConfigureAwait(false); } - catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } - catch (Exception ex) + catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested) { + if (IsConnectionLevel(ex)) throw Abort(phase, $"tag '{tag.Name}'", ex); error = Describe(ex); } var status = runtime.GetStatus(); byte[]? raw = null; object? decoded = null; + string? decodeError = null; if (error is null) { - raw = Try(() => RawBufferOf(runtime).ToArray()); - if (tag.Type is { } type) - decoded = Try(() => tag.IsArray ? runtime.DecodeArray(type, tag.ElementCount) : runtime.DecodeValue(type, null)); + try + { + raw = RawBufferOf(runtime).ToArray(); + if (tag.Type is { } type) + decoded = tag.IsArray ? runtime.DecodeArray(type, tag.ElementCount) : runtime.DecodeValue(type, null); + } + catch (Exception ex) + { + decodeError = Describe(ex); // recorded, and a NO verdict (review r1 M3) + } } - into.Reads[tag.Name] = new ReadObservation(status, error, raw, decoded); + into.Reads[tag.Name] = new ReadObservation(status, error, raw, decoded, decodeError); } finally { runtime.Dispose(); } + afterEach?.Invoke($"tag '{tag.Name}'"); } } @@ -276,14 +413,49 @@ internal static class DifferentialRun _ => throw new InvalidOperationException($"Unexpected runtime {runtime.GetType().Name}."), }; + /// True when the failure is about the session, not one tag: the run must stop. + internal static bool IsConnectionLevel(Exception ex) => ex switch + { + AbTransportException ate => !TagLevelStatuses.Contains(ate.Status), + // Local, pre-I/O refusals of one tag name (malformed path, unsupported type): no session involved. + ArgumentException or FormatException or NotSupportedException => false, + _ => true, + }; + + private static DifferentialAbortedException Abort(string phase, string operation, Exception ex) => + new($"Differential ABORTED in the {phase} phase at {operation}: {Describe(ex)}. " + + "Stopped on the first connection-level failure; nothing was retried.", ex); + + private static Task PaceAsync(DifferentialTarget target, CancellationToken ct) => + target.PaceMs > 0 ? Task.Delay(target.PaceMs, ct) : Task.CompletedTask; + + /// Resolves the endpoint once, bounded (an IP literal needs no lookup). + private static async Task> ResolveAsync(DifferentialTarget target, CancellationToken ct) + { + if (IPAddress.TryParse(target.Host, out var literal)) return new HashSet { literal.MapToIPv6() }; + using var bounded = CancellationTokenSource.CreateLinkedTokenSource(ct); + bounded.CancelAfter(ResolveBudget); + try + { + var resolved = await Dns.GetHostAddressesAsync(target.Host, bounded.Token).ConfigureAwait(false); + return resolved.Select(x => x.MapToIPv6()).ToHashSet(); + } + catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested) + { + throw new DifferentialAbortedException( + $"Differential ABORTED before any I/O: could not resolve '{target.Host}' within {ResolveBudget.TotalSeconds:0}s ({ex.Message}).", ex); + } + } + /// - /// libplctag closes its session on a background thread some time after the last handle is - /// destroyed. Wait until the endpoint's TCP connection count is back at the pre-run baseline, so - /// the two transports are never connected to the controller at the same time. + /// libplctag closes its session on a background thread some time after the last handle is destroyed. + /// Wait until no TCP connection to the endpoint remains that was not already there before the run + /// (compared by local endpoint, not by count — review r1 M6), so the two transports are never + /// connected to the controller at the same time. /// /// The connection count observed when native work may start (null when unobservable). private static async Task WaitForLibplctagSessionToCloseAsync( - DifferentialTarget target, int? baseline, CancellationToken ct) + DifferentialTarget target, IReadOnlySet addresses, IReadOnlySet? baseline, CancellationToken ct) { if (baseline is null) { @@ -293,26 +465,29 @@ internal static class DifferentialRun var deadline = Stopwatch.StartNew(); while (true) { - var now = CountConnections(target); - if (now is null || now <= baseline) return now; + var now = LocalEndpointsTo(target, addresses); + if (now is null) return null; + var added = now.Where(e => !baseline.Contains(e)).ToList(); + if (added.Count == 0) return now.Count; if (deadline.Elapsed > LibplctagDrainBudget) - throw new InvalidOperationException( + throw new DifferentialAbortedException( $"libplctag's session to {target.Host}:{target.Port} was still open {LibplctagDrainBudget.TotalSeconds:0}s after its " + - $"handles were disposed ({now} connection(s), baseline {baseline}); refusing to start the native phase alongside it."); + $"handles were disposed (local {string.Join(", ", added)}); refusing to start the native phase alongside it."); await Task.Delay(TimeSpan.FromMilliseconds(250), ct).ConfigureAwait(false); } } - /// Open TCP connections from this machine to the target, or null when the OS cannot say. - internal static int? CountConnections(DifferentialTarget target) + /// Local endpoints of this machine's open TCP connections to the target, or null when the OS cannot say. + private static IReadOnlySet? LocalEndpointsTo(DifferentialTarget target, IReadOnlySet addresses) { try { - var addresses = Dns.GetHostAddresses(target.Host).Select(a => a.MapToIPv6()).ToHashSet(); - return IPGlobalProperties.GetIPGlobalProperties().GetActiveTcpConnections().Count(c => - c.RemoteEndPoint.Port == target.Port - && addresses.Contains(c.RemoteEndPoint.Address.MapToIPv6()) - && c.State is not (TcpState.Closed or TcpState.TimeWait or TcpState.Unknown)); + return IPGlobalProperties.GetIPGlobalProperties().GetActiveTcpConnections() + .Where(c => c.RemoteEndPoint.Port == target.Port + && addresses.Contains(c.RemoteEndPoint.Address.MapToIPv6()) + && c.State is not (TcpState.Closed or TcpState.TimeWait or TcpState.Unknown)) + .Select(c => c.LocalEndPoint) + .ToHashSet(); } catch (Exception ex) when (ex is PlatformNotSupportedException or NetworkInformationException or NotImplementedException) { @@ -339,18 +514,35 @@ internal static class DifferentialRun ? StructuralComparisons.StructuralEqualityComparer.Equals(a, b) : Equals(a, b); + /// Equal only when BOTH decoded: an undecodable shape is never parity (review r1 I3). internal static bool ShapesEqual(AbCipUdtShape? a, AbCipUdtShape? b) => - a is null || b is null - ? a is null && b is null - : a.TypeName == b.TypeName && a.TotalSize == b.TotalSize && a.Members.SequenceEqual(b.Members); + a is not null && b is not null + && a.TypeName == b.TypeName && a.TotalSize == b.TotalSize && a.Members.SequenceEqual(b.Members); - private static T? Try(Func f) + /// + /// Template-parity problems; empty means parity. Vacuity is a problem: browse found structured tags + /// but no template id was compared (review r1 I3). + /// + internal static List TemplateProblems(TransportObservations lib, TransportObservations native) { - try { return f(); } - catch (Exception) { return default; } + var problems = new List(); + var ids = lib.Shapes.Keys.Concat(lib.TemplateErrors.Keys) + .Concat(native.Shapes.Keys).Concat(native.TemplateErrors.Keys) + .Distinct().Order().ToList(); + var structured = lib.Browse.Concat(native.Browse).Count(t => t.DataType == AbCipDataType.Structure); + if (structured > 0 && ids.Count == 0) + problems.Add($"vacuous: browse found {structured} structured tag(s) but no template id was compared"); + foreach (var id in ids) + { + var libShape = lib.Shapes.GetValueOrDefault(id); + var nativeShape = native.Shapes.GetValueOrDefault(id); + if (!ShapesEqual(libShape, nativeShape)) + problems.Add($"template {id}: " + (libShape is null || nativeShape is null ? "undecodable or unread on a side" : "shapes differ")); + } + return problems; } - private static string Describe(Exception ex) => ex is AbTransportException ate + internal static string Describe(Exception ex) => ex is AbTransportException ate ? $"{ate.Status}: {ate.Message}" : $"{ex.GetType().Name}: {ex.Message}"; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialSafetyTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialSafetyTests.cs new file mode 100644 index 00000000..6f48e563 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialSafetyTests.cs @@ -0,0 +1,179 @@ +using System.Net; +using System.Net.Sockets; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.Differential; + +/// +/// Offline pins for the differential's press-safety rules (Task 18 review r1). No controller is touched: +/// the "endpoint" is a loopback listener that accepts and immediately hangs up. +/// +[Collection(DifferentialCollection.Name)] +public sealed class DifferentialSafetyTests +{ + [Fact] + public async Task A_failing_endpoint_aborts_the_native_phase_after_exactly_one_connect() + { + await using var endpoint = HangUpListener.Start(); + var target = Target(endpoint.Port); + IReadOnlyList selection = + [ + new("A", AbCipDataType.DInt, 1, false, "scalar"), + new("B", AbCipDataType.DInt, 1, false, "scalar"), + new("C", AbCipDataType.DInt, 1, false, "scalar"), + ]; + + var ex = await Should.ThrowAsync( + () => DifferentialRun.RunNativePhaseAsync(target, selection, TestContext.Current.CancellationToken)); + + ex.Message.ShouldContain("native phase at browse"); + // Long enough for a throttled (≥ 1 s) reconnect to have reached the listener, had one been allowed. + await Task.Delay(TimeSpan.FromSeconds(2.5), TestContext.Current.CancellationToken); + endpoint.Connections.ShouldBe(1); + Volatile.Read(ref CipConnection.OpenAttemptBudget).ShouldBe(-1, "the budget is restored after the phase"); + } + + [Fact] + public async Task A_failing_endpoint_aborts_the_libplctag_phase_before_any_tag_is_read() + { + await using var endpoint = HangUpListener.Start(); + var target = Target(endpoint.Port); + + var ex = await Should.ThrowAsync( + () => DifferentialRun.RunLibplctagPhaseAsync(target, TestContext.Current.CancellationToken)); + + ex.Message.ShouldContain("libplctag phase at"); + await Task.Delay(TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + // libplctag's own session layer may redial inside its one anchor-create timeout; the harness cannot + // reach below that. Recorded, and bounded: one anchor create, never one per tag. + TestContext.Current.TestOutputHelper?.WriteLine($"libplctag connects before the abort: {endpoint.Connections}"); + endpoint.Connections.ShouldBeInRange(1, 2); + } + + [Fact] + public void The_default_selection_keeps_arrays_and_udt_members_at_any_cap() + { + var browsed = new TransportObservations(); + for (var i = 0; i < 300; i++) browsed.Browse.Add(new AbCipDiscoveredTag($"S{i}", null, AbCipDataType.DInt, false)); + for (var i = 0; i < 30; i++) + browsed.Browse.Add(new AbCipDiscoveredTag($"A{i}", null, AbCipDataType.Real, false, ElementCount: 4, IsArray: true)); + browsed.Browse.Add(new AbCipDiscoveredTag("U", null, AbCipDataType.Structure, false, TemplateInstanceId: 7)); + browsed.Shapes[7] = new AbCipUdtShape("Udt", 400, + [.. Enumerable.Range(0, 25).Select(i => new AbCipUdtMember($"M{i}", i * 4, AbCipDataType.DInt, 1))]); + + var at200 = DifferentialRun.Select(Target(1) with { MaxTags = 200 }, browsed); + at200.Count.ShouldBe(200); + at200.Count(t => t.Origin == "array").ShouldBe(20); + at200.Count(t => t.Origin == "udt member").ShouldBe(20); + at200.Count(t => t.Origin == "scalar").ShouldBe(160); + + var at25 = DifferentialRun.Select(Target(1) with { MaxTags = 25 }, browsed); + at25.Count.ShouldBe(25); + at25.ShouldContain(t => t.Origin == "array"); + at25.ShouldContain(t => t.Origin == "udt member"); + at25.ShouldContain(t => t.Origin == "scalar"); + DifferentialRun.SelectionCoverageProblems(Target(1) with { MaxTags = 25 }, browsed, at25).ShouldBeEmpty(); + + // The coverage check is not vacuous: a selection of scalars only is flagged for both kinds. + DifferentialRun.SelectionCoverageProblems(Target(1), browsed, [.. at200.Where(t => t.Origin == "scalar")]) + .Count.ShouldBe(2); + } + + [Fact] + public void Template_parity_fails_when_vacuous_or_undecodable() + { + var lib = new TransportObservations(); + var native = new TransportObservations(); + lib.Browse.Add(new AbCipDiscoveredTag("U", null, AbCipDataType.Structure, false, TemplateInstanceId: 7)); + DifferentialRun.TemplateProblems(lib, native).ShouldHaveSingleItem().ShouldContain("vacuous"); + + lib.Shapes[7] = null; + native.Shapes[7] = null; + DifferentialRun.TemplateProblems(lib, native).ShouldHaveSingleItem().ShouldContain("undecodable"); + + var shape = new AbCipUdtShape("Udt", 4, [new AbCipUdtMember("M", 0, AbCipDataType.DInt, 1)]); + lib.Shapes[7] = shape; + native.Shapes[7] = shape with { Members = [new AbCipUdtMember("M", 0, AbCipDataType.DInt, 1)] }; + DifferentialRun.TemplateProblems(lib, native).ShouldBeEmpty(); + } + + [Fact] + public void Volume_knobs_fail_fast_above_the_ceiling_instead_of_clamping() + { + var saved = Snapshot(DifferentialTarget.EndpointVar, DifferentialTarget.MaxTagsVar, DifferentialTarget.MaxTemplatesVar, DifferentialTarget.PaceVar); + try + { + Environment.SetEnvironmentVariable(DifferentialTarget.EndpointVar, "127.0.0.1:1"); + Environment.SetEnvironmentVariable(DifferentialTarget.MaxTagsVar, "501"); + Should.Throw(DifferentialTarget.FromEnvironment).Message.ShouldContain(DifferentialTarget.MaxTagsVar); + + Environment.SetEnvironmentVariable(DifferentialTarget.MaxTagsVar, "500"); + Environment.SetEnvironmentVariable(DifferentialTarget.MaxTemplatesVar, "9999"); + Should.Throw(DifferentialTarget.FromEnvironment).Message.ShouldContain(DifferentialTarget.MaxTemplatesVar); + + Environment.SetEnvironmentVariable(DifferentialTarget.MaxTemplatesVar, null); + Environment.SetEnvironmentVariable(DifferentialTarget.PaceVar, null); + var t = DifferentialTarget.FromEnvironment()!; + t.MaxTags.ShouldBe(500); + t.MaxTemplates.ShouldBe(DifferentialTarget.DefaultMaxTemplates); + t.PaceMs.ShouldBe(DifferentialTarget.DefaultPaceMs); + } + finally + { + foreach (var (k, v) in saved) Environment.SetEnvironmentVariable(k, v); + } + } + + private static DifferentialTarget Target(int port) => + new("127.0.0.1", port, "1,0", AbCipPlcFamily.CompactLogix, null, 200, CountOpens: false, MaxTemplates: 50, PaceMs: 0); + + private static (string, string?)[] Snapshot(params string[] names) => + [.. names.Select(n => (n, Environment.GetEnvironmentVariable(n)))]; + + /// Accepts every connection and closes it at once, counting them. + private sealed class HangUpListener : IAsyncDisposable + { + private readonly TcpListener _listener = new(IPAddress.Loopback, 0); + private readonly CancellationTokenSource _stop = new(); + private readonly Task _loop; + private int _connections; + + private HangUpListener() + { + _listener.Start(); + _loop = Task.Run(AcceptLoopAsync); + } + + public static HangUpListener Start() => new(); + + public int Port => ((IPEndPoint)_listener.LocalEndpoint).Port; + + public int Connections => Volatile.Read(ref _connections); + + private async Task AcceptLoopAsync() + { + try + { + while (!_stop.IsCancellationRequested) + { + using var socket = await _listener.AcceptSocketAsync(_stop.Token); + Interlocked.Increment(ref _connections); + socket.Close(); + } + } + catch (OperationCanceledException) { } + catch (SocketException) { } + catch (ObjectDisposedException) { } + } + + public async ValueTask DisposeAsync() + { + await _stop.CancelAsync(); + _listener.Stop(); + try { await _loop; } catch { /* stopped */ } + _stop.Dispose(); + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialTarget.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialTarget.cs index b977d6a0..c72617ea 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialTarget.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialTarget.cs @@ -10,7 +10,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.Differential; /// AB_DIFF_PATH — CIP path, default 1,0. /// AB_DIFF_FAMILY — an name, default CompactLogix. /// AB_DIFF_TAGS — optional comma list of tag names; absent ⇒ selected from browse. -/// AB_DIFF_MAX_TAGS — cap on the selected tags, default 200. +/// AB_DIFF_MAX_TAGS — cap on the selected tags, default 200, at most 500 (higher fails fast). +/// AB_DIFF_MAX_TEMPLATES — cap on template reads per transport, default 50, at most 500. +/// AB_DIFF_PACE_MS — delay before every harness-issued request, default 20 ms; 0 for ab_server. /// AB_DIFF_COUNT_OPENS — 1 ⇒ assert exactly one native Forward Open over the run. /// /// @@ -21,7 +23,9 @@ internal sealed record DifferentialTarget( AbCipPlcFamily Family, IReadOnlyList? Tags, int MaxTags, - bool CountOpens) + bool CountOpens, + int MaxTemplates = DifferentialTarget.DefaultMaxTemplates, + int PaceMs = DifferentialTarget.DefaultPaceMs) { public const string EndpointVar = "AB_DIFF_ENDPOINT"; public const string PathVar = "AB_DIFF_PATH"; @@ -29,6 +33,13 @@ internal sealed record DifferentialTarget( public const string TagsVar = "AB_DIFF_TAGS"; public const string MaxTagsVar = "AB_DIFF_MAX_TAGS"; public const string CountOpensVar = "AB_DIFF_COUNT_OPENS"; + public const string MaxTemplatesVar = "AB_DIFF_MAX_TEMPLATES"; + public const string PaceVar = "AB_DIFF_PACE_MS"; + + /// Hard ceiling on AB_DIFF_MAX_TAGS / AB_DIFF_MAX_TEMPLATES: a production press is the target. + public const int Ceiling = 500; + public const int DefaultMaxTemplates = 50; + public const int DefaultPaceMs = 20; private const int DefaultPort = 44818; private const string DefaultPath = "1,0"; @@ -69,13 +80,23 @@ internal sealed record DifferentialTarget( if (Environment.GetEnvironmentVariable(TagsVar) is { Length: > 0 } rawTags) tags = rawTags.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); - var max = DefaultMaxTags; - if (Environment.GetEnvironmentVariable(MaxTagsVar) is { Length: > 0 } rawMax - && (!int.TryParse(rawMax, NumberStyles.None, CultureInfo.InvariantCulture, out max) || max < 1)) - throw new InvalidOperationException($"{MaxTagsVar}='{rawMax}' is not a positive integer."); + var max = BoundedInt(MaxTagsVar, DefaultMaxTags, 1, Ceiling); + var maxTemplates = BoundedInt(MaxTemplatesVar, DefaultMaxTemplates, 1, Ceiling); + var pace = BoundedInt(PaceVar, DefaultPaceMs, 0, 10_000); + if (tags is { Count: > Ceiling }) + throw new InvalidOperationException($"{TagsVar} names {tags.Count} tags; the ceiling is {Ceiling}."); var countOpens = Environment.GetEnvironmentVariable(CountOpensVar) == "1"; - return new DifferentialTarget(host, port, path, family, tags, max, countOpens); + return new DifferentialTarget(host, port, path, family, tags, max, countOpens, maxTemplates, pace); + } + + /// Reads an integer knob; out of range FAILS rather than clamping, so a typo never silently widens a press run. + private static int BoundedInt(string name, int fallback, int min, int max) + { + if (Environment.GetEnvironmentVariable(name) is not { Length: > 0 } raw) return fallback; + if (!int.TryParse(raw.Trim(), NumberStyles.None, CultureInfo.InvariantCulture, out var value) || value < min || value > max) + throw new InvalidOperationException($"{name}='{raw}' must be an integer in [{min}, {max}]."); + return value; } /// The family profile the driver would use for this target. @@ -99,5 +120,6 @@ internal sealed record DifferentialTarget( /// public override string ToString() => - $"{Host}:{Port} path '{CipPath}' family {Family} ({Profile.LibplctagPlcAttribute}, connection size {Profile.DefaultConnectionSize})"; + $"{Host}:{Port} path '{CipPath}' family {Family} ({Profile.LibplctagPlcAttribute}, connection size {Profile.DefaultConnectionSize}), " + + $"max tags {MaxTags}, max templates {MaxTemplates}, pace {PaceMs} ms"; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/TransportDifferentialTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/TransportDifferentialTests.cs index 0129a198..b5b33e47 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/TransportDifferentialTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/TransportDifferentialTests.cs @@ -70,24 +70,22 @@ public sealed class TransportDifferentialTests(DifferentialRunFixture fixture) var ids = r.Libplctag.Shapes.Keys.Concat(r.Libplctag.TemplateErrors.Keys) .Concat(r.Native.Shapes.Keys).Concat(r.Native.TemplateErrors.Keys) .Distinct().Order().ToList(); - Log($"## Templates — {ids.Count} id(s)"); + Log($"## Templates — {ids.Count} id(s); truncated by {DifferentialTarget.MaxTemplatesVar}: " + + $"libplctag {r.Libplctag.TemplatesTruncated}, native {r.Native.TemplatesTruncated}"); if (ids.Count > 0) { Log("| Template | libplctag | native | Equal? |"); Log("|---|---|---|---|"); } - - var unequal = new List(); foreach (var id in ids) { - var lib = Shape(r.Libplctag, id); - var nat = Shape(r.Native, id); - var equal = r.Libplctag.Shapes.ContainsKey(id) && r.Native.Shapes.ContainsKey(id) - && DifferentialRun.ShapesEqual(r.Libplctag.Shapes[id], r.Native.Shapes[id]); - if (!equal) unequal.Add(id); - Log($"| {id} | {lib} | {nat} | {(equal ? "yes" : "**NO**")} |"); + var equal = DifferentialRun.ShapesEqual(r.Libplctag.Shapes.GetValueOrDefault(id), r.Native.Shapes.GetValueOrDefault(id)); + Log($"| {id} | {Shape(r.Libplctag, id)} | {Shape(r.Native, id)} | {(equal ? "yes" : "**NO**")} |"); } - unequal.ShouldBeEmpty(); + + var problems = DifferentialRun.TemplateProblems(r.Libplctag, r.Native); + foreach (var p in problems) Log($"- PROBLEM: {p}"); + problems.ShouldBeEmpty(); } [Fact] @@ -95,6 +93,8 @@ public sealed class TransportDifferentialTests(DifferentialRunFixture fixture) { var r = await RunOrSkipAsync(); Log($"## Reads — {r.Selection.Count} tag(s), {r.Target}"); + Log("Selection by origin: " + string.Join(", ", + r.Selection.GroupBy(t => t.Origin).Select(g => $"{g.Key} {g.Count()}"))); Log("| Tag | Type | libplctag status | native status | Equal? |"); Log("|---|---|---|---|---|"); @@ -116,8 +116,12 @@ public sealed class TransportDifferentialTests(DifferentialRunFixture fixture) foreach (var e in expected) Log($"- expected: {e}"); foreach (var u in unexpected) Log($"- UNEXPECTED: {u}"); + var coverage = DifferentialRun.SelectionCoverageProblems(r.Target, r.Libplctag, r.Selection); + foreach (var c in coverage) Log($"- COVERAGE: {c}"); + r.Selection.ShouldNotBeEmpty("a run that reads nothing proves nothing"); unexpected.ShouldBeEmpty(); + coverage.ShouldBeEmpty(); } [Fact] @@ -158,6 +162,9 @@ public sealed class TransportDifferentialTests(DifferentialRunFixture fixture) [ "Write" + "Async", "Encode" + "Value", "WriteBit" + "InDInt", "Send" + "Async", "AbCip" + "Driver", ]; + // IL half (review r1 M1): method names match by prefix; the driver type matches by declaring type, + // since the driver type's own members are named .ctor / ReadAsync / ..., never the type name. + var forbiddenType = "AbCip" + "Driver"; var dir = Path.GetDirectoryName(ThisFile())!; var sources = Directory.GetFiles(dir, "*.cs"); @@ -175,7 +182,8 @@ public sealed class TransportDifferentialTests(DifferentialRunFixture fixture) var callHits = (from t in harnessTypes from m in MethodsOf(t) from callee in CalleesOf(m) - where forbidden.Contains(callee.Name, StringComparer.Ordinal) + where forbidden.Any(f => callee.Name.StartsWith(f, StringComparison.Ordinal)) + || callee.DeclaringType?.Name.StartsWith(forbiddenType, StringComparison.Ordinal) == true select $"{t.Name}.{m.Name} calls {callee.DeclaringType?.Name}.{callee.Name}").ToList(); callHits.ShouldBeEmpty(); } @@ -223,13 +231,14 @@ public sealed class TransportDifferentialTests(DifferentialRunFixture fixture) if (lib.Status != nat.Status) return "**NO: status**"; if (!lib.Succeeded || !nat.Succeeded) return lib.Succeeded == nat.Succeeded ? "yes" : "**NO: error**"; + if (lib.DecodeError is not null || nat.DecodeError is not null) return "**NO: decode**"; if (lib.Raw is null || nat.Raw is null || !lib.Raw.AsSpan().SequenceEqual(nat.Raw)) return "**NO: raw**"; if (tag.Type is not null && !DifferentialRun.ValuesEqual(lib.Decoded, nat.Decoded)) return "**NO: value**"; return "yes"; } private static string Detail(ReadObservation o) => - o.Error ?? $"{o.Raw?.Length.ToString(CultureInfo.InvariantCulture) ?? "-"} byte(s) {Hex(o.Raw)} = {Show(o.Decoded)}"; + o.Error ?? o.DecodeError ?? $"{o.Raw?.Length.ToString(CultureInfo.InvariantCulture) ?? "-"} byte(s) {Hex(o.Raw)} = {Show(o.Decoded)}"; private static string Hex(byte[]? raw) => raw is null ? "-" : Convert.ToHexString(raw.AsSpan(0, Math.Min(raw.Length, 32))) + (raw.Length > 32 ? "…" : ""); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/README.md b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/README.md index 1c9b3844..3a6bad3f 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/README.md +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/README.md @@ -121,6 +121,19 @@ records a skip reason when unreachable, so tests stay green on a fresh clone without the container running. Tests use `[AbServerFact]` / `[AbServerTheory]` which check the same probe. +**Writes are opt-in.** Every write-capable test (today the Logix Emulate ALMD +test, which pulses `SimulateAlarm`) also requires `AB_SERVER_ALLOW_WRITES=1`, +and refuses while `AB_DIFF_ENDPOINT` is set. Simulator / Emulate runs set it; +**never** set it when `AB_SERVER_ENDPOINT` points at production hardware. The +ab_server controllogix matrix itself is read-only and needs no opt-in. + +**Read-only differential (Task 18).** `Differential/` compares libplctag and the +native transport against one endpoint, configured only by `AB_DIFF_*` +variables (see `Differential/DifferentialTarget.cs`). Always run it with +`--filter "FullyQualifiedName~Differential"`, and keep the sibling suites' +discovery probes off-network with `AB_SERVER_ENDPOINT=127.0.0.1:1` when the +target is a real controller. + ## What each family seeds Tag sets match `AbServerProfile.cs` exactly — changing seeds in one diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Emulate/AbCipEmulateAlmdTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Emulate/AbCipEmulateAlmdTests.cs index b74feaa6..0be30426 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Emulate/AbCipEmulateAlmdTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Emulate/AbCipEmulateAlmdTests.cs @@ -46,6 +46,7 @@ public sealed class AbCipEmulateAlmdTests public async Task Real_ALMD_raise_fires_OnAlarmEvent_through_the_driver_projection(AbCipTransport transport) { AbServerProfileGate.SkipUnless(AbServerProfileGate.Emulate); + IntegrationWriteGate.SkipUnlessWritesAllowed(); // pulses SimulateAlarm on the controller var endpoint = Environment.GetEnvironmentVariable("AB_SERVER_ENDPOINT") ?? throw new InvalidOperationException( diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/IntegrationWriteGate.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/IntegrationWriteGate.cs new file mode 100644 index 00000000..b316ca54 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/IntegrationWriteGate.cs @@ -0,0 +1,26 @@ +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests; + +/// +/// Opt-in gate for every integration test that WRITES to the endpoint under test. A write test runs only +/// when AB_SERVER_ALLOW_WRITES=1, in addition to its own reachability / profile conditions, so +/// pointing an endpoint variable at a real controller (a production press in Task 21) can never write to +/// it by accident. It also refuses while AB_DIFF_ENDPOINT is set, because that variable means a +/// read-only differential run is in progress. The ab_server / simulator runs set the opt-in explicitly. +/// +public static class IntegrationWriteGate +{ + public const string AllowWritesVar = "AB_SERVER_ALLOW_WRITES"; + public const string DifferentialEndpointVar = "AB_DIFF_ENDPOINT"; + + /// Skips the calling test unless writes are explicitly allowed for this run. + public static void SkipUnlessWritesAllowed() + { + if (Environment.GetEnvironmentVariable(DifferentialEndpointVar) is { Length: > 0 }) + Assert.Skip($"Write test refused: {DifferentialEndpointVar} is set, so a read-only differential run is in progress."); + if (Environment.GetEnvironmentVariable(AllowWritesVar) != "1") + Assert.Skip($"Write test: set {AllowWritesVar}=1 to allow writes to the endpoint under test " + + "(simulator runs only — never a production controller)."); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadSmokeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadSmokeTests.cs index 04030e32..16592fa8 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadSmokeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadSmokeTests.cs @@ -71,6 +71,7 @@ public sealed class AbLegacyReadSmokeTests(AbLegacyServerFixture sim) // container held the port; it survives below only for real-hardware runs, where // AB_LEGACY_ENDPOINT pins one device and the family IS a property of that device. if (sim.SkipReasonFor(KnownProfiles.Slc500) is { } skip) Assert.Skip(skip); + IntegrationWriteGate.SkipUnlessWritesAllowed(); // writes N7:5 var only = Environment.GetEnvironmentVariable("AB_LEGACY_COMPOSE_PROFILE"); if (sim.EndpointPinned && !string.IsNullOrEmpty(only) && diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/README.md b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/README.md index 84edcf90..3aec0909 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/README.md +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/README.md @@ -53,6 +53,7 @@ families stop the current service + start another. |---|---|---| | `AB_LEGACY_ENDPOINT` | *unset* — per-family ports on `10.100.0.35` | `host:port` of a single PCCC device, pinning **every** family to it (real-hardware mode). | | `AB_LEGACY_CIP_PATH` | `1,0` | CIP routing path portion of the `ab://host:port/` URI. ab_server rejects empty paths at the CIP unconnected-send layer; real SLC/MicroLogix/PLC-5 hardware accepts empty (no backplane). Set to empty (`AB_LEGACY_CIP_PATH=`) when pointing at real hardware. | +| `AB_SERVER_ALLOW_WRITES` | *unset* | **Write opt-in.** Every write-capable test (today: the SLC500 N7:5 write-then-read round trip) skips unless this is `1`. Set it for simulator runs; **never** set it when an endpoint variable points at production hardware. Write tests also refuse while `AB_DIFF_ENDPOINT` (the AbCip read-only differential) is set. | | `AB_LEGACY_COMPOSE_PROFILE` | *unset* | Only consulted when `AB_LEGACY_ENDPOINT` pins an endpoint: names which family that one device is, so the SLC500-only write test skips against other hardware. Against the Docker fixture it is unnecessary — each family has its own port, and each test skips itself if its own port is down. | ## Run the integration tests @@ -63,6 +64,7 @@ running so only the matching theory-parameterization executes: ```powershell cd C:\Users\dohertj2\Desktop\lmxopcua $env:AB_LEGACY_COMPOSE_PROFILE = "slc500" # or "micrologix" / "plc5" +$env:AB_SERVER_ALLOW_WRITES = "1" # simulator only: lets the write round-trip test run dotnet test tests\Drivers\ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests ``` diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/IntegrationWriteGate.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/IntegrationWriteGate.cs new file mode 100644 index 00000000..4a183ca8 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/IntegrationWriteGate.cs @@ -0,0 +1,26 @@ +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests; + +/// +/// Opt-in gate for every integration test that WRITES to the endpoint under test. A write test runs only +/// when AB_SERVER_ALLOW_WRITES=1, in addition to its own reachability / profile conditions, so +/// pointing an endpoint variable at a real controller (a production press in Task 21) can never write to +/// it by accident. It also refuses while AB_DIFF_ENDPOINT is set, because that variable means a +/// read-only differential run is in progress. The ab_server / simulator runs set the opt-in explicitly. +/// +public static class IntegrationWriteGate +{ + public const string AllowWritesVar = "AB_SERVER_ALLOW_WRITES"; + public const string DifferentialEndpointVar = "AB_DIFF_ENDPOINT"; + + /// Skips the calling test unless writes are explicitly allowed for this run. + public static void SkipUnlessWritesAllowed() + { + if (Environment.GetEnvironmentVariable(DifferentialEndpointVar) is { Length: > 0 }) + Assert.Skip($"Write test refused: {DifferentialEndpointVar} is set, so a read-only differential run is in progress."); + if (Environment.GetEnvironmentVariable(AllowWritesVar) != "1") + Assert.Skip($"Write test: set {AllowWritesVar}=1 to allow writes to the endpoint under test " + + "(simulator runs only — never a production controller)."); + } +}