fix(driver-focas): resolve Medium code-review finding (Driver.FOCAS-012)
Add FocasDriverMediumFindingsTests.cs with regression coverage for the five Medium findings: - 003: InitializeAsync throws when tag's DeviceHostAddress is absent from Devices (two variants: typo host, wrong port; also happy path) - 004: DiscoverAsync emits ViewOnly for tags with Writable:true - 005: GetHealth() is consistent after ten concurrent ReadAsync calls - 006: Read recovers after the client is externally disposed, creating a fresh client rather than wedging with BadCommunicationError - 012: Factory full-round-trip with all three opt-in config sections (FixedTree + AlarmProjection + HandleRecycle) with all subfields Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,263 @@
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||
using ZB.MOM.WW.OtOpcUa.Driver.FOCAS;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Regression coverage for Medium code-review findings:
|
||||
/// <list type="bullet">
|
||||
/// <item>Driver.FOCAS-003 — unknown DeviceHostAddress detected at init</item>
|
||||
/// <item>Driver.FOCAS-004 — all FOCAS tags advertised as ViewOnly</item>
|
||||
/// <item>Driver.FOCAS-005 — GetHealth() survives concurrent updates</item>
|
||||
/// <item>Driver.FOCAS-006 — disposed client is recreated by EnsureConnectedAsync</item>
|
||||
/// <item>Driver.FOCAS-012 — factory round-trip + reconnect coverage</item>
|
||||
/// </list>
|
||||
/// </summary>
|
||||
[Trait("Category", "Unit")]
|
||||
public sealed class FocasDriverMediumFindingsTests
|
||||
{
|
||||
// ---- Driver.FOCAS-003: unknown DeviceHostAddress fails at InitializeAsync ----
|
||||
|
||||
[Fact]
|
||||
public async Task InitializeAsync_throws_when_tag_DeviceHostAddress_not_in_Devices()
|
||||
{
|
||||
var drv = new FocasDriver(new FocasDriverOptions
|
||||
{
|
||||
Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")],
|
||||
Tags =
|
||||
[
|
||||
// DeviceHostAddress has a port typo — not in Devices
|
||||
new FocasTagDefinition("X", "focas://10.0.0.5:9999", "R100", FocasDataType.Byte),
|
||||
],
|
||||
Probe = new FocasProbeOptions { Enabled = false },
|
||||
}, "drv-1", new FakeFocasClientFactory());
|
||||
|
||||
var ex = await Should.ThrowAsync<InvalidOperationException>(
|
||||
() => drv.InitializeAsync("{}", CancellationToken.None));
|
||||
|
||||
ex.Message.ShouldContain("10.0.0.5:9999");
|
||||
ex.Message.ShouldContain("not in the Devices list");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task InitializeAsync_throws_naming_the_offending_tag()
|
||||
{
|
||||
var drv = new FocasDriver(new FocasDriverOptions
|
||||
{
|
||||
Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")],
|
||||
Tags =
|
||||
[
|
||||
// Correct address so address validation passes
|
||||
new FocasTagDefinition("TypoTag", "focas://10.0.0.99:8193", "R100", FocasDataType.Byte),
|
||||
],
|
||||
Probe = new FocasProbeOptions { Enabled = false },
|
||||
}, "drv-1", new FakeFocasClientFactory());
|
||||
|
||||
var ex = await Should.ThrowAsync<InvalidOperationException>(
|
||||
() => drv.InitializeAsync("{}", CancellationToken.None));
|
||||
|
||||
ex.Message.ShouldContain("TypoTag");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task InitializeAsync_succeeds_when_all_tags_reference_declared_devices()
|
||||
{
|
||||
var drv = new FocasDriver(new FocasDriverOptions
|
||||
{
|
||||
Devices =
|
||||
[
|
||||
new FocasDeviceOptions("focas://10.0.0.5:8193"),
|
||||
new FocasDeviceOptions("focas://10.0.0.6:8193"),
|
||||
],
|
||||
Tags =
|
||||
[
|
||||
new FocasTagDefinition("A", "focas://10.0.0.5:8193", "R100", FocasDataType.Byte),
|
||||
new FocasTagDefinition("B", "focas://10.0.0.6:8193", "R100", FocasDataType.Byte),
|
||||
],
|
||||
Probe = new FocasProbeOptions { Enabled = false },
|
||||
}, "drv-1", new FakeFocasClientFactory());
|
||||
|
||||
// Should not throw
|
||||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||
await drv.ShutdownAsync(CancellationToken.None);
|
||||
}
|
||||
|
||||
// ---- Driver.FOCAS-004: all FOCAS user tags advertised as ViewOnly ----
|
||||
|
||||
[Fact]
|
||||
public async Task DiscoverAsync_all_user_tags_are_ViewOnly_regardless_of_Writable_field()
|
||||
{
|
||||
var builder = new RecordingBuilder();
|
||||
var drv = new FocasDriver(new FocasDriverOptions
|
||||
{
|
||||
Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")],
|
||||
Tags =
|
||||
[
|
||||
// Writable: true is the default — must still be ViewOnly
|
||||
new FocasTagDefinition("Speed", "focas://10.0.0.5:8193", "R100", FocasDataType.Int16, Writable: true),
|
||||
new FocasTagDefinition("Alarm", "focas://10.0.0.5:8193", "R200", FocasDataType.Byte, Writable: false),
|
||||
],
|
||||
Probe = new FocasProbeOptions { Enabled = false },
|
||||
}, "drv-1", new FakeFocasClientFactory());
|
||||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||
|
||||
await drv.DiscoverAsync(builder, CancellationToken.None);
|
||||
|
||||
builder.Variables.Single(v => v.BrowseName == "Speed").Info.SecurityClass
|
||||
.ShouldBe(SecurityClassification.ViewOnly,
|
||||
"FOCAS is read-only by design; Writable:true must not emit Operate");
|
||||
builder.Variables.Single(v => v.BrowseName == "Alarm").Info.SecurityClass
|
||||
.ShouldBe(SecurityClassification.ViewOnly);
|
||||
}
|
||||
|
||||
// ---- Driver.FOCAS-005: Volatile-guarded _health survives concurrent reads ----
|
||||
|
||||
[Fact]
|
||||
public async Task GetHealth_reflects_state_updated_from_concurrent_reads()
|
||||
{
|
||||
var factory = new FakeFocasClientFactory
|
||||
{
|
||||
Customise = () => new FakeFocasClient { Values = { ["R100"] = (sbyte)1 } },
|
||||
};
|
||||
var drv = new FocasDriver(new FocasDriverOptions
|
||||
{
|
||||
Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")],
|
||||
Tags = [new FocasTagDefinition("X", "focas://10.0.0.5:8193", "R100", FocasDataType.Byte)],
|
||||
Probe = new FocasProbeOptions { Enabled = false },
|
||||
}, "drv-1", factory);
|
||||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||
|
||||
// Fire concurrent reads — the volatile _health field must not produce a
|
||||
// stale/null reference when accessed simultaneously from multiple tasks.
|
||||
await Task.WhenAll(Enumerable.Range(0, 10).Select(_ =>
|
||||
drv.ReadAsync(["X"], CancellationToken.None)));
|
||||
|
||||
var health = drv.GetHealth();
|
||||
health.ShouldNotBeNull();
|
||||
health.State.ShouldBe(DriverState.Healthy);
|
||||
health.LastSuccessfulRead.ShouldNotBeNull();
|
||||
}
|
||||
|
||||
// ---- Driver.FOCAS-006: EnsureConnectedAsync recreates a disposed/stale client ----
|
||||
|
||||
[Fact]
|
||||
public async Task Read_recovers_after_client_is_externally_disposed()
|
||||
{
|
||||
var factory = new FakeFocasClientFactory
|
||||
{
|
||||
Customise = () => new FakeFocasClient { Values = { ["R100"] = (sbyte)42 } },
|
||||
};
|
||||
var drv = new FocasDriver(new FocasDriverOptions
|
||||
{
|
||||
Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")],
|
||||
Tags = [new FocasTagDefinition("X", "focas://10.0.0.5:8193", "R100", FocasDataType.Byte)],
|
||||
Probe = new FocasProbeOptions { Enabled = false },
|
||||
}, "drv-1", factory);
|
||||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||
|
||||
// Establish first connection via a read.
|
||||
var first = await drv.ReadAsync(["X"], CancellationToken.None);
|
||||
first.Single().StatusCode.ShouldBe(FocasStatusMapper.Good);
|
||||
factory.Clients.Count.ShouldBe(1);
|
||||
|
||||
// Simulate an external dispose (e.g. HandleRecycle or a race) — the client is
|
||||
// now disconnected. The next read must detect the stale client, dispose it,
|
||||
// create a fresh one, and reconnect rather than failing permanently.
|
||||
var staleClient = factory.Clients[0];
|
||||
staleClient.Dispose(); // sets IsConnected=false
|
||||
|
||||
var second = await drv.ReadAsync(["X"], CancellationToken.None);
|
||||
second.Single().StatusCode.ShouldBe(FocasStatusMapper.Good);
|
||||
second.Single().Value.ShouldBe((sbyte)42);
|
||||
|
||||
// A second (fresh) client must have been created to recover.
|
||||
factory.Clients.Count.ShouldBe(2);
|
||||
factory.Clients[1].ConnectCount.ShouldBe(1);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Read_disposes_stale_client_before_creating_fresh_one()
|
||||
{
|
||||
var factory = new FakeFocasClientFactory
|
||||
{
|
||||
Customise = () => new FakeFocasClient { Values = { ["R100"] = (sbyte)1 } },
|
||||
};
|
||||
var drv = new FocasDriver(new FocasDriverOptions
|
||||
{
|
||||
Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")],
|
||||
Tags = [new FocasTagDefinition("X", "focas://10.0.0.5:8193", "R100", FocasDataType.Byte)],
|
||||
Probe = new FocasProbeOptions { Enabled = false },
|
||||
}, "drv-1", factory);
|
||||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||
|
||||
await drv.ReadAsync(["X"], CancellationToken.None);
|
||||
var first = factory.Clients[0];
|
||||
|
||||
// Disconnect the first client without going through DisposeClient so the driver
|
||||
// still holds a reference to it.
|
||||
first.Dispose();
|
||||
|
||||
await drv.ReadAsync(["X"], CancellationToken.None);
|
||||
|
||||
// The original stale client must have been disposed exactly once (by our call)
|
||||
// plus EnsureConnectedAsync must have disposed it again before re-creating it,
|
||||
// or at minimum only the new client connected. Here we verify we have two clients
|
||||
// (the stale one was replaced) and the new one connected successfully.
|
||||
factory.Clients.Count.ShouldBe(2, "a fresh client must replace the disposed one");
|
||||
factory.Clients[1].IsConnected.ShouldBeTrue();
|
||||
}
|
||||
|
||||
// ---- Driver.FOCAS-012: factory round-trip for all three opt-in sections ----
|
||||
|
||||
[Fact]
|
||||
public void CreateInstance_full_round_trip_all_opt_in_sections()
|
||||
{
|
||||
const string json = """
|
||||
{
|
||||
"Backend": "unimplemented",
|
||||
"Devices": [{ "HostAddress": "focas://10.0.0.1:8193" }],
|
||||
"FixedTree": { "Enabled": true, "PollInterval": "00:00:00.200", "ProgramPollInterval": "00:00:05", "TimerPollInterval": "00:01:00" },
|
||||
"AlarmProjection": { "Enabled": true, "PollInterval": "00:00:03" },
|
||||
"HandleRecycle": { "Enabled": true, "Interval": "00:30:00" }
|
||||
}
|
||||
""";
|
||||
|
||||
var drv = FocasDriverFactoryExtensions.CreateInstance("drv-rt", json);
|
||||
|
||||
drv.Options.FixedTree.Enabled.ShouldBeTrue();
|
||||
drv.Options.FixedTree.PollInterval.ShouldBe(TimeSpan.FromMilliseconds(200));
|
||||
drv.Options.FixedTree.ProgramPollInterval.ShouldBe(TimeSpan.FromSeconds(5));
|
||||
drv.Options.FixedTree.TimerPollInterval.ShouldBe(TimeSpan.FromMinutes(1));
|
||||
|
||||
drv.Options.AlarmProjection.Enabled.ShouldBeTrue();
|
||||
drv.Options.AlarmProjection.PollInterval.ShouldBe(TimeSpan.FromSeconds(3));
|
||||
|
||||
drv.Options.HandleRecycle.Enabled.ShouldBeTrue();
|
||||
drv.Options.HandleRecycle.Interval.ShouldBe(TimeSpan.FromMinutes(30));
|
||||
}
|
||||
|
||||
// ---- helpers ----
|
||||
|
||||
private sealed class RecordingBuilder : IAddressSpaceBuilder
|
||||
{
|
||||
public List<(string BrowseName, DriverAttributeInfo Info)> Variables { get; } = new();
|
||||
public List<(string BrowseName, string DisplayName)> Folders { get; } = new();
|
||||
|
||||
public IAddressSpaceBuilder Folder(string browseName, string displayName)
|
||||
{ Folders.Add((browseName, displayName)); return this; }
|
||||
|
||||
public IVariableHandle Variable(string browseName, string displayName, DriverAttributeInfo info)
|
||||
{ Variables.Add((browseName, info)); return new Handle(info.FullName); }
|
||||
|
||||
public void AddProperty(string _, DriverDataType __, object? ___) { }
|
||||
|
||||
private sealed class Handle(string fullRef) : IVariableHandle
|
||||
{
|
||||
public string FullReference => fullRef;
|
||||
public IAlarmConditionSink MarkAsAlarmCondition(AlarmConditionInfo info) => new NullSink();
|
||||
}
|
||||
private sealed class NullSink : IAlarmConditionSink { public void OnTransition(AlarmEventArgs args) { } }
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user