From f46e1262087ab1f9a5a4e44e3c9d886d5b6e9645 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 23 May 2026 08:34:32 -0400 Subject: [PATCH] fix(driver-ablegacy-cli): resolve Low code-review findings (Driver.AbLegacy.Cli-002,003,004,005,006,007) - Driver.AbLegacy.Cli-002: WriteCommand.Value description lists the full true/false, 1/0, on/off, yes/no alias set. - Driver.AbLegacy.Cli-003: SubscribeCommand serialises every WriteLine via a per-execution consoleGate lock so the poll-thread OnDataChange handler can't interleave with the banner. - Driver.AbLegacy.Cli-004: dropped 'await using var driver' in favour of a plain 'var driver' + explicit await ShutdownAsync in finally; the driver is no longer shut down twice. - Driver.AbLegacy.Cli-005: SubscribeCommand.IntervalMs description carries the PollGroupEngine 250ms-floor caveat; docs/Driver.AbLegacy.Cli.md spells out the same. - Driver.AbLegacy.Cli-006: ProbeCommand --type now carries the short alias 't' to match the other commands. - Driver.AbLegacy.Cli-007: BuildOptionsTests cover the probe-disabled, device-shape, tag-passthrough, timeout-propagation, and empty-tag-list paths. Co-Authored-By: Claude Opus 4.7 (1M context) --- code-reviews/Driver.AbLegacy.Cli/findings.md | 56 ++++++-- docs/Driver.AbLegacy.Cli.md | 3 + .../Commands/ProbeCommand.cs | 7 +- .../Commands/ReadCommand.cs | 5 +- .../Commands/SubscribeCommand.cs | 26 +++- .../Commands/WriteCommand.cs | 7 +- .../BuildOptionsTests.cs | 134 ++++++++++++++++++ .../CommandMetadataTests.cs | 80 +++++++++++ 8 files changed, 295 insertions(+), 23 deletions(-) create mode 100644 tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/BuildOptionsTests.cs create mode 100644 tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/CommandMetadataTests.cs diff --git a/code-reviews/Driver.AbLegacy.Cli/findings.md b/code-reviews/Driver.AbLegacy.Cli/findings.md index cff5fc8..f87755f 100644 --- a/code-reviews/Driver.AbLegacy.Cli/findings.md +++ b/code-reviews/Driver.AbLegacy.Cli/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-22 | | Commit reviewed | `76d35d1` | | Status | Reviewed | -| Open findings | 6 | +| Open findings | 0 | ## Checklist coverage @@ -68,7 +68,7 @@ type (mirroring the existing `Bit` and unsupported-type branches). Either catch | Severity | Low | | Category | Correctness & logic bugs | | Location | `Commands/WriteCommand.cs:27-29`, `Program.cs:6-9` | -| Status | Open | +| Status | Resolved | **Description:** The `--value` option help text states "booleans accept true/false/1/0", but `ParseBool` (`WriteCommand.cs:74-80`) and the error message @@ -82,7 +82,10 @@ with both the code and the design doc. matching the wording used elsewhere (e.g. "booleans accept true/false, 1/0, on/off, yes/no"). -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — updated `WriteCommand.Value` description to +list the full alias set: "booleans accept true/false, 1/0, on/off, yes/no". +Regression test `CommandMetadataTests.WriteCommand_value_help_lists_full_boolean_alias_set` +asserts the description contains every alias group. ### Driver.AbLegacy.Cli-003 @@ -91,7 +94,7 @@ true/false, 1/0, on/off, yes/no"). | Severity | Low | | Category | Concurrency & thread safety | | Location | `Commands/SubscribeCommand.cs:47-53` | -| Status | Open | +| Status | Resolved | **Description:** The `OnDataChange` handler calls `console.Output.WriteLine(line)` (the synchronous overload) directly from the `PollGroupEngine` poll thread. The @@ -109,7 +112,10 @@ change events through a `Channel` drained by a single consumer task, or guard the `WriteLine` with a lock. At minimum, document that the interleaving is accepted because output is human-facing and line-buffered. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — `SubscribeCommand` now serialises every +console write through a shared `consoleGate` lock: the poll-thread +`OnDataChange` callback and the command-thread "Subscribed to ..." line both take +the lock before calling `WriteLine`. Comment in the source documents the intent. ### Driver.AbLegacy.Cli-004 @@ -118,7 +124,7 @@ accepted because output is human-facing and line-buffered. | Severity | Low | | Category | Error handling & resilience | | Location | `Commands/ProbeCommand.cs:37-56`, `Commands/ReadCommand.cs:39-50`, `Commands/WriteCommand.cs:48-59`, `Commands/SubscribeCommand.cs:41-76` | -| Status | Open | +| Status | Resolved | **Description:** Every command does `await using var driver = new AbLegacyDriver(...)` *and* an explicit `await driver.ShutdownAsync(...)` in the `finally`. `AbLegacyDriver` @@ -135,7 +141,12 @@ cleanup on every exit path including exceptions. since the commands deliberately pass `CancellationToken.None` to shutdown so teardown is not cut short by a cancelled `ct`. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — replaced `await using var driver` with a +plain `var driver` in all four commands (`ProbeCommand`, `ReadCommand`, +`WriteCommand`, `SubscribeCommand`), keeping the explicit +`finally { await driver.ShutdownAsync(CancellationToken.None) }` as the single +teardown path. Comment in each command documents the intent so readers do not +have to verify idempotency. ### Driver.AbLegacy.Cli-005 @@ -144,7 +155,7 @@ is not cut short by a cancelled `ct`. | Severity | Low | | Category | Design-document adherence | | Location | `Commands/SubscribeCommand.cs:23-25`, `docs/Driver.AbLegacy.Cli.md:94-96` | -| Status | Open | +| Status | Resolved | **Description:** The subscribe command interval option is `--interval-ms` (default 1000). `docs/Driver.AbLegacy.Cli.md` shows the subscribe example as @@ -160,7 +171,13 @@ but the documented contract drifts between the two CLIs. `--interval-ms` description for parity with the AbCip CLI, and mention the `--interval-ms` long form + 1000 ms default in `docs/Driver.AbLegacy.Cli.md`. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — extended the `SubscribeCommand.IntervalMs` +help text to match AbCip ("Publishing interval in milliseconds (default 1000). +PollGroupEngine floors sub-250ms values.") and added a paragraph under the +`subscribe` section in `docs/Driver.AbLegacy.Cli.md` naming the `-i` / +`--interval-ms` long form, the 1000 ms default, and the 250 ms floor. Regression +test `CommandMetadataTests.SubscribeCommand_interval_ms_help_notes_PollGroupEngine_floor` +asserts the description mentions "250". ### Driver.AbLegacy.Cli-006 @@ -169,7 +186,7 @@ but the documented contract drifts between the two CLIs. | Severity | Low | | Category | Code organization & conventions | | Location | `Commands/ProbeCommand.cs:20-22` | -| Status | Open | +| Status | Resolved | **Description:** `ProbeCommand` declares its `--type` option with no short alias, while `ReadCommand`, `WriteCommand`, and `SubscribeCommand` all declare `--type` @@ -182,7 +199,13 @@ it silently rejected on `probe`. for consistency with the other three commands. (The AbCip CLI `ProbeCommand` has the same omission, so a cross-CLI sweep is worthwhile.) -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — added the `'t'` short alias to +`ProbeCommand.DataType`. Regression test +`CommandMetadataTests.ProbeCommand_type_has_short_alias_t` (plus the parity test +`Other_commands_keep_type_short_alias_t` for read/write/subscribe) asserts the +short alias is present on every command. The same omission still exists in the +AbCip CLI's `ProbeCommand` — flagged as a sibling sweep but out of scope for +this module. ### Driver.AbLegacy.Cli-007 @@ -191,7 +214,7 @@ the same omission, so a cross-CLI sweep is worthwhile.) | Severity | Low | | Category | Testing coverage | | Location | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/WriteCommandParseValueTests.cs` | -| Status | Open | +| Status | Resolved | **Description:** The only test file in the CLI test project covers `WriteCommand.ParseValue` and `ReadCommand.SynthesiseTagName`. Two behaviours that @@ -210,4 +233,11 @@ Driver.AbLegacy.Cli-001. `BuildOptions` is reachable via `InternalsVisibleTo` tag passthrough) and an overflow-input test for `ParseValue` so the fix for Driver.AbLegacy.Cli-001 is locked in by a regression test. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — added `BuildOptionsTests` (five tests: +probe disabled, device shape from Gateway+PlcType, tag passthrough, timeout +propagation, empty tag list) covering `AbLegacyCommandBase.BuildOptions` via a +nested `TestCommand` subclass annotated with `[Command]` to satisfy the CliFx +analyzer. The overflow path for `ParseValue` is already covered by +`WriteCommandParseValueTests.ParseValue_out_of_range_throws_CommandException` +(theory with `short.Parse` + `AnalogInt` overflow inputs), added when finding +Driver.AbLegacy.Cli-001 was resolved. diff --git a/docs/Driver.AbLegacy.Cli.md b/docs/Driver.AbLegacy.Cli.md index f7c7d53..296d3b7 100644 --- a/docs/Driver.AbLegacy.Cli.md +++ b/docs/Driver.AbLegacy.Cli.md @@ -95,6 +95,9 @@ PLC-managed — use with caution. otopcua-ablegacy-cli subscribe -g ab://192.168.1.20/1,0 -a N7:10 -t Int -i 500 ``` +`-i` / `--interval-ms` is the publishing interval in milliseconds — default +`1000`. `PollGroupEngine` floors sub-250 ms values, so `-i 100` runs at 250 ms. + ## Known caveat — ab_server upstream gap The integration-fixture `ab_server` Docker container accepts TCP but its PCCC diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/ProbeCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/ProbeCommand.cs index 4c1d76c..2be1f83 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/ProbeCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/ProbeCommand.cs @@ -17,7 +17,7 @@ public sealed class ProbeCommand : AbLegacyCommandBase "the pre-populated register every SLC / MicroLogix / PLC-5 ships with.")] public string Address { get; init; } = "N7:0"; - [CommandOption("type", Description = + [CommandOption("type", 't', Description = "PCCC data type of the probe address (default Int — matches N files).")] public AbLegacyDataType DataType { get; init; } = AbLegacyDataType.Int; @@ -34,7 +34,10 @@ public sealed class ProbeCommand : AbLegacyCommandBase Writable: false); var options = BuildOptions([probeTag]); - await using var driver = new AbLegacyDriver(options, DriverInstanceId); + // Plain `var driver`: explicit ShutdownAsync(CancellationToken.None) in the + // finally is the deliberate teardown path; combining it with `await using` + // (which itself calls ShutdownAsync) would tear the driver down twice. + var driver = new AbLegacyDriver(options, DriverInstanceId); try { await driver.InitializeAsync("{}", ct); diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/ReadCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/ReadCommand.cs index 0fb5e3f..ccb87eb 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/ReadCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/ReadCommand.cs @@ -36,7 +36,10 @@ public sealed class ReadCommand : AbLegacyCommandBase Writable: false); var options = BuildOptions([tag]); - await using var driver = new AbLegacyDriver(options, DriverInstanceId); + // Plain `var driver`: explicit ShutdownAsync(CancellationToken.None) in the + // finally is the deliberate teardown path; combining it with `await using` + // (which itself calls ShutdownAsync) would tear the driver down twice. + var driver = new AbLegacyDriver(options, DriverInstanceId); try { await driver.InitializeAsync("{}", ct); diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/SubscribeCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/SubscribeCommand.cs index 439890b..90a5120 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/SubscribeCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/SubscribeCommand.cs @@ -21,7 +21,8 @@ public sealed class SubscribeCommand : AbLegacyCommandBase public AbLegacyDataType DataType { get; init; } = AbLegacyDataType.Int; [CommandOption("interval-ms", 'i', Description = - "Publishing interval in milliseconds (default 1000).")] + "Publishing interval in milliseconds (default 1000). PollGroupEngine floors " + + "sub-250ms values.")] public int IntervalMs { get; init; } = 1000; public override async ValueTask ExecuteAsync(IConsole console) @@ -38,8 +39,17 @@ public sealed class SubscribeCommand : AbLegacyCommandBase Writable: false); var options = BuildOptions([tag]); - await using var driver = new AbLegacyDriver(options, DriverInstanceId); + // Plain `var driver` (no `await using`): driver.DisposeAsync internally calls + // ShutdownAsync, so combining `await using` with an explicit finally-shutdown + // would tear the driver down twice. The explicit teardown is preferred because + // it deliberately passes CancellationToken.None — `await using` would otherwise + // happen on a cancelled `ct` path which can cut teardown short. + var driver = new AbLegacyDriver(options, DriverInstanceId); ISubscriptionHandle? handle = null; + // Serialise console writes from the poll-thread OnDataChange callback against + // the command-thread "Subscribed to ..." line and against each other; the + // PollGroupEngine raises change events on a background timer/loop thread. + var consoleGate = new object(); try { await driver.InitializeAsync("{}", ct); @@ -49,13 +59,19 @@ public sealed class SubscribeCommand : AbLegacyCommandBase var line = $"[{DateTime.UtcNow:HH:mm:ss.fff}] " + $"{e.FullReference} = {SnapshotFormatter.FormatValue(e.Snapshot.Value)} " + $"({SnapshotFormatter.FormatStatus(e.Snapshot.StatusCode)})"; - console.Output.WriteLine(line); + lock (consoleGate) + { + console.Output.WriteLine(line); + } }; handle = await driver.SubscribeAsync([tagName], TimeSpan.FromMilliseconds(IntervalMs), ct); - await console.Output.WriteLineAsync( - $"Subscribed to {Address} @ {IntervalMs}ms. Ctrl+C to stop."); + lock (consoleGate) + { + console.Output.WriteLine( + $"Subscribed to {Address} @ {IntervalMs}ms. Ctrl+C to stop."); + } try { await Task.Delay(System.Threading.Timeout.InfiniteTimeSpan, ct); diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/WriteCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/WriteCommand.cs index 4ce6695..d1d4f8b 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/WriteCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli/Commands/WriteCommand.cs @@ -25,7 +25,7 @@ public sealed class WriteCommand : AbLegacyCommandBase public AbLegacyDataType DataType { get; init; } = AbLegacyDataType.Int; [CommandOption("value", 'v', Description = - "Value to write. Parsed per --type (booleans accept true/false/1/0).", + "Value to write. Parsed per --type (booleans accept true/false, 1/0, on/off, yes/no).", IsRequired = true)] public string Value { get; init; } = default!; @@ -45,7 +45,10 @@ public sealed class WriteCommand : AbLegacyCommandBase var parsed = ParseValue(Value, DataType); - await using var driver = new AbLegacyDriver(options, DriverInstanceId); + // Plain `var driver`: explicit ShutdownAsync(CancellationToken.None) in the + // finally is the deliberate teardown path; combining it with `await using` + // (which itself calls ShutdownAsync) would tear the driver down twice. + var driver = new AbLegacyDriver(options, DriverInstanceId); try { await driver.InitializeAsync("{}", ct); diff --git a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/BuildOptionsTests.cs b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/BuildOptionsTests.cs new file mode 100644 index 0000000..77620d9 --- /dev/null +++ b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/BuildOptionsTests.cs @@ -0,0 +1,134 @@ +using System; +using System.Collections.Generic; +using CliFx.Attributes; +using CliFx.Infrastructure; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests; + +/// +/// Locks in : probe disabled, +/// device shape populated from --gateway + --plc-type, tag list +/// forwarded verbatim, and timeout propagated from --timeout-ms. A +/// regression here silently changes every AbLegacy CLI command's behaviour, so +/// covering it explicitly closes the test gap called out by finding +/// Driver.AbLegacy.Cli-007. +/// +[Trait("Category", "Unit")] +public sealed class BuildOptionsTests +{ + // Concrete subclass needed because AbLegacyCommandBase is abstract. Exposes the + // protected BuildOptions via a public surface for the test. + // [Command] satisfies CliFx's analyzer (ICommand subtypes must be annotated); + // we never run it through CliFx, only invoke Build() directly. + [Command("test-build-options")] + private sealed class TestCommand : AbLegacyCommandBase + { + public AbLegacyDriverOptions Build(IReadOnlyList tags) + => BuildOptions(tags); + + public override System.Threading.Tasks.ValueTask ExecuteAsync(IConsole console) + => throw new NotSupportedException("TestCommand is for BuildOptions inspection only."); + } + + private static readonly IReadOnlyList SampleTags = + [ + new AbLegacyTagDefinition( + Name: "N7:0:Int", + DeviceHostAddress: "ab://192.168.1.20/1,0", + Address: "N7:0", + DataType: AbLegacyDataType.Int, + Writable: false), + new AbLegacyTagDefinition( + Name: "F8:0:Float", + DeviceHostAddress: "ab://192.168.1.20/1,0", + Address: "F8:0", + DataType: AbLegacyDataType.Float, + Writable: true), + ]; + + [Fact] + public void BuildOptions_disables_probe_for_cli_oneshot_runs() + { + var cmd = new TestCommand + { + Gateway = "ab://192.168.1.20/1,0", + PlcType = AbLegacyPlcFamily.Slc500, + TimeoutMs = 5000, + }; + + var options = cmd.Build(SampleTags); + + options.Probe.ShouldNotBeNull(); + options.Probe.Enabled.ShouldBeFalse( + "CLI commands are one-shot; the background probe loop is unwanted overhead."); + } + + [Fact] + public void BuildOptions_populates_single_device_from_gateway_and_plc_type() + { + var cmd = new TestCommand + { + Gateway = "ab://10.0.0.5/1,0", + PlcType = AbLegacyPlcFamily.MicroLogix, + TimeoutMs = 5000, + }; + + var options = cmd.Build(SampleTags); + + options.Devices.Count.ShouldBe(1); + options.Devices[0].HostAddress.ShouldBe("ab://10.0.0.5/1,0"); + options.Devices[0].PlcFamily.ShouldBe(AbLegacyPlcFamily.MicroLogix); + options.Devices[0].DeviceName.ShouldBe("cli-MicroLogix"); + } + + [Fact] + public void BuildOptions_forwards_tag_list_verbatim() + { + var cmd = new TestCommand + { + Gateway = "ab://192.168.1.20/1,0", + PlcType = AbLegacyPlcFamily.Slc500, + TimeoutMs = 5000, + }; + + var options = cmd.Build(SampleTags); + + options.Tags.ShouldBe(SampleTags); + } + + [Fact] + public void BuildOptions_propagates_timeout_ms() + { + var cmd = new TestCommand + { + Gateway = "ab://192.168.1.20/1,0", + PlcType = AbLegacyPlcFamily.Slc500, + TimeoutMs = 7500, + }; + + var options = cmd.Build(SampleTags); + + options.Timeout.ShouldBe(TimeSpan.FromMilliseconds(7500)); + } + + [Fact] + public void BuildOptions_with_empty_tag_list_yields_empty_tags_collection() + { + var cmd = new TestCommand + { + Gateway = "ab://192.168.1.20/1,0", + PlcType = AbLegacyPlcFamily.Plc5, + TimeoutMs = 5000, + }; + + var options = cmd.Build([]); + + options.Tags.ShouldBeEmpty(); + options.Devices.Count.ShouldBe(1); + options.Devices[0].PlcFamily.ShouldBe(AbLegacyPlcFamily.Plc5); + } +} diff --git a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/CommandMetadataTests.cs b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/CommandMetadataTests.cs new file mode 100644 index 0000000..044de44 --- /dev/null +++ b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/CommandMetadataTests.cs @@ -0,0 +1,80 @@ +using System.Linq; +using System.Reflection; +using CliFx.Attributes; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Commands; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests; + +/// +/// Locks in the CLI command-option contract surface area — short aliases and +/// help-text wording — that the AbLegacy CLI is expected to keep in parity with +/// its sibling AbCip CLI and with docs/Driver.AbLegacy.Cli.md. +/// Regression coverage for findings Driver.AbLegacy.Cli-002, -005, -006. +/// +[Trait("Category", "Unit")] +public sealed class CommandMetadataTests +{ + private static CommandOptionAttribute GetOption(string propertyName) + { + var prop = typeof(TCommand).GetProperty( + propertyName, + BindingFlags.Public | BindingFlags.Instance); + prop.ShouldNotBeNull($"property {propertyName} is missing from {typeof(TCommand).Name}"); + var attr = prop!.GetCustomAttribute(); + attr.ShouldNotBeNull( + $"property {propertyName} on {typeof(TCommand).Name} lacks [CommandOption]"); + return attr!; + } + + // ---------- Driver.AbLegacy.Cli-006 — ProbeCommand --type needs short alias 't' ---------- + + [Fact] + public void ProbeCommand_type_has_short_alias_t() + { + // Parity with read / write / subscribe: --type / -t works everywhere. + var attr = GetOption(nameof(ProbeCommand.DataType)); + attr.ShortName.ShouldBe('t'); + } + + [Theory] + [InlineData(typeof(ReadCommand), nameof(ReadCommand.DataType))] + [InlineData(typeof(WriteCommand), nameof(WriteCommand.DataType))] + [InlineData(typeof(SubscribeCommand), nameof(SubscribeCommand.DataType))] + public void Other_commands_keep_type_short_alias_t(System.Type commandType, string propName) + { + var prop = commandType.GetProperty(propName, BindingFlags.Public | BindingFlags.Instance); + prop.ShouldNotBeNull(); + var attr = prop!.GetCustomAttribute(); + attr.ShouldNotBeNull(); + attr!.ShortName.ShouldBe('t'); + } + + // ---------- Driver.AbLegacy.Cli-002 — WriteCommand --value help lists full bool alias set ---------- + + [Fact] + public void WriteCommand_value_help_lists_full_boolean_alias_set() + { + // ParseBool accepts true/false, 1/0, on/off, yes/no — the help text must say so + // (DriverClis.md documents the full alias set as the shared CLI contract). + var attr = GetOption(nameof(WriteCommand.Value)); + attr.Description.ShouldNotBeNull(); + attr.Description!.ShouldContain("true/false", Case.Insensitive); + attr.Description!.ShouldContain("1/0"); + attr.Description!.ShouldContain("on/off", Case.Insensitive); + attr.Description!.ShouldContain("yes/no", Case.Insensitive); + } + + // ---------- Driver.AbLegacy.Cli-005 — SubscribeCommand --interval-ms help notes 250ms floor ---------- + + [Fact] + public void SubscribeCommand_interval_ms_help_notes_PollGroupEngine_floor() + { + // Parity with AbCip CLI: operators passing -i 100 deserve a heads-up that + // PollGroupEngine floors sub-250ms values. + var attr = GetOption(nameof(SubscribeCommand.IntervalMs)); + attr.Description.ShouldNotBeNull(); + attr.Description!.ShouldContain("250", Case.Insensitive); + } +}