dohertj2/lmxopcua
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

Merge #863 TwinCAT Test Connect: drop the AmsNetId-as-IP TCP pre-check; probe via the local AMS router ADS handshake

Browse Source 
Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk
This commit is contained in:
Joseph Doherty
2026-11-11 12:14:58 -05:00
parent 70bc960111 fdc9473819
commit 233e4bab25
4 changed files with 298 additions and 334 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDriverProbeTests.cs
+132 -174
View File
@@ -1,22 +1,17 @@
// Unit tests for TwinCATDriverProbe — the degrade-guarded ADS ReadState Test-Connect probe.
// Unit tests for TwinCATDriverProbe — the ADS ReadState Test-Connect probe (#863).
//
// Coverage here is the set of outcomes that are *deterministic on a CI/dev box with no TwinCAT
// runtime*: invalid JSON, missing host/port, an unreachable TCP target, and — the crux — the
// DEGRADE path. On this box there is no AMS router, so once the TCP preflight succeeds against a
// reachable listener the managed ADS client cannot initialise (it throws a "Check for a running
// TwinCAT router instance!" server exception, NOT an ADS device rejection). The probe MUST treat
// that as "TCP reachability only" (Ok=true) so it never regresses below the old TCP-only probe.
//
// The two paths that REQUIRE a live ADS target are LIVE-VERIFY DEFERRED (no TwinCAT runtime on the
// rig): (a) the happy "ADS state: Run/Config/Stop" path (Ok=true with the AdsState name), and
// (b) the route/auth-reject RED path (a reachable router that refuses the route, surfaced as an
// AdsErrorException carrying a TargetPortNotFound / route AdsErrorCode → Ok=false with the
// "check the target's ADS route table" message). Those are exercised against a real TwinCAT XAR /
// remote runtime, out of scope for this unit suite.
// The probe takes exactly the driver's path: AdsClient.Connect(netId, port) through the LOCAL AMS
// router, then one ReadStateAsync. It must never open a socket of its own, and never to a host derived
// from the AMS Net ID (the old first-phase check dialled the Net ID's first four octets on the AMS
// port — an unrelated host, often a public one, on a port ADS does not use — so Test Connect timed out
// against every real runtime). The classification and budget tests drive the handshake through the
// internal seam; the no-dial and no-router tests use the real AdsClient on a box with no AMS router.
using System.Diagnostics;
using System.Net;
using System.Net.Sockets;
using Shouldly;
using TwinCAT.Ads;
using Xunit;
namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests;
@@ -24,199 +19,162 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests;
[Trait("Category", "Unit")]
public sealed class TwinCATDriverProbeTests
{
private static readonly TwinCATDriverProbe Probe = new();
private const string Z35474Like = "{\"devices\":[{\"hostAddress\":\"ads://5.118.56.93.1.1:851\"}]}";
// -------------------------------------------------------------------------
// 1. Invalid JSON
// -------------------------------------------------------------------------
private static TwinCATDriverProbe ProbeWith(Func<TwinCATAmsAddress, TimeSpan, CancellationToken, Task<AdsState>> handshake)
=> new(handshake);
// ---- config validation ----------------------------------------------------------------
/// <summary>Invalid JSON returns Ok=false with a message containing "invalid".</summary>
[Fact]
public async Task InvalidJson_Returns_OkFalse_WithInvalidMessage()
{
var result = await Probe.ProbeAsync(
"not-valid-json{{{",
TimeSpan.FromSeconds(3),
TestContext.Current.CancellationToken);
var result = await new TwinCATDriverProbe().ProbeAsync(
"not-valid-json{{{", TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken);
result.Ok.ShouldBeFalse();
result.Message.ShouldNotBeNull();
result.Message!.ToLowerInvariant().ShouldContain("invalid");
result.Latency.ShouldBeNull();
}
// -------------------------------------------------------------------------
// 2. Config with no device / no host
// -------------------------------------------------------------------------
/// <summary>An empty Devices list returns Ok=false with a "no host/port" message.</summary>
[Fact]
public async Task NoDevices_Returns_OkFalse_WithNoHostPortMessage()
[Theory]
[InlineData("{\"devices\":[]}")]
[InlineData("{\"devices\":[{\"hostAddress\":\"not-an-ads-url\"}]}")]
public async Task NoUsableDevice_Returns_OkFalse_WithNoHostPortMessage(string json)
{
var result = await Probe.ProbeAsync(
"{\"devices\":[]}",
TimeSpan.FromSeconds(3),
TestContext.Current.CancellationToken);
var called = false;
var result = await ProbeWith((_, _, _) => { called = true; return Task.FromResult(AdsState.Run); })
.ProbeAsync(json, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken);
result.Ok.ShouldBeFalse();
result.Message.ShouldNotBeNull();
result.Message!.ShouldContain("no host/port");
result.Latency.ShouldBeNull();
called.ShouldBeFalse();
}
/// <summary>
/// A first device with a malformed (non-<c>ads://</c>) host address returns Ok=false with a
/// "no host/port" message because <see cref="TwinCATAmsAddress.TryParse"/> returns null.
/// </summary>
[Fact]
public async Task MalformedHostAddress_Returns_OkFalse_WithNoHostPortMessage()
{
var result = await Probe.ProbeAsync(
"{\"devices\":[{\"hostAddress\":\"not-an-ads-url\"}]}",
TimeSpan.FromSeconds(3),
TestContext.Current.CancellationToken);
result.Ok.ShouldBeFalse();
result.Message.ShouldNotBeNull();
result.Message!.ShouldContain("no host/port");
result.Latency.ShouldBeNull();
}
// -------------------------------------------------------------------------
// 3. Unreachable target — TCP preflight fails first
// -------------------------------------------------------------------------
// ---- #863: never dial a host derived from the AMS Net ID --------------------------------
/// <summary>
/// A closed port causes the TCP preflight <c>ConnectAsync</c> to throw
/// <see cref="SocketException"/>; the probe returns Ok=false with "Connect failed".
/// The port is reserved then released so the OS refuses the connection immediately.
/// Regression for #863. A listener sits on exactly the endpoint the old probe dialled
/// (Net ID octets 1–4 as IPv4, AMS port as TCP port). The real probe must never connect to
/// it — the old TCP pre-check connected here first, so this fails on the old behaviour.
/// </summary>
[Fact]
public async Task ClosedPort_Returns_OkFalse_WithConnectFailedMessage()
public async Task Probe_NeverConnects_ToAmsNetIdDerivedEndpoint()
{
// Reserve an ephemeral port then release it so the port is definitely closed.
var reserved = new TcpListener(IPAddress.Loopback, 0);
reserved.Start();
var port = ((IPEndPoint)reserved.LocalEndpoint).Port;
reserved.Stop();
// 127.0.0.1 as the first four AMS-Net-ID octets → the probe's TCP target is 127.0.0.1.
var configJson = $"{{\"devices\":[{{\"hostAddress\":\"ads://127.0.0.1.1.1:{port}\"}}]}}";
using var cts = CancellationTokenSource.CreateLinkedTokenSource(
TestContext.Current.CancellationToken);
cts.CancelAfter(TimeSpan.FromSeconds(5));
var result = await Probe.ProbeAsync(
configJson,
TimeSpan.FromSeconds(3),
cts.Token);
result.Ok.ShouldBeFalse();
result.Message.ShouldNotBeNull();
// Either a fast SocketException ("Connect failed") or, if the OS stalls, a timeout.
var lower = result.Message!.ToLowerInvariant();
(lower.Contains("connect failed") || lower.Contains("timed out")).ShouldBeTrue(
$"Expected 'Connect failed' or 'timed out' but got: {result.Message}");
result.Latency.ShouldBeNull();
}
/// <summary>
/// A target on a non-routable IP (192.0.2.1 — TEST-NET-1, RFC 5737) with a short
/// cancellation window times out (or fails fast on EHOSTUNREACH) at the TCP preflight.
/// </summary>
[Fact]
public async Task BlackHoleTarget_Returns_OkFalse_WithTimedOutOrConnectFailed()
{
using var cts = CancellationTokenSource.CreateLinkedTokenSource(
TestContext.Current.CancellationToken);
cts.CancelAfter(TimeSpan.FromSeconds(1));
var configJson = "{\"devices\":[{\"hostAddress\":\"ads://192.0.2.1.1.1:851\"}]}";
var result = await Probe.ProbeAsync(
configJson,
TimeSpan.FromSeconds(1),
cts.Token);
result.Ok.ShouldBeFalse();
result.Message.ShouldNotBeNull();
var lower = result.Message!.ToLowerInvariant();
(lower.Contains("timed out") || lower.Contains("connect failed")).ShouldBeTrue(
$"Expected 'timed out' or 'Connect failed' but got: {result.Message}");
result.Latency.ShouldBeNull();
}
// -------------------------------------------------------------------------
// 4. DEGRADE path — TCP reachable, but the ADS handshake can't be attempted here
// -------------------------------------------------------------------------
/// <summary>
/// The crux. A reachable TCP listener satisfies the preflight, but on this box (no TwinCAT
/// AMS router) the managed ADS client cannot establish a session — <c>AdsClient.Connect</c>
/// throws an environment/router exception (NOT an ADS device rejection). The probe MUST
/// DEGRADE: <c>Ok=true</c> with the "TCP reachability only" note, never a false RED. This
/// guarantees the new probe never reports worse than the old TCP-only probe on a host with
/// no ADS runtime.
/// </summary>
[Fact]
public async Task ReachableTcp_NoAdsRuntime_Degrades_To_OkTrue_WithTcpOnlyNote()
{
// Stand up a real TCP listener and accept (then drop) connections so the preflight
// ConnectAsync succeeds. We never speak ADS — the ADS handshake is what must degrade.
var listener = new TcpListener(IPAddress.Loopback, 0);
listener.Start();
var port = ((IPEndPoint)listener.LocalEndpoint).Port;
using var acceptCts = CancellationTokenSource.CreateLinkedTokenSource(
TestContext.Current.CancellationToken);
var acceptLoop = Task.Run(async () =>
{
try
{
while (true)
{
var client = await listener.AcceptTcpClientAsync(acceptCts.Token);
// Hold the socket briefly so the preflight sees a clean accept, then close.
_ = Task.Run(async () =>
{
await Task.Delay(200);
client.Dispose();
});
}
}
catch (OperationCanceledException) { /* accept cancelled — expected on teardown */ }
catch (ObjectDisposedException) { /* listener stopped — expected on teardown */ }
catch (SocketException) { /* listener stopped — expected on teardown */ }
}, TestContext.Current.CancellationToken);
try
{
var configJson = $"{{\"devices\":[{{\"hostAddress\":\"ads://127.0.0.1.1.1:{port}\"}}]}}";
using var cts = CancellationTokenSource.CreateLinkedTokenSource(
TestContext.Current.CancellationToken);
using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken);
cts.CancelAfter(TimeSpan.FromSeconds(10));
await new TwinCATDriverProbe().ProbeAsync(
$"{{\"devices\":[{{\"hostAddress\":\"ads://127.0.0.1.1.1:{port}\"}}]}}",
TimeSpan.FromSeconds(3), cts.Token);
var result = await Probe.ProbeAsync(
configJson,
TimeSpan.FromSeconds(3),
cts.Token);
// DEGRADE guard: must be Ok=true (never worse than TCP-only), with the explicit note
// and a latency. If on some box AdsClient instead threw synchronously at construction,
// the probe still degrades the same way — this assertion holds either way.
result.Ok.ShouldBeTrue(
$"Degrade guard violated — probe returned a worse-than-TCP result: {result.Message}");
result.Message.ShouldNotBeNull();
result.Message!.ShouldContain("ADS handshake unavailable on this host");
result.Message!.ShouldContain("TCP reachability only");
result.Latency.ShouldNotBeNull();
listener.Pending().ShouldBeFalse(
"the probe opened a TCP connection to the AmsNetId-derived address — it must only use the local AMS router");
}
finally
{
await acceptCts.CancelAsync();
listener.Stop();
try { await acceptLoop; } catch { /* ignore teardown races */ }
}
}
/// <summary>The handshake receives the AMS address unchanged — the probe resolves no host itself.</summary>
[Fact]
public async Task Handshake_IsHandedTheAmsAddress_Unchanged()
{
TwinCATAmsAddress? seen = null;
var result = await ProbeWith((t, _, _) => { seen = t; return Task.FromResult(AdsState.Run); })
.ProbeAsync(Z35474Like, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken);
result.Ok.ShouldBeTrue();
seen.ShouldBe(new TwinCATAmsAddress("5.118.56.93.1.1", 851));
}
// ---- outcome classification -------------------------------------------------------------
[Fact]
public async Task ReadStateOk_Returns_OkTrue_WithStateAndLatency()
{
var result = await ProbeWith((_, _, _) => Task.FromResult(AdsState.Run))
.ProbeAsync(Z35474Like, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken);
result.Ok.ShouldBeTrue();
result.Message.ShouldBe("ADS state: Run");
result.Latency.ShouldNotBeNull();
}
/// <summary>
/// On a box with no AMS router the real AdsClient cannot connect. That is a RED naming the
/// missing router — no longer a green "TCP reachability only", since nothing was contacted.
/// </summary>
[Fact]
public async Task NoLocalRouter_RealClient_Returns_OkFalse_NamingTheRouter()
{
var result = await new TwinCATDriverProbe().ProbeAsync(
Z35474Like, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken);
result.Ok.ShouldBeFalse();
result.Message!.ShouldContain("No local AMS router");
result.Message!.ShouldContain("not contacted");
result.Latency.ShouldBeNull();
}
[Fact]
public async Task AmsServerException_Classified_AsNoLocalRouter()
{
var result = await ProbeWith((_, _, _) => throw new global::TwinCAT.Ams.AmsServerException("Connect to Server failed!"))
.ProbeAsync(Z35474Like, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken);
result.Ok.ShouldBeFalse();
result.Message!.ShouldContain("No local AMS router");
}
[Theory]
[InlineData(AdsErrorCode.ClientPortNotOpen, "No local AMS router")]
[InlineData(AdsErrorCode.TargetMachineNotFound, "No route to 5.118.56.93.1.1:851 or target unreachable")]
[InlineData(AdsErrorCode.TargetPortNotFound, "No route to 5.118.56.93.1.1:851 or target unreachable")]
[InlineData(AdsErrorCode.WSA_HostUnreachable, "No route to 5.118.56.93.1.1:851 or target unreachable")]
[InlineData(AdsErrorCode.ClientSyncTimeOut, "timed out")]
[InlineData(AdsErrorCode.DeviceInvalidGroup, "ADS handshake with 5.118.56.93.1.1:851 failed")]
public async Task AdsErrorCodes_AreClassified(AdsErrorCode code, string expected)
{
var result = await ProbeWith((_, _, _) => throw new AdsErrorException("x", code))
.ProbeAsync(Z35474Like, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken);
result.Ok.ShouldBeFalse();
result.Message!.ShouldContain(expected);
result.Message!.ShouldContain(code.ToString());
result.Latency.ShouldBeNull();
}
// ---- time budget --------------------------------------------------------------------------
/// <summary>A handshake that never returns (and ignores its token) is cut off at the budget.</summary>
[Fact]
public async Task HangingHandshake_IsBoundedByTimeout()
{
var never = new TaskCompletionSource<AdsState>();
var sw = Stopwatch.StartNew();
var result = await ProbeWith((_, _, _) => never.Task)
.ProbeAsync(Z35474Like, TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken);
sw.Stop();
result.Ok.ShouldBeFalse();
result.Message!.ShouldContain("timed out after 1s");
sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(5));
}
[Fact]
public async Task CallerCancellation_Returns_TimedOut()
{
using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(200));
var result = await ProbeWith(async (_, _, ct) => { await Task.Delay(Timeout.Infinite, ct); return AdsState.Run; })
.ProbeAsync(Z35474Like, TimeSpan.FromSeconds(10), cts.Token);
result.Ok.ShouldBeFalse();
result.Message!.ShouldContain("Probe timed out after 10s");
}
}