From b4480bf45a5ce24c73ec3f8d98c6ce6324a4c4b2 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 06:56:16 -0400 Subject: [PATCH] test(mtconnect): live write legs against the AllowPut fixture The fixture Agent flips to AllowPut = true (its comment rewritten: writes are in scope now that the driver implements IWritable) and the suite grows three env-gated write legs, all still Assert.Skip-clean with no Agent reachable. Devices.xml gains a DataItem, and that is the deliberate part. The write target has to be one the SHDR adapter NEVER feeds, or the assertion races the stream -- but it must NOT be the existing never-fed fixture_block: the first successful write takes an item off UNAVAILABLE for the life of the container, which would silently rob Read_maps_an_unavailable_observation_to_BadNoCommunication of its subject. So fixture_writeback (free-text EVENT, PROGRAM_COMMENT) is a separate, dedicated target, overridable via MTCONNECT_WRITE_TARGET for a run pointed at a real machine tool. The three legs: * Round trip -- driver write accepted, then the injected value read back off /current over plain HTTP (not through the parser under test) AND through the driver's own ReadAsync. The value carries a TickCount64 suffix: the Agent keeps an injected observation for the container's life, so a constant would be satisfied by a previous run's leftover. * Agent refusal -> BadCommunicationError. The design doc doubted this leg was constructible ("an id in the probe but refused by the agent"); it is, and the mechanism is BuildWriteTargets. Write targets are derived from the tags the BINDING serves, with the device resolved from the probe model and falling back to the configured deviceName -- so an authored tag naming an id the Agent never declared, on a driver whose deviceName IS set, resolves to a real URL and is POSTed. That is exactly the shape a stale tag has after a DataItem is renamed. The second half of the test is the falsifiability control: the SAME unknown id with NO deviceName has no URL and must be refused locally as BadNodeIdUnknown. Two different codes is what proves the first write actually reached the Agent rather than being short-circuited into a look-alike. * Disabled gate -- write.enabled absent returns BadNotWritable AND /current is unchanged from the snapshot taken just before, which is what distinguishes "refused" from "reported refused and sent anyway". RAN LIVE, not just compiled: the fixture was brought up locally (OrbStack, MTCONNECT_AGENT_HOST_PORT=5555 -- macOS AirPlay squats :5000) and the whole suite passed 15/15, 0 skipped. Evidence beyond the green: the Agent reports OTOPCUA-WRITE-2CBCB530 and its log carries "Error processing request: /OtFixtureCnc?otopcua_no_such_data_item_2CBCB3D9=refuse-me" -- the refusal leg demonstrably went to the wire. Offline (no Agent) the suite is 15 skipped, 0 failed; build 0 warnings / 0 errors. Note for anyone reading agent.cfg: BufferSize = 14 is an EXPONENT (2^14 = 16384 observations), not a 14-deep ring, and /current is answered from the Agent's per-DataItem checkpoint anyway -- so eviction is not a hazard for a write read-back even on this busy fixture stream. LiveOptions/ConfigJson/LiveDriverAsync gained writeEnabled + deviceName parameters, both defaulted off/null so every pre-existing test still runs against the shipped read-only driver whatever the Agent's AllowPut says. The config JSON sources write.enabled FROM the options object, so the parsed config can never disagree with the options a test thinks it built. --- .../Docker/Devices.xml | 16 +- .../Docker/agent.cfg | 14 +- .../MTConnectAgentIntegrationTests.cs | 338 +++++++++++++++++- 3 files changed, 358 insertions(+), 10 deletions(-) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/Docker/Devices.xml b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/Docker/Devices.xml index 19979d1a..8b7238b2 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/Docker/Devices.xml +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/Docker/Devices.xml @@ -25,12 +25,20 @@ EVENT, PART_COUNT -> Int64 fixture_partcount (the regression case) EVENT, LINE_NUMBER -> Int64 fixture_linenumber EVENT, controlled vocabulary -> String fixture_execution, fixture_avail, fixture_mode - EVENT, free text -> String fixture_program, fixture_block (never fed) + EVENT, free text -> String fixture_program, fixture_block (never fed), + fixture_writeback (never fed; write target) EVENT, DATA_SET representation -> String fixture_varset CONDITION (any type) -> String + IsAlarm fixture_x_travel, fixture_logic fixture_block is deliberately NEVER fed by the adapter, so the Agent reports it UNAVAILABLE forever — the live half of the UNAVAILABLE -> BadNoCommunication mapping. + + fixture_writeback is likewise never fed, and exists for a different reason: it is the + target the write legs POST to (agent.cfg has AllowPut = true). It is a SEPARATE item from + fixture_block on purpose — the first successful write would take fixture_block off + UNAVAILABLE for the life of the container and quietly rob the mapping test above of its + subject. Because no adapter ever writes fixture_writeback, the value a write leg reads back + out of /current can only have come from that write. --> + + diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/Docker/agent.cfg b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/Docker/agent.cfg index bbc22692..7243dc11 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/Docker/agent.cfg +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/Docker/agent.cfg @@ -22,8 +22,18 @@ BufferSize = 14 CheckpointFrequency = 1000 MaxAssets = 128 -# MTConnect is a read-only source for this driver; the Agent must not accept writes. -AllowPut = false +# Writes ARE in scope for this fixture, because the driver now implements IWritable (see +# docs/plans/2026-08-05-mtconnect-writeback-design.md). The mechanism is this very key: with +# AllowPut on, POST {agent}/{device}?{dataItemId}={value} sets what the Agent REPORTS for that +# observation. It is injection into the Agent's buffer -- visible to every consumer of the +# Agent, overwritten by the next SHDR update for the same DataItem -- and never a command to a +# machine. The integration suite's write legs (round-trip, agent refusal, disabled gate) need +# it on; with the driver's own write.enabled off, the shipped read-only behaviour is unchanged. +# +# No AllowPutFrom is set, so any host that can reach the port may write. Acceptable for a +# throwaway test Agent on the fixture network; NOT acceptable for a plant Agent, where the +# allow-list is the second half of the gate. +AllowPut = true SchemaVersion = 2.0 MonitorConfigFiles = false diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs index 2e08a622..48c909a1 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs @@ -55,14 +55,23 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests; /// PART_COUNT) Assert.Skips rather than passing vacuously. /// /// -/// Two things this suite deliberately does NOT cover, with reasons. -/// (a) MTConnectError under HTTP 200. The driver handles it and canned tests +/// One thing this suite deliberately does NOT cover, with a reason. +/// MTConnectError under HTTP 200. The driver handles it and canned tests /// pin it, but a modern Agent cannot be made to produce it: mtconnect/agent 2.7 /// answers an unknown device with 404 and an out-of-range sequence with 400, /// each carrying a well-formed MTConnectError body (verified live). The under-200 /// shape belongs to older/third-party Agents and stays canned-only. -/// (b) Writes. MTConnect is a read-only source protocol and the driver implements no -/// IWritable; the fixture Agent runs AllowPut = false to match. +/// +/// +/// Writes ARE covered now — the driver gained IWritable and the fixture Agent +/// runs AllowPut = true to match (Docker/agent.cfg). Three legs: the round trip +/// (a driver write the Agent then reports back), the Agent's own refusal of an undeclared +/// DataItem mapping to BadCommunicationError, and the write.enabled gate +/// refusing with BadNotWritable without touching the Agent. The injected target is +/// fixture_writeback, a DataItem the fixture's adapter never feeds, so an observed +/// value can only have come from the write under test. A write is observation +/// injection, not machine command: it sets what the Agent reports, which is exactly why +/// a live Agent is the only thing that can prove it. /// /// [Collection(MTConnectAgentCollection.Name)] @@ -83,9 +92,40 @@ public sealed class MTConnectAgentIntegrationTests(MTConnectAgentFixture agent) private const uint BadNoCommunication = 0x80310000u; + /// The Agent answered and refused the write (non-2xx, or an MTConnectError under 200). + private const uint BadCommunicationError = 0x80050000u; + + /// No DataItem this driver serves answers to the reference — decided locally, before the wire. + private const uint BadNodeIdUnknown = 0x80340000u; + + /// write.enabled is off (or the DataItem is CONDITION-category): nothing is attempted. + private const uint BadNotWritable = 0x803B0000u; + + /// + /// Overrides the DataItem the write legs inject into, for a run pointed at a real Agent whose + /// model has no fixture_writeback. + /// + private const string WriteTargetEnvVar = "MTCONNECT_WRITE_TARGET"; + + /// + /// The fixture's dedicated write target: a free-text EVENT the SHDR adapter NEVER feeds, so a + /// value observed on it can only have come from the write under test. Deliberately NOT + /// fixture_block — the first successful write would take that item off + /// UNAVAILABLE for the life of the container and rob + /// of its subject. + /// + private const string DefaultWriteTargetId = "fixture_writeback"; + /// How long a subscription test waits for the live /sample stream to move a value. private static readonly TimeSpan StreamWait = TimeSpan.FromSeconds(45); + /// + /// How long a write leg waits for the Agent to report the injected observation on + /// /current. A PUT lands in the Agent's buffer synchronously, so this is slack for the + /// HTTP round trips rather than for any processing the Agent has to schedule. + /// + private static readonly TimeSpan WriteVisibleWait = TimeSpan.FromSeconds(15); + private static CancellationToken Ct => TestContext.Current.CancellationToken; // ---- reachability ---- @@ -537,6 +577,169 @@ public sealed class MTConnectAgentIntegrationTests(MTConnectAgentFixture agent) } } + // ---- writes: the Agent's AllowPut surface ---- + + /// + /// The write round trip, and the only thing that can prove the PUT surface at all. A + /// driver write must be accepted by the Agent and must then be what the Agent + /// reports for that DataItem — read back off /current over plain HTTP (not + /// through the parser under test), and then through the driver's own read path. + /// + /// + /// + /// The value is unique per run (a suffix). + /// The Agent keeps an injected observation for the life of the container, so a constant + /// would be satisfied by a previous run's leftover and the test would pass against a + /// driver whose write never left the process. + /// + /// + /// The target is a DataItem the SHDR adapter never feeds, so no observation but + /// this write can put a value on it. Ring-buffer eviction is not a hazard here even + /// though the fixture's stream is busy: BufferSize = 14 in agent.cfg is an + /// exponent (2^14 = 16384 observations), and /current is answered from the + /// Agent's checkpoint — the newest observation per DataItem — which survives the ring + /// wrapping past it. + /// + /// + [Fact(Timeout = TestTimeoutMs)] + public async Task Write_injects_an_observation_the_agent_then_reports() + { + SkipIfDown(); + + var target = ResolveWriteTarget(); + if (target is null) + { + Assert.Skip(NoWriteTargetReason()); + } + + var (dataItemId, deviceName) = target!.Value; + var value = $"OTOPCUA-WRITE-{Environment.TickCount64:X}"; + + await using var live = await LiveDriverAsync([Tag(dataItemId, DriverDataType.String)], writeEnabled: true); + + var results = await live.Driver.WriteAsync([new WriteRequest(dataItemId, value)], Ct); + + results.Count.ShouldBe(1); + results[0].StatusCode.ShouldBe( + Good, + $"the Agent at {agent.AgentUri} must accept a write to DataItem '{dataItemId}' on device " + + $"'{deviceName}'. A refusal here is most often AllowPut still false in the Agent's config."); + + var observed = await WaitForObservationAsync(dataItemId, value, WriteVisibleWait); + observed.ShouldBe( + value, + $"the Agent must REPORT the injected value on /current within {WriteVisibleWait.TotalSeconds:F0}s; " + + "a Good write whose value never appears is a write that went nowhere"); + + var reads = await live.Driver.ReadAsync([dataItemId], Ct); + reads.Count.ShouldBe(1); + reads[0].StatusCode.ShouldBe(Good, "the injected observation must read back through the driver's own path"); + reads[0].Value.ShouldBe(value); + } + + /// + /// The Agent's own refusal, mapped to BadCommunicationError. A write naming a + /// DataItem the Agent does not declare reaches the wire and comes back refused. + /// + /// + /// + /// Why this reaches the Agent at all, when an unknown reference is normally answered + /// locally. BuildWriteTargets derives a write target for every DataItem the + /// binding serves — i.e. every authored tag — resolving the device from the probe + /// model and falling back to the configured deviceName. So an authored tag naming + /// an id the Agent never declared, on a driver whose deviceName IS configured, + /// resolves to a real URL and is POSTed. That is precisely the shape a stale tag has after + /// a machine's DataItem is renamed, and the Agent's answer to it is the refusal this leg + /// pins. + /// + /// + /// The second half is the falsifiability control, and without it the leg proves + /// nothing: the SAME unknown id on a driver with NO configured device name has no URL to + /// POST to and must be refused locally as BadNodeIdUnknown. Two different codes for + /// the same id is what shows the first write actually went to the Agent rather than being + /// short-circuited into a look-alike failure. + /// + /// + [Fact(Timeout = TestTimeoutMs)] + public async Task Write_to_a_data_item_the_agent_does_not_declare_reports_the_agents_refusal() + { + SkipIfDown(); + + var target = ResolveWriteTarget(); + if (target is null) + { + Assert.Skip(NoWriteTargetReason()); + } + + var deviceName = target!.Value.DeviceName; + var unknownId = $"otopcua_no_such_data_item_{Environment.TickCount64:X}"; + + await using (var live = await LiveDriverAsync( + [Tag(unknownId, DriverDataType.String)], writeEnabled: true, deviceName: deviceName)) + { + var refused = await live.Driver.WriteAsync([new WriteRequest(unknownId, "refuse-me")], Ct); + + refused.Count.ShouldBe(1); + refused[0].StatusCode.ShouldBe( + BadCommunicationError, + $"the Agent at {agent.AgentUri} must refuse a write to DataItem '{unknownId}', which device " + + $"'{deviceName}' does not declare, and the driver must report that refusal as an Agent " + + "failure rather than as success"); + } + + await using var noDevice = await LiveDriverAsync([Tag(unknownId, DriverDataType.String)], writeEnabled: true); + + var local = await noDevice.Driver.WriteAsync([new WriteRequest(unknownId, "refuse-me")], Ct); + + local.Count.ShouldBe(1); + local[0].StatusCode.ShouldBe( + BadNodeIdUnknown, + "with no device resolvable there is no URL to POST to, so the refusal must be decided locally — " + + "and a run where BOTH writes report the same code proves nothing about the Agent"); + } + + /// + /// The driver-side gate, which is the half an operator controls. With + /// write.enabled absent the same write is refused BadNotWritable — and the + /// Agent's /current is untouched, which is the assertion that distinguishes "refused" + /// from "reported as refused and sent anyway". + /// + [Fact(Timeout = TestTimeoutMs)] + public async Task Write_is_refused_without_touching_the_agent_when_write_is_not_enabled() + { + SkipIfDown(); + + var target = ResolveWriteTarget(); + if (target is null) + { + Assert.Skip(NoWriteTargetReason()); + } + + var dataItemId = target!.Value.DataItemId; + var value = $"OTOPCUA-DISABLED-{Environment.TickCount64:X}"; + var before = await ObservationValueAsync(dataItemId); + + // No writeEnabled: the driver's own gate, with the Agent's AllowPut left on. + await using var live = await LiveDriverAsync([Tag(dataItemId, DriverDataType.String)]); + + var results = await live.Driver.WriteAsync([new WriteRequest(dataItemId, value)], Ct); + + results.Count.ShouldBe(1); + results[0].StatusCode.ShouldBe( + BadNotWritable, + "write.enabled is absent, so the honest answer is 'this is not writable' — not a comms failure"); + + // Long enough that a write which HAD been dispatched would be visible: the round-trip leg + // observes one inside WriteVisibleWait, and this waits for the same value to appear. + var after = await WaitForObservationAsync(dataItemId, value, TimeSpan.FromSeconds(3)); + + after.ShouldNotBe(value, "a gated write must never reach the Agent's observation buffer"); + after.ShouldBe( + before, + $"nothing else feeds DataItem '{dataItemId}', so its reported value must be exactly what it was " + + "before the gated write"); + } + // ---- helpers ---- private void SkipIfDown() @@ -569,13 +772,17 @@ public sealed class MTConnectAgentIntegrationTests(MTConnectAgentFixture agent) return port; } - private MTConnectDriverOptions LiveOptions(string? deviceName = null) => + private MTConnectDriverOptions LiveOptions(string? deviceName = null, bool writeEnabled = false) => new() { AgentUri = agent.AgentUri, DeviceName = deviceName, RequestTimeoutMs = 15_000, + // Off unless a write leg asks for it: every other test in this suite must run against the + // shipped read-only driver, whatever the fixture Agent's AllowPut says. + Write = new MTConnectWriteOptions { Enabled = writeEnabled }, + // Faster than the 1000 ms default so a subscription sees several distinct chunks inside // StreamWait; the heartbeat stays well under it so a silent Agent is caught, not waited on. SampleIntervalMs = 200, @@ -597,6 +804,10 @@ public sealed class MTConnectAgentIntegrationTests(MTConnectAgentFixture agent) sampleCount = options.SampleCount, heartbeatMs = options.HeartbeatMs, probe = new { enabled = false }, + + // Sourced from the options rather than a separate parameter, so the config the driver + // parses can never disagree with the options object a test thinks it built. + write = new { enabled = options.Write.Enabled }, tags = (tags ?? []).Select(t => new { fullName = t.FullName, @@ -607,9 +818,16 @@ public sealed class MTConnectAgentIntegrationTests(MTConnectAgentFixture agent) private static MTConnectTagDefinition Tag(string dataItemId, DriverDataType type) => new(dataItemId, type); /// Initializes a driver against the live Agent with the supplied authored tags. - private async Task LiveDriverAsync(IEnumerable tags) + /// The authored tags, by DataItem id. + /// The driver's own write.enabled gate; off by default. + /// + /// The configured device scope, and the write path's device fallback for a DataItem the probe + /// model does not declare. Null (the default) leaves the driver unscoped. + /// + private async Task LiveDriverAsync( + IEnumerable tags, bool writeEnabled = false, string? deviceName = null) { - var options = LiveOptions(); + var options = LiveOptions(deviceName, writeEnabled); var driver = new MTConnectDriver(options, "mtconnect-live"); await driver.InitializeAsync(ConfigJson(options, tags), Ct); @@ -650,6 +868,112 @@ public sealed class MTConnectAgentIntegrationTests(MTConnectAgentFixture agent) return XDocument.Parse(body); } + /// + /// The DataItem the write legs inject into, and the device that owns it — or null when + /// this Agent declares nothing suitable, which is a skip rather than a failure. + /// + /// + /// Resolved from the Agent's own /probe like every other expectation in this suite, and + /// overridable via so a run pointed at a real machine tool can + /// name a DataItem its operator is willing to have injected. CONDITION-category items and the + /// TIME_SERIES / DATA_SET / TABLE representations are excluded because WriteAsync + /// itself refuses them — pointing a write leg at one would assert the wrong refusal. + /// + private (string DataItemId, string DeviceName)? ResolveWriteTarget() + { + var id = (Environment.GetEnvironmentVariable(WriteTargetEnvVar) ?? DefaultWriteTargetId).Trim(); + + var declared = agent.DeclaredDataItems + .FirstOrDefault(d => string.Equals(d.Id, id, StringComparison.Ordinal)); + + if (declared is null || + Is(declared.Category, "CONDITION") || + Is(declared.Representation, "TIME_SERIES") || + Is(declared.Representation, "DATA_SET") || + Is(declared.Representation, "TABLE")) + { + return null; + } + + var device = OwningDeviceName(id); + + return device is null ? null : (id, device); + } + + /// The skip message for a run whose Agent declares no usable write target. + private string NoWriteTargetReason() => + $"The Agent at {agent.AgentUri} declares no writable DataItem " + + $"'{Environment.GetEnvironmentVariable(WriteTargetEnvVar) ?? DefaultWriteTargetId}' " + + $"(a non-CONDITION, scalar item under a Device). Set {WriteTargetEnvVar} to a DataItem this " + + "Agent declares and is safe to inject an observation into, or run against the fixture's own " + + "Devices.xml, which carries a never-fed one for exactly this purpose."; + + /// + /// The device a DataItem is declared under, addressed the way the Agent's PUT URL addresses + /// it: by name, falling back to id for a nameless device. + /// + private string? OwningDeviceName(string dataItemId) + { + var element = agent.ProbeDocument? + .Descendants() + .FirstOrDefault(e => + e.Name.LocalName == "DataItem" && + string.Equals((string?)e.Attribute("id"), dataItemId, StringComparison.Ordinal)); + + var device = element?.Ancestors().FirstOrDefault(a => a.Name.LocalName == "Device"); + var name = (string?)device?.Attribute("name") ?? (string?)device?.Attribute("id"); + + return string.IsNullOrWhiteSpace(name) ? null : name; + } + + /// + /// What the Agent currently reports for one DataItem, straight off /current — the raw + /// observation text (UNAVAILABLE included), or null when the snapshot carries no + /// observation for it at all. + /// + private async Task ObservationValueAsync(string dataItemId) => + (await CurrentAsync()) + .Descendants() + .Where(e => !e.HasElements) + .Where(e => string.Equals((string?)e.Attribute("dataItemId"), dataItemId, StringComparison.Ordinal)) + .Select(e => e.Value.Trim()) + .LastOrDefault(); + + /// + /// Polls /current until the DataItem reports or the wait + /// runs out, returning whatever it last observed either way. + /// + /// + /// Returning the last observation rather than a bool is deliberate: a failing assertion can + /// then print what the Agent WAS reporting, which is the difference between "the write never + /// landed" and "the write landed as something else" (a formatting defect). + /// + private async Task WaitForObservationAsync(string dataItemId, string expected, TimeSpan timeout) + { + using var deadline = CancellationTokenSource.CreateLinkedTokenSource(Ct); + deadline.CancelAfter(timeout); + + string? observed; + do + { + observed = await ObservationValueAsync(dataItemId); + if (string.Equals(observed, expected, StringComparison.Ordinal)) + { + return observed; + } + + try + { + await Task.Delay(TimeSpan.FromMilliseconds(500), deadline.Token); + } + catch (OperationCanceledException) when (!Ct.IsCancellationRequested) + { + return observed; + } + } + while (true); + } + /// /// The DataItem ids the Agent is currently reporting a parseable number for: the live subset /// a read or subscription can make a non-vacuous statement about.