87a7414df7
The init-time tag guards THREW: one mis-authored tag (DB1.DBZ0, DB1.DBW0 typed Float64, T5 typed Int32, a writable Bool array) faulted the whole driver and the actor retried the same init every 10 s forever, darkening every tag on the PLC. Neither the editor nor the deploy gate checked any of it. - S7AddressParser moves to S7.Contracts (same namespace) and a new S7TagShapeRules there is the single authority on servable shapes. - S7TagDefinitionFactory.FromTagConfig applies it, so the AdminUI editor, the deploy gate (Inspect, with the specific reason) and the driver init refuse the same tags. Init now skips a refused tag (ISkippedTagReporter) instead of throwing; the throwing RejectUnsupportedTagConfigs is gone. - New rules on top of the old guards: a narrow type at the wrong access width (every read failed) and a Bool array not starting at bit 0 (U4: DB1.DBX0.3 silently read bit 0 of bytes 0..n). - AdminUI test fixtures that used addresses the driver could never serve (DB1.DBS0, a browse "SomeAddress", Int32/Float32 on DBW) now use real ones. Refs #742 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
172 lines
8.4 KiB
C#
172 lines
8.4 KiB
C#
using Shouldly;
|
|
using Xunit;
|
|
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
|
|
|
namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests;
|
|
|
|
/// <summary>
|
|
/// Regression tests for the High code-review findings closed against the S7 driver:
|
|
/// Driver.S7-001 (Timer/Counter tags rejected at init), Driver.S7-006 (shutdown drains
|
|
/// the probe/poll loops before disposing the gate), Driver.S7-007 (PUT/GET-disabled maps
|
|
/// to a config alert, not a transient fault), and Driver.S7-011 (Initialize/Reinitialize
|
|
/// honour the supplied driverConfigJson).
|
|
/// </summary>
|
|
[Trait("Category", "Unit")]
|
|
public sealed class S7DriverCodeReviewFixTests
|
|
{
|
|
// ---- Driver.S7-001 — Timer/Counter tags must be rejected at init ----
|
|
|
|
/// <summary>Verifies that initialize rejects timer or counter tags with NotSupportedException.</summary>
|
|
/// <param name="address">The S7 Timer or Counter address to test.</param>
|
|
[Theory]
|
|
[InlineData("T0")]
|
|
[InlineData("T15")]
|
|
[InlineData("C0")]
|
|
[InlineData("C10")]
|
|
public async Task Initialize_rejects_timer_or_counter_tag_with_NotSupportedException(string address)
|
|
{
|
|
// A Timer/Counter address typed anything but Float64 (Timer) / Int32 (Counter) cannot be decoded.
|
|
// It is refused PER TAG at every gate; it used to throw at init and fault every tag on the PLC.
|
|
await S7TagSkipAssert.RefusedPerTagAsync(
|
|
new S7TagDefinition("Quirk", address, S7DataType.Int16, Writable: false), address);
|
|
}
|
|
|
|
/// <summary>Verifies that initialize accepts DB and MIQ addresses without triggering the unsupported guard.</summary>
|
|
[Fact]
|
|
public async Task Initialize_accepts_DB_and_MIQ_addresses_without_the_unsupported_guard_tripping()
|
|
{
|
|
// Sanity check the guard is targeted — DB/M/I/Q tags must NOT be rejected by it.
|
|
// The connect still fails (reserved host), so we assert the failure is the connect,
|
|
// NOT a NotSupportedException from the address guard.
|
|
var opts = new S7DriverOptions
|
|
{
|
|
Host = "192.0.2.1",
|
|
Timeout = TimeSpan.FromMilliseconds(250),
|
|
RawTags = S7RawTags.Entries(
|
|
new S7TagDefinition("Word", "DB1.DBW0", S7DataType.Int16),
|
|
new S7TagDefinition("Bit", "M0.0", S7DataType.Bool)),
|
|
};
|
|
using var drv = new S7Driver(opts, "s7-ok");
|
|
|
|
var ex = await Should.ThrowAsync<Exception>(async () =>
|
|
await drv.InitializeAsync("{}", TestContext.Current.CancellationToken));
|
|
ex.ShouldNotBeOfType<NotSupportedException>(
|
|
"DB/M/I/Q tags are supported — the failure must be the connect, not the address guard");
|
|
}
|
|
|
|
// ---- Driver.S7-011 — driverConfigJson must be applied on Initialize ----
|
|
|
|
/// <summary>Verifies that initialize applies the supplied driverConfigJson over constructor options.</summary>
|
|
[Fact]
|
|
public async Task Initialize_applies_the_supplied_driverConfigJson_over_the_constructor_options()
|
|
{
|
|
// Constructor options point at a real-looking host; the JSON config points at a
|
|
// reserved-for-documentation host with a tiny timeout. If InitializeAsync honours the
|
|
// JSON (the IDriver contract), the connect fails fast against 192.0.2.x. If it ignored
|
|
// the JSON it would hang on the constructor host instead.
|
|
var ctorOpts = new S7DriverOptions { Host = "10.255.255.1", Timeout = TimeSpan.FromSeconds(30) };
|
|
using var drv = new S7Driver(ctorOpts, "s7-cfg");
|
|
|
|
const string json = """
|
|
{ "Host": "192.0.2.1", "TimeoutMs": 250,
|
|
"Tags": [ { "Name": "W", "Address": "DB1.DBW0", "DataType": "Int16" } ] }
|
|
""";
|
|
|
|
var sw = System.Diagnostics.Stopwatch.StartNew();
|
|
await Should.ThrowAsync<Exception>(async () =>
|
|
await drv.InitializeAsync(json, TestContext.Current.CancellationToken));
|
|
sw.Stop();
|
|
|
|
// A 30 s constructor timeout would dominate; the 250 ms JSON timeout proves the JSON won.
|
|
sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(10),
|
|
"InitializeAsync must apply the driverConfigJson timeout, not the constructor's");
|
|
}
|
|
|
|
/// <summary>Verifies that initialize rejects a timer tag supplied only through driverConfigJson.</summary>
|
|
[Fact]
|
|
public async Task Initialize_rejects_a_timer_tag_supplied_only_through_driverConfigJson()
|
|
{
|
|
// The shape gate must run against the *re-parsed* config, not just the constructor options —
|
|
// proves the per-tag skip and Driver.S7-011 compose correctly.
|
|
var ctorOpts = new S7DriverOptions { Host = "192.0.2.1", Timeout = TimeSpan.FromMilliseconds(250) };
|
|
using var drv = new S7Driver(ctorOpts, "s7-cfg-tc", new ScriptedS7PlcFactory());
|
|
|
|
const string json = """
|
|
{ "Host": "192.0.2.1", "TimeoutMs": 250, "Probe": { "Enabled": false },
|
|
"RawTags": [ { "RawPath": "TimerTag", "TagConfig": "{\"address\":\"T5\",\"dataType\":\"Int16\"}", "WriteIdempotent": false } ] }
|
|
""";
|
|
|
|
await drv.InitializeAsync(json, TestContext.Current.CancellationToken);
|
|
drv.GetHealth().State.ShouldBe(DriverState.Healthy);
|
|
var skipped = drv.GetSkippedTags().ShouldNotBeNull().Sample.ShouldHaveSingleItem();
|
|
skipped.RawPath.ShouldBe("TimerTag");
|
|
skipped.Reason.ShouldContain("T5");
|
|
}
|
|
|
|
/// <summary>Verifies that reinitialize applies a changed driverConfigJson.</summary>
|
|
[Fact]
|
|
public async Task Reinitialize_applies_a_changed_driverConfigJson()
|
|
{
|
|
// ReinitializeAsync is the only Core-initiated in-process recovery path; a config
|
|
// change delivered through it must not be silently discarded.
|
|
var ctorOpts = new S7DriverOptions { Host = "10.255.255.1", Timeout = TimeSpan.FromSeconds(30) };
|
|
using var drv = new S7Driver(ctorOpts, "s7-reinit");
|
|
|
|
const string changed = """{ "Host": "192.0.2.1", "TimeoutMs": 250 }""";
|
|
|
|
var sw = System.Diagnostics.Stopwatch.StartNew();
|
|
await Should.ThrowAsync<Exception>(async () =>
|
|
await drv.ReinitializeAsync(changed, TestContext.Current.CancellationToken));
|
|
sw.Stop();
|
|
|
|
sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(10),
|
|
"ReinitializeAsync must re-parse and apply the new driverConfigJson");
|
|
}
|
|
|
|
// ---- Driver.S7-006 — Shutdown drains probe/poll loops before disposing the gate ----
|
|
|
|
/// <summary>Verifies that shutdown completes cleanly with active subscriptions and no disposal race.</summary>
|
|
[Fact]
|
|
public async Task Shutdown_completes_cleanly_with_active_subscriptions_and_no_disposal_race()
|
|
{
|
|
// SubscribeAsync starts a poll loop; ShutdownAsync must cancel AND await it before
|
|
// disposing the shared semaphore. A regression here surfaces as an
|
|
// ObjectDisposedException escaping ShutdownAsync / DisposeAsync.
|
|
var opts = new S7DriverOptions
|
|
{
|
|
Host = "192.0.2.1",
|
|
Probe = new S7ProbeOptions { Enabled = false },
|
|
};
|
|
var drv = new S7Driver(opts, "s7-drain");
|
|
|
|
await drv.SubscribeAsync(["A"], TimeSpan.FromMilliseconds(100), TestContext.Current.CancellationToken);
|
|
await drv.SubscribeAsync(["B"], TimeSpan.FromMilliseconds(100), TestContext.Current.CancellationToken);
|
|
|
|
// Let the poll loops actually start churning so cancellation has something to race.
|
|
await Task.Delay(150, TestContext.Current.CancellationToken);
|
|
|
|
// Must not throw — the drain awaits the poll tasks before disposing _gate.
|
|
await Should.NotThrowAsync(async () => await drv.ShutdownAsync(CancellationToken.None));
|
|
await Should.NotThrowAsync(async () => await drv.DisposeAsync());
|
|
}
|
|
|
|
/// <summary>Verifies that dispose after subscribe does not throw ObjectDisposedException.</summary>
|
|
[Fact]
|
|
public async Task Dispose_after_subscribe_does_not_throw_ObjectDisposedException()
|
|
{
|
|
// The synchronous Dispose() path round-trips through DisposeAsync → ShutdownAsync;
|
|
// it must also drain the poll loop rather than dispose the gate from under it.
|
|
var opts = new S7DriverOptions
|
|
{
|
|
Host = "192.0.2.1",
|
|
Probe = new S7ProbeOptions { Enabled = false },
|
|
};
|
|
var drv = new S7Driver(opts, "s7-dispose");
|
|
|
|
await drv.SubscribeAsync(["A"], TimeSpan.FromMilliseconds(100), TestContext.Current.CancellationToken);
|
|
await Task.Delay(150, TestContext.Current.CancellationToken);
|
|
|
|
Should.NotThrow(() => drv.Dispose());
|
|
}
|
|
}
|